* [PATCH v2 0/6] media: Implement UVC v1.5 ROI
@ 2022-05-16 14:04 Yunke Cao
2022-05-16 14:04 ` [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control Yunke Cao
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Yunke Cao @ 2022-05-16 14:04 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao
This patch set implements UVC v1.5 regioin of interest using V4L2
control API.
ROI control is consisted an auto control with type bitmask and a
rectangle control with a newly added type V4L2_CTRL_TYPE_RECT.
V4L2_CTRL_WHICH_MIN/MAX_VAL is added to support the rectangle control.
A rectangle control is also added to the vivid test driver.
Tested on two different usb cameras using v4l2-ctl and calling ioctls.
Yunke Cao (6):
media: v4l2_ctrl: Add region of interest rectangle control
media: v4l2_ctrl: Add region of interest auto control
media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL
media: uvcvideo: implement UVC v1.5 ROI
media: uvcvideo: Initialize roi to default value
media: vivid: Add a roi rectangle control
.../userspace-api/media/drivers/uvcvideo.rst | 1 +
.../media/v4l/ext-ctrls-camera.rst | 29 +++
.../media/v4l/vidioc-g-ext-ctrls.rst | 14 +-
.../media/videodev2.h.rst.exceptions | 3 +
drivers/media/i2c/imx214.c | 4 +-
.../media/test-drivers/vivid/vivid-ctrls.c | 34 +++
drivers/media/usb/uvc/uvc_ctrl.c | 213 ++++++++++++++++--
drivers/media/usb/uvc/uvc_v4l2.c | 12 +-
drivers/media/usb/uvc/uvcvideo.h | 10 +-
drivers/media/v4l2-core/v4l2-ctrls-api.c | 51 ++++-
drivers/media/v4l2-core/v4l2-ctrls-core.c | 155 ++++++++++++-
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 6 +
drivers/media/v4l2-core/v4l2-ioctl.c | 4 +-
include/media/v4l2-ctrls.h | 32 ++-
include/uapi/linux/usb/video.h | 1 +
include/uapi/linux/uvcvideo.h | 1 +
include/uapi/linux/v4l2-controls.h | 11 +
include/uapi/linux/videodev2.h | 4 +
18 files changed, 532 insertions(+), 53 deletions(-)
--
2.36.0.550.gb090851708-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control
2022-05-16 14:04 [PATCH v2 0/6] media: Implement UVC v1.5 ROI Yunke Cao
@ 2022-05-16 14:04 ` Yunke Cao
2022-05-16 19:02 ` Nicolas Dufresne
2022-05-16 14:04 ` [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control Yunke Cao
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Yunke Cao @ 2022-05-16 14:04 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao
Including:
1. Add a control ID.
2. Add p_rect to struct v4l2_ext_control with basic support in
v4l2-ctrls.
Signed-off-by: Yunke Cao <yunkec@google.com>
---
.../media/v4l/ext-ctrls-camera.rst | 4 ++++
.../media/v4l/vidioc-g-ext-ctrls.rst | 4 ++++
.../media/videodev2.h.rst.exceptions | 1 +
drivers/media/v4l2-core/v4l2-ctrls-core.c | 20 +++++++++++++++++++
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 ++++
include/media/v4l2-ctrls.h | 2 ++
include/uapi/linux/v4l2-controls.h | 2 ++
include/uapi/linux/videodev2.h | 2 ++
8 files changed, 39 insertions(+)
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
index 4c5061aa9cd4..86a1f09a8a1c 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
@@ -661,3 +661,7 @@ enum v4l2_scene_mode -
.. [#f1]
This control may be changed to a menu control in the future, if more
options are required.
+
+``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
+ This control determines the region of interest. Region of interest is an
+ rectangular area represented by a struct v4l2_rect.
diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
index 29971a45a2d4..f4e205ead0a2 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
@@ -189,6 +189,10 @@ still cause this situation.
- ``p_area``
- A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
of type ``V4L2_CTRL_TYPE_AREA``.
+ * - struct :c:type:`v4l2_rect` *
+ - ``p_area``
+ - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
+ of type ``V4L2_CTRL_TYPE_RECT``.
* - struct :c:type:`v4l2_ctrl_h264_sps` *
- ``p_h264_sps``
- A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 9cbb7a0c354a..7b423475281d 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
+replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index 8968cec8454e..dcde405c2713 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
return ptr1.p_u16[idx] == ptr2.p_u16[idx];
case V4L2_CTRL_TYPE_U32:
return ptr1.p_u32[idx] == ptr2.p_u32[idx];
+ case V4L2_CTRL_TYPE_RECT:
+ return ptr1.p_rect->top == ptr2.p_rect->top &&
+ ptr1.p_rect->left == ptr2.p_rect->left &&
+ ptr1.p_rect->height == ptr2.p_rect->height &&
+ ptr1.p_rect->width == ptr2.p_rect->width;
default:
if (ctrl->is_int)
return ptr1.p_s32[idx] == ptr2.p_s32[idx];
@@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
case V4L2_CTRL_TYPE_VP9_FRAME:
pr_cont("VP9_FRAME");
break;
+ case V4L2_CTRL_TYPE_RECT:
+ pr_cont("l: %d, t: %d, w: %u, h: %u",
+ ptr.p_rect->left, ptr.p_rect->top,
+ ptr.p_rect->width, ptr.p_rect->height);
+ break;
default:
pr_cont("unknown type %d", ctrl->type);
break;
@@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
struct v4l2_area *area;
+ struct v4l2_rect *rect;
void *p = ptr.p + idx * ctrl->elem_size;
unsigned int i;
@@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
return -EINVAL;
break;
+ case V4L2_CTRL_TYPE_RECT:
+ rect = p;
+ if (!rect->width || !rect->height)
+ return -EINVAL;
+ break;
+
default:
return -EINVAL;
}
@@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
case V4L2_CTRL_TYPE_AREA:
elem_size = sizeof(struct v4l2_area);
break;
+ case V4L2_CTRL_TYPE_RECT:
+ elem_size = sizeof(struct v4l2_rect);
+ break;
default:
if (type < V4L2_CTRL_COMPOUND_TYPES)
elem_size = sizeof(s32);
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 54ca4e6b820b..95f39a2d2ad2 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -1042,6 +1042,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_UNIT_CELL_SIZE: return "Unit Cell Size";
case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
+ case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
/* FM Radio Modulator controls */
/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1524,6 +1525,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
*type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
break;
+ case V4L2_CID_REGION_OF_INTEREST_RECT:
+ *type = V4L2_CTRL_TYPE_RECT;
+ break;
default:
*type = V4L2_CTRL_TYPE_INTEGER;
break;
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index b3ce438f1329..919e104de50b 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -58,6 +58,7 @@ struct video_device;
* @p_hdr10_cll: Pointer to an HDR10 Content Light Level structure.
* @p_hdr10_mastering: Pointer to an HDR10 Mastering Display structure.
* @p_area: Pointer to an area.
+ * @p_rect: Pointer to a rectangle.
* @p: Pointer to a compound value.
* @p_const: Pointer to a constant compound value.
*/
@@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
struct v4l2_area *p_area;
+ struct v4l2_rect *p_rect;
void *p;
const void *p_const;
};
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index bb40129446d4..499fcddb6254 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1008,6 +1008,8 @@ enum v4l2_auto_focus_range {
#define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
+#define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
+
/* FM Modulator class control IDs */
#define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3768a0a80830..b712412cf763 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
__u16 __user *p_u16;
__u32 __user *p_u32;
struct v4l2_area __user *p_area;
+ struct v4l2_rect __user *p_rect;
struct v4l2_ctrl_h264_sps __user *p_h264_sps;
struct v4l2_ctrl_h264_pps *p_h264_pps;
struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
@@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
V4L2_CTRL_TYPE_U16 = 0x0101,
V4L2_CTRL_TYPE_U32 = 0x0102,
V4L2_CTRL_TYPE_AREA = 0x0106,
+ V4L2_CTRL_TYPE_RECT = 0x0107,
V4L2_CTRL_TYPE_HDR10_CLL_INFO = 0x0110,
V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY = 0x0111,
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control
2022-05-16 14:04 [PATCH v2 0/6] media: Implement UVC v1.5 ROI Yunke Cao
2022-05-16 14:04 ` [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control Yunke Cao
@ 2022-05-16 14:04 ` Yunke Cao
2022-05-17 13:23 ` Nicolas Dufresne
2022-05-16 14:04 ` [PATCH v2 3/6] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Yunke Cao @ 2022-05-16 14:04 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao
Signed-off-by: Yunke Cao <yunkec@google.com>
---
.../media/v4l/ext-ctrls-camera.rst | 25 +++++++++++++++++++
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
include/uapi/linux/v4l2-controls.h | 9 +++++++
3 files changed, 36 insertions(+)
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
index 86a1f09a8a1c..3da66e1e1fc7 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
@@ -665,3 +665,28 @@ enum v4l2_scene_mode -
``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
This control determines the region of interest. Region of interest is an
rectangular area represented by a struct v4l2_rect.
+
+``V4L2_CID_REGION_OF_INTEREST_AUTO (bitmask)``
+ This determines which, if any, on board features should track to the
+ Region of Interest.
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE``
+ - Auto Exposure.
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS``
+ - Auto Iris.
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE``
+ - Auto White Balance.
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS``
+ - Auto Focus.
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT``
+ - Auto Face Detect.
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK``
+ - Auto Detect and Track.
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION``
+ - Image Stabilization.
+ * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
+ - Higher Quality.
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 95f39a2d2ad2..f28763bf95e9 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -1043,6 +1043,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
+ case V4L2_CID_REGION_OF_INTEREST_AUTO: return "Region Of Interest Auto Controls";
/* FM Radio Modulator controls */
/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1415,6 +1416,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_JPEG_ACTIVE_MARKER:
case V4L2_CID_3A_LOCK:
case V4L2_CID_AUTO_FOCUS_STATUS:
+ case V4L2_CID_REGION_OF_INTEREST_AUTO:
case V4L2_CID_DV_TX_HOTPLUG:
case V4L2_CID_DV_TX_RXSENSE:
case V4L2_CID_DV_TX_EDID_PRESENT:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 499fcddb6254..f6938e4de129 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1009,6 +1009,15 @@ enum v4l2_auto_focus_range {
#define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
#define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO (V4L2_CID_CAMERA_CLASS_BASE+37)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE (1 << 0)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS (1 << 1)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE (1 << 2)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS (1 << 3)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT (1 << 4)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK (1 << 5)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION (1 << 6)
+#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY (1 << 7)
/* FM Modulator class control IDs */
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/6] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL
2022-05-16 14:04 [PATCH v2 0/6] media: Implement UVC v1.5 ROI Yunke Cao
2022-05-16 14:04 ` [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control Yunke Cao
2022-05-16 14:04 ` [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control Yunke Cao
@ 2022-05-16 14:04 ` Yunke Cao
2022-05-16 14:04 ` [PATCH v2 4/6] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Yunke Cao @ 2022-05-16 14:04 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao
Added V4L2_CTRL_WHICH_MIN/MAX_VAL and basic support in v4l2-core.
Mostly reusing a previous attempt:
https://lore.kernel.org/all/20191220134843.25977-1-m.tretter@pengutronix.de/T/#m5c83970af8b774a4b1ea5f2dca4c0a534da4ccbe
Signed-off-by: Yunke Cao <yunkec@google.com>
---
.../media/v4l/vidioc-g-ext-ctrls.rst | 10 +-
.../media/videodev2.h.rst.exceptions | 2 +
drivers/media/i2c/imx214.c | 4 +-
drivers/media/v4l2-core/v4l2-ctrls-api.c | 51 +++++--
drivers/media/v4l2-core/v4l2-ctrls-core.c | 135 ++++++++++++++++--
drivers/media/v4l2-core/v4l2-ioctl.c | 4 +-
include/media/v4l2-ctrls.h | 30 +++-
include/uapi/linux/videodev2.h | 2 +
8 files changed, 210 insertions(+), 28 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
index f4e205ead0a2..11336f5d69ed 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
@@ -284,14 +284,18 @@ still cause this situation.
- Which value of the control to get/set/try.
* - :cspan:`2` ``V4L2_CTRL_WHICH_CUR_VAL`` will return the current value of
the control, ``V4L2_CTRL_WHICH_DEF_VAL`` will return the default
- value of the control and ``V4L2_CTRL_WHICH_REQUEST_VAL`` indicates that
+ value of the control, ``V4L2_CTRL_WHICH_MIN_VAL`` will return the minimum
+ value of the control, ``V4L2_CTRL_WHICH_MAX_VAL`` will return the maximum
+ value of the control and ``V4L2_CTRL_WHICH_REQUEST_VAL`` indicates that
these controls have to be retrieved from a request or tried/set for
a request. In the latter case the ``request_fd`` field contains the
file descriptor of the request that should be used. If the device
does not support requests, then ``EACCES`` will be returned.
- When using ``V4L2_CTRL_WHICH_DEF_VAL`` be aware that you can only
- get the default value of the control, you cannot set or try it.
+ When using ``V4L2_CTRL_WHICH_DEF_VAL``, ``V4L2_CTRL_WHICH_MIN_VAL``
+ or ``V4L2_CTRL_WHICH_MAX_VAL`` be aware that you can only
+ get the default/minimum/maximum value of the control, you cannot set
+ or try it.
For backwards compatibility you can also use a control class here
(see :ref:`ctrl-class`). In that case all controls have to
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 7b423475281d..e2dde31d76df 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -553,6 +553,8 @@ ignore define V4L2_CTRL_DRIVER_PRIV
ignore define V4L2_CTRL_MAX_DIMS
ignore define V4L2_CTRL_WHICH_CUR_VAL
ignore define V4L2_CTRL_WHICH_DEF_VAL
+ignore define V4L2_CTRL_WHICH_MIN_VAL
+ignore define V4L2_CTRL_WHICH_MAX_VAL
ignore define V4L2_CTRL_WHICH_REQUEST_VAL
ignore define V4L2_OUT_CAP_CUSTOM_TIMINGS
ignore define V4L2_CID_MAX_CTRLS
diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index 83c1737abeec..1541a81dcd46 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -1037,7 +1037,9 @@ static int imx214_probe(struct i2c_client *client)
imx214->unit_size = v4l2_ctrl_new_std_compound(&imx214->ctrls,
NULL,
V4L2_CID_UNIT_CELL_SIZE,
- v4l2_ctrl_ptr_create((void *)&unit_size));
+ v4l2_ctrl_ptr_create((void *)&unit_size),
+ v4l2_ctrl_ptr_create(NULL),
+ v4l2_ctrl_ptr_create(NULL));
ret = imx214->ctrls.error;
if (ret) {
dev_err(&client->dev, "%s control init failed (%d)\n",
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c
index db9baa0bd05f..8a9c816b0dab 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
@@ -97,6 +97,28 @@ static int def_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
return ptr_to_user(c, ctrl, ctrl->p_new);
}
+/* Helper function: copy the minimum control value back to the caller */
+static int min_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
+{
+ int idx;
+
+ for (idx = 0; idx < ctrl->elems; idx++)
+ ctrl->type_ops->minimum(ctrl, idx, ctrl->p_new);
+
+ return ptr_to_user(c, ctrl, ctrl->p_new);
+}
+
+/* Helper function: copy the maximum control value back to the caller */
+static int max_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
+{
+ int idx;
+
+ for (idx = 0; idx < ctrl->elems; idx++)
+ ctrl->type_ops->maximum(ctrl, idx, ctrl->p_new);
+
+ return ptr_to_user(c, ctrl, ctrl->p_new);
+}
+
/* Helper function: copy the caller-provider value to the given control value */
static int user_to_ptr(struct v4l2_ext_control *c,
struct v4l2_ctrl *ctrl,
@@ -220,8 +242,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
cs->error_idx = i;
if (cs->which &&
- cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
- cs->which != V4L2_CTRL_WHICH_REQUEST_VAL &&
+ (cs->which < V4L2_CTRL_WHICH_DEF_VAL ||
+ cs->which > V4L2_CTRL_WHICH_MAX_VAL) &&
V4L2_CTRL_ID2WHICH(id) != cs->which) {
dprintk(vdev,
"invalid which 0x%x or control id 0x%x\n",
@@ -335,8 +357,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
*/
static int class_check(struct v4l2_ctrl_handler *hdl, u32 which)
{
- if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL ||
- which == V4L2_CTRL_WHICH_REQUEST_VAL)
+ if (which == 0 || (which >= V4L2_CTRL_WHICH_DEF_VAL &&
+ which <= V4L2_CTRL_WHICH_MAX_VAL))
return 0;
return find_ref_lock(hdl, which | 1) ? 0 : -EINVAL;
}
@@ -356,10 +378,12 @@ int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl,
struct v4l2_ctrl_helper *helpers = helper;
int ret;
int i, j;
- bool is_default, is_request;
+ bool is_default, is_request, is_min, is_max;
is_default = (cs->which == V4L2_CTRL_WHICH_DEF_VAL);
is_request = (cs->which == V4L2_CTRL_WHICH_REQUEST_VAL);
+ is_min = (cs->which == V4L2_CTRL_WHICH_MIN_VAL);
+ is_max = (cs->which == V4L2_CTRL_WHICH_MAX_VAL);
cs->error_idx = cs->count;
cs->which = V4L2_CTRL_ID2WHICH(cs->which);
@@ -399,13 +423,14 @@ int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl,
/*
* g_volatile_ctrl will update the new control values.
- * This makes no sense for V4L2_CTRL_WHICH_DEF_VAL and
+ * This makes no sense for V4L2_CTRL_WHICH_DEF_VAL,
+ * V4L2_CTRL_WHICH_MIN_VAL, V4L2_CTRL_WHICH_MAX_VAL and
* V4L2_CTRL_WHICH_REQUEST_VAL. In the case of requests
* it is v4l2_ctrl_request_complete() that copies the
* volatile controls at the time of request completion
* to the request, so you don't want to do that again.
*/
- if (!is_default && !is_request &&
+ if (!is_default && !is_request && !is_min && !is_max &&
((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
(master->has_volatiles && !is_cur_manual(master)))) {
for (j = 0; j < master->ncontrols; j++)
@@ -432,6 +457,10 @@ int v4l2_g_ext_ctrls_common(struct v4l2_ctrl_handler *hdl,
ret = def_to_user(cs->controls + idx, ref->ctrl);
else if (is_request && ref->valid_p_req)
ret = req_to_user(cs->controls + idx, ref);
+ else if (is_min)
+ ret = min_to_user(cs->controls + idx, ref->ctrl);
+ else if (is_max)
+ ret = max_to_user(cs->controls + idx, ref->ctrl);
else if (is_volatile)
ret = new_to_user(cs->controls + idx, ref->ctrl);
else
@@ -523,9 +552,11 @@ int try_set_ext_ctrls_common(struct v4l2_fh *fh,
cs->error_idx = cs->count;
- /* Default value cannot be changed */
- if (cs->which == V4L2_CTRL_WHICH_DEF_VAL) {
- dprintk(vdev, "%s: cannot change default value\n",
+ /* Default/minimum/maximum values cannot be changed */
+ if (cs->which == V4L2_CTRL_WHICH_DEF_VAL ||
+ cs->which == V4L2_CTRL_WHICH_MIN_VAL ||
+ cs->which == V4L2_CTRL_WHICH_MAX_VAL) {
+ dprintk(vdev, "%s: cannot change default/min/max value\n",
video_device_node_name(vdev));
return -EINVAL;
}
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index dcde405c2713..d056bd6a1561 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -186,6 +186,28 @@ static void std_init_compound(const struct v4l2_ctrl *ctrl, u32 idx,
}
}
+static void std_min_compound(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ void *p = ptr.p + idx * ctrl->elem_size;
+
+ if (ctrl->p_min.p_const)
+ memcpy(p, ctrl->p_min.p_const, ctrl->elem_size);
+ else
+ memset(p, 0, ctrl->elem_size);
+}
+
+static void std_max_compound(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ void *p = ptr.p + idx * ctrl->elem_size;
+
+ if (ctrl->p_max.p_const)
+ memcpy(p, ctrl->p_max.p_const, ctrl->elem_size);
+ else
+ memset(p, 0, ctrl->elem_size);
+}
+
static void std_init(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr)
{
@@ -224,6 +246,82 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32 idx,
}
}
+static void std_minimum(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ switch (ctrl->type) {
+ case V4L2_CTRL_TYPE_STRING:
+ idx *= ctrl->elem_size;
+ memset(ptr.p_char + idx, ' ', ctrl->minimum);
+ ptr.p_char[idx + ctrl->minimum] = '\0';
+ break;
+ case V4L2_CTRL_TYPE_INTEGER64:
+ ptr.p_s64[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_INTEGER:
+ case V4L2_CTRL_TYPE_INTEGER_MENU:
+ case V4L2_CTRL_TYPE_MENU:
+ case V4L2_CTRL_TYPE_BITMASK:
+ case V4L2_CTRL_TYPE_BOOLEAN:
+ ptr.p_s32[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_BUTTON:
+ case V4L2_CTRL_TYPE_CTRL_CLASS:
+ ptr.p_s32[idx] = 0;
+ break;
+ case V4L2_CTRL_TYPE_U8:
+ ptr.p_u8[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_U16:
+ ptr.p_u16[idx] = ctrl->minimum;
+ break;
+ case V4L2_CTRL_TYPE_U32:
+ ptr.p_u32[idx] = ctrl->minimum;
+ break;
+ default:
+ std_min_compound(ctrl, idx, ptr);
+ break;
+ }
+}
+
+static void std_maximum(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr)
+{
+ switch (ctrl->type) {
+ case V4L2_CTRL_TYPE_STRING:
+ idx *= ctrl->elem_size;
+ memset(ptr.p_char + idx, ' ', ctrl->maximum);
+ ptr.p_char[idx + ctrl->maximum] = '\0';
+ break;
+ case V4L2_CTRL_TYPE_INTEGER64:
+ ptr.p_s64[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_INTEGER:
+ case V4L2_CTRL_TYPE_INTEGER_MENU:
+ case V4L2_CTRL_TYPE_MENU:
+ case V4L2_CTRL_TYPE_BITMASK:
+ case V4L2_CTRL_TYPE_BOOLEAN:
+ ptr.p_s32[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_BUTTON:
+ case V4L2_CTRL_TYPE_CTRL_CLASS:
+ ptr.p_s32[idx] = 0;
+ break;
+ case V4L2_CTRL_TYPE_U8:
+ ptr.p_u8[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_U16:
+ ptr.p_u16[idx] = ctrl->maximum;
+ break;
+ case V4L2_CTRL_TYPE_U32:
+ ptr.p_u32[idx] = ctrl->maximum;
+ break;
+ default:
+ std_max_compound(ctrl, idx, ptr);
+ break;
+ }
+}
+
static void std_log(const struct v4l2_ctrl *ctrl)
{
union v4l2_ctrl_ptr ptr = ctrl->p_cur;
@@ -986,6 +1084,8 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
static const struct v4l2_ctrl_type_ops std_type_ops = {
.equal = std_equal,
.init = std_init,
+ .minimum = std_minimum,
+ .maximum = std_maximum,
.log = std_log,
.validate = std_validate,
};
@@ -1369,7 +1469,10 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
s64 min, s64 max, u64 step, s64 def,
const u32 dims[V4L2_CTRL_MAX_DIMS], u32 elem_size,
u32 flags, const char * const *qmenu,
- const s64 *qmenu_int, const union v4l2_ctrl_ptr p_def,
+ const s64 *qmenu_int,
+ const union v4l2_ctrl_ptr p_def,
+ const union v4l2_ctrl_ptr p_min,
+ const union v4l2_ctrl_ptr p_max,
void *priv)
{
struct v4l2_ctrl *ctrl;
@@ -1516,7 +1619,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
sz_extra += 2 * tot_ctrl_size;
if (type >= V4L2_CTRL_COMPOUND_TYPES && p_def.p_const)
- sz_extra += elem_size;
+ sz_extra += elem_size * 3;
ctrl = kvzalloc(sizeof(*ctrl) + sz_extra, GFP_KERNEL);
if (ctrl == NULL) {
@@ -1566,6 +1669,13 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
ctrl->p_def.p = ctrl->p_cur.p + tot_ctrl_size;
memcpy(ctrl->p_def.p, p_def.p_const, elem_size);
}
+ if (type >= V4L2_CTRL_COMPOUND_TYPES &&
+ p_min.p_const && p_max.p_const) {
+ ctrl->p_min.p = ctrl->p_cur.p + 2 * tot_ctrl_size;
+ memcpy(ctrl->p_min.p, p_min.p_const, elem_size);
+ ctrl->p_max.p = ctrl->p_cur.p + 3 * tot_ctrl_size;
+ memcpy(ctrl->p_max.p, p_max.p_const, elem_size);
+ }
for (idx = 0; idx < elems; idx++) {
ctrl->type_ops->init(ctrl, idx, ctrl->p_cur);
@@ -1618,7 +1728,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl,
type, min, max,
is_menu ? cfg->menu_skip_mask : step, def,
cfg->dims, cfg->elem_size,
- flags, qmenu, qmenu_int, cfg->p_def, priv);
+ flags, qmenu, qmenu_int, cfg->p_def, cfg->p_min,
+ cfg->p_max, priv);
if (ctrl)
ctrl->is_private = cfg->is_private;
return ctrl;
@@ -1643,7 +1754,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
min, max, step, def, NULL, 0,
- flags, NULL, NULL, ptr_null, NULL);
+ flags, NULL, NULL, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std);
@@ -1676,7 +1788,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
0, max, mask, def, NULL, 0,
- flags, qmenu, qmenu_int, ptr_null, NULL);
+ flags, qmenu, qmenu_int, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
@@ -1708,7 +1821,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
0, max, mask, def, NULL, 0,
- flags, qmenu, NULL, ptr_null, NULL);
+ flags, qmenu, NULL, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items);
@@ -1716,7 +1830,9 @@ EXPORT_SYMBOL(v4l2_ctrl_new_std_menu_items);
/* Helper function for standard compound controls */
struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
const struct v4l2_ctrl_ops *ops, u32 id,
- const union v4l2_ctrl_ptr p_def)
+ const union v4l2_ctrl_ptr p_def,
+ const union v4l2_ctrl_ptr p_min,
+ const union v4l2_ctrl_ptr p_max)
{
const char *name;
enum v4l2_ctrl_type type;
@@ -1730,7 +1846,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
min, max, step, def, NULL, 0,
- flags, NULL, NULL, p_def, NULL);
+ flags, NULL, NULL, p_def, p_min, p_max, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_std_compound);
@@ -1754,7 +1870,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl,
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,
0, max, 0, def, NULL, 0,
- flags, NULL, qmenu_int, ptr_null, NULL);
+ flags, NULL, qmenu_int, ptr_null, ptr_null,
+ ptr_null, NULL);
}
EXPORT_SYMBOL(v4l2_ctrl_new_int_menu);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 96e307fe3aab..6ed6ef87c7ff 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -892,7 +892,9 @@ static bool check_ext_ctrls(struct v4l2_ext_controls *c, unsigned long ioctl)
return false;
break;
case V4L2_CTRL_WHICH_DEF_VAL:
- /* Default value cannot be changed */
+ case V4L2_CTRL_WHICH_MIN_VAL:
+ case V4L2_CTRL_WHICH_MAX_VAL:
+ /* Default, minimum or maximum value cannot be changed */
if (ioctl == VIDIOC_S_EXT_CTRLS ||
ioctl == VIDIOC_TRY_EXT_CTRLS) {
c->error_idx = c->count;
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 919e104de50b..bd3bab55b766 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -131,6 +131,8 @@ struct v4l2_ctrl_ops {
*
* @equal: return true if both values are equal.
* @init: initialize the value.
+ * @minimum: set the value to the minimum value of the control.
+ * @maximum: set the value to the maximum value of the control.
* @log: log the value.
* @validate: validate the value. Return 0 on success and a negative value
* otherwise.
@@ -141,6 +143,10 @@ struct v4l2_ctrl_type_ops {
union v4l2_ctrl_ptr ptr2);
void (*init)(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr);
+ void (*minimum)(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr);
+ void (*maximum)(const struct v4l2_ctrl *ctrl, u32 idx,
+ union v4l2_ctrl_ptr ptr);
void (*log)(const struct v4l2_ctrl *ctrl);
int (*validate)(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr);
@@ -237,6 +243,12 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);
* @p_def: The control's default value represented via a union which
* provides a standard way of accessing control types
* through a pointer (for compound controls only).
+ * @p_min: The control's minimum value represented via a union which
+ * provides a standard way of accessing control types
+ * through a pointer (for compound controls only).
+ * @p_max: The control's maximum value represented via a union which
+ * provides a standard way of accessing control types
+ * through a pointer (for compound controls only).
* @p_cur: The control's current value represented via a union which
* provides a standard way of accessing control types
* through a pointer.
@@ -292,6 +304,8 @@ struct v4l2_ctrl {
} cur;
union v4l2_ctrl_ptr p_def;
+ union v4l2_ctrl_ptr p_min;
+ union v4l2_ctrl_ptr p_max;
union v4l2_ctrl_ptr p_new;
union v4l2_ctrl_ptr p_cur;
};
@@ -398,6 +412,8 @@ struct v4l2_ctrl_handler {
* @step: The control's step value for non-menu controls.
* @def: The control's default value.
* @p_def: The control's default value for compound controls.
+ * @p_min: The control's minimum value for compound controls.
+ * @p_max: The control's maximum value for compound controls.
* @dims: The size of each dimension.
* @elem_size: The size in bytes of the control.
* @flags: The control's flags.
@@ -427,6 +443,8 @@ struct v4l2_ctrl_config {
u64 step;
s64 def;
union v4l2_ctrl_ptr p_def;
+ union v4l2_ctrl_ptr p_min;
+ union v4l2_ctrl_ptr p_max;
u32 dims[V4L2_CTRL_MAX_DIMS];
u32 elem_size;
u32 flags;
@@ -696,17 +714,21 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl,
* @ops: The control ops.
* @id: The control ID.
* @p_def: The control's default value.
+ * @p_min: The control's default value.
+ * @p_ma: The control's default value.
*
* Sames as v4l2_ctrl_new_std(), but with support to compound controls, thanks
- * to the @p_def field. Use v4l2_ctrl_ptr_create() to create @p_def from a
- * pointer. Use v4l2_ctrl_ptr_create(NULL) if the default value of the
- * compound control should be all zeroes.
+ * to the @p_def/min/max field. Use v4l2_ctrl_ptr_create() to create
+ * @p_def/min/max from a pointer. Use v4l2_ctrl_ptr_create(NULL) if the default
+ * value of the compound control should be all zeroes.
*
*/
struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl,
const struct v4l2_ctrl_ops *ops,
u32 id,
- const union v4l2_ctrl_ptr p_def);
+ const union v4l2_ctrl_ptr p_def,
+ const union v4l2_ctrl_ptr p_min,
+ const union v4l2_ctrl_ptr p_max);
/**
* v4l2_ctrl_new_int_menu() - Create a new standard V4L2 integer menu control.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index b712412cf763..d22ebb0102d4 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1793,6 +1793,8 @@ struct v4l2_ext_controls {
#define V4L2_CTRL_WHICH_CUR_VAL 0
#define V4L2_CTRL_WHICH_DEF_VAL 0x0f000000
#define V4L2_CTRL_WHICH_REQUEST_VAL 0x0f010000
+#define V4L2_CTRL_WHICH_MIN_VAL 0x0f020000
+#define V4L2_CTRL_WHICH_MAX_VAL 0x0f030000
enum v4l2_ctrl_type {
V4L2_CTRL_TYPE_INTEGER = 1,
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/6] media: uvcvideo: implement UVC v1.5 ROI
2022-05-16 14:04 [PATCH v2 0/6] media: Implement UVC v1.5 ROI Yunke Cao
` (2 preceding siblings ...)
2022-05-16 14:04 ` [PATCH v2 3/6] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
@ 2022-05-16 14:04 ` Yunke Cao
2022-05-16 14:04 ` [PATCH v2 5/6] media: uvcvideo: Initialize roi to default value Yunke Cao
2022-05-16 14:04 ` [PATCH v2 6/6] media: vivid: Add a roi rectangle control Yunke Cao
5 siblings, 0 replies; 15+ messages in thread
From: Yunke Cao @ 2022-05-16 14:04 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao
Supports GET_CUR, GET_DEF, GET_MIN and GET_MAX requests for UVC v1.5
using V4L2 control API.
References a rejected attempt that uses v4l2 selection API:
https://lore.kernel.org/lkml/20210501082001.100533-2-senozhatsky@chromium.org
Signed-off-by: Yunke Cao <yunkec@google.com>
---
.../userspace-api/media/drivers/uvcvideo.rst | 1 +
drivers/media/usb/uvc/uvc_ctrl.c | 173 ++++++++++++++++--
drivers/media/usb/uvc/uvc_v4l2.c | 12 +-
drivers/media/usb/uvc/uvcvideo.h | 10 +-
include/uapi/linux/usb/video.h | 1 +
include/uapi/linux/uvcvideo.h | 1 +
6 files changed, 173 insertions(+), 25 deletions(-)
diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst
index e5fd8fad333c..43b8431b0d9f 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
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index b4f6edf968bc..c3d816985f13 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -355,6 +355,15 @@ static const struct uvc_control_info uvc_ctrls[] = {
.flags = UVC_CTRL_FLAG_GET_CUR
| UVC_CTRL_FLAG_AUTO_UPDATE,
},
+ {
+ .entity = UVC_GUID_UVC_CAMERA,
+ .selector = UVC_CT_REGION_OF_INTEREST_CONTROL,
+ .index = 21,
+ .size = 10,
+ .flags = UVC_CTRL_FLAG_SET_CUR | UVC_CTRL_FLAG_GET_CUR
+ | UVC_CTRL_FLAG_GET_MIN | UVC_CTRL_FLAG_GET_MAX
+ | UVC_CTRL_FLAG_GET_DEF
+ },
};
static const u32 uvc_control_classes[] = {
@@ -728,6 +737,24 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
.v4l2_type = V4L2_CTRL_TYPE_BOOLEAN,
.data_type = UVC_CTRL_DATA_TYPE_BOOLEAN,
},
+ {
+ .id = V4L2_CID_REGION_OF_INTEREST_RECT,
+ .entity = UVC_GUID_UVC_CAMERA,
+ .selector = UVC_CT_REGION_OF_INTEREST_CONTROL,
+ .size = 64,
+ .offset = 0,
+ .v4l2_type = V4L2_CTRL_TYPE_RECT,
+ .data_type = UVC_CTRL_DATA_TYPE_RECT,
+ },
+ {
+ .id = V4L2_CID_REGION_OF_INTEREST_AUTO,
+ .entity = UVC_GUID_UVC_CAMERA,
+ .selector = UVC_CT_REGION_OF_INTEREST_CONTROL,
+ .size = 16,
+ .offset = 64,
+ .v4l2_type = V4L2_CTRL_TYPE_BITMASK,
+ .data_type = UVC_CTRL_DATA_TYPE_BITMASK,
+ },
};
/* ------------------------------------------------------------------------
@@ -749,6 +776,33 @@ static inline void uvc_clear_bit(u8 *data, int bit)
data[bit >> 3] &= ~(1 << (bit & 7));
}
+static void uvc_to_v4l2_rect(struct v4l2_rect *v4l2_rect,
+ const struct uvc_rect *uvc_rect)
+{
+ v4l2_rect->top = uvc_rect->top;
+ v4l2_rect->left = uvc_rect->left;
+ v4l2_rect->height = uvc_rect->bottom - uvc_rect->top + 1;
+ v4l2_rect->width = uvc_rect->right - uvc_rect->left + 1;
+}
+
+static int v4l2_to_uvc_rect(struct uvc_rect *uvc_rect,
+ const struct v4l2_rect *v4l2_rect)
+{
+ // Safely converts s32 and u32 to u16.
+ if (v4l2_rect->top > U16_MAX || v4l2_rect->top < 0 ||
+ v4l2_rect->left > U16_MAX || v4l2_rect->left < 0 ||
+ v4l2_rect->height > U16_MAX || v4l2_rect->width > U16_MAX ||
+ v4l2_rect->height + v4l2_rect->top - 1 > U16_MAX ||
+ v4l2_rect->width + v4l2_rect->left - 1 > U16_MAX)
+ return -ERANGE;
+
+ uvc_rect->top = v4l2_rect->top;
+ uvc_rect->left = v4l2_rect->left;
+ uvc_rect->bottom = v4l2_rect->height + v4l2_rect->top - 1;
+ uvc_rect->right = v4l2_rect->width + v4l2_rect->left - 1;
+ return 0;
+}
+
/* Extract the bit string specified by mapping->offset and mapping->size
* from the little-endian data stored at 'data' and return the result as
* a signed 32bit integer. Sign extension will be performed if the mapping
@@ -963,11 +1017,23 @@ static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping,
return value;
}
+static void __uvc_ctrl_get_v4l2_rect(struct uvc_control *ctrl,
+ struct uvc_control_mapping *mapping,
+ u32 id,
+ struct v4l2_rect *rect)
+{
+ struct uvc_rect *uvc_rect =
+ (struct uvc_rect *)(uvc_ctrl_data(ctrl, id)
+ + mapping->offset / 8);
+ uvc_to_v4l2_rect(rect, uvc_rect);
+}
+
static int __uvc_ctrl_get(struct uvc_video_chain *chain,
struct uvc_control *ctrl, struct uvc_control_mapping *mapping,
- s32 *value)
+ struct v4l2_ext_control *xctrl)
{
int ret;
+ struct v4l2_rect v4l2_rect;
if ((ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR) == 0)
return -EACCES;
@@ -993,8 +1059,17 @@ static int __uvc_ctrl_get(struct uvc_video_chain *chain,
ctrl->loaded = 1;
}
- *value = __uvc_ctrl_get_value(mapping,
+ switch (mapping->v4l2_type) {
+ case V4L2_CTRL_TYPE_RECT:
+ __uvc_ctrl_get_v4l2_rect(ctrl, mapping, UVC_CTRL_DATA_CURRENT,
+ &v4l2_rect);
+ return copy_to_user(xctrl->p_rect, &v4l2_rect,
+ sizeof(v4l2_rect));
+
+ default:
+ xctrl->value = __uvc_ctrl_get_value(mapping,
uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
+ }
return 0;
}
@@ -1104,13 +1179,14 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
__uvc_find_control(ctrl->entity, mapping->master_id,
&master_map, &master_ctrl, 0);
if (master_ctrl && (master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) {
- s32 val;
- int ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
+ struct v4l2_ext_control xctrl;
+ int ret =
+ __uvc_ctrl_get(chain, master_ctrl, master_map, &xctrl);
if (ret < 0)
return ret;
- if (val != mapping->master_manual)
- v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
+ if (xctrl.value != mapping->master_manual)
+ v4l2_ctrl->flags |= V4L2_CTRL_FLAG_INACTIVE;
}
if (!ctrl->cached) {
@@ -1344,16 +1420,16 @@ static void uvc_ctrl_send_slave_event(struct uvc_video_chain *chain,
struct uvc_control_mapping *mapping = NULL;
struct uvc_control *ctrl = NULL;
u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
- s32 val = 0;
+ struct v4l2_ext_control xctrl;
__uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
if (ctrl == NULL)
return;
- if (__uvc_ctrl_get(chain, ctrl, mapping, &val) == 0)
+ if (__uvc_ctrl_get(chain, ctrl, mapping, &xctrl) == 0)
changes |= V4L2_EVENT_CTRL_CH_VALUE;
- uvc_ctrl_send_event(chain, handle, ctrl, mapping, val, changes);
+ uvc_ctrl_send_event(chain, handle, ctrl, mapping, xctrl.value, changes);
}
void uvc_ctrl_status_event(struct uvc_video_chain *chain,
@@ -1515,13 +1591,13 @@ static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
if (sev->flags & V4L2_EVENT_SUB_FL_SEND_INITIAL) {
struct v4l2_event ev;
u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
- s32 val = 0;
+ struct v4l2_ext_control xctrl;
- if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
+ if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &xctrl) == 0)
changes |= V4L2_EVENT_CTRL_CH_VALUE;
- uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, val,
- changes);
+ uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping,
+ xctrl.value, changes);
/* Mark the queue as active, allowing this initial
event to be accepted. */
sev->elems = elems;
@@ -1682,10 +1758,14 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
}
int uvc_ctrl_get(struct uvc_video_chain *chain,
- struct v4l2_ext_control *xctrl)
+ struct v4l2_ext_control *xctrl, u32 v4l2_which)
{
struct uvc_control *ctrl;
struct uvc_control_mapping *mapping;
+ int ret;
+ u32 flag;
+ u32 id;
+ u8 query;
if (__uvc_query_v4l2_class(chain, xctrl->id, 0) >= 0)
return -EACCES;
@@ -1694,7 +1774,46 @@ int uvc_ctrl_get(struct uvc_video_chain *chain,
if (ctrl == NULL)
return -EINVAL;
- return __uvc_ctrl_get(chain, ctrl, mapping, &xctrl->value);
+ switch (v4l2_which) {
+ case V4L2_CTRL_WHICH_DEF_VAL:
+ flag = UVC_CTRL_FLAG_GET_DEF;
+ id = UVC_CTRL_DATA_DEF;
+ query = UVC_GET_DEF;
+ break;
+ case V4L2_CTRL_WHICH_MIN_VAL:
+ flag = UVC_CTRL_FLAG_GET_MIN;
+ id = UVC_CTRL_DATA_MIN;
+ query = UVC_GET_MIN;
+ break;
+ case V4L2_CTRL_WHICH_MAX_VAL:
+ flag = UVC_CTRL_FLAG_GET_MAX;
+ id = UVC_CTRL_DATA_MAX;
+ query = UVC_GET_MAX;
+ break;
+ case V4L2_CTRL_WHICH_CUR_VAL:
+ default:
+ return __uvc_ctrl_get(chain, ctrl, mapping, xctrl);
+ }
+
+ if (!ctrl->cached) {
+ ret = uvc_ctrl_populate_cache(chain, ctrl);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (!(ctrl->info.flags & flag))
+ return -EACCES;
+
+ if (mapping->v4l2_type == V4L2_CTRL_TYPE_RECT) {
+ struct v4l2_rect rect;
+
+ __uvc_ctrl_get_v4l2_rect(ctrl, mapping, id, &rect);
+ return copy_to_user(xctrl->p_rect, &rect, sizeof(rect));
+ }
+
+ xctrl->value = mapping->get(mapping, query, uvc_ctrl_data(ctrl, id));
+
+ return 0;
}
int uvc_ctrl_set(struct uvc_fh *handle,
@@ -1703,6 +1822,8 @@ int uvc_ctrl_set(struct uvc_fh *handle,
struct uvc_video_chain *chain = handle->chain;
struct uvc_control *ctrl;
struct uvc_control_mapping *mapping;
+ struct v4l2_rect v4l2_rect;
+ struct uvc_rect uvc_rect;
s32 value;
u32 step;
s32 min;
@@ -1774,6 +1895,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
break;
+ case V4L2_CTRL_TYPE_RECT:
+ ret = copy_from_user(&v4l2_rect, xctrl->p_rect,
+ sizeof(v4l2_rect));
+ if (ret < 0)
+ return ret;
+ ret = v4l2_to_uvc_rect(&uvc_rect, &v4l2_rect);
+ if (ret < 0)
+ return ret;
+ break;
+
default:
value = xctrl->value;
break;
@@ -1807,8 +1938,16 @@ int uvc_ctrl_set(struct uvc_fh *handle,
ctrl->info.size);
}
- mapping->set(mapping, value,
- uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
+ switch (mapping->data_type) {
+ case UVC_CTRL_DATA_TYPE_RECT:
+ memcpy(uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT)
+ + mapping->offset / 8,
+ &uvc_rect, sizeof(uvc_rect));
+ break;
+ default:
+ mapping->set(mapping, value,
+ uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT));
+ }
if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
ctrl->handle = handle;
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 711556d13d03..a88d3fe6de93 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1038,17 +1038,15 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
if (ret < 0)
return ret;
- if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
+ if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL ||
+ ctrls->which == V4L2_CTRL_WHICH_MIN_VAL ||
+ ctrls->which == V4L2_CTRL_WHICH_MAX_VAL) {
for (i = 0; i < ctrls->count; ++ctrl, ++i) {
- struct v4l2_queryctrl qc = { .id = ctrl->id };
-
- ret = uvc_query_v4l2_ctrl(chain, &qc);
+ ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
if (ret < 0) {
ctrls->error_idx = i;
return ret;
}
-
- ctrl->value = qc.default_value;
}
return 0;
@@ -1059,7 +1057,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
return ret;
for (i = 0; i < ctrls->count; ++ctrl, ++i) {
- ret = uvc_ctrl_get(chain, ctrl);
+ ret = uvc_ctrl_get(chain, ctrl, ctrls->which);
if (ret < 0) {
uvc_ctrl_rollback(handle);
ctrls->error_idx = i;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 143230b3275b..f414ad7d57b2 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -424,6 +424,13 @@ struct uvc_streaming_header {
u8 bTriggerUsage;
};
+struct uvc_rect {
+ u16 top;
+ u16 left;
+ u16 bottom;
+ u16 right;
+} __packed;
+
enum uvc_buffer_state {
UVC_BUF_STATE_IDLE = 0,
UVC_BUF_STATE_QUEUED = 1,
@@ -897,7 +904,8 @@ static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
return __uvc_ctrl_commit(handle, 1, NULL);
}
-int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl);
+int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl,
+ u32 v4l2_which);
int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl);
int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
bool read);
diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
index bfdae12cdacf..9076a444758a 100644
--- a/include/uapi/linux/usb/video.h
+++ b/include/uapi/linux/usb/video.h
@@ -104,6 +104,7 @@
#define UVC_CT_ROLL_ABSOLUTE_CONTROL 0x0f
#define UVC_CT_ROLL_RELATIVE_CONTROL 0x10
#define UVC_CT_PRIVACY_CONTROL 0x11
+#define UVC_CT_REGION_OF_INTEREST_CONTROL 0x14
/* A.9.5. Processing Unit Control Selectors */
#define UVC_PU_CONTROL_UNDEFINED 0x00
diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
index 8288137387c0..d7d77602a5e7 100644
--- a/include/uapi/linux/uvcvideo.h
+++ b/include/uapi/linux/uvcvideo.h
@@ -16,6 +16,7 @@
#define UVC_CTRL_DATA_TYPE_BOOLEAN 3
#define UVC_CTRL_DATA_TYPE_ENUM 4
#define UVC_CTRL_DATA_TYPE_BITMASK 5
+#define UVC_CTRL_DATA_TYPE_RECT 6
/* Control flags */
#define UVC_CTRL_FLAG_SET_CUR (1 << 0)
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 5/6] media: uvcvideo: Initialize roi to default value
2022-05-16 14:04 [PATCH v2 0/6] media: Implement UVC v1.5 ROI Yunke Cao
` (3 preceding siblings ...)
2022-05-16 14:04 ` [PATCH v2 4/6] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
@ 2022-05-16 14:04 ` Yunke Cao
2022-05-16 14:04 ` [PATCH v2 6/6] media: vivid: Add a roi rectangle control Yunke Cao
5 siblings, 0 replies; 15+ messages in thread
From: Yunke Cao @ 2022-05-16 14:04 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao
For the devices we tested with, firmwares have wrong initial
ROI configuration. Initialize roi by setting to default value.
Signed-off-by: Yunke Cao <yunkec@google.com>
---
Changelog since v1:
- Fix a build error introduced in rebasing.
drivers/media/usb/uvc/uvc_ctrl.c | 40 ++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c3d816985f13..2e37bb8c784c 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2505,6 +2505,40 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev,
}
}
+static int uvc_ctrl_init_roi(struct uvc_device *dev, struct uvc_control *ctrl)
+{
+ int ret;
+
+ ret = uvc_query_ctrl(dev, UVC_GET_DEF, ctrl->entity->id, dev->intfnum,
+ UVC_CT_REGION_OF_INTEREST_CONTROL,
+ uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
+ ctrl->info.size);
+ if (ret)
+ goto out;
+
+ /*
+ * Some firmwares have wrong GET_CURRENT configuration. E.g. it's
+ * below GET_MIN, or have rectangle coordinates mixed up. This
+ * causes problems sometimes, because we are unable to set
+ * auto-controls value without first setting ROI rectangle to
+ * valid configuration.
+ *
+ * We expect that default configuration is always correct and
+ * is within the GET_MIN / GET_MAX boundaries.
+ *
+ * Set current ROI configuration to GET_DEF, so that we will
+ * always have properly configured ROI.
+ */
+ ret = uvc_query_ctrl(dev, UVC_SET_CUR, 1, dev->intfnum,
+ UVC_CT_REGION_OF_INTEREST_CONTROL,
+ uvc_ctrl_data(ctrl, UVC_CTRL_DATA_DEF),
+ ctrl->info.size);
+out:
+ if (ret)
+ dev_err(&dev->udev->dev, "Failed to fixup ROI (%d).\n", ret);
+ return ret;
+}
+
/*
* Add control information and hardcoded stock control mappings to the given
* device.
@@ -2517,6 +2551,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
const struct uvc_control_mapping *mend =
mapping + ARRAY_SIZE(uvc_ctrl_mappings);
+ const u8 camera_entity[16] = UVC_GUID_UVC_CAMERA;
/* XU controls initialization requires querying the device for control
* information. As some buggy UVC devices will crash when queried
@@ -2537,6 +2572,11 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
* GET_INFO on standard controls.
*/
uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
+
+ if (ctrl->info.selector ==
+ UVC_CT_REGION_OF_INTEREST_CONTROL &&
+ uvc_entity_match_guid(ctrl->entity, camera_entity))
+ uvc_ctrl_init_roi(chain->dev, ctrl);
break;
}
}
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 6/6] media: vivid: Add a roi rectangle control
2022-05-16 14:04 [PATCH v2 0/6] media: Implement UVC v1.5 ROI Yunke Cao
` (4 preceding siblings ...)
2022-05-16 14:04 ` [PATCH v2 5/6] media: uvcvideo: Initialize roi to default value Yunke Cao
@ 2022-05-16 14:04 ` Yunke Cao
5 siblings, 0 replies; 15+ messages in thread
From: Yunke Cao @ 2022-05-16 14:04 UTC (permalink / raw)
To: Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media, Yunke Cao
The control supports current, default, minimum and maximum.
Tested by calling ioctls from the user space.
Signed-off-by: Yunke Cao <yunkec@google.com>
---
.../media/test-drivers/vivid/vivid-ctrls.c | 34 +++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c
index e7516dc1227b..79093882d386 100644
--- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
+++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
@@ -34,6 +34,7 @@
#define VIVID_CID_U8_4D_ARRAY (VIVID_CID_CUSTOM_BASE + 10)
#define VIVID_CID_AREA (VIVID_CID_CUSTOM_BASE + 11)
#define VIVID_CID_RO_INTEGER (VIVID_CID_CUSTOM_BASE + 12)
+#define VIVID_CID_RECT (VIVID_CID_CUSTOM_BASE + 13)
#define VIVID_CID_VIVID_BASE (0x00f00000 | 0xf000)
#define VIVID_CID_VIVID_CLASS (0x00f00000 | 1)
@@ -292,6 +293,38 @@ static const struct v4l2_ctrl_config vivid_ctrl_area = {
.p_def.p_const = &area,
};
+static const struct v4l2_rect def_rect = {
+ .left = 0,
+ .top = 0,
+ .width = 1000,
+ .height = 2000,
+};
+
+static const struct v4l2_rect min_rect = {
+ .left = 0,
+ .top = 0,
+ .width = 1,
+ .height = 2,
+};
+
+static const struct v4l2_rect max_rect = {
+ .left = 0,
+ .top = 0,
+ .width = 2000,
+ .height = 4000,
+};
+
+static const struct v4l2_ctrl_config vivid_ctrl_rect = {
+ .ops = &vivid_user_gen_ctrl_ops,
+ .id = VIVID_CID_RECT,
+ .name = "Region of Interest Rectangle",
+ .type = V4L2_CTRL_TYPE_RECT,
+ .p_def.p_const = &def_rect,
+ .p_min.p_const = &min_rect,
+ .p_max.p_const = &max_rect,
+};
+
+
static const struct v4l2_ctrl_config vivid_ctrl_ro_int32 = {
.ops = &vivid_user_gen_ctrl_ops,
.id = VIVID_CID_RO_INTEGER,
@@ -1611,6 +1644,7 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap,
dev->int_menu = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_int_menu, NULL);
dev->ro_int32 = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_ro_int32, NULL);
v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_area, NULL);
+ v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_rect, NULL);
v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u32_array, NULL);
v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u16_matrix, NULL);
v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u8_4d_array, NULL);
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control
2022-05-16 14:04 ` [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control Yunke Cao
@ 2022-05-16 19:02 ` Nicolas Dufresne
2022-05-17 7:11 ` Yunke Cao
2022-05-18 5:19 ` Tomasz Figa
0 siblings, 2 replies; 15+ messages in thread
From: Nicolas Dufresne @ 2022-05-16 19:02 UTC (permalink / raw)
To: Yunke Cao, Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media
Hi,
thanks for working on this, see my comments below ...
Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> Including:
> 1. Add a control ID.
> 2. Add p_rect to struct v4l2_ext_control with basic support in
> v4l2-ctrls.
>
> Signed-off-by: Yunke Cao <yunkec@google.com>
> ---
> .../media/v4l/ext-ctrls-camera.rst | 4 ++++
> .../media/v4l/vidioc-g-ext-ctrls.rst | 4 ++++
> .../media/videodev2.h.rst.exceptions | 1 +
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 20 +++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 ++++
> include/media/v4l2-ctrls.h | 2 ++
> include/uapi/linux/v4l2-controls.h | 2 ++
> include/uapi/linux/videodev2.h | 2 ++
> 8 files changed, 39 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> index 4c5061aa9cd4..86a1f09a8a1c 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> @@ -661,3 +661,7 @@ enum v4l2_scene_mode -
> .. [#f1]
> This control may be changed to a menu control in the future, if more
> options are required.
> +
> +``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> + This control determines the region of interest. Region of interest is an
> + rectangular area represented by a struct v4l2_rect.
This control documentation is missing some important information. Notably, what
will happen if this rectangle is set ? Is there a value to unset it ?
The name is very generic and I would expect that to be usable in general. But it
won't work for encoders, as you only allow 1 rectangle and it would be missing
some QP delta parameter. I think I would prefer if we specialize this type of
control a bit more. In your case, I'm guessing you only care about 1 ROI when
taking a picture, and this ROI will be used for automatic focus. If my guess is
right, perhaps a FOCUS_AERA could be a better name ?
regards,
Nicolas
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> index 29971a45a2d4..f4e205ead0a2 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -189,6 +189,10 @@ still cause this situation.
> - ``p_area``
> - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
> of type ``V4L2_CTRL_TYPE_AREA``.
> + * - struct :c:type:`v4l2_rect` *
> + - ``p_area``
> + - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
> + of type ``V4L2_CTRL_TYPE_RECT``.
> * - struct :c:type:`v4l2_ctrl_h264_sps` *
> - ``p_h264_sps``
> - A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 9cbb7a0c354a..7b423475281d 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
> replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
> replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
> replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
> +replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
> replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
> replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
> replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index 8968cec8454e..dcde405c2713 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
> return ptr1.p_u16[idx] == ptr2.p_u16[idx];
> case V4L2_CTRL_TYPE_U32:
> return ptr1.p_u32[idx] == ptr2.p_u32[idx];
> + case V4L2_CTRL_TYPE_RECT:
> + return ptr1.p_rect->top == ptr2.p_rect->top &&
> + ptr1.p_rect->left == ptr2.p_rect->left &&
> + ptr1.p_rect->height == ptr2.p_rect->height &&
> + ptr1.p_rect->width == ptr2.p_rect->width;
> default:
> if (ctrl->is_int)
> return ptr1.p_s32[idx] == ptr2.p_s32[idx];
> @@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> case V4L2_CTRL_TYPE_VP9_FRAME:
> pr_cont("VP9_FRAME");
> break;
> + case V4L2_CTRL_TYPE_RECT:
> + pr_cont("l: %d, t: %d, w: %u, h: %u",
> + ptr.p_rect->left, ptr.p_rect->top,
> + ptr.p_rect->width, ptr.p_rect->height);
> + break;
> default:
> pr_cont("unknown type %d", ctrl->type);
> break;
> @@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
> struct v4l2_area *area;
> + struct v4l2_rect *rect;
> void *p = ptr.p + idx * ctrl->elem_size;
> unsigned int i;
>
> @@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> return -EINVAL;
> break;
>
> + case V4L2_CTRL_TYPE_RECT:
> + rect = p;
> + if (!rect->width || !rect->height)
> + return -EINVAL;
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> case V4L2_CTRL_TYPE_AREA:
> elem_size = sizeof(struct v4l2_area);
> break;
> + case V4L2_CTRL_TYPE_RECT:
> + elem_size = sizeof(struct v4l2_rect);
> + break;
> default:
> if (type < V4L2_CTRL_COMPOUND_TYPES)
> elem_size = sizeof(s32);
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 54ca4e6b820b..95f39a2d2ad2 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1042,6 +1042,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_UNIT_CELL_SIZE: return "Unit Cell Size";
> case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> + case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
>
> /* FM Radio Modulator controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1524,6 +1525,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
> *type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
> break;
> + case V4L2_CID_REGION_OF_INTEREST_RECT:
> + *type = V4L2_CTRL_TYPE_RECT;
> + break;
> default:
> *type = V4L2_CTRL_TYPE_INTEGER;
> break;
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index b3ce438f1329..919e104de50b 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -58,6 +58,7 @@ struct video_device;
> * @p_hdr10_cll: Pointer to an HDR10 Content Light Level structure.
> * @p_hdr10_mastering: Pointer to an HDR10 Mastering Display structure.
> * @p_area: Pointer to an area.
> + * @p_rect: Pointer to a rectangle.
> * @p: Pointer to a compound value.
> * @p_const: Pointer to a constant compound value.
> */
> @@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
> struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
> struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> struct v4l2_area *p_area;
> + struct v4l2_rect *p_rect;
> void *p;
> const void *p_const;
> };
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index bb40129446d4..499fcddb6254 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1008,6 +1008,8 @@ enum v4l2_auto_focus_range {
>
> #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
>
> +#define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> +
> /* FM Modulator class control IDs */
>
> #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 3768a0a80830..b712412cf763 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
> __u16 __user *p_u16;
> __u32 __user *p_u32;
> struct v4l2_area __user *p_area;
> + struct v4l2_rect __user *p_rect;
> struct v4l2_ctrl_h264_sps __user *p_h264_sps;
> struct v4l2_ctrl_h264_pps *p_h264_pps;
> struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
> @@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
> V4L2_CTRL_TYPE_U16 = 0x0101,
> V4L2_CTRL_TYPE_U32 = 0x0102,
> V4L2_CTRL_TYPE_AREA = 0x0106,
> + V4L2_CTRL_TYPE_RECT = 0x0107,
>
> V4L2_CTRL_TYPE_HDR10_CLL_INFO = 0x0110,
> V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY = 0x0111,
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control
2022-05-16 19:02 ` Nicolas Dufresne
@ 2022-05-17 7:11 ` Yunke Cao
2022-05-18 5:19 ` Tomasz Figa
1 sibling, 0 replies; 15+ messages in thread
From: Yunke Cao @ 2022-05-17 7:11 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Hans Verkuil, Laurent Pinchart, Tomasz Figa, Sergey Senozhatsky,
Ricardo Ribalda, linux-media
Hi Nicolas,
Thanks for the feedback. I will send v3 updating the documentation.
Yes, the current implementation only cares about 1 ROI.
For naming, my worry about using something like FOCUS_AREA is that the
ROI does more than auto focus.
See the list of controls V4L2_CID_REGION_OF_INTEREST_AUTO_* in Patch 2/6.
Let me know what you think.
Best,
Yunke
On Tue, May 17, 2022 at 4:02 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Hi,
>
> thanks for working on this, see my comments below ...
>
> Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> > Including:
> > 1. Add a control ID.
> > 2. Add p_rect to struct v4l2_ext_control with basic support in
> > v4l2-ctrls.
> >
> > Signed-off-by: Yunke Cao <yunkec@google.com>
> > ---
> > .../media/v4l/ext-ctrls-camera.rst | 4 ++++
> > .../media/v4l/vidioc-g-ext-ctrls.rst | 4 ++++
> > .../media/videodev2.h.rst.exceptions | 1 +
> > drivers/media/v4l2-core/v4l2-ctrls-core.c | 20 +++++++++++++++++++
> > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 ++++
> > include/media/v4l2-ctrls.h | 2 ++
> > include/uapi/linux/v4l2-controls.h | 2 ++
> > include/uapi/linux/videodev2.h | 2 ++
> > 8 files changed, 39 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > index 4c5061aa9cd4..86a1f09a8a1c 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > @@ -661,3 +661,7 @@ enum v4l2_scene_mode -
> > .. [#f1]
> > This control may be changed to a menu control in the future, if more
> > options are required.
> > +
> > +``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> > + This control determines the region of interest. Region of interest is an
> > + rectangular area represented by a struct v4l2_rect.
>
> This control documentation is missing some important information. Notably, what
> will happen if this rectangle is set ? Is there a value to unset it ?
>
> The name is very generic and I would expect that to be usable in general. But it
> won't work for encoders, as you only allow 1 rectangle and it would be missing
> some QP delta parameter. I think I would prefer if we specialize this type of
> control a bit more. In your case, I'm guessing you only care about 1 ROI when
> taking a picture, and this ROI will be used for automatic focus. If my guess is
> right, perhaps a FOCUS_AERA could be a better name ?
>
> regards,
> Nicolas
>
> > diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > index 29971a45a2d4..f4e205ead0a2 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > @@ -189,6 +189,10 @@ still cause this situation.
> > - ``p_area``
> > - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
> > of type ``V4L2_CTRL_TYPE_AREA``.
> > + * - struct :c:type:`v4l2_rect` *
> > + - ``p_area``
> > + - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
> > + of type ``V4L2_CTRL_TYPE_RECT``.
> > * - struct :c:type:`v4l2_ctrl_h264_sps` *
> > - ``p_h264_sps``
> > - A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
> > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > index 9cbb7a0c354a..7b423475281d 100644
> > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > @@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
> > +replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > index 8968cec8454e..dcde405c2713 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > @@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
> > return ptr1.p_u16[idx] == ptr2.p_u16[idx];
> > case V4L2_CTRL_TYPE_U32:
> > return ptr1.p_u32[idx] == ptr2.p_u32[idx];
> > + case V4L2_CTRL_TYPE_RECT:
> > + return ptr1.p_rect->top == ptr2.p_rect->top &&
> > + ptr1.p_rect->left == ptr2.p_rect->left &&
> > + ptr1.p_rect->height == ptr2.p_rect->height &&
> > + ptr1.p_rect->width == ptr2.p_rect->width;
> > default:
> > if (ctrl->is_int)
> > return ptr1.p_s32[idx] == ptr2.p_s32[idx];
> > @@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> > case V4L2_CTRL_TYPE_VP9_FRAME:
> > pr_cont("VP9_FRAME");
> > break;
> > + case V4L2_CTRL_TYPE_RECT:
> > + pr_cont("l: %d, t: %d, w: %u, h: %u",
> > + ptr.p_rect->left, ptr.p_rect->top,
> > + ptr.p_rect->width, ptr.p_rect->height);
> > + break;
> > default:
> > pr_cont("unknown type %d", ctrl->type);
> > break;
> > @@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
> > struct v4l2_area *area;
> > + struct v4l2_rect *rect;
> > void *p = ptr.p + idx * ctrl->elem_size;
> > unsigned int i;
> >
> > @@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > return -EINVAL;
> > break;
> >
> > + case V4L2_CTRL_TYPE_RECT:
> > + rect = p;
> > + if (!rect->width || !rect->height)
> > + return -EINVAL;
> > + break;
> > +
> > default:
> > return -EINVAL;
> > }
> > @@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> > case V4L2_CTRL_TYPE_AREA:
> > elem_size = sizeof(struct v4l2_area);
> > break;
> > + case V4L2_CTRL_TYPE_RECT:
> > + elem_size = sizeof(struct v4l2_rect);
> > + break;
> > default:
> > if (type < V4L2_CTRL_COMPOUND_TYPES)
> > elem_size = sizeof(s32);
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > index 54ca4e6b820b..95f39a2d2ad2 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > @@ -1042,6 +1042,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_UNIT_CELL_SIZE: return "Unit Cell Size";
> > case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> > case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> > + case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
> >
> > /* FM Radio Modulator controls */
> > /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> > @@ -1524,6 +1525,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
> > *type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
> > break;
> > + case V4L2_CID_REGION_OF_INTEREST_RECT:
> > + *type = V4L2_CTRL_TYPE_RECT;
> > + break;
> > default:
> > *type = V4L2_CTRL_TYPE_INTEGER;
> > break;
> > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > index b3ce438f1329..919e104de50b 100644
> > --- a/include/media/v4l2-ctrls.h
> > +++ b/include/media/v4l2-ctrls.h
> > @@ -58,6 +58,7 @@ struct video_device;
> > * @p_hdr10_cll: Pointer to an HDR10 Content Light Level structure.
> > * @p_hdr10_mastering: Pointer to an HDR10 Mastering Display structure.
> > * @p_area: Pointer to an area.
> > + * @p_rect: Pointer to a rectangle.
> > * @p: Pointer to a compound value.
> > * @p_const: Pointer to a constant compound value.
> > */
> > @@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
> > struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
> > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > struct v4l2_area *p_area;
> > + struct v4l2_rect *p_rect;
> > void *p;
> > const void *p_const;
> > };
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index bb40129446d4..499fcddb6254 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -1008,6 +1008,8 @@ enum v4l2_auto_focus_range {
> >
> > #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
> >
> > +#define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> > +
> > /* FM Modulator class control IDs */
> >
> > #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 3768a0a80830..b712412cf763 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
> > __u16 __user *p_u16;
> > __u32 __user *p_u32;
> > struct v4l2_area __user *p_area;
> > + struct v4l2_rect __user *p_rect;
> > struct v4l2_ctrl_h264_sps __user *p_h264_sps;
> > struct v4l2_ctrl_h264_pps *p_h264_pps;
> > struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
> > @@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
> > V4L2_CTRL_TYPE_U16 = 0x0101,
> > V4L2_CTRL_TYPE_U32 = 0x0102,
> > V4L2_CTRL_TYPE_AREA = 0x0106,
> > + V4L2_CTRL_TYPE_RECT = 0x0107,
> >
> > V4L2_CTRL_TYPE_HDR10_CLL_INFO = 0x0110,
> > V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY = 0x0111,
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control
2022-05-16 14:04 ` [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control Yunke Cao
@ 2022-05-17 13:23 ` Nicolas Dufresne
2022-05-18 1:11 ` Yunke Cao
0 siblings, 1 reply; 15+ messages in thread
From: Nicolas Dufresne @ 2022-05-17 13:23 UTC (permalink / raw)
To: Yunke Cao, Hans Verkuil, Laurent Pinchart
Cc: Tomasz Figa, Sergey Senozhatsky, Ricardo Ribalda, linux-media
Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> Signed-off-by: Yunke Cao <yunkec@google.com>
> ---
> .../media/v4l/ext-ctrls-camera.rst | 25 +++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
> include/uapi/linux/v4l2-controls.h | 9 +++++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> index 86a1f09a8a1c..3da66e1e1fc7 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> @@ -665,3 +665,28 @@ enum v4l2_scene_mode -
> ``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> This control determines the region of interest. Region of interest is an
> rectangular area represented by a struct v4l2_rect.
> +
> +``V4L2_CID_REGION_OF_INTEREST_AUTO (bitmask)``
> + This determines which, if any, on board features should track to the
> + Region of Interest.
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> +
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE``
> + - Auto Exposure.
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS``
> + - Auto Iris.
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE``
> + - Auto White Balance.
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS``
> + - Auto Focus.
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT``
> + - Auto Face Detect.
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK``
> + - Auto Detect and Track.
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION``
> + - Image Stabilization.
> + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
> + - Higher Quality.
Now I see the usage, the control is missing cross-reference and behaviour.
Consider that someone may have to use or implement your feature on different HW
and in different context in the future. Right now you aren't writing a
specification, but barely listing things that are already encoded in the item
names. For each of this, add human readable prose that explain what is expected
behaviour when the bit is set. This way, future implementation can check their
behaviour and cross-over with the documentation to make sure it is a fit, or if
another bit need to be allocated.
I still believe REGION_OF_INTEREST is too generic of a name for the purpose, as
in the context of the V4L2 API, we also support video encoders, and none of this
(perhaps except QUALITY, but with encoder you have to specify a delta for that).
The name really needs to be specialized to be implemented this way. Otherwise,
it create confusion, and makes the V4L2 uAPI poorer over time. Naming is hard,
but I need to make a suggestion, perhaps CAMERA_ROI ? We have classes, perhaps a
class for the CAMERA controls is needed ?
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 95f39a2d2ad2..f28763bf95e9 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1043,6 +1043,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
> + case V4L2_CID_REGION_OF_INTEREST_AUTO: return "Region Of Interest Auto Controls";
>
> /* FM Radio Modulator controls */
> /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1415,6 +1416,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> case V4L2_CID_JPEG_ACTIVE_MARKER:
> case V4L2_CID_3A_LOCK:
> case V4L2_CID_AUTO_FOCUS_STATUS:
> + case V4L2_CID_REGION_OF_INTEREST_AUTO:
> case V4L2_CID_DV_TX_HOTPLUG:
> case V4L2_CID_DV_TX_RXSENSE:
> case V4L2_CID_DV_TX_EDID_PRESENT:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 499fcddb6254..f6938e4de129 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1009,6 +1009,15 @@ enum v4l2_auto_focus_range {
> #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
>
> #define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO (V4L2_CID_CAMERA_CLASS_BASE+37)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE (1 << 0)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS (1 << 1)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE (1 << 2)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS (1 << 3)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT (1 << 4)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK (1 << 5)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION (1 << 6)
> +#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY (1 << 7)
>
> /* FM Modulator class control IDs */
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control
2022-05-17 13:23 ` Nicolas Dufresne
@ 2022-05-18 1:11 ` Yunke Cao
2022-05-19 18:26 ` Nicolas Dufresne
0 siblings, 1 reply; 15+ messages in thread
From: Yunke Cao @ 2022-05-18 1:11 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Hans Verkuil, Laurent Pinchart, Tomasz Figa, Sergey Senozhatsky,
Ricardo Ribalda, linux-media
Thanks, I will add detailed behavior in the follow up version.
On Tue, May 17, 2022 at 10:23 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> > Signed-off-by: Yunke Cao <yunkec@google.com>
> > ---
> > .../media/v4l/ext-ctrls-camera.rst | 25 +++++++++++++++++++
> > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
> > include/uapi/linux/v4l2-controls.h | 9 +++++++
> > 3 files changed, 36 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > index 86a1f09a8a1c..3da66e1e1fc7 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > @@ -665,3 +665,28 @@ enum v4l2_scene_mode -
> > ``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> > This control determines the region of interest. Region of interest is an
> > rectangular area represented by a struct v4l2_rect.
> > +
> > +``V4L2_CID_REGION_OF_INTEREST_AUTO (bitmask)``
> > + This determines which, if any, on board features should track to the
> > + Region of Interest.
> > +
> > +.. flat-table::
> > + :header-rows: 0
> > + :stub-columns: 0
> > +
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE``
> > + - Auto Exposure.
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS``
> > + - Auto Iris.
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE``
> > + - Auto White Balance.
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS``
> > + - Auto Focus.
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT``
> > + - Auto Face Detect.
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK``
> > + - Auto Detect and Track.
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION``
> > + - Image Stabilization.
> > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
> > + - Higher Quality.
>
> Now I see the usage, the control is missing cross-reference and behaviour.
> Consider that someone may have to use or implement your feature on different HW
> and in different context in the future. Right now you aren't writing a
> specification, but barely listing things that are already encoded in the item
> names. For each of this, add human readable prose that explain what is expected
> behaviour when the bit is set. This way, future implementation can check their
> behaviour and cross-over with the documentation to make sure it is a fit, or if
> another bit need to be allocated.
>
> I still believe REGION_OF_INTEREST is too generic of a name for the purpose, as
> in the context of the V4L2 API, we also support video encoders, and none of this
> (perhaps except QUALITY, but with encoder you have to specify a delta for that).
> The name really needs to be specialized to be implemented this way. Otherwise,
> it create confusion, and makes the V4L2 uAPI poorer over time. Naming is hard,
> but I need to make a suggestion, perhaps CAMERA_ROI ? We have classes, perhaps a
> class for the CAMERA controls is needed ?
>
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > index 95f39a2d2ad2..f28763bf95e9 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > @@ -1043,6 +1043,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> > case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> > case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
> > + case V4L2_CID_REGION_OF_INTEREST_AUTO: return "Region Of Interest Auto Controls";
> >
> > /* FM Radio Modulator controls */
> > /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> > @@ -1415,6 +1416,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_JPEG_ACTIVE_MARKER:
> > case V4L2_CID_3A_LOCK:
> > case V4L2_CID_AUTO_FOCUS_STATUS:
> > + case V4L2_CID_REGION_OF_INTEREST_AUTO:
> > case V4L2_CID_DV_TX_HOTPLUG:
> > case V4L2_CID_DV_TX_RXSENSE:
> > case V4L2_CID_DV_TX_EDID_PRESENT:
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index 499fcddb6254..f6938e4de129 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -1009,6 +1009,15 @@ enum v4l2_auto_focus_range {
> > #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
> >
> > #define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO (V4L2_CID_CAMERA_CLASS_BASE+37)
The ROI controls are in V4L2_CID_CAMERA_CLASS
"(V4L2_CID_CAMERA_CLASS_BASE+36)" unless I miss anything.
Correct me if I'm wrong but I don't see "CAMERA" included in the name
of other camera controls.
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE (1 << 0)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS (1 << 1)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE (1 << 2)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS (1 << 3)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT (1 << 4)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK (1 << 5)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION (1 << 6)
> > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY (1 << 7)
> >
> > /* FM Modulator class control IDs */
> >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control
2022-05-16 19:02 ` Nicolas Dufresne
2022-05-17 7:11 ` Yunke Cao
@ 2022-05-18 5:19 ` Tomasz Figa
2022-05-19 18:23 ` Nicolas Dufresne
1 sibling, 1 reply; 15+ messages in thread
From: Tomasz Figa @ 2022-05-18 5:19 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Yunke Cao, Hans Verkuil, Laurent Pinchart, Sergey Senozhatsky,
Ricardo Ribalda, linux-media
Hi Nicolas,
On Tue, May 17, 2022 at 4:02 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Hi,
>
> thanks for working on this, see my comments below ...
>
> Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> > Including:
> > 1. Add a control ID.
> > 2. Add p_rect to struct v4l2_ext_control with basic support in
> > v4l2-ctrls.
> >
> > Signed-off-by: Yunke Cao <yunkec@google.com>
> > ---
> > .../media/v4l/ext-ctrls-camera.rst | 4 ++++
> > .../media/v4l/vidioc-g-ext-ctrls.rst | 4 ++++
> > .../media/videodev2.h.rst.exceptions | 1 +
> > drivers/media/v4l2-core/v4l2-ctrls-core.c | 20 +++++++++++++++++++
> > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 ++++
> > include/media/v4l2-ctrls.h | 2 ++
> > include/uapi/linux/v4l2-controls.h | 2 ++
> > include/uapi/linux/videodev2.h | 2 ++
> > 8 files changed, 39 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > index 4c5061aa9cd4..86a1f09a8a1c 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > @@ -661,3 +661,7 @@ enum v4l2_scene_mode -
> > .. [#f1]
> > This control may be changed to a menu control in the future, if more
> > options are required.
> > +
> > +``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> > + This control determines the region of interest. Region of interest is an
> > + rectangular area represented by a struct v4l2_rect.
>
> This control documentation is missing some important information. Notably, what
> will happen if this rectangle is set ? Is there a value to unset it ?
>
Thanks for the review!
In V4L2 all the controls are always set to something. There is no way
to unset a control. Since controls have default values, perhaps the
way to "unset" (or rather reset it to the initial configuration) is to
set it to the default value?
> The name is very generic and I would expect that to be usable in general. But it
> won't work for encoders, as you only allow 1 rectangle and it would be missing
> some QP delta parameter. I think I would prefer if we specialize this type of
> control a bit more. In your case, I'm guessing you only care about 1 ROI when
> taking a picture, and this ROI will be used for automatic focus. If my guess is
> right, perhaps a FOCUS_AERA could be a better name ?
I wonder if an array control is what we need here to make it flexible
enough? For a UVC camera obviously we would only need 1 element, but
other devices could accept more.
Best regards,
Tomasz
>
> regards,
> Nicolas
>
> > diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > index 29971a45a2d4..f4e205ead0a2 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > @@ -189,6 +189,10 @@ still cause this situation.
> > - ``p_area``
> > - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
> > of type ``V4L2_CTRL_TYPE_AREA``.
> > + * - struct :c:type:`v4l2_rect` *
> > + - ``p_area``
> > + - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
> > + of type ``V4L2_CTRL_TYPE_RECT``.
> > * - struct :c:type:`v4l2_ctrl_h264_sps` *
> > - ``p_h264_sps``
> > - A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
> > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > index 9cbb7a0c354a..7b423475281d 100644
> > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > @@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
> > +replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
> > replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > index 8968cec8454e..dcde405c2713 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > @@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
> > return ptr1.p_u16[idx] == ptr2.p_u16[idx];
> > case V4L2_CTRL_TYPE_U32:
> > return ptr1.p_u32[idx] == ptr2.p_u32[idx];
> > + case V4L2_CTRL_TYPE_RECT:
> > + return ptr1.p_rect->top == ptr2.p_rect->top &&
> > + ptr1.p_rect->left == ptr2.p_rect->left &&
> > + ptr1.p_rect->height == ptr2.p_rect->height &&
> > + ptr1.p_rect->width == ptr2.p_rect->width;
> > default:
> > if (ctrl->is_int)
> > return ptr1.p_s32[idx] == ptr2.p_s32[idx];
> > @@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> > case V4L2_CTRL_TYPE_VP9_FRAME:
> > pr_cont("VP9_FRAME");
> > break;
> > + case V4L2_CTRL_TYPE_RECT:
> > + pr_cont("l: %d, t: %d, w: %u, h: %u",
> > + ptr.p_rect->left, ptr.p_rect->top,
> > + ptr.p_rect->width, ptr.p_rect->height);
> > + break;
> > default:
> > pr_cont("unknown type %d", ctrl->type);
> > break;
> > @@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
> > struct v4l2_area *area;
> > + struct v4l2_rect *rect;
> > void *p = ptr.p + idx * ctrl->elem_size;
> > unsigned int i;
> >
> > @@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > return -EINVAL;
> > break;
> >
> > + case V4L2_CTRL_TYPE_RECT:
> > + rect = p;
> > + if (!rect->width || !rect->height)
> > + return -EINVAL;
> > + break;
> > +
> > default:
> > return -EINVAL;
> > }
> > @@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> > case V4L2_CTRL_TYPE_AREA:
> > elem_size = sizeof(struct v4l2_area);
> > break;
> > + case V4L2_CTRL_TYPE_RECT:
> > + elem_size = sizeof(struct v4l2_rect);
> > + break;
> > default:
> > if (type < V4L2_CTRL_COMPOUND_TYPES)
> > elem_size = sizeof(s32);
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > index 54ca4e6b820b..95f39a2d2ad2 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > @@ -1042,6 +1042,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_UNIT_CELL_SIZE: return "Unit Cell Size";
> > case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> > case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> > + case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
> >
> > /* FM Radio Modulator controls */
> > /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> > @@ -1524,6 +1525,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
> > *type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
> > break;
> > + case V4L2_CID_REGION_OF_INTEREST_RECT:
> > + *type = V4L2_CTRL_TYPE_RECT;
> > + break;
> > default:
> > *type = V4L2_CTRL_TYPE_INTEGER;
> > break;
> > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > index b3ce438f1329..919e104de50b 100644
> > --- a/include/media/v4l2-ctrls.h
> > +++ b/include/media/v4l2-ctrls.h
> > @@ -58,6 +58,7 @@ struct video_device;
> > * @p_hdr10_cll: Pointer to an HDR10 Content Light Level structure.
> > * @p_hdr10_mastering: Pointer to an HDR10 Mastering Display structure.
> > * @p_area: Pointer to an area.
> > + * @p_rect: Pointer to a rectangle.
> > * @p: Pointer to a compound value.
> > * @p_const: Pointer to a constant compound value.
> > */
> > @@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
> > struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
> > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > struct v4l2_area *p_area;
> > + struct v4l2_rect *p_rect;
> > void *p;
> > const void *p_const;
> > };
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index bb40129446d4..499fcddb6254 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -1008,6 +1008,8 @@ enum v4l2_auto_focus_range {
> >
> > #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
> >
> > +#define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> > +
> > /* FM Modulator class control IDs */
> >
> > #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 3768a0a80830..b712412cf763 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
> > __u16 __user *p_u16;
> > __u32 __user *p_u32;
> > struct v4l2_area __user *p_area;
> > + struct v4l2_rect __user *p_rect;
> > struct v4l2_ctrl_h264_sps __user *p_h264_sps;
> > struct v4l2_ctrl_h264_pps *p_h264_pps;
> > struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
> > @@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
> > V4L2_CTRL_TYPE_U16 = 0x0101,
> > V4L2_CTRL_TYPE_U32 = 0x0102,
> > V4L2_CTRL_TYPE_AREA = 0x0106,
> > + V4L2_CTRL_TYPE_RECT = 0x0107,
> >
> > V4L2_CTRL_TYPE_HDR10_CLL_INFO = 0x0110,
> > V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY = 0x0111,
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control
2022-05-18 5:19 ` Tomasz Figa
@ 2022-05-19 18:23 ` Nicolas Dufresne
2022-05-20 2:44 ` Tomasz Figa
0 siblings, 1 reply; 15+ messages in thread
From: Nicolas Dufresne @ 2022-05-19 18:23 UTC (permalink / raw)
To: Tomasz Figa
Cc: Yunke Cao, Hans Verkuil, Laurent Pinchart, Sergey Senozhatsky,
Ricardo Ribalda, linux-media
Le mercredi 18 mai 2022 à 14:19 +0900, Tomasz Figa a écrit :
> Hi Nicolas,
>
> On Tue, May 17, 2022 at 4:02 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> >
> > Hi,
> >
> > thanks for working on this, see my comments below ...
> >
> > Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> > > Including:
> > > 1. Add a control ID.
> > > 2. Add p_rect to struct v4l2_ext_control with basic support in
> > > v4l2-ctrls.
> > >
> > > Signed-off-by: Yunke Cao <yunkec@google.com>
> > > ---
> > > .../media/v4l/ext-ctrls-camera.rst | 4 ++++
> > > .../media/v4l/vidioc-g-ext-ctrls.rst | 4 ++++
> > > .../media/videodev2.h.rst.exceptions | 1 +
> > > drivers/media/v4l2-core/v4l2-ctrls-core.c | 20 +++++++++++++++++++
> > > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 ++++
> > > include/media/v4l2-ctrls.h | 2 ++
> > > include/uapi/linux/v4l2-controls.h | 2 ++
> > > include/uapi/linux/videodev2.h | 2 ++
> > > 8 files changed, 39 insertions(+)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > index 4c5061aa9cd4..86a1f09a8a1c 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > @@ -661,3 +661,7 @@ enum v4l2_scene_mode -
> > > .. [#f1]
> > > This control may be changed to a menu control in the future, if more
> > > options are required.
> > > +
> > > +``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> > > + This control determines the region of interest. Region of interest is an
> > > + rectangular area represented by a struct v4l2_rect.
> >
> > This control documentation is missing some important information. Notably, what
> > will happen if this rectangle is set ? Is there a value to unset it ?
> >
>
> Thanks for the review!
>
> In V4L2 all the controls are always set to something. There is no way
> to unset a control. Since controls have default values, perhaps the
> way to "unset" (or rather reset it to the initial configuration) is to
> set it to the default value?
That's why a lot of similar controls comes with a companion boolean control to
enable/disable the feature, or an enum that specify what else is to be used,
could be an "automatic" default or something.
>
> > The name is very generic and I would expect that to be usable in general. But it
> > won't work for encoders, as you only allow 1 rectangle and it would be missing
> > some QP delta parameter. I think I would prefer if we specialize this type of
> > control a bit more. In your case, I'm guessing you only care about 1 ROI when
> > taking a picture, and this ROI will be used for automatic focus. If my guess is
> > right, perhaps a FOCUS_AERA could be a better name ?
>
> I wonder if an array control is what we need here to make it flexible
> enough? For a UVC camera obviously we would only need 1 element, but
> other devices could accept more.
See Hans and Laurent's comment on V3. They don't believe camera region of
interest is likely to exists outside of UVC, and suggested making all these
control entirely UVC specific, which also allow making it 1:1 with UVC without
any headache.
And this way we don't need to think about encoder ROI just yet.
>
> Best regards,
> Tomasz
>
> >
> > regards,
> > Nicolas
> >
> > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > > index 29971a45a2d4..f4e205ead0a2 100644
> > > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > > @@ -189,6 +189,10 @@ still cause this situation.
> > > - ``p_area``
> > > - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
> > > of type ``V4L2_CTRL_TYPE_AREA``.
> > > + * - struct :c:type:`v4l2_rect` *
> > > + - ``p_area``
> > > + - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
> > > + of type ``V4L2_CTRL_TYPE_RECT``.
> > > * - struct :c:type:`v4l2_ctrl_h264_sps` *
> > > - ``p_h264_sps``
> > > - A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
> > > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > index 9cbb7a0c354a..7b423475281d 100644
> > > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > @@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
> > > replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
> > > replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
> > > replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
> > > +replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
> > > replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
> > > replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
> > > replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
> > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > > index 8968cec8454e..dcde405c2713 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > > @@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
> > > return ptr1.p_u16[idx] == ptr2.p_u16[idx];
> > > case V4L2_CTRL_TYPE_U32:
> > > return ptr1.p_u32[idx] == ptr2.p_u32[idx];
> > > + case V4L2_CTRL_TYPE_RECT:
> > > + return ptr1.p_rect->top == ptr2.p_rect->top &&
> > > + ptr1.p_rect->left == ptr2.p_rect->left &&
> > > + ptr1.p_rect->height == ptr2.p_rect->height &&
> > > + ptr1.p_rect->width == ptr2.p_rect->width;
> > > default:
> > > if (ctrl->is_int)
> > > return ptr1.p_s32[idx] == ptr2.p_s32[idx];
> > > @@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> > > case V4L2_CTRL_TYPE_VP9_FRAME:
> > > pr_cont("VP9_FRAME");
> > > break;
> > > + case V4L2_CTRL_TYPE_RECT:
> > > + pr_cont("l: %d, t: %d, w: %u, h: %u",
> > > + ptr.p_rect->left, ptr.p_rect->top,
> > > + ptr.p_rect->width, ptr.p_rect->height);
> > > + break;
> > > default:
> > > pr_cont("unknown type %d", ctrl->type);
> > > break;
> > > @@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > > struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
> > > struct v4l2_area *area;
> > > + struct v4l2_rect *rect;
> > > void *p = ptr.p + idx * ctrl->elem_size;
> > > unsigned int i;
> > >
> > > @@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > > return -EINVAL;
> > > break;
> > >
> > > + case V4L2_CTRL_TYPE_RECT:
> > > + rect = p;
> > > + if (!rect->width || !rect->height)
> > > + return -EINVAL;
> > > + break;
> > > +
> > > default:
> > > return -EINVAL;
> > > }
> > > @@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> > > case V4L2_CTRL_TYPE_AREA:
> > > elem_size = sizeof(struct v4l2_area);
> > > break;
> > > + case V4L2_CTRL_TYPE_RECT:
> > > + elem_size = sizeof(struct v4l2_rect);
> > > + break;
> > > default:
> > > if (type < V4L2_CTRL_COMPOUND_TYPES)
> > > elem_size = sizeof(s32);
> > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > index 54ca4e6b820b..95f39a2d2ad2 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > @@ -1042,6 +1042,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > > case V4L2_CID_UNIT_CELL_SIZE: return "Unit Cell Size";
> > > case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> > > case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> > > + case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
> > >
> > > /* FM Radio Modulator controls */
> > > /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> > > @@ -1524,6 +1525,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > > case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
> > > *type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
> > > break;
> > > + case V4L2_CID_REGION_OF_INTEREST_RECT:
> > > + *type = V4L2_CTRL_TYPE_RECT;
> > > + break;
> > > default:
> > > *type = V4L2_CTRL_TYPE_INTEGER;
> > > break;
> > > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > > index b3ce438f1329..919e104de50b 100644
> > > --- a/include/media/v4l2-ctrls.h
> > > +++ b/include/media/v4l2-ctrls.h
> > > @@ -58,6 +58,7 @@ struct video_device;
> > > * @p_hdr10_cll: Pointer to an HDR10 Content Light Level structure.
> > > * @p_hdr10_mastering: Pointer to an HDR10 Mastering Display structure.
> > > * @p_area: Pointer to an area.
> > > + * @p_rect: Pointer to a rectangle.
> > > * @p: Pointer to a compound value.
> > > * @p_const: Pointer to a constant compound value.
> > > */
> > > @@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
> > > struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
> > > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > > struct v4l2_area *p_area;
> > > + struct v4l2_rect *p_rect;
> > > void *p;
> > > const void *p_const;
> > > };
> > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > > index bb40129446d4..499fcddb6254 100644
> > > --- a/include/uapi/linux/v4l2-controls.h
> > > +++ b/include/uapi/linux/v4l2-controls.h
> > > @@ -1008,6 +1008,8 @@ enum v4l2_auto_focus_range {
> > >
> > > #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
> > >
> > > +#define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> > > +
> > > /* FM Modulator class control IDs */
> > >
> > > #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
> > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > index 3768a0a80830..b712412cf763 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
> > > __u16 __user *p_u16;
> > > __u32 __user *p_u32;
> > > struct v4l2_area __user *p_area;
> > > + struct v4l2_rect __user *p_rect;
> > > struct v4l2_ctrl_h264_sps __user *p_h264_sps;
> > > struct v4l2_ctrl_h264_pps *p_h264_pps;
> > > struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
> > > @@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
> > > V4L2_CTRL_TYPE_U16 = 0x0101,
> > > V4L2_CTRL_TYPE_U32 = 0x0102,
> > > V4L2_CTRL_TYPE_AREA = 0x0106,
> > > + V4L2_CTRL_TYPE_RECT = 0x0107,
> > >
> > > V4L2_CTRL_TYPE_HDR10_CLL_INFO = 0x0110,
> > > V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY = 0x0111,
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control
2022-05-18 1:11 ` Yunke Cao
@ 2022-05-19 18:26 ` Nicolas Dufresne
0 siblings, 0 replies; 15+ messages in thread
From: Nicolas Dufresne @ 2022-05-19 18:26 UTC (permalink / raw)
To: Yunke Cao
Cc: Hans Verkuil, Laurent Pinchart, Tomasz Figa, Sergey Senozhatsky,
Ricardo Ribalda, linux-media
Le mercredi 18 mai 2022 à 10:11 +0900, Yunke Cao a écrit :
> Thanks, I will add detailed behavior in the follow up version.
>
> On Tue, May 17, 2022 at 10:23 PM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> >
> > Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> > > Signed-off-by: Yunke Cao <yunkec@google.com>
> > > ---
> > > .../media/v4l/ext-ctrls-camera.rst | 25 +++++++++++++++++++
> > > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
> > > include/uapi/linux/v4l2-controls.h | 9 +++++++
> > > 3 files changed, 36 insertions(+)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > index 86a1f09a8a1c..3da66e1e1fc7 100644
> > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > @@ -665,3 +665,28 @@ enum v4l2_scene_mode -
> > > ``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> > > This control determines the region of interest. Region of interest is an
> > > rectangular area represented by a struct v4l2_rect.
> > > +
> > > +``V4L2_CID_REGION_OF_INTEREST_AUTO (bitmask)``
> > > + This determines which, if any, on board features should track to the
> > > + Region of Interest.
> > > +
> > > +.. flat-table::
> > > + :header-rows: 0
> > > + :stub-columns: 0
> > > +
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE``
> > > + - Auto Exposure.
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS``
> > > + - Auto Iris.
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE``
> > > + - Auto White Balance.
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS``
> > > + - Auto Focus.
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT``
> > > + - Auto Face Detect.
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK``
> > > + - Auto Detect and Track.
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION``
> > > + - Image Stabilization.
> > > + * - ``V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY``
> > > + - Higher Quality.
> >
> > Now I see the usage, the control is missing cross-reference and behaviour.
> > Consider that someone may have to use or implement your feature on different HW
> > and in different context in the future. Right now you aren't writing a
> > specification, but barely listing things that are already encoded in the item
> > names. For each of this, add human readable prose that explain what is expected
> > behaviour when the bit is set. This way, future implementation can check their
> > behaviour and cross-over with the documentation to make sure it is a fit, or if
> > another bit need to be allocated.
> >
> > I still believe REGION_OF_INTEREST is too generic of a name for the purpose, as
> > in the context of the V4L2 API, we also support video encoders, and none of this
> > (perhaps except QUALITY, but with encoder you have to specify a delta for that).
> > The name really needs to be specialized to be implemented this way. Otherwise,
> > it create confusion, and makes the V4L2 uAPI poorer over time. Naming is hard,
> > but I need to make a suggestion, perhaps CAMERA_ROI ? We have classes, perhaps a
> > class for the CAMERA controls is needed ?
> >
> > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > index 95f39a2d2ad2..f28763bf95e9 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > @@ -1043,6 +1043,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > > case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> > > case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> > > case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
> > > + case V4L2_CID_REGION_OF_INTEREST_AUTO: return "Region Of Interest Auto Controls";
> > >
> > > /* FM Radio Modulator controls */
> > > /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> > > @@ -1415,6 +1416,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > > case V4L2_CID_JPEG_ACTIVE_MARKER:
> > > case V4L2_CID_3A_LOCK:
> > > case V4L2_CID_AUTO_FOCUS_STATUS:
> > > + case V4L2_CID_REGION_OF_INTEREST_AUTO:
> > > case V4L2_CID_DV_TX_HOTPLUG:
> > > case V4L2_CID_DV_TX_RXSENSE:
> > > case V4L2_CID_DV_TX_EDID_PRESENT:
> > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > > index 499fcddb6254..f6938e4de129 100644
> > > --- a/include/uapi/linux/v4l2-controls.h
> > > +++ b/include/uapi/linux/v4l2-controls.h
> > > @@ -1009,6 +1009,15 @@ enum v4l2_auto_focus_range {
> > > #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
> > >
> > > #define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO (V4L2_CID_CAMERA_CLASS_BASE+37)
>
> The ROI controls are in V4L2_CID_CAMERA_CLASS
> "(V4L2_CID_CAMERA_CLASS_BASE+36)" unless I miss anything.
> Correct me if I'm wrong but I don't see "CAMERA" included in the name
> of other camera controls.
Namespace should be extended when its obviously going to clash with other
similar concept. In this case it obviously clash with video encoder ROI.
regards,
Nicolas
>
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_EXPOSURE (1 << 0)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_IRIS (1 << 1)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_WHITE_BALANCE (1 << 2)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_FOCUS (1 << 3)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_FACE_DETECT (1 << 4)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_DETECT_AND_TRACK (1 << 5)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_IMAGE_STABILIZATION (1 << 6)
> > > +#define V4L2_CID_REGION_OF_INTEREST_AUTO_HIGHER_QUALITY (1 << 7)
> > >
> > > /* FM Modulator class control IDs */
> > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control
2022-05-19 18:23 ` Nicolas Dufresne
@ 2022-05-20 2:44 ` Tomasz Figa
0 siblings, 0 replies; 15+ messages in thread
From: Tomasz Figa @ 2022-05-20 2:44 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Yunke Cao, Hans Verkuil, Laurent Pinchart, Sergey Senozhatsky,
Ricardo Ribalda, linux-media
On Fri, May 20, 2022 at 3:23 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Le mercredi 18 mai 2022 à 14:19 +0900, Tomasz Figa a écrit :
> > Hi Nicolas,
> >
> > On Tue, May 17, 2022 at 4:02 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > >
> > > Hi,
> > >
> > > thanks for working on this, see my comments below ...
> > >
> > > Le lundi 16 mai 2022 à 23:04 +0900, Yunke Cao a écrit :
> > > > Including:
> > > > 1. Add a control ID.
> > > > 2. Add p_rect to struct v4l2_ext_control with basic support in
> > > > v4l2-ctrls.
> > > >
> > > > Signed-off-by: Yunke Cao <yunkec@google.com>
> > > > ---
> > > > .../media/v4l/ext-ctrls-camera.rst | 4 ++++
> > > > .../media/v4l/vidioc-g-ext-ctrls.rst | 4 ++++
> > > > .../media/videodev2.h.rst.exceptions | 1 +
> > > > drivers/media/v4l2-core/v4l2-ctrls-core.c | 20 +++++++++++++++++++
> > > > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 ++++
> > > > include/media/v4l2-ctrls.h | 2 ++
> > > > include/uapi/linux/v4l2-controls.h | 2 ++
> > > > include/uapi/linux/videodev2.h | 2 ++
> > > > 8 files changed, 39 insertions(+)
> > > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > > index 4c5061aa9cd4..86a1f09a8a1c 100644
> > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> > > > @@ -661,3 +661,7 @@ enum v4l2_scene_mode -
> > > > .. [#f1]
> > > > This control may be changed to a menu control in the future, if more
> > > > options are required.
> > > > +
> > > > +``V4L2_CID_REGION_OF_INTEREST_RECT (struct)``
> > > > + This control determines the region of interest. Region of interest is an
> > > > + rectangular area represented by a struct v4l2_rect.
> > >
> > > This control documentation is missing some important information. Notably, what
> > > will happen if this rectangle is set ? Is there a value to unset it ?
> > >
> >
> > Thanks for the review!
> >
> > In V4L2 all the controls are always set to something. There is no way
> > to unset a control. Since controls have default values, perhaps the
> > way to "unset" (or rather reset it to the initial configuration) is to
> > set it to the default value?
>
> That's why a lot of similar controls comes with a companion boolean control to
> enable/disable the feature, or an enum that specify what else is to be used,
> could be an "automatic" default or something.
>
True. I think the other bitmask control is the companion control in
this case, but it indeed wouldn't work for other use cases.
> >
> > > The name is very generic and I would expect that to be usable in general. But it
> > > won't work for encoders, as you only allow 1 rectangle and it would be missing
> > > some QP delta parameter. I think I would prefer if we specialize this type of
> > > control a bit more. In your case, I'm guessing you only care about 1 ROI when
> > > taking a picture, and this ROI will be used for automatic focus. If my guess is
> > > right, perhaps a FOCUS_AERA could be a better name ?
> >
> > I wonder if an array control is what we need here to make it flexible
> > enough? For a UVC camera obviously we would only need 1 element, but
> > other devices could accept more.
>
> See Hans and Laurent's comment on V3. They don't believe camera region of
> interest is likely to exists outside of UVC, and suggested making all these
> control entirely UVC specific, which also allow making it 1:1 with UVC without
> any headache.
>
> And this way we don't need to think about encoder ROI just yet.
>
Okay, I think that also works for us. :)
Best regards,
Tomasz
> >
> > Best regards,
> > Tomasz
> >
> > >
> > > regards,
> > > Nicolas
> > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > > > index 29971a45a2d4..f4e205ead0a2 100644
> > > > --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> > > > @@ -189,6 +189,10 @@ still cause this situation.
> > > > - ``p_area``
> > > > - A pointer to a struct :c:type:`v4l2_area`. Valid if this control is
> > > > of type ``V4L2_CTRL_TYPE_AREA``.
> > > > + * - struct :c:type:`v4l2_rect` *
> > > > + - ``p_area``
> > > > + - A pointer to a struct :c:type:`v4l2_rect`. Valid if this control is
> > > > + of type ``V4L2_CTRL_TYPE_RECT``.
> > > > * - struct :c:type:`v4l2_ctrl_h264_sps` *
> > > > - ``p_h264_sps``
> > > > - A pointer to a struct :c:type:`v4l2_ctrl_h264_sps`. Valid if this control is
> > > > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > > index 9cbb7a0c354a..7b423475281d 100644
> > > > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > > @@ -147,6 +147,7 @@ replace symbol V4L2_CTRL_TYPE_HEVC_SPS :c:type:`v4l2_ctrl_type`
> > > > replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
> > > > replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
> > > > replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
> > > > +replace symbol V4L2_CTRL_TYPE_RECT :c:type:`v4l2_ctrl_type`
> > > > replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
> > > > replace symbol V4L2_CTRL_TYPE_VP8_FRAME :c:type:`v4l2_ctrl_type`
> > > > replace symbol V4L2_CTRL_TYPE_VP9_COMPRESSED_HDR :c:type:`v4l2_ctrl_type`
> > > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > > > index 8968cec8454e..dcde405c2713 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> > > > @@ -84,6 +84,11 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
> > > > return ptr1.p_u16[idx] == ptr2.p_u16[idx];
> > > > case V4L2_CTRL_TYPE_U32:
> > > > return ptr1.p_u32[idx] == ptr2.p_u32[idx];
> > > > + case V4L2_CTRL_TYPE_RECT:
> > > > + return ptr1.p_rect->top == ptr2.p_rect->top &&
> > > > + ptr1.p_rect->left == ptr2.p_rect->left &&
> > > > + ptr1.p_rect->height == ptr2.p_rect->height &&
> > > > + ptr1.p_rect->width == ptr2.p_rect->width;
> > > > default:
> > > > if (ctrl->is_int)
> > > > return ptr1.p_s32[idx] == ptr2.p_s32[idx];
> > > > @@ -307,6 +312,11 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> > > > case V4L2_CTRL_TYPE_VP9_FRAME:
> > > > pr_cont("VP9_FRAME");
> > > > break;
> > > > + case V4L2_CTRL_TYPE_RECT:
> > > > + pr_cont("l: %d, t: %d, w: %u, h: %u",
> > > > + ptr.p_rect->left, ptr.p_rect->top,
> > > > + ptr.p_rect->width, ptr.p_rect->height);
> > > > + break;
> > > > default:
> > > > pr_cont("unknown type %d", ctrl->type);
> > > > break;
> > > > @@ -525,6 +535,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > > > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > > > struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
> > > > struct v4l2_area *area;
> > > > + struct v4l2_rect *rect;
> > > > void *p = ptr.p + idx * ctrl->elem_size;
> > > > unsigned int i;
> > > >
> > > > @@ -888,6 +899,12 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
> > > > return -EINVAL;
> > > > break;
> > > >
> > > > + case V4L2_CTRL_TYPE_RECT:
> > > > + rect = p;
> > > > + if (!rect->width || !rect->height)
> > > > + return -EINVAL;
> > > > + break;
> > > > +
> > > > default:
> > > > return -EINVAL;
> > > > }
> > > > @@ -1456,6 +1473,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> > > > case V4L2_CTRL_TYPE_AREA:
> > > > elem_size = sizeof(struct v4l2_area);
> > > > break;
> > > > + case V4L2_CTRL_TYPE_RECT:
> > > > + elem_size = sizeof(struct v4l2_rect);
> > > > + break;
> > > > default:
> > > > if (type < V4L2_CTRL_COMPOUND_TYPES)
> > > > elem_size = sizeof(s32);
> > > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > > index 54ca4e6b820b..95f39a2d2ad2 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > > > @@ -1042,6 +1042,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > > > case V4L2_CID_UNIT_CELL_SIZE: return "Unit Cell Size";
> > > > case V4L2_CID_CAMERA_ORIENTATION: return "Camera Orientation";
> > > > case V4L2_CID_CAMERA_SENSOR_ROTATION: return "Camera Sensor Rotation";
> > > > + case V4L2_CID_REGION_OF_INTEREST_RECT: return "Region Of Interest Rectangle";
> > > >
> > > > /* FM Radio Modulator controls */
> > > > /* Keep the order of the 'case's the same as in v4l2-controls.h! */
> > > > @@ -1524,6 +1525,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > > > case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
> > > > *type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
> > > > break;
> > > > + case V4L2_CID_REGION_OF_INTEREST_RECT:
> > > > + *type = V4L2_CTRL_TYPE_RECT;
> > > > + break;
> > > > default:
> > > > *type = V4L2_CTRL_TYPE_INTEGER;
> > > > break;
> > > > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > > > index b3ce438f1329..919e104de50b 100644
> > > > --- a/include/media/v4l2-ctrls.h
> > > > +++ b/include/media/v4l2-ctrls.h
> > > > @@ -58,6 +58,7 @@ struct video_device;
> > > > * @p_hdr10_cll: Pointer to an HDR10 Content Light Level structure.
> > > > * @p_hdr10_mastering: Pointer to an HDR10 Mastering Display structure.
> > > > * @p_area: Pointer to an area.
> > > > + * @p_rect: Pointer to a rectangle.
> > > > * @p: Pointer to a compound value.
> > > > * @p_const: Pointer to a constant compound value.
> > > > */
> > > > @@ -87,6 +88,7 @@ union v4l2_ctrl_ptr {
> > > > struct v4l2_ctrl_hdr10_cll_info *p_hdr10_cll;
> > > > struct v4l2_ctrl_hdr10_mastering_display *p_hdr10_mastering;
> > > > struct v4l2_area *p_area;
> > > > + struct v4l2_rect *p_rect;
> > > > void *p;
> > > > const void *p_const;
> > > > };
> > > > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > > > index bb40129446d4..499fcddb6254 100644
> > > > --- a/include/uapi/linux/v4l2-controls.h
> > > > +++ b/include/uapi/linux/v4l2-controls.h
> > > > @@ -1008,6 +1008,8 @@ enum v4l2_auto_focus_range {
> > > >
> > > > #define V4L2_CID_CAMERA_SENSOR_ROTATION (V4L2_CID_CAMERA_CLASS_BASE+35)
> > > >
> > > > +#define V4L2_CID_REGION_OF_INTEREST_RECT (V4L2_CID_CAMERA_CLASS_BASE+36)
> > > > +
> > > > /* FM Modulator class control IDs */
> > > >
> > > > #define V4L2_CID_FM_TX_CLASS_BASE (V4L2_CTRL_CLASS_FM_TX | 0x900)
> > > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > > index 3768a0a80830..b712412cf763 100644
> > > > --- a/include/uapi/linux/videodev2.h
> > > > +++ b/include/uapi/linux/videodev2.h
> > > > @@ -1751,6 +1751,7 @@ struct v4l2_ext_control {
> > > > __u16 __user *p_u16;
> > > > __u32 __user *p_u32;
> > > > struct v4l2_area __user *p_area;
> > > > + struct v4l2_rect __user *p_rect;
> > > > struct v4l2_ctrl_h264_sps __user *p_h264_sps;
> > > > struct v4l2_ctrl_h264_pps *p_h264_pps;
> > > > struct v4l2_ctrl_h264_scaling_matrix __user *p_h264_scaling_matrix;
> > > > @@ -1810,6 +1811,7 @@ enum v4l2_ctrl_type {
> > > > V4L2_CTRL_TYPE_U16 = 0x0101,
> > > > V4L2_CTRL_TYPE_U32 = 0x0102,
> > > > V4L2_CTRL_TYPE_AREA = 0x0106,
> > > > + V4L2_CTRL_TYPE_RECT = 0x0107,
> > > >
> > > > V4L2_CTRL_TYPE_HDR10_CLL_INFO = 0x0110,
> > > > V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY = 0x0111,
> > >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-05-20 2:44 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16 14:04 [PATCH v2 0/6] media: Implement UVC v1.5 ROI Yunke Cao
2022-05-16 14:04 ` [PATCH v2 1/6] media: v4l2_ctrl: Add region of interest rectangle control Yunke Cao
2022-05-16 19:02 ` Nicolas Dufresne
2022-05-17 7:11 ` Yunke Cao
2022-05-18 5:19 ` Tomasz Figa
2022-05-19 18:23 ` Nicolas Dufresne
2022-05-20 2:44 ` Tomasz Figa
2022-05-16 14:04 ` [PATCH v2 2/6] media: v4l2_ctrl: Add region of interest auto control Yunke Cao
2022-05-17 13:23 ` Nicolas Dufresne
2022-05-18 1:11 ` Yunke Cao
2022-05-19 18:26 ` Nicolas Dufresne
2022-05-16 14:04 ` [PATCH v2 3/6] media: v4l2_ctrl: Add V4L2_CTRL_WHICH_MIN/MAX_VAL Yunke Cao
2022-05-16 14:04 ` [PATCH v2 4/6] media: uvcvideo: implement UVC v1.5 ROI Yunke Cao
2022-05-16 14:04 ` [PATCH v2 5/6] media: uvcvideo: Initialize roi to default value Yunke Cao
2022-05-16 14:04 ` [PATCH v2 6/6] media: vivid: Add a roi rectangle control Yunke Cao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.