| Message ID | 20241129095142.87196-6-sakari.ailus@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Sub-device configuration models | expand |
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 > >
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
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 >
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 --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