diff mbox series

[RFC,v3,5/9] media: Documentation: Add scaling and post-scaler crop for common raw

Message ID 20241129095142.87196-6-sakari.ailus@linux.intel.com (mailing list archive)
State New
Headers show
Series Sub-device configuration models | expand

Commit Message

Sakari Ailus Nov. 29, 2024, 9:51 a.m. UTC 
Document scaling and post-scaler digital crop operations for the common
raw sensor model.
Signedg-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../media/v4l/subdev-config-model.rst         | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

Comments

Jacopo Mondi Dec. 4, 2024, 11:25 a.m. UTC | #1 
Hi Sakari
  thanks for the update

On Fri, Nov 29, 2024 at 11:51:38AM +0200, Sakari Ailus wrote:
> Document scaling and post-scaler digital crop operations for the common
> raw sensor model.
> Signedg-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../media/v4l/subdev-config-model.rst         | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/subdev-config-model.rst b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> index 4ddf98e3143c..1ae20800f34b 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> @@ -119,9 +119,13 @@ The digital crop operation takes place after binning and sub-sampling. It is
>  configured by setting the ``V4L2_SEL_TGT_CROP`` rectangle on (pad, stream) pair
>  0/0. The resulting image size is further output by the sensor.

I think this last line "The resulting image size is further output by
the sensor." doesn't apply anymore now that we have [digital crop +
digital scaling + post-scaling crop].

>
> +The digital scaling operation is performed after the digital crop. It is
> +configured by setting the ``V4L2_SEL_TGT_COMPOSE`` rectangle on (pad, stream) pair
> +0/0, relative to the digital crop.
> +

"to the digital crop rectangle." maybe ?


>  The sensor's output mbus code is configured by setting the format on the (pad,
> -stream) pair 0/0. When setting the format, always use the same width and height
> -as for the digital crop setting.
> +stream) pair 0/0. The width and height fields are used to configure post-scaler
> +digital crop, affecting the right side and the bottom of the frame.
>

I would introduce the (optional) presence of an additional post-scaler
digital crop step explicitly, also I get this is kind of rare feature,
isn't it ?

-------------------------------------------------------------------------------
The digital scaling operation is performed after the digital crop. It
is configured by setting the ``V4L2_SEL_TGT_COMPOSE`` rectangle on
(pad, stream) pair 0/0, relative to the digital crop rectangle.

The sensor's output mbus code is configured by setting the format on the (pad,
stream) pair 0/0. An optional post-scaler crop step might be performed by
specifying a width and height smaller than the digital scaling
rectangle. If post-scaler cropping is not supported the format's sizes
will always match the compose rectangle sizes applied on the same 0/0
(pad, stream) pair.
-------------------------------------------------------------------------------

I didn't get when we spoke about it where the "affecting the right
side and bottom of the frame" comes from when talking about
post-scaler cropping.


>  Drivers may only support some of even none of these configurations, in which
>  case they do not expose the corresponding selection rectangles. If any selection
> @@ -179,12 +183,19 @@ Also refer to :ref:`Selection targets <v4l2-selection-targets-table>`.
>        - X
>        - Digital crop. This rectangle is relative to the ``V4L2_SEL_TGT_COMPOSE``
>          rectangle on (pad, stream) pair 1/0.
> +    * - 0/0
> +      - ``V4L2_SEL_TGT_COMPOSE``
> +      - \-
> +      - X
> +      - Scaling. This rectangle is relative to the ``V4L2_SEL_TGT_CROP``

"Digital scaling" ? or is not necessary ?

> +        rectangle on (pad, stream) pair 0/0.
>      * - 0/0
>        - Format
>        - X
>        - X
> -      - Image data source format. Always assign the width and height fields of
> -        the format to the same values than for the ``V4L2_SEL_TGT_CROP``
> +      - Image data source format and post-scaler crop. The width and height
> +        fields of the format, used to configure post-scaler crop on the right
> +        and bottom edges of the image, are related to the ``V4L2_SEL_TGT_COMPOSE``
>          rectangle on (pad, stream) pair 0/0. The media bus code reflects the
>          pixel data output of the sensor.
>      * - 0/1
> --
> 2.39.5
>
>
Sakari Ailus Dec. 4, 2024, 2:15 p.m. UTC | #2 
Hi Jacopo,

Thank you for the review.

