From: Sergey Senozhatsky <[email protected]>
Document new v4l2-selection target which will be used for the
Region of Interest v4l2 control.
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
.../media/v4l/selection-api-configuration.rst | 23 +++++++++++++++++++
.../media/v4l/v4l2-selection-targets.rst | 21 +++++++++++++++++
include/uapi/linux/v4l2-common.h | 8 +++++++
3 files changed, 52 insertions(+)
diff --git a/Documentation/userspace-api/media/v4l/selection-api-configuration.rst b/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
index fee49bf1a1c0..9f69d71803f6 100644
--- a/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
+++ b/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
@@ -135,3 +135,26 @@ and the height of rectangles obtained using ``V4L2_SEL_TGT_CROP`` and
``V4L2_SEL_TGT_COMPOSE`` targets. If these are not equal then the
scaling is applied. The application can compute the scaling ratios using
these values.
+
+Configuration of Region of Interest (ROI)
+=========================================
+
+The range of coordinates of the top left corner, width and height of
+areas that can be ROI is given by the ``V4L2_SEL_TGT_ROI_BOUNDS`` target.
+It is recommended for the driver developers to put the top/left corner
+at position ``(0,0)``. The rectangle's coordinates are in global sensor
+coordinates. The units are in pixels and independent of the field of view.
+They are not impacted by any cropping or scaling that is currently being
+used.
+
+The top left corner, width and height of the Region of Interest area
+currently being employed by the device is given by the
+``V4L2_SEL_TGT_ROI_CURRENT`` target. It uses the same coordinate system
+as ``V4L2_SEL_TGT_ROI_BOUNDS``.
+
+In order to change active ROI top left, width and height coordinates
+use ``V4L2_SEL_TGT_ROI`` target.
+
+Each capture device has a default ROI rectangle, given by the
+``V4L2_SEL_TGT_ROI_DEFAULT`` target. Drivers shall set the ROI rectangle
+to the default when the driver is first loaded, but not later.
diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
index e877ebbdb32e..cb3809418fa6 100644
--- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
+++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
@@ -69,3 +69,24 @@ of the two interfaces they are used.
modified by hardware.
- Yes
- No
+ * - ``V4L2_SEL_TGT_ROI_CURRENT``
+ - 0x0200
+ - Current Region of Interest rectangle.
+ - Yes
+ - No
+ * - ``V4L2_SEL_TGT_ROI_DEFAULT``
+ - 0x0201
+ - Suggested Region of Interest rectangle.
+ - Yes
+ - No
+ * - ``V4L2_SEL_TGT_ROI_BOUNDS``
+ - 0x0202
+ - Bounds of the Region of Interest rectangle. All valid ROI rectangles fit
+ inside the ROI bounds rectangle.
+ - Yes
+ - No
+ * - ``V4L2_SEL_TGT_ROI``
+ - 0x0203
+ - Sets the new Region of Interest rectangle.
+ - Yes
+ - No
diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
index 7d21c1634b4d..d0c108fba638 100644
--- a/include/uapi/linux/v4l2-common.h
+++ b/include/uapi/linux/v4l2-common.h
@@ -78,6 +78,14 @@
#define V4L2_SEL_TGT_COMPOSE_BOUNDS 0x0102
/* Current composing area plus all padding pixels */
#define V4L2_SEL_TGT_COMPOSE_PADDED 0x0103
+/* Current Region of Interest area */
+#define V4L2_SEL_TGT_ROI_CURRENT 0x0200
+/* Default Region of Interest area */
+#define V4L2_SEL_TGT_ROI_DEFAULT 0x0201
+/* Region of Interest bounds */
+#define V4L2_SEL_TGT_ROI_BOUNDS 0x0202
+/* Set Region of Interest area */
+#define V4L2_SEL_TGT_ROI 0x0203
/* Selection flags */
#define V4L2_SEL_FLAG_GE (1 << 0)
--
2.30.0
Hi Sergey
Thanks for the patch!
On Mon, Feb 8, 2021 at 6:21 AM Sergey Senozhatsky
<[email protected]> wrote:
>
> From: Sergey Senozhatsky <[email protected]>
>
> Document new v4l2-selection target which will be used for the
> Region of Interest v4l2 control.
>
> Signed-off-by: Sergey Senozhatsky <[email protected]>
> ---
> .../media/v4l/selection-api-configuration.rst | 23 +++++++++++++++++++
> .../media/v4l/v4l2-selection-targets.rst | 21 +++++++++++++++++
> include/uapi/linux/v4l2-common.h | 8 +++++++
> 3 files changed, 52 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/selection-api-configuration.rst b/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
> index fee49bf1a1c0..9f69d71803f6 100644
> --- a/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
> +++ b/Documentation/userspace-api/media/v4l/selection-api-configuration.rst
> @@ -135,3 +135,26 @@ and the height of rectangles obtained using ``V4L2_SEL_TGT_CROP`` and
> ``V4L2_SEL_TGT_COMPOSE`` targets. If these are not equal then the
> scaling is applied. The application can compute the scaling ratios using
> these values.
> +
> +Configuration of Region of Interest (ROI)
> +=========================================
> +
> +The range of coordinates of the top left corner, width and height of
> +areas that can be ROI is given by the ``V4L2_SEL_TGT_ROI_BOUNDS`` target.
> +It is recommended for the driver developers to put the top/left corner
> +at position ``(0,0)``. The rectangle's coordinates are in global sensor
> +coordinates. The units are in pixels and independent of the field of view.
> +They are not impacted by any cropping or scaling that is currently being
> +used.
Can we also mention binning here?
> +
> +The top left corner, width and height of the Region of Interest area
> +currently being employed by the device is given by the
> +``V4L2_SEL_TGT_ROI_CURRENT`` target. It uses the same coordinate system
> +as ``V4L2_SEL_TGT_ROI_BOUNDS``.
Why do we need current? Cant we just read back V4L2_SEL_TGT_ROI ?
> +
> +In order to change active ROI top left, width and height coordinates
> +use ``V4L2_SEL_TGT_ROI`` target.
> +
> +Each capture device has a default ROI rectangle, given by the
> +``V4L2_SEL_TGT_ROI_DEFAULT`` target. Drivers shall set the ROI rectangle
> +to the default when the driver is first loaded, but not later.
> diff --git a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
> index e877ebbdb32e..cb3809418fa6 100644
> --- a/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
> +++ b/Documentation/userspace-api/media/v4l/v4l2-selection-targets.rst
> @@ -69,3 +69,24 @@ of the two interfaces they are used.
> modified by hardware.
> - Yes
> - No
> + * - ``V4L2_SEL_TGT_ROI_CURRENT``
> + - 0x0200
> + - Current Region of Interest rectangle.
> + - Yes
> + - No
> + * - ``V4L2_SEL_TGT_ROI_DEFAULT``
> + - 0x0201
> + - Suggested Region of Interest rectangle.
> + - Yes
> + - No
> + * - ``V4L2_SEL_TGT_ROI_BOUNDS``
> + - 0x0202
> + - Bounds of the Region of Interest rectangle. All valid ROI rectangles fit
> + inside the ROI bounds rectangle.
> + - Yes
> + - No
> + * - ``V4L2_SEL_TGT_ROI``
> + - 0x0203
> + - Sets the new Region of Interest rectangle.
> + - Yes
> + - No
As mentioned before I think we should not have TGT_ROI_CURRENT and TGT_ROI
> diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
> index 7d21c1634b4d..d0c108fba638 100644
> --- a/include/uapi/linux/v4l2-common.h
> +++ b/include/uapi/linux/v4l2-common.h
> @@ -78,6 +78,14 @@
> #define V4L2_SEL_TGT_COMPOSE_BOUNDS 0x0102
> /* Current composing area plus all padding pixels */
> #define V4L2_SEL_TGT_COMPOSE_PADDED 0x0103
> +/* Current Region of Interest area */
> +#define V4L2_SEL_TGT_ROI_CURRENT 0x0200
> +/* Default Region of Interest area */
> +#define V4L2_SEL_TGT_ROI_DEFAULT 0x0201
> +/* Region of Interest bounds */
> +#define V4L2_SEL_TGT_ROI_BOUNDS 0x0202
> +/* Set Region of Interest area */
> +#define V4L2_SEL_TGT_ROI 0x0203
Nit: Maybe it could be a good idea to split doc and code. This way the
backports/fixes are easier.
>
> /* Selection flags */
> #define V4L2_SEL_FLAG_GE (1 << 0)
> --
> 2.30.0
>
--
Ricardo Ribalda
On (21/03/16 19:19), Ricardo Ribalda Delgado wrote:
> > +Configuration of Region of Interest (ROI)
> > +=========================================
> > +
> > +The range of coordinates of the top left corner, width and height of
> > +areas that can be ROI is given by the ``V4L2_SEL_TGT_ROI_BOUNDS`` target.
> > +It is recommended for the driver developers to put the top/left corner
> > +at position ``(0,0)``. The rectangle's coordinates are in global sensor
> > +coordinates. The units are in pixels and independent of the field of view.
> > +They are not impacted by any cropping or scaling that is currently being
> > +used.
>
> Can we also mention binning here?
What's binning? Is it in the UVC spec?
> > +The top left corner, width and height of the Region of Interest area
> > +currently being employed by the device is given by the
> > +``V4L2_SEL_TGT_ROI_CURRENT`` target. It uses the same coordinate system
> > +as ``V4L2_SEL_TGT_ROI_BOUNDS``.
>
> Why do we need current? Cant we just read back V4L2_SEL_TGT_ROI ?
We don't. Will remove it.
> > + * - ``V4L2_SEL_TGT_ROI_CURRENT``
> > + - 0x0200
> > + - Current Region of Interest rectangle.
> > + - Yes
> > + - No
> > + * - ``V4L2_SEL_TGT_ROI_DEFAULT``
> > + - 0x0201
> > + - Suggested Region of Interest rectangle.
> > + - Yes
> > + - No
> > + * - ``V4L2_SEL_TGT_ROI_BOUNDS``
> > + - 0x0202
> > + - Bounds of the Region of Interest rectangle. All valid ROI rectangles fit
> > + inside the ROI bounds rectangle.
> > + - Yes
> > + - No
> > + * - ``V4L2_SEL_TGT_ROI``
> > + - 0x0203
> > + - Sets the new Region of Interest rectangle.
> > + - Yes
> > + - No
> As mentioned before I think we should not have TGT_ROI_CURRENT and TGT_ROI
Agreed.
> > diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
> > index 7d21c1634b4d..d0c108fba638 100644
> > --- a/include/uapi/linux/v4l2-common.h
> > +++ b/include/uapi/linux/v4l2-common.h
> > @@ -78,6 +78,14 @@
> > #define V4L2_SEL_TGT_COMPOSE_BOUNDS 0x0102
> > /* Current composing area plus all padding pixels */
> > #define V4L2_SEL_TGT_COMPOSE_PADDED 0x0103
> > +/* Current Region of Interest area */
> > +#define V4L2_SEL_TGT_ROI_CURRENT 0x0200
> > +/* Default Region of Interest area */
> > +#define V4L2_SEL_TGT_ROI_DEFAULT 0x0201
> > +/* Region of Interest bounds */
> > +#define V4L2_SEL_TGT_ROI_BOUNDS 0x0202
> > +/* Set Region of Interest area */
> > +#define V4L2_SEL_TGT_ROI 0x0203
>
> Nit: Maybe it could be a good idea to split doc and code. This way the
> backports/fixes are easier.
I'm quite sure this is the first time I'm being asked to split code
and documentation :) I'm usually asked to do the opposite - merge code
and documentation.
Hi
On Wed, Mar 17, 2021 at 2:31 AM Sergey Senozhatsky
<[email protected]> wrote:
>
> On (21/03/16 19:19), Ricardo Ribalda Delgado wrote:
> > > +Configuration of Region of Interest (ROI)
> > > +=========================================
> > > +
> > > +The range of coordinates of the top left corner, width and height of
> > > +areas that can be ROI is given by the ``V4L2_SEL_TGT_ROI_BOUNDS`` target.
> > > +It is recommended for the driver developers to put the top/left corner
> > > +at position ``(0,0)``. The rectangle's coordinates are in global sensor
> > > +coordinates. The units are in pixels and independent of the field of view.
> > > +They are not impacted by any cropping or scaling that is currently being
> > > +used.
> >
> > Can we also mention binning here?
>
> What's binning? Is it in the UVC spec?
Binning is when you reduce an image by adding up surrounding pixels.
So you have a 100x100 image that you convert to a 50x50 but showing
the same area of interest.
>
> > > +The top left corner, width and height of the Region of Interest area
> > > +currently being employed by the device is given by the
> > > +``V4L2_SEL_TGT_ROI_CURRENT`` target. It uses the same coordinate system
> > > +as ``V4L2_SEL_TGT_ROI_BOUNDS``.
> >
> > Why do we need current? Cant we just read back V4L2_SEL_TGT_ROI ?
>
> We don't. Will remove it.
>
> > > + * - ``V4L2_SEL_TGT_ROI_CURRENT``
> > > + - 0x0200
> > > + - Current Region of Interest rectangle.
> > > + - Yes
> > > + - No
> > > + * - ``V4L2_SEL_TGT_ROI_DEFAULT``
> > > + - 0x0201
> > > + - Suggested Region of Interest rectangle.
> > > + - Yes
> > > + - No
> > > + * - ``V4L2_SEL_TGT_ROI_BOUNDS``
> > > + - 0x0202
> > > + - Bounds of the Region of Interest rectangle. All valid ROI rectangles fit
> > > + inside the ROI bounds rectangle.
> > > + - Yes
> > > + - No
> > > + * - ``V4L2_SEL_TGT_ROI``
> > > + - 0x0203
> > > + - Sets the new Region of Interest rectangle.
> > > + - Yes
> > > + - No
> > As mentioned before I think we should not have TGT_ROI_CURRENT and TGT_ROI
>
> Agreed.
>
> > > diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
> > > index 7d21c1634b4d..d0c108fba638 100644
> > > --- a/include/uapi/linux/v4l2-common.h
> > > +++ b/include/uapi/linux/v4l2-common.h
> > > @@ -78,6 +78,14 @@
> > > #define V4L2_SEL_TGT_COMPOSE_BOUNDS 0x0102
> > > /* Current composing area plus all padding pixels */
> > > #define V4L2_SEL_TGT_COMPOSE_PADDED 0x0103
> > > +/* Current Region of Interest area */
> > > +#define V4L2_SEL_TGT_ROI_CURRENT 0x0200
> > > +/* Default Region of Interest area */
> > > +#define V4L2_SEL_TGT_ROI_DEFAULT 0x0201
> > > +/* Region of Interest bounds */
> > > +#define V4L2_SEL_TGT_ROI_BOUNDS 0x0202
> > > +/* Set Region of Interest area */
> > > +#define V4L2_SEL_TGT_ROI 0x0203
> >
> > Nit: Maybe it could be a good idea to split doc and code. This way the
> > backports/fixes are easier.
>
> I'm quite sure this is the first time I'm being asked to split code
> and documentation :) I'm usually asked to do the opposite - merge code
> and documentation.
I got answered in both directions. I prefer to split it because the
doc can go to different audience than the code, and then it makes my
life easier when backporting.
But if you or Laurent prefer otherwise I am of course happy with any option ;)
--
Ricardo Ribalda
On (21/03/17 09:04), Ricardo Ribalda Delgado wrote:
> On Wed, Mar 17, 2021 at 2:31 AM Sergey Senozhatsky
> <[email protected]> wrote:
> >
> > On (21/03/16 19:19), Ricardo Ribalda Delgado wrote:
> > > > +Configuration of Region of Interest (ROI)
> > > > +=========================================
> > > > +
> > > > +The range of coordinates of the top left corner, width and height of
> > > > +areas that can be ROI is given by the ``V4L2_SEL_TGT_ROI_BOUNDS`` target.
> > > > +It is recommended for the driver developers to put the top/left corner
> > > > +at position ``(0,0)``. The rectangle's coordinates are in global sensor
> > > > +coordinates. The units are in pixels and independent of the field of view.
> > > > +They are not impacted by any cropping or scaling that is currently being
> > > > +used.
> > >
> > > Can we also mention binning here?
> >
> > What's binning? Is it in the UVC spec?
>
> Binning is when you reduce an image by adding up surrounding pixels.
>
> So you have a 100x100 image that you convert to a 50x50 but showing
> the same area of interest.
I see. Hmm, not sure if I can comment on this. It's not in the spec,
so it might be up to the firmware, maybe. What do you think?
> > > > diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
> > > > index 7d21c1634b4d..d0c108fba638 100644
> > > > --- a/include/uapi/linux/v4l2-common.h
> > > > +++ b/include/uapi/linux/v4l2-common.h
> > > > @@ -78,6 +78,14 @@
> > > > #define V4L2_SEL_TGT_COMPOSE_BOUNDS 0x0102
> > > > /* Current composing area plus all padding pixels */
> > > > #define V4L2_SEL_TGT_COMPOSE_PADDED 0x0103
> > > > +/* Current Region of Interest area */
> > > > +#define V4L2_SEL_TGT_ROI_CURRENT 0x0200
> > > > +/* Default Region of Interest area */
> > > > +#define V4L2_SEL_TGT_ROI_DEFAULT 0x0201
> > > > +/* Region of Interest bounds */
> > > > +#define V4L2_SEL_TGT_ROI_BOUNDS 0x0202
> > > > +/* Set Region of Interest area */
> > > > +#define V4L2_SEL_TGT_ROI 0x0203
> > >
> > > Nit: Maybe it could be a good idea to split doc and code. This way the
> > > backports/fixes are easier.
> >
> > I'm quite sure this is the first time I'm being asked to split code
> > and documentation :) I'm usually asked to do the opposite - merge code
> > and documentation.
>
> I got answered in both directions. I prefer to split it because the
> doc can go to different audience than the code, and then it makes my
> life easier when backporting.
>
> But if you or Laurent prefer otherwise I am of course happy with any option ;)
Either way works for me. Laurent, any preferences?
-ss