Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Dafna Hirschfeld <dafna3@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Helen Koike <helen.koike@collabora.com>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	kernel@collabora.com, linux-rockchip@lists.infradead.org,
	mchehab@kernel.org
Subject: Re: [PATCH 1/2] media: media.h: Add a pad flag MEDIA_PAD_FL_METADATA
Date: Fri, 27 Mar 2020 10:08:58 +0200
Message-ID: <20200327080858.GB23713@kekkonen.localdomain> (raw)
In-Reply-To: <CAJ1myNS5jdqCXAtueA_j+ULkiioDxhegfCLQWFXrvL6_AYPwFg@mail.gmail.com>

Dafna, Laurent,

On Thu, Mar 26, 2020 at 08:59:04AM +0100, Dafna Hirschfeld wrote:
> On Wed, Mar 25, 2020 at 11:26 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Dafna,
> >
> > Thank you for the patch.
> >
> > On Wed, Mar 25, 2020 at 10:27:03PM +0100, Dafna Hirschfeld wrote:
> > > Add a flag to the flags field of 'struct media_pad_desc'
> > > that indicates that the data transmitted by the pad is
> > > metadata.
> > >
> > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> > > ---
> > >  Documentation/media/uapi/mediactl/media-types.rst | 4 ++++
> > >  include/uapi/linux/media.h                        | 1 +
> > >  2 files changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst
> > > index 3af6a414b501..4ca902478971 100644
> > > --- a/Documentation/media/uapi/mediactl/media-types.rst
> > > +++ b/Documentation/media/uapi/mediactl/media-types.rst
> > > @@ -361,6 +361,7 @@ Types and flags used to represent the media graph elements
> > >  .. _MEDIA-PAD-FL-SINK:
> > >  .. _MEDIA-PAD-FL-SOURCE:
> > >  .. _MEDIA-PAD-FL-MUST-CONNECT:
> > > +.. _MEDIA-PAD-FL-METADATA:
> > >
> > >  .. flat-table:: Media pad flags
> > >      :header-rows:  0
> > > @@ -381,6 +382,9 @@ Types and flags used to represent the media graph elements
> > >         configuration dependent) for the pad to need enabled links even
> > >         when this flag isn't set; the absence of the flag doesn't imply
> > >         there is none.
> > > +    *  -  ``MEDIA_PAD_FL_METADATA``
> > > +       -  This flag indicates that the data transmitted by the pad is of
> > > +          type metadata.
> > >
> > >
> > >  One and only one of ``MEDIA_PAD_FL_SINK`` and ``MEDIA_PAD_FL_SOURCE``
> > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > > index 383ac7b7d8f0..ae37226eb5c9 100644
> > > --- a/include/uapi/linux/media.h
> > > +++ b/include/uapi/linux/media.h
> > > @@ -210,6 +210,7 @@ struct media_entity_desc {
> > >  #define MEDIA_PAD_FL_SINK                    (1 << 0)
> > >  #define MEDIA_PAD_FL_SOURCE                  (1 << 1)
> > >  #define MEDIA_PAD_FL_MUST_CONNECT            (1 << 2)
> > > +#define MEDIA_PAD_FL_METADATA                        (1 << 3)
> >
> > I think we need to reserve a few bits here. We'll have more than
> > metadata. Audio comes to mind, there will likely be more. Having
> > independent flags would not only waste a bit of space in the bitfield
> > (not that we're about to run out of bits, but still), but would make it
> > possible to specify invalid configurations such as MEDIA_PAD_FL_METADATA
> > | MEDIA_PAD_FL_AUDIO. And now that I've written this, I realize that
> > audio metadata could be a thing, so maybe a metadata flag is actually
> > the best option :-)
> hehe, ok, but drivers that set the METADATA flag should also set the media
> bus code to MEDIA_BUS_FMT_FIXED ? If yes then setting
> the METADATA flag with a different media bus is also an invalid configuration.

That may be currently the case, but not all non-image data (metadata in
practice) will be using MEDIA_BUS_FMT_FIXED, for instance sensor embedded
data when we support that in upstream.

Note that whether metadata flows over a pad is dynamic configuration. I
wonder if it is useful to tell this to the user, as the user, in many
cases, will be making the configuration affecting this. There definitely
are devices where this configuration would be static, but many of those
cases (ISPs in particular) have DMAs (i.e. video nodes) directly connected
over a link, where you'll find this information on the video node.

-- 
Regards,

Sakari Ailus

  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 21:27 [PATCH 0/2] media: staging: rkisp1: Fix formats for metadata pads Dafna Hirschfeld
2020-03-25 21:27 ` [PATCH 1/2] media: media.h: Add a pad flag MEDIA_PAD_FL_METADATA Dafna Hirschfeld
2020-03-25 22:26   ` Laurent Pinchart
2020-03-26  7:59     ` Dafna Hirschfeld
2020-03-27  8:08       ` Sakari Ailus [this message]
2020-03-30 12:42         ` Dafna Hirschfeld
2020-03-25 21:27 ` [PATCH 2/2] media: staging: rkisp1: add the flag MEDIA_PAD_FL_METADATA to the metadata pads Dafna Hirschfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200327080858.GB23713@kekkonen.localdomain \
    --to=sakari.ailus@linux.intel.com \
    --cc=dafna.hirschfeld@collabora.com \
    --cc=dafna3@gmail.com \
    --cc=ezequiel@collabora.com \
    --cc=helen.koike@collabora.com \
    --cc=hverkuil@xs4all.nl \
    --cc=kernel@collabora.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mchehab@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git