Re: [PATCH 4/7] media: uapi: v4l: Document source routes

Jacopo Mondi
2023-07-10T12:54:02Z

Hi Sakari

On Sat, Jul 08, 2023 at 12:13:17PM +0000, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Mon, Jul 03, 2023 at 09:47:37AM +0200, Jacopo Mondi wrote:
> > Hi Sakari
> >
> > On Fri, Jun 30, 2023 at 11:43:35PM +0300, Sakari Ailus wrote:
> > > Document how internal pads are used on source routes.
> > >
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > > .../userspace-api/media/v4l/dev-subdev.rst | 20 +++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > index a4f1df7093e8..5a46c9a9d352 100644
> > > --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> > > @@ -551,6 +551,26 @@ A stream at a specific point in the media pipeline is identified by the
> > > sub-device and a (pad, stream) pair. For sub-devices that do not support
> > > multiplexed streams the 'stream' field is always 0.
> > >
> > > +.. _v4l2-subdev-source-routes:
> > > +
> > > +Source routes
> > > +^^^^^^^^^^^^^
> >
> > I always found the concept of source routes a bit confusing, should we
> > instead just present internal pads ?
> >
> > > +
> > > +Cases where a single sub-device pad is a source of multiple streams are special
> > > +as there is no external sink pad for such a route. In those cases, the sources
> > > +of the streams are indicated by source routes that have an internal source pad
> > > +as the sink pad of such a route. Internal source pads have the
> > > +:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> > > +pad flags set.
> >
> > All this last part is a little bit hard to parse, not your fault but
> > the fact "internal source pads" are actually "SINK" pads is a bit
> > confusing ?
> >
> > Can we remove the "source route" concept to avoid mixing source/sink ?
> >
> > This can be rewritten as
> >
> > Internal pads
> > ^^^^^^^^^^^^^
> >
> > Cases where a single sub-device pad is a source of multiple streams are special
> > as there is no external sink pad for such a route. A typical example is a
> > sensor device which produces a video stream and a metadata stream of
> > embedded data. To support such cases internal pads are introduced as
> > sink pads of such internally generated streams.
> > Internal source pads have the :ref:`MEDIA_PAD_FL_INTERNAL
> > <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK`` pad flags set.
> >
> > > +Internal source pads have all the properties of a sink pad in such case,
> >
> > Also here, "Internal source pads" are actually sinks :)
> >
> > I would drop "source" from "Internal source pads"
>
> How about this (compared to the patch):
>
> diff --git a/Documentation/userspace-api/media/v4l/dev-subdev.rst b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> index 5a46c9a9d352..9d544a29e78a 100644
> --- a/Documentation/userspace-api/media/v4l/dev-subdev.rst
> +++ b/Documentation/userspace-api/media/v4l/dev-subdev.rst
> @@ -553,21 +553,23 @@ multiplexed streams the 'stream' field is always 0.
>
> .. _v4l2-subdev-source-routes:
>
> -Source routes
> -^^^^^^^^^^^^^
> -
> -Cases where a single sub-device pad is a source of multiple streams are special
> -as there is no external sink pad for such a route. In those cases, the sources
> -of the streams are indicated by source routes that have an internal source pad
> -as the sink pad of such a route. Internal source pads have the
> -:ref:`MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>` and ``MEDIA_PAD_FL_SINK``
> -pad flags set.
> -
> -Internal source pads have all the properties of a sink pad in such case,
> -including formats and selections. The format in this case is the source format
> -of the stream. An internal pad always has a single stream only (0).
> -
> -Generally source routes are not modifiable but they can be activated and
> +Internal pads and source routes
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Cases where a single sub-device source pad is traversed by multiple streams one
> +or more of which originate from within the sub-device itself are special as
> +there is no external sink pad for such routes. In those cases, the sources of

"external" made me think of "a different entity". Can we just drop it
or am I too easy to confuse ?

> +the internally generated streams are indicated by internal pads, pads which have

s/indicated by/represented by/ ?

'internal pads, pads...'

or

/internal pads, which are sink pads with the
:ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set.'

and drop the "on the sink pad of the route" ?

> +a :ref:`MEDIA_PAD_FL_INTERNAL` <MEDIA-PAD-FL-INTERNAL> pad flag set on the sink
> +pad of the route. A typical use case for these is a camera sensor device which
> +produces a pixel data stream and an embedded data stream.

This case is represented with two internal sink pads for the video and
the data streams and a single multiplexed source pad that connects to
the next entity in the pipeline.

> +
> +Internal pads have all the properties of an external pad, including formats and
> +selections. The format in this case is the source format of the stream. An
> +internal pad always has a single stream only (0).
> +
> +/Source routes/ are routes from an internal sink pad to a(n external) source
> +pad. Generally source routes are not modifiable but they can be activated and
> deactivated using the :ref:`V4L2_SUBDEV_ROUTE_FL_ACTIVE
> <v4l2-subdev-routing-flags>` flag, depending on driver capabilities.

If you want to keep the part about source routes, this form is ok with
me!

>
> I'll also check the ACTIVE route flag, it wasn't merged with the rest of
> the Tomi's streams series.
>
> --
> Kind regards,
>
> Sakari Ailus