On Wed, Dec 04, 2024 at 12:25:11PM +0100, Jacopo Mondi wrote:
> Hi Sakari
>   thanks for the update
> 
> On Fri, Nov 29, 2024 at 11:51:38AM +0200, Sakari Ailus wrote:
> > Document scaling and post-scaler digital crop operations for the common
> > raw sensor model.
> > Signedg-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  .../media/v4l/subdev-config-model.rst         | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/subdev-config-model.rst b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> > index 4ddf98e3143c..1ae20800f34b 100644
> > --- a/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> > +++ b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> > @@ -119,9 +119,13 @@ The digital crop operation takes place after binning and sub-sampling. It is
> >  configured by setting the ``V4L2_SEL_TGT_CROP`` rectangle on (pad, stream) pair
> >  0/0. The resulting image size is further output by the sensor.
> 
> I think this last line "The resulting image size is further output by
> the sensor." doesn't apply anymore now that we have [digital crop +
> digital scaling + post-scaling crop].

Indeed. I'll address this in v4.

> 
> >
> > +The digital scaling operation is performed after the digital crop. It is
> > +configured by setting the ``V4L2_SEL_TGT_COMPOSE`` rectangle on (pad, stream) pair
> > +0/0, relative to the digital crop.
> > +
> 
> "to the digital crop rectangle." maybe ?

Yes.

> 
> 
> >  The sensor's output mbus code is configured by setting the format on the (pad,
> > -stream) pair 0/0. When setting the format, always use the same width and height
> > -as for the digital crop setting.
> > +stream) pair 0/0. The width and height fields are used to configure post-scaler
> > +digital crop, affecting the right side and the bottom of the frame.
> >
> 
> I would introduce the (optional) presence of an additional post-scaler
> digital crop step explicitly, also I get this is kind of rare feature,
> isn't it ?
> 
> -------------------------------------------------------------------------------
> The digital scaling operation is performed after the digital crop. It
> is configured by setting the ``V4L2_SEL_TGT_COMPOSE`` rectangle on
> (pad, stream) pair 0/0, relative to the digital crop rectangle.
> 
> The sensor's output mbus code is configured by setting the format on the (pad,
> stream) pair 0/0. An optional post-scaler crop step might be performed by
> specifying a width and height smaller than the digital scaling
> rectangle. If post-scaler cropping is not supported the format's sizes
> will always match the compose rectangle sizes applied on the same 0/0
> (pad, stream) pair.

Right, even if the post-scaler crop is optional, the format is always
there but unmodifiable (directly). I'd refer to driver support instead of
somewhat nebulous "might be performed".

How about:

The sensor's output mbus code is configured by setting the format on the
(pad, stream) pair 0/0. The width and height fields are used to configure
post-scaler digital crop if supported by the driver, affecting the right
and bottom edges of the frame. If post-scaler digital crop is not
supported, the width and height fields of the format will match the compose
rectangle sizes applied on the same 0/0 (pad, stream) pair.

> -------------------------------------------------------------------------------
> 
> I didn't get when we spoke about it where the "affecting the right
> side and bottom of the frame" comes from when talking about
> post-scaler cropping.
> 
> 
> >  Drivers may only support some of even none of these configurations, in which
> >  case they do not expose the corresponding selection rectangles. If any selection
> > @@ -179,12 +183,19 @@ Also refer to :ref:`Selection targets <v4l2-selection-targets-table>`.
> >        - X
> >        - Digital crop. This rectangle is relative to the ``V4L2_SEL_TGT_COMPOSE``
> >          rectangle on (pad, stream) pair 1/0.
> > +    * - 0/0
> > +      - ``V4L2_SEL_TGT_COMPOSE``
> > +      - \-
> > +      - X
> > +      - Scaling. This rectangle is relative to the ``V4L2_SEL_TGT_CROP``
> 
> "Digital scaling" ? or is not necessary ?

We don't mention it in the context of sub-sampling either. Digital and
analogue variants are separately documented where they exist, I think this
should be fine. What do you think?

> 
> > +        rectangle on (pad, stream) pair 0/0.
> >      * - 0/0
> >        - Format
> >        - X
> >        - X
> > -      - Image data source format. Always assign the width and height fields of
> > -        the format to the same values than for the ``V4L2_SEL_TGT_CROP``
> > +      - Image data source format and post-scaler crop. The width and height
> > +        fields of the format, used to configure post-scaler crop on the right
> > +        and bottom edges of the image, are related to the ``V4L2_SEL_TGT_COMPOSE``
> >          rectangle on (pad, stream) pair 0/0. The media bus code reflects the
> >          pixel data output of the sensor.
> >      * - 0/1
Jacopo Mondi Dec. 4, 2024, 3:33 p.m. UTC | #3 
Hi Sakari

On Wed, Dec 04, 2024 at 02:15:07PM +0000, Sakari Ailus wrote:
> Hi Jacopo,
>
> Thank you for the review.
>
> On Wed, Dec 04, 2024 at 12:25:11PM +0100, Jacopo Mondi wrote:
> > Hi Sakari
> >   thanks for the update
> >
> > On Fri, Nov 29, 2024 at 11:51:38AM +0200, Sakari Ailus wrote:
> > > Document scaling and post-scaler digital crop operations for the common
> > > raw sensor model.
> > > Signedg-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >  .../media/v4l/subdev-config-model.rst         | 19 +++++++++++++++----
> > >  1 file changed, 15 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/subdev-config-model.rst b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> > > index 4ddf98e3143c..1ae20800f34b 100644
> > > --- a/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> > > +++ b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
> > > @@ -119,9 +119,13 @@ The digital crop operation takes place after binning and sub-sampling. It is
> > >  configured by setting the ``V4L2_SEL_TGT_CROP`` rectangle on (pad, stream) pair
> > >  0/0. The resulting image size is further output by the sensor.
> >
> > I think this last line "The resulting image size is further output by
> > the sensor." doesn't apply anymore now that we have [digital crop +
> > digital scaling + post-scaling crop].
>
> Indeed. I'll address this in v4.
>
> >
> > >
> > > +The digital scaling operation is performed after the digital crop. It is
> > > +configured by setting the ``V4L2_SEL_TGT_COMPOSE`` rectangle on (pad, stream) pair
> > > +0/0, relative to the digital crop.
> > > +
> >
> > "to the digital crop rectangle." maybe ?
>
> Yes.
>
> >
> >
> > >  The sensor's output mbus code is configured by setting the format on the (pad,
> > > -stream) pair 0/0. When setting the format, always use the same width and height
> > > -as for the digital crop setting.
> > > +stream) pair 0/0. The width and height fields are used to configure post-scaler
> > > +digital crop, affecting the right side and the bottom of the frame.
> > >
> >
> > I would introduce the (optional) presence of an additional post-scaler
> > digital crop step explicitly, also I get this is kind of rare feature,
> > isn't it ?
> >
> > -------------------------------------------------------------------------------
> > The digital scaling operation is performed after the digital crop. It
> > is configured by setting the ``V4L2_SEL_TGT_COMPOSE`` rectangle on
> > (pad, stream) pair 0/0, relative to the digital crop rectangle.
> >
> > The sensor's output mbus code is configured by setting the format on the (pad,
> > stream) pair 0/0. An optional post-scaler crop step might be performed by
> > specifying a width and height smaller than the digital scaling
> > rectangle. If post-scaler cropping is not supported the format's sizes
> > will always match the compose rectangle sizes applied on the same 0/0
> > (pad, stream) pair.
>
> Right, even if the post-scaler crop is optional, the format is always
> there but unmodifiable (directly). I'd refer to driver support instead of
> somewhat nebulous "might be performed".
>
> How about:
>
> The sensor's output mbus code is configured by setting the format on the
> (pad, stream) pair 0/0. The width and height fields are used to configure
> post-scaler digital crop if supported by the driver, affecting the right
> and bottom edges of the frame. If post-scaler digital crop is not
> supported, the width and height fields of the format will match the compose
> rectangle sizes applied on the same 0/0 (pad, stream) pair.
>

Fine indeed!

> > -------------------------------------------------------------------------------
> >
> > I didn't get when we spoke about it where the "affecting the right
> > side and bottom of the frame" comes from when talking about
> > post-scaler cropping.
> >
> >
> > >  Drivers may only support some of even none of these configurations, in which
> > >  case they do not expose the corresponding selection rectangles. If any selection
> > > @@ -179,12 +183,19 @@ Also refer to :ref:`Selection targets <v4l2-selection-targets-table>`.
> > >        - X
> > >        - Digital crop. This rectangle is relative to the ``V4L2_SEL_TGT_COMPOSE``
> > >          rectangle on (pad, stream) pair 1/0.
> > > +    * - 0/0
> > > +      - ``V4L2_SEL_TGT_COMPOSE``
> > > +      - \-
> > > +      - X
> > > +      - Scaling. This rectangle is relative to the ``V4L2_SEL_TGT_CROP``
> >
> > "Digital scaling" ? or is not necessary ?
>
> We don't mention it in the context of sub-sampling either. Digital and
> analogue variants are separately documented where they exist, I think this
> should be fine. What do you think?
>

I'm fine, after all there's no "analogue scaling" this can be confused
with, right ?

> >
> > > +        rectangle on (pad, stream) pair 0/0.
> > >      * - 0/0
> > >        - Format
> > >        - X
> > >        - X
> > > -      - Image data source format. Always assign the width and height fields of
> > > -        the format to the same values than for the ``V4L2_SEL_TGT_CROP``
> > > +      - Image data source format and post-scaler crop. The width and height
> > > +        fields of the format, used to configure post-scaler crop on the right
> > > +        and bottom edges of the image, are related to the ``V4L2_SEL_TGT_COMPOSE``
> > >          rectangle on (pad, stream) pair 0/0. The media bus code reflects the
> > >          pixel data output of the sensor.
> > >      * - 0/1
>
> --
> Kind regards,
>
> Sakari Ailus
>
Sakari Ailus Dec. 5, 2024, 6:56 a.m. UTC | #4 
Hi Jacopo,

On Wed, Dec 04, 2024 at 04:33:22PM +0100, Jacopo Mondi wrote:
> > > > @@ -179,12 +183,19 @@ Also refer to :ref:`Selection targets <v4l2-selection-targets-table>`.
> > > >        - X
> > > >        - Digital crop. This rectangle is relative to the ``V4L2_SEL_TGT_COMPOSE``
> > > >          rectangle on (pad, stream) pair 1/0.
> > > > +    * - 0/0
> > > > +      - ``V4L2_SEL_TGT_COMPOSE``
> > > > +      - \-
> > > > +      - X
> > > > +      - Scaling. This rectangle is relative to the ``V4L2_SEL_TGT_CROP``
> > >
> > > "Digital scaling" ? or is not necessary ?
> >
> > We don't mention it in the context of sub-sampling either. Digital and
> > analogue variants are separately documented where they exist, I think this
> > should be fine. What do you think?
> >
> 
> I'm fine, after all there's no "analogue scaling" this can be confused
> with, right ?

Correct.
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/v4l/subdev-config-model.rst b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
index 4ddf98e3143c..1ae20800f34b 100644
--- a/Documentation/userspace-api/media/v4l/subdev-config-model.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-config-model.rst
@@ -119,9 +119,13 @@  The digital crop operation takes place after binning and sub-sampling. It is
 configured by setting the ``V4L2_SEL_TGT_CROP`` rectangle on (pad, stream) pair
 0/0. The resulting image size is further output by the sensor.
 
+The digital scaling operation is performed after the digital crop. It is
+configured by setting the ``V4L2_SEL_TGT_COMPOSE`` rectangle on (pad, stream) pair
+0/0, relative to the digital crop.
+
 The sensor's output mbus code is configured by setting the format on the (pad,
-stream) pair 0/0. When setting the format, always use the same width and height
-as for the digital crop setting.
+stream) pair 0/0. The width and height fields are used to configure post-scaler
+digital crop, affecting the right side and the bottom of the frame.
 
 Drivers may only support some of even none of these configurations, in which
 case they do not expose the corresponding selection rectangles. If any selection
@@ -179,12 +183,19 @@  Also refer to :ref:`Selection targets <v4l2-selection-targets-table>`.
       - X
       - Digital crop. This rectangle is relative to the ``V4L2_SEL_TGT_COMPOSE``
         rectangle on (pad, stream) pair 1/0.
+    * - 0/0
+      - ``V4L2_SEL_TGT_COMPOSE``
+      - \-
+      - X
+      - Scaling. This rectangle is relative to the ``V4L2_SEL_TGT_CROP``
+        rectangle on (pad, stream) pair 0/0.
     * - 0/0
       - Format
       - X
       - X
-      - Image data source format. Always assign the width and height fields of
-        the format to the same values than for the ``V4L2_SEL_TGT_CROP``
+      - Image data source format and post-scaler crop. The width and height
+        fields of the format, used to configure post-scaler crop on the right
+        and bottom edges of the image, are related to the ``V4L2_SEL_TGT_COMPOSE``
         rectangle on (pad, stream) pair 0/0. The media bus code reflects the
         pixel data output of the sensor.
     * - 0/1