Hi Laurent, On Fri, Oct 27, 2023 at 03:48:04PM +0300, Laurent Pinchart wrote: > 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 > ... Thanks for spotting this, I'll address it in v7. > > > where it indicates that the :ref:``stream <media-glossary-stream>`` > > originates from within the entity. > -- Regards, Sakari Ailus