Re: [PATCH v6 01/28] media: mc: Add INTERNAL pad flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari,

On Wed, Oct 25, 2023 at 09:17:15PM +0000, Sakari Ailus wrote:
> On Thu, Oct 05, 2023 at 01:16:27PM +0200, Hans Verkuil wrote:
> > On 05/10/2023 12:22, Sakari Ailus wrote:
> > > On Thu, Oct 05, 2023 at 11:52:08AM +0200, Hans Verkuil wrote:
> > >> On 03/10/2023 13:52, Sakari Ailus wrote:
> > >>> Internal source pads will be used as routing endpoints in V4L2
> > >>> [GS]_ROUTING IOCTLs, to indicate that the stream begins in the entity.
> > >>>
> > >>> Also prevent creating links to pads that have been flagged as internal and
> > >>> initialising source pads with internal flag set.
> > >>>
> > >>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > >>> ---
> > >>>  Documentation/userspace-api/media/glossary.rst         |  6 ++++++
> > >>>  .../userspace-api/media/mediactl/media-types.rst       |  6 ++++++
> > >>>  drivers/media/mc/mc-entity.c                           | 10 ++++++++--
> > >>>  include/uapi/linux/media.h                             |  1 +
> > >>>  4 files changed, 21 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/Documentation/userspace-api/media/glossary.rst b/Documentation/userspace-api/media/glossary.rst
> > >>> index 96a360edbf3b..f7b99a4527c7 100644
> > >>> --- a/Documentation/userspace-api/media/glossary.rst
> > >>> +++ b/Documentation/userspace-api/media/glossary.rst
> > >>> @@ -173,6 +173,12 @@ Glossary
> > >>>  	An integrated circuit that integrates all components of a computer
> > >>>  	or other electronic systems.
> > >>>  
> > >>> +_media-glossary-stream:
> > >>> +    Stream
> > >>> +	A distinct flow of data (image data or metadata) over a media pipeline
> > >>> +	from source to sink. A source may be e.g. an image sensor and a sink
> > >>> +	e.g. a memory buffer.
> > >>
> > >> Hmm, I think this is a bit confusing. I think it would be better to replace
> > >> "from source to sink" with "from the initial source to the final sink".
> > > 
> > > Seems fine to me. I'll use that in v7.
> > > 
> > >>
> > >> The original text doesn't make it clear that there can be many hops in
> > >> between.
> > >>
> > >> So also: "A source" -> "An initial source", and "a sink" -> "a final sink".
> > >>
> > >> Note that "media pipeline" isn't defined either. Should that be added?
> > > 
> > > I can add that, too...
> > > 
> > >>
> > >> Finally, I think this should be a separate patch as it has nothing to do with
> > >> adding the INTERNAL pad flag.
> > > 
> > > I agree, will split in v7.
> > > 
> > >>
> > >>> +
> > >>>      V4L2 API
> > >>>  	**V4L2 userspace API**
> > >>>  
> > >>> diff --git a/Documentation/userspace-api/media/mediactl/media-types.rst b/Documentation/userspace-api/media/mediactl/media-types.rst
> > >>> index 0ffeece1e0c8..28941da27790 100644
> > >>> --- a/Documentation/userspace-api/media/mediactl/media-types.rst
> > >>> +++ b/Documentation/userspace-api/media/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-INTERNAL:
> > >>>  
> > >>>  .. flat-table:: Media pad flags
> > >>>      :header-rows:  0
> > >>> @@ -382,6 +383,11 @@ Types and flags used to represent the media graph elements
> > >>>  	  when this flag isn't set; the absence of the flag doesn't imply
> > >>>  	  there is none.
> > >>>  
> > >>> +    *  -  ``MEDIA_PAD_FL_INTERNAL``
> > >>> +       -  The internal flag indicates an internal pad that has no external
> > >>> +	  connections. Such a pad shall not be connected with a link. The
> > >>> +	  internal flag indicates that the :ref:``stream
> > >>> +	  <media-glossary-stream>`` either starts or ends in the entity.
> > >>
> > >> This suggests that INTERNAL can be used for both sinks and sources, but...
> > > 
> > > Right. The INTERNAL flag in UAPI shouldn't be limited to sources, but at
> > > the same time we don't have use for it in sinks. I'd prefer to leave this
> > > open in the future. We could of course say that explicitly.
> > 
> > I think it is better to mention that it is for streams that start in the
> > entity only, but that in the future it might be extended to support streams
> > that end in the entity.
> > 
> > When we add support for that, we need to update the documentation as well,
> > at minimum with one or more examples of how that would be used.
> 
> I'll use this instead of the original sentence:
> 
> 	  The internal flag may currently be present only in a source pad only

Double "only". 

	The internal flag may currently be present only in source pads, where
	...

> 	  where it indicates that the :ref:``stream <media-glossary-stream>``
> 	  originates from within the entity.

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux