Re: [PATCH v14 11/11] media: uvcvideo: document UVC v1.5 ROI

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Yunke,

Thank you for the patch.

On Fri, Dec 01, 2023 at 04:19:02PM +0900, Yunke Cao wrote:
> Added documentation of V4L2_CID_UVC_REGION_OF_INTEREST_RECT and
> V4L2_CID_UVC_REGION_OF_INTEREST_AUTO.

It would be useful to point here to the userspace code that uses those
controls. A link to the appropriate Chrome OS git tree would be useful,
to showcase a real user.

> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Signed-off-by: Yunke Cao <yunkec@google.com>
> ---
> Changelog since v11:
> - No change.
> Changelog since v10:
> - Added Reviewed-by from Sergey.
> Changelog since v9:
> - No change.
> Changelog since v8:
> - No change.
> Changelog since v7:
> - Fix documentation for automatic exposure based on comment in v7.
> 
>  .../userspace-api/media/drivers/uvcvideo.rst  | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst
> index aab4304e6bb5..3dc062221f8b 100644
> --- a/Documentation/userspace-api/media/drivers/uvcvideo.rst
> +++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst
> @@ -181,6 +181,7 @@ Argument: struct uvc_xu_control_mapping
>  	UVC_CTRL_DATA_TYPE_BOOLEAN	Boolean
>  	UVC_CTRL_DATA_TYPE_ENUM		Enumeration
>  	UVC_CTRL_DATA_TYPE_BITMASK	Bitmask
> +	UVC_CTRL_DATA_TYPE_RECT		Rectangular area
>  
>  
>  UVCIOC_CTRL_QUERY - Query a UVC XU control
> @@ -255,3 +256,64 @@ Argument: struct uvc_xu_control_query
>  	__u8	query		Request code to send to the device
>  	__u16	size		Control data size (in bytes)
>  	__u8	*data		Control value
> +
> +
> +Driver-specific V4L2 controls
> +-----------------------------
> +
> +The uvcvideo driver implements the following UVC-specific controls:
> +
> +``V4L2_CID_UVC_REGION_OF_INTEREST_RECT (struct)``
> +	This control determines the region of interest (ROI). ROI is a
> +	rectangular area represented by a struct :c:type:`v4l2_rect`. The
> +	rectangle is in global sensor coordinates and pixel units. It is
> +	independent of the field of view, not impacted by any cropping or
> +	scaling.
> +
> +	Use ``V4L2_CTRL_WHICH_MIN_VAL`` and ``V4L2_CTRL_WHICH_MAX_VAL`` to query
> +	the range of rectangle sizes. The left/top coordinates of a minimum or
> +	maximum rectangle are always 0. For example, a device can have a minimum
> +	ROI rectangle of 1x1@0x0 and a maximum of 640x480@0x0.

Is that actually true ? The UVC 1.5 specification states

    GET_MAX shall return the current Window as specified by
    CT_DIGITAL_WINDOW_CONTROL.

And the window can have non-zero left and right coordinates.

> +
> +	Setting a ROI allows the camera to optimize the capture for the region.
> +	The value of ``V4L2_CID_REGION_OF_INTEREST_AUTO`` control determines
> +	the detailed behavior.
> +
> +
> +``V4L2_CID_UVC_REGION_OF_INTEREST_AUTO (bitmask)``
> +	This determines which, if any, on board features should track to the

s/on board/on-board/

> +	Region of Interest specified by the current value of
> +	``V4L2_CID_UVD__REGION_OF_INTEREST_RECT``.
> +
> +	Max value is a mask indicating all supported Auto Controls.
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_EXPOSURE``
> +      - Setting this to true causes automatic exposure to track the region of

Maybe "Setting this bit causes..." as those values are bit flags, not
booleans. Same below.

> +	interest instead of the whole image.
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_IRIS``
> +      - Setting this to true causes automatic iris to track the region of
> +	interest instead of the whole image.
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_WHITE_BALANCE``
> +      - Setting this to true causes automatic white balance to track the region
> +	of interest instead of the whole image.
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_FOCUS``
> +      - Setting this to true causes automatic focus adjustment to track the
> +	region of interest instead of the whole image.
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_FACE_DETECT``
> +      - Setting this to true causes automatic face detection to track the
> +	region of interest instead of the whole image.
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK``
> +      - Setting this to true enables automatic face detection and tracking. The
> +	current value of ``V4L2_CID_REGION_OF_INTEREST_RECT`` may be updated by
> +	the driver.
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION``
> +      - Setting this to true enables automatic image stabilization. The
> +	current value of ``V4L2_CID_REGION_OF_INTEREST_RECT`` may be updated by
> +	the driver.
> +    * - ``V4L2_UVC_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
> +      - Setting this to true enables automatically capture the specified region

I wonder what this means, the UVC 1.5 specification doesn't document
this bit clearly :-(

> +	with higher quality if possible.

-- 
Regards,

Laurent Pinchart