Hi Hans, On Thu, Oct 05, 2023 at 01:16:27PM +0200, Hans Verkuil wrote: > On 05/10/2023 12:22, Sakari Ailus wrote: > > Hi Hans, > > > > Thanks for the review! > > > > 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 where it indicates that the :ref:``stream <media-glossary-stream>`` originates from within the entity. -- Regards, Sakari Ailus
