Skip to contentSkip to conversation

media: add pixel_size control

PATCH Superseded Ricardo Ribalda Delgado 3 participants 10 messages Aug 19, 2019 – Aug 22, 2019

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Ricardo Ribalda Delgado

author maintainer
#0 lore raw

PATCH 1/3 media: add pixel_size control

This control returns the pixel size in nanometres. The struct provides the width and the height in separated fields to take into consideration asymmetric pixels and/or hardware binning.
This control is required for automatic calibration of the sensor.

---
 drivers/media/v4l2-core/v4l2-ctrls.c | 11 +++++++++++
 include/media/v4l2-ctrls.h           |  2 ++
 include/uapi/linux/v4l2-controls.h   |  3 +++
 include/uapi/linux/videodev2.h       | 11 +++++++++++
 4 files changed, 27 insertions(+)
drivers/media/v4l2-core/v4l2-ctrls.c +11 -0
index cd1ae016706f..a3a0086c96ff 100644
@@ -978,6 +978,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_AUTO_FOCUS_RANGE:		return "Auto Focus, Range";
 	case V4L2_CID_PAN_SPEED:		return "Pan, Speed";
 	case V4L2_CID_TILT_SPEED:		return "Tilt, Speed";
+	case V4L2_CID_PIXEL_SIZE:		return "Pixel Size";
 
 	/* FM Radio Modulator controls */
 	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1357,6 +1358,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
 		*type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
 		break;
+	case V4L2_CID_PIXEL_SIZE:
+		*type = V4L2_CTRL_TYPE_PIXEL_SIZE;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1423,6 +1427,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_RDS_RX_TRAFFIC_ANNOUNCEMENT:
 	case V4L2_CID_RDS_RX_TRAFFIC_PROGRAM:
 	case V4L2_CID_RDS_RX_MUSIC_SPEECH:
+	case V4L2_CID_PIXEL_SIZE:
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
 	case V4L2_CID_RF_TUNER_PLL_LOCK:
@@ -1705,6 +1710,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 	case V4L2_CTRL_TYPE_FWHT_PARAMS:
 		break;
 
+	case V4L2_CTRL_TYPE_PIXEL_SIZE:
+		break;
+
 	case V4L2_CTRL_TYPE_H264_SPS:
 	case V4L2_CTRL_TYPE_H264_PPS:
 	case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
@@ -2403,6 +2411,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
 		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
 		break;
+	case V4L2_CTRL_TYPE_PIXEL_SIZE:
+		elem_size = sizeof(struct v4l2_pixel_size);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);
include/media/v4l2-ctrls.h +2 -0
index 570ff4b0205a..63de780398b8 100644
@@ -50,6 +50,7 @@ struct poll_table_struct;
  * @p_h264_slice_params:	Pointer to a struct v4l2_ctrl_h264_slice_params.
  * @p_h264_decode_params:	Pointer to a struct v4l2_ctrl_h264_decode_params.
  * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
+ * @p_pixel_size:		Pointer to a pixel_size value.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -68,6 +69,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
 	struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
 	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
+	struct v4l2_pixel_size *p_pixel_size;
 	void *p;
 };
 
include/uapi/linux/v4l2-controls.h +3 -0
index a2669b79b294..13f0410df4c6 100644
@@ -912,6 +912,9 @@ enum v4l2_auto_focus_range {
 #define V4L2_CID_PAN_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+32)
 #define V4L2_CID_TILT_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+33)
 
+#define V4L2_CID_PIXEL_SIZE			(V4L2_CID_CAMERA_CLASS_BASE+34)
+
+
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
include/uapi/linux/videodev2.h +11 -0
index 2427bc4d8eba..21f4846dca0b 100644
@@ -422,6 +422,11 @@ struct v4l2_fract {
 	__u32   denominator;
 };
 
+struct v4l2_pixel_size {
+	__u32   width;
+	__u32   height;
+};
+
 /**
   * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
   *
@@ -1718,6 +1723,12 @@ enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_U8	     = 0x0100,
 	V4L2_CTRL_TYPE_U16	     = 0x0101,
 	V4L2_CTRL_TYPE_U32	     = 0x0102,
+	/*
+	 * V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
+	 * V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
+	 * V4L2_CTRL_TYPE_FWHT_PARAMS = 0x0105,
+	 */
+	V4L2_CTRL_TYPE_PIXEL_SIZE    = 0x0106,
 };
 
 /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
Signature · 1 line
2.23.0.rc1
author maintainer
#1 lore raw

PATCH 2/3 Documentation: Describe V4L2_CID_PIXEL_SIZE

New control to pass to userspace the width/height of a pixel. Which is needed for 3D calibration and lens selection.

---
 Documentation/media/uapi/v4l/ext-ctrls-camera.rst | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/media/uapi/v4l/ext-ctrls-camera.rst +6 -0
index 51c1d5c9eb00..670c57a6f622 100644
@@ -510,6 +510,12 @@ enum v4l2_scene_mode -
     value down. A value of zero stops the motion if one is in progress
     and has no effect otherwise.
 
+``V4L2_CID_PIXEL_SIZE (struct)``
+    This control returns the pixel size in nanometres. The struct provides
+    the width and the height in separated fields to take into consideration
+    asymmetric pixels and/or hardware binning.
+    This control is required for automatic calibration of the sensor.
+
 .. [#f1]
    This control may be changed to a menu control in the future, if more
    options are required.
Signature · 1 line
2.23.0.rc1
#2 lore raw

Re: [PATCH 2/3] Documentation: Describe V4L2_CID_PIXEL_SIZE

Quoted 15 lines · Ricardo Ribalda Delgado

New control to pass to userspace the width/height of a pixel. Which is needed for 3D calibration and lens selection.

---
 Documentation/media/uapi/v4l/ext-ctrls-camera.rst | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/media/uapi/v4l/ext-ctrls-camera.rst +0 -0
index 51c1d5c9eb00..670c57a6f622 100644
@@ -510,6 +510,12 @@ enum v4l2_scene_mode -
     value down. A value of zero stops the motion if one is in progress
     and has no effect otherwise.

Documentation/media/uapi/v4l/ext-ctrls-camera.rst enum v4l2_scene_mode - · line 521

Quoted 10 lines
Documentation/media/uapi/v4l/ext-ctrls-camera.rst +6 -0
 
+``V4L2_CID_PIXEL_SIZE (struct)``
+    This control returns the pixel size in nanometres. The struct provides
+    the width and the height in separated fields to take into consideration
+    asymmetric pixels and/or hardware binning.
+    This control is required for automatic calibration of the sensor.
+
 .. [#f1]
    This control may be changed to a menu control in the future, if more
    options are required.

I suppose this is a common term, but should it be mentioned that pixel size is the same as unit cell size, and not necessarily the size of the light sensitive area? Just in case the effective fill-factor is < 100%.

regards
Philipp

author maintainer
#3 lore raw

Re: [PATCH 2/3] Documentation: Describe V4L2_CID_PIXEL_SIZE

Completely agree. Maybe we should call the control unit cell size?, in case in the future we need a control for the light sensitive area.

Best regards

Quoted 34 lines · Philipp Zabel
Quoted 15 lines · Ricardo Ribalda Delgado

New control to pass to userspace the width/height of a pixel. Which is needed for 3D calibration and lens selection.

---
 Documentation/media/uapi/v4l/ext-ctrls-camera.rst | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/media/uapi/v4l/ext-ctrls-camera.rst +0 -0
index 51c1d5c9eb00..670c57a6f622 100644
@@ -510,6 +510,12 @@ enum v4l2_scene_mode -
     value down. A value of zero stops the motion if one is in progress
     and has no effect otherwise.

Documentation/media/uapi/v4l/ext-ctrls-camera.rst enum v4l2_scene_mode - · line 521

Quoted 10 lines
Documentation/media/uapi/v4l/ext-ctrls-camera.rst +6 -0
 
+``V4L2_CID_PIXEL_SIZE (struct)``
+    This control returns the pixel size in nanometres. The struct provides
+    the width and the height in separated fields to take into consideration
+    asymmetric pixels and/or hardware binning.
+    This control is required for automatic calibration of the sensor.
+
 .. [#f1]
    This control may be changed to a menu control in the future, if more
    options are required.

I suppose this is a common term, but should it be mentioned that pixel size is the same as unit cell size, and not necessarily the size of the light sensitive area? Just in case the effective fill-factor is < 100%.

regards
Philipp

author maintainer
#4 lore raw

PATCH 3/3 media: imx214: Add new control with V4L2_CID_PIXEL_SIZE

In reply to Ricardo Ribalda Delgado

According to the product brief, the unit cell size is 1120 nanometers^2.

https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf

---
 drivers/media/i2c/imx214.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
drivers/media/i2c/imx214.c +23 -0
index 159a3a604f0e..b2f6bd2d8d7d 100644
@@ -47,6 +47,7 @@ struct imx214 {
 	struct v4l2_ctrl *pixel_rate;
 	struct v4l2_ctrl *link_freq;
 	struct v4l2_ctrl *exposure;
+	struct v4l2_ctrl *pixel_size;
 
 	struct regulator_bulk_data	supplies[IMX214_NUM_SUPPLIES];
 
@@ -941,6 +942,26 @@ static int __maybe_unused imx214_resume(struct device *dev)
 	return ret;
 }
 
+static void pixel_size_init(const struct v4l2_ctrl *ctrl, u32 idx,
+		     union v4l2_ctrl_ptr ptr)
+{
+	ptr.p_pixel_size->width = 1120;
+	ptr.p_pixel_size->height = 1120;
+}
+
+static const struct v4l2_ctrl_type_ops pixel_size_ops = {
+	.init = pixel_size_init,
+};
+
+static struct v4l2_ctrl *new_pixel_size_ctrl(struct v4l2_ctrl_handler *handler)
+{
+	static struct v4l2_ctrl_config ctrl = {
+		.id = V4L2_CID_PIXEL_SIZE,
+		.type_ops = &pixel_size_ops,
+	};
+
+	return v4l2_ctrl_new_custom(handler, &ctrl, NULL);
+}
 static int imx214_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -1029,6 +1050,8 @@ static int imx214_probe(struct i2c_client *client)
 					     V4L2_CID_EXPOSURE,
 					     0, 3184, 1, 0x0c70);
 
+	imx214->pixel_size = new_pixel_size_ctrl(&imx214->ctrls);
+
 	ret = imx214->ctrls.error;
 	if (ret) {
 		dev_err(&client->dev, "%s control init failed (%d)\n",
Signature · 1 line
2.23.0.rc1
#5 lore raw

Re: [PATCH 3/3] media: imx214: Add new control with V4L2_CID_PIXEL_SIZE

Hi Ricardo,

Quoted 1 line · Ricardo Ribalda Delgado

According to the product brief, the unit cell size is 1120 nanometers^2.

Should this information come from DT ?

I'm asking as I've a series in review that adds an helper that collectes dt properties and register controls for them. It currently only supports the newly proposed camera location control, but there might be others like the rotation, for which we already have a DT property.

https://patchwork.kernel.org/project/linux-media/list/?series=160901

This new one is indeed an HW property of the sensor, I wonder if having it in the firmware interface would make any sense or not...

Thanks
j

Quoted 59 lines

https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf

---
 drivers/media/i2c/imx214.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
drivers/media/i2c/imx214.c +23 -1
index 159a3a604f0e..b2f6bd2d8d7d 100644
@@ -47,6 +47,7 @@ struct imx214 {
 	struct v4l2_ctrl *pixel_rate;
 	struct v4l2_ctrl *link_freq;
 	struct v4l2_ctrl *exposure;
+	struct v4l2_ctrl *pixel_size;
 
 	struct regulator_bulk_data	supplies[IMX214_NUM_SUPPLIES];
 
@@ -941,6 +942,26 @@ static int __maybe_unused imx214_resume(struct device *dev)
 	return ret;
 }
 
+static void pixel_size_init(const struct v4l2_ctrl *ctrl, u32 idx,
+		     union v4l2_ctrl_ptr ptr)
+{
+	ptr.p_pixel_size->width = 1120;
+	ptr.p_pixel_size->height = 1120;
+}
+
+static const struct v4l2_ctrl_type_ops pixel_size_ops = {
+	.init = pixel_size_init,
+};
+
+static struct v4l2_ctrl *new_pixel_size_ctrl(struct v4l2_ctrl_handler *handler)
+{
+	static struct v4l2_ctrl_config ctrl = {
+		.id = V4L2_CID_PIXEL_SIZE,
+		.type_ops = &pixel_size_ops,
+	};
+
+	return v4l2_ctrl_new_custom(handler, &ctrl, NULL);
+}
 static int imx214_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -1029,6 +1050,8 @@ static int imx214_probe(struct i2c_client *client)
 					     V4L2_CID_EXPOSURE,
 					     0, 3184, 1, 0x0c70);
 
+	imx214->pixel_size = new_pixel_size_ctrl(&imx214->ctrls);
+
 	ret = imx214->ctrls.error;
 	if (ret) {
 		dev_err(&client->dev, "%s control init failed (%d)\n",
--

2.23.0.rc1

author maintainer
#6 lore raw

Re: [PATCH 3/3] media: imx214: Add new control with V4L2_CID_PIXEL_SIZE

Hi Jacopo

Quoted 7 lines · Jacopo Mondi

Hi Ricardo,

Quoted 1 line · Ricardo Ribalda Delgado

According to the product brief, the unit cell size is 1120 nanometers^2.

Should this information come from DT ?

I do not think so. You cannot change this value and it needs to be defined also in sensors/cameras that might not have a DT, like a usb webcam.

It would be like adding to the DT the min/max exposure time...

But of course we can discuss it ;)

Best regards

Quoted 74 lines

I'm asking as I've a series in review that adds an helper that collectes dt properties and register controls for them. It currently only supports the newly proposed camera location control, but there might be others like the rotation, for which we already have a DT property.

https://patchwork.kernel.org/project/linux-media/list/?series=160901

This new one is indeed an HW property of the sensor, I wonder if having it in the firmware interface would make any sense or not...

Thanks
j

Quoted 59 lines

https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf

---
 drivers/media/i2c/imx214.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
drivers/media/i2c/imx214.c +23 -1
index 159a3a604f0e..b2f6bd2d8d7d 100644
@@ -47,6 +47,7 @@ struct imx214 {
      struct v4l2_ctrl *pixel_rate;
      struct v4l2_ctrl *link_freq;
      struct v4l2_ctrl *exposure;
+     struct v4l2_ctrl *pixel_size;
 
      struct regulator_bulk_data      supplies[IMX214_NUM_SUPPLIES];
 
@@ -941,6 +942,26 @@ static int __maybe_unused imx214_resume(struct device *dev)
      return ret;
 }
 
+static void pixel_size_init(const struct v4l2_ctrl *ctrl, u32 idx,
+                  union v4l2_ctrl_ptr ptr)
+{
+     ptr.p_pixel_size->width = 1120;
+     ptr.p_pixel_size->height = 1120;
+}
+
+static const struct v4l2_ctrl_type_ops pixel_size_ops = {
+     .init = pixel_size_init,
+};
+
+static struct v4l2_ctrl *new_pixel_size_ctrl(struct v4l2_ctrl_handler *handler)
+{
+     static struct v4l2_ctrl_config ctrl = {
+             .id = V4L2_CID_PIXEL_SIZE,
+             .type_ops = &pixel_size_ops,
+     };
+
+     return v4l2_ctrl_new_custom(handler, &ctrl, NULL);
+}
 static int imx214_probe(struct i2c_client *client)
 {
      struct device *dev = &client->dev;
@@ -1029,6 +1050,8 @@ static int imx214_probe(struct i2c_client *client)
                                           V4L2_CID_EXPOSURE,
                                           0, 3184, 1, 0x0c70);
 
+     imx214->pixel_size = new_pixel_size_ctrl(&imx214->ctrls);
+
      ret = imx214->ctrls.error;
      if (ret) {
              dev_err(&client->dev, "%s control init failed (%d)\n",
--

2.23.0.rc1

#7 lore raw

Re: [PATCH 3/3] media: imx214: Add new control with V4L2_CID_PIXEL_SIZE

Hi Ricardo,

Quoted 7 lines · Ricardo Ribalda Delgado

Hi Jacopo

Quoted 3 lines · Jacopo Mondi

Hi Ricardo,

Quoted 8 lines
Quoted 4 lines · Ricardo Ribalda Delgado
Quoted 1 line

According to the product brief, the unit cell size is 1120 nanometers^2.

Should this information come from DT ?

I do not think so. You cannot change this value and it needs to be defined also in sensors/cameras that might not have a DT, like a usb webcam.

You're probably right. I got this thinking because the camera location/orientation are two read only parameters that come from DT, but their value depends on the design of the device where the camera is installed on, so they're configurable, while this and other physical properties are not, and it doesn't make much sense to have them in DT.

Thanks
j

Quoted 81 lines

It would be like adding to the DT the min/max exposure time...

But of course we can discuss it ;)

Best regards

Quoted 74 lines

I'm asking as I've a series in review that adds an helper that collectes dt properties and register controls for them. It currently only supports the newly proposed camera location control, but there might be others like the rotation, for which we already have a DT property.

https://patchwork.kernel.org/project/linux-media/list/?series=160901

This new one is indeed an HW property of the sensor, I wonder if having it in the firmware interface would make any sense or not...

Thanks
j

Quoted 59 lines

https://www.sony-semicon.co.jp/products_en/IS/sensor1/img/products/ProductBrief_IMX214_20150428.pdf

---
 drivers/media/i2c/imx214.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
drivers/media/i2c/imx214.c +23 -1
index 159a3a604f0e..b2f6bd2d8d7d 100644
@@ -47,6 +47,7 @@ struct imx214 {
      struct v4l2_ctrl *pixel_rate;
      struct v4l2_ctrl *link_freq;
      struct v4l2_ctrl *exposure;
+     struct v4l2_ctrl *pixel_size;
 
      struct regulator_bulk_data      supplies[IMX214_NUM_SUPPLIES];
 
@@ -941,6 +942,26 @@ static int __maybe_unused imx214_resume(struct device *dev)
      return ret;
 }
 
+static void pixel_size_init(const struct v4l2_ctrl *ctrl, u32 idx,
+                  union v4l2_ctrl_ptr ptr)
+{
+     ptr.p_pixel_size->width = 1120;
+     ptr.p_pixel_size->height = 1120;
+}
+
+static const struct v4l2_ctrl_type_ops pixel_size_ops = {
+     .init = pixel_size_init,
+};
+
+static struct v4l2_ctrl *new_pixel_size_ctrl(struct v4l2_ctrl_handler *handler)
+{
+     static struct v4l2_ctrl_config ctrl = {
+             .id = V4L2_CID_PIXEL_SIZE,
+             .type_ops = &pixel_size_ops,
+     };
+
+     return v4l2_ctrl_new_custom(handler, &ctrl, NULL);
+}
 static int imx214_probe(struct i2c_client *client)
 {
      struct device *dev = &client->dev;
@@ -1029,6 +1050,8 @@ static int imx214_probe(struct i2c_client *client)
                                           V4L2_CID_EXPOSURE,
                                           0, 3184, 1, 0x0c70);
 
+     imx214->pixel_size = new_pixel_size_ctrl(&imx214->ctrls);
+
      ret = imx214->ctrls.error;
      if (ret) {
              dev_err(&client->dev, "%s control init failed (%d)\n",
--

2.23.0.rc1

#8 lore raw
In reply to Ricardo Ribalda Delgado

Hi Ricardo,

Quoted 18 lines · Ricardo Ribalda Delgado

This control returns the pixel size in nanometres. The struct provides the width and the height in separated fields to take into consideration asymmetric pixels and/or hardware binning.
This control is required for automatic calibration of the sensor.

---
 drivers/media/v4l2-core/v4l2-ctrls.c | 11 +++++++++++
 include/media/v4l2-ctrls.h           |  2 ++
 include/uapi/linux/v4l2-controls.h   |  3 +++
 include/uapi/linux/videodev2.h       | 11 +++++++++++
 4 files changed, 27 insertions(+)
drivers/media/v4l2-core/v4l2-ctrls.c +0 -0
index cd1ae016706f..a3a0086c96ff 100644

drivers/media/v4l2-core/v4l2-ctrls.c const char *v4l2_ctrl_get_name(u32 id) · line 981

Quoted 4 lines
drivers/media/v4l2-core/v4l2-ctrls.c +1 -0
@@ -978,6 +978,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_AUTO_FOCUS_RANGE:		return "Auto Focus, Range";
 	case V4L2_CID_PAN_SPEED:		return "Pan, Speed";
 	case V4L2_CID_TILT_SPEED:		return "Tilt, Speed";
+	case V4L2_CID_PIXEL_SIZE:		return "Pixel Size";

Is this a camera class control or an image source one ?
Also, isn't pixel size a bit too generic? I would somehow specify this is the physical pixel size

drivers/media/v4l2-core/v4l2-ctrls.c void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, · line 1362

Quoted 9 lines
drivers/media/v4l2-core/v4l2-ctrls.c +2 -0
 	/* FM Radio Modulator controls */
 	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1357,6 +1358,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
 		*type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
 		break;
+	case V4L2_CID_PIXEL_SIZE:
+		*type = V4L2_CTRL_TYPE_PIXEL_SIZE;

Isn't this a read-only control?

drivers/media/v4l2-core/v4l2-ctrls.c void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, · line 1431

Quoted 10 lines
drivers/media/v4l2-core/v4l2-ctrls.c +2 -0
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1423,6 +1427,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_RDS_RX_TRAFFIC_ANNOUNCEMENT:
 	case V4L2_CID_RDS_RX_TRAFFIC_PROGRAM:
 	case V4L2_CID_RDS_RX_MUSIC_SPEECH:
+	case V4L2_CID_PIXEL_SIZE:
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;

Ah yes, you set flags here... I don't get why we have two switches here.. Anyway, I would set both type and flags in a single case, otherwise one should jump back and forth...

Quoted 47 lines
drivers/media/v4l2-core/v4l2-ctrls.c +6 -0
 		break;
 	case V4L2_CID_RF_TUNER_PLL_LOCK:
@@ -1705,6 +1710,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 	case V4L2_CTRL_TYPE_FWHT_PARAMS:
 		break;
 
+	case V4L2_CTRL_TYPE_PIXEL_SIZE:
+		break;
+
 	case V4L2_CTRL_TYPE_H264_SPS:
 	case V4L2_CTRL_TYPE_H264_PPS:
 	case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
@@ -2403,6 +2411,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
 		elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
 		break;
+	case V4L2_CTRL_TYPE_PIXEL_SIZE:
+		elem_size = sizeof(struct v4l2_pixel_size);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);
include/media/v4l2-ctrls.h +2 -0
index 570ff4b0205a..63de780398b8 100644
@@ -50,6 +50,7 @@ struct poll_table_struct;
  * @p_h264_slice_params:	Pointer to a struct v4l2_ctrl_h264_slice_params.
  * @p_h264_decode_params:	Pointer to a struct v4l2_ctrl_h264_decode_params.
  * @p_vp8_frame_header:		Pointer to a VP8 frame header structure.
+ * @p_pixel_size:		Pointer to a pixel_size value.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -68,6 +69,7 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
 	struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
 	struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
+	struct v4l2_pixel_size *p_pixel_size;
 	void *p;
 };
 
include/uapi/linux/v4l2-controls.h +0 -0
index a2669b79b294..13f0410df4c6 100644

include/uapi/linux/v4l2-controls.h enum v4l2_auto_focus_range { · line 917

Quoted 6 lines
include/uapi/linux/v4l2-controls.h +3 -0
@@ -912,6 +912,9 @@ enum v4l2_auto_focus_range {
 #define V4L2_CID_PAN_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+32)
 #define V4L2_CID_TILT_SPEED			(V4L2_CID_CAMERA_CLASS_BASE+33)
 
+#define V4L2_CID_PIXEL_SIZE			(V4L2_CID_CAMERA_CLASS_BASE+34)
+
+

Double empty line

include/uapi/linux/videodev2.h struct v4l2_fract { · line 425

Quoted 12 lines
include/uapi/linux/v4l2-controls.h +0 -0
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
include/uapi/linux/videodev2.h +1 -0
index 2427bc4d8eba..21f4846dca0b 100644
@@ -422,6 +422,11 @@ struct v4l2_fract {
 	__u32   denominator;
 };
 
+struct v4l2_pixel_size {

I wonder if instead of defining a 'pixel size' this shouldn't be a more generic v4l2_size or something similar. Or even a v4l2_rect with left=top=0.

Quoted 8 lines
include/uapi/linux/videodev2.h +4 -0
+	__u32   width;
+	__u32   height;
+};
+
 /**
   * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
   *

include/uapi/linux/videodev2.h enum v4l2_ctrl_type { · line 1730

Quoted 8 lines
include/uapi/linux/videodev2.h +5 -0
@@ -1718,6 +1723,12 @@ enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_U8	     = 0x0100,
 	V4L2_CTRL_TYPE_U16	     = 0x0101,
 	V4L2_CTRL_TYPE_U32	     = 0x0102,
+	/*
+	 * V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
+	 * V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
+	 * V4L2_CTRL_TYPE_FWHT_PARAMS = 0x0105,
+	 */

leftovers ?

include/uapi/linux/videodev2.h enum v4l2_ctrl_type { · line 1731

Quoted 1 line
include/uapi/linux/videodev2.h +1 -0
+	V4L2_CTRL_TYPE_PIXEL_SIZE    = 0x0106,

I don't see other compound controls with a custom payload adding their types here. What am I missing?

Thanks
j

Quoted 6 lines
include/uapi/linux/videodev2.h +0 -1
 };
 
 /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
--

2.23.0.rc1

author maintainer
#9 lore raw

Hi Jacopo

[sorry for the multiple send, working on a train and fighting with the mail client and the 4g connection :S]

Quoted 17 lines · On Wed, 21 Aug 2019, 18:10 Jacopo Mondi, <[email protected]> wrote:

Hi Ricardo,

Quoted 13 lines · Ricardo Ribalda Delgado

This control returns the pixel size in nanometres. The struct provides the width and the height in separated fields to take into consideration asymmetric pixels and/or hardware binning.
This control is required for automatic calibration of the sensor.

---
 drivers/media/v4l2-core/v4l2-ctrls.c | 11 +++++++++++
 include/media/v4l2-ctrls.h           |  2 ++
 include/uapi/linux/v4l2-controls.h   |  3 +++
 include/uapi/linux/videodev2.h       | 11 +++++++++++
 4 files changed, 27 insertions(+)
Quoted 13 lines

drivers/media/v4l2-core/v4l2-ctrls.c const char *v4l2_ctrl_get_name(u32 id) · line 981

Quoted 9 lines
drivers/media/v4l2-core/v4l2-ctrls.c +1 -0
index cd1ae016706f..a3a0086c96ff 100644
@@ -978,6 +978,7 @@ const char *v4l2_ctrl_get_name(u32 id)
      case V4L2_CID_AUTO_FOCUS_RANGE:         return "Auto Focus, Range";
      case V4L2_CID_PAN_SPEED:                return "Pan, Speed";
      case V4L2_CID_TILT_SPEED:               return "Tilt, Speed";
+     case V4L2_CID_PIXEL_SIZE:               return "Pixel Size";

Is this a camera class control or an image source one ?
Also, isn't pixel size a bit too generic? I would somehow specify this is the physical pixel size

Please review v2. It has changed from pixel_size to unit size, which is more clear.

Quoted 14 lines

drivers/media/v4l2-core/v4l2-ctrls.c void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, · line 1362

Quoted 11 lines
drivers/media/v4l2-core/v4l2-ctrls.c +2 -0
      /* FM Radio Modulator controls */
      /* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1357,6 +1358,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
      case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
              *type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
              break;
+     case V4L2_CID_PIXEL_SIZE:
+             *type = V4L2_CTRL_TYPE_PIXEL_SIZE;

Isn't this a read-only control?

Quoted 14 lines

drivers/media/v4l2-core/v4l2-ctrls.c void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, · line 1431

Quoted 10 lines
drivers/media/v4l2-core/v4l2-ctrls.c +2 -0
+             break;
      default:
              *type = V4L2_CTRL_TYPE_INTEGER;
              break;
@@ -1423,6 +1427,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
      case V4L2_CID_RDS_RX_TRAFFIC_ANNOUNCEMENT:
      case V4L2_CID_RDS_RX_TRAFFIC_PROGRAM:
      case V4L2_CID_RDS_RX_MUSIC_SPEECH:
+     case V4L2_CID_PIXEL_SIZE:
              *flags |= V4L2_CTRL_FLAG_READ_ONLY;

Ah yes, you set flags here... I don't get why we have two switches here.. Anyway, I would set both type and flags in a single case, otherwise one should jump back and forth...

Sure. will do on v3

Quoted 51 lines
Quoted 51 lines
drivers/media/v4l2-core/v4l2-ctrls.c +6 -0
              break;
      case V4L2_CID_RF_TUNER_PLL_LOCK:
@@ -1705,6 +1710,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
      case V4L2_CTRL_TYPE_FWHT_PARAMS:
              break;
 
+     case V4L2_CTRL_TYPE_PIXEL_SIZE:
+             break;
+
      case V4L2_CTRL_TYPE_H264_SPS:
      case V4L2_CTRL_TYPE_H264_PPS:
      case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
@@ -2403,6 +2411,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
      case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
              elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
              break;
+     case V4L2_CTRL_TYPE_PIXEL_SIZE:
+             elem_size = sizeof(struct v4l2_pixel_size);
+             break;
      default:
              if (type < V4L2_CTRL_COMPOUND_TYPES)
                      elem_size = sizeof(s32);
include/media/v4l2-ctrls.h +2 -0
index 570ff4b0205a..63de780398b8 100644
@@ -50,6 +50,7 @@ struct poll_table_struct;
  * @p_h264_slice_params:     Pointer to a struct v4l2_ctrl_h264_slice_params.
  * @p_h264_decode_params:    Pointer to a struct v4l2_ctrl_h264_decode_params.
  * @p_vp8_frame_header:              Pointer to a VP8 frame header structure.
+ * @p_pixel_size:            Pointer to a pixel_size value.
  * @p:                               Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -68,6 +69,7 @@ union v4l2_ctrl_ptr {
      struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
      struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
      struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
+     struct v4l2_pixel_size *p_pixel_size;
      void *p;
 };
 
include/uapi/linux/v4l2-controls.h +0 -0
index a2669b79b294..13f0410df4c6 100644
@@ -912,6 +912,9 @@ enum v4l2_auto_focus_range {
 #define V4L2_CID_PAN_SPEED                   (V4L2_CID_CAMERA_CLASS_BASE+32)
 #define V4L2_CID_TILT_SPEED                  (V4L2_CID_CAMERA_CLASS_BASE+33)
 
Quoted 5 lines

include/uapi/linux/v4l2-controls.h enum v4l2_auto_focus_range { · line 917

Quoted 3 lines
include/uapi/linux/v4l2-controls.h +3 -0
+#define V4L2_CID_PIXEL_SIZE                  (V4L2_CID_CAMERA_CLASS_BASE+34)
+
+

Double empty line

Thanks , will fix in v3

Quoted 12 lines
Quoted 12 lines
include/uapi/linux/v4l2-controls.h +0 -0
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE            (V4L2_CTRL_CLASS_FM_TX | 0x900)
include/uapi/linux/videodev2.h +0 -0
index 2427bc4d8eba..21f4846dca0b 100644
@@ -422,6 +422,11 @@ struct v4l2_fract {
      __u32   denominator;
 };
 
Quoted 5 lines

include/uapi/linux/videodev2.h struct v4l2_fract { · line 425

Quoted 1 line
include/uapi/linux/videodev2.h +1 -0
+struct v4l2_pixel_size {

I wonder if instead of defining a 'pixel size' this shouldn't be a more generic v4l2_size or something similar. Or even a v4l2_rect with left=top=0.

Fixed on v2

Quoted 11 lines
Quoted 11 lines
include/uapi/linux/videodev2.h +4 -0
+     __u32   width;
+     __u32   height;
+};
+
 /**
   * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
   *
@@ -1718,6 +1723,12 @@ enum v4l2_ctrl_type {
      V4L2_CTRL_TYPE_U8            = 0x0100,
      V4L2_CTRL_TYPE_U16           = 0x0101,
Quoted 8 lines

include/uapi/linux/videodev2.h enum v4l2_ctrl_type { · line 1730

Quoted 6 lines
include/uapi/linux/videodev2.h +5 -0
      V4L2_CTRL_TYPE_U32           = 0x0102,
+     /*
+      * V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
+      * V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
+      * V4L2_CTRL_TYPE_FWHT_PARAMS = 0x0105,
+      */

leftovers ?

They are defined on other .h. I thought it was nice to know why I did not use 0x0103

include/media/mpeg2-ctrls.h:#define V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS 0x0103 include/media/mpeg2-ctrls.h:#define V4L2_CTRL_TYPE_MPEG2_QUANTIZATION 0x0104 include/media/fwht-ctrls.h:#define V4L2_CTRL_TYPE_FWHT_PARAMS 0x0105

Quoted 5 lines

include/uapi/linux/videodev2.h enum v4l2_ctrl_type { · line 1731

Quoted 2 lines
include/uapi/linux/videodev2.h +1 -0
+     V4L2_CTRL_TYPE_PIXEL_SIZE    = 0x0106,

I don't see other compound controls with a custom payload adding their types here. What am I missing?

@Hans could you clarify here what is the best practice?

Thanks!!

Quoted 150 lines · Jacopo Mondi

Hi Ricardo,

Quoted 18 lines · Ricardo Ribalda Delgado

This control returns the pixel size in nanometres. The struct provides the width and the height in separated fields to take into consideration asymmetric pixels and/or hardware binning.
This control is required for automatic calibration of the sensor.

---
 drivers/media/v4l2-core/v4l2-ctrls.c | 11 +++++++++++
 include/media/v4l2-ctrls.h           |  2 ++
 include/uapi/linux/v4l2-controls.h   |  3 +++
 include/uapi/linux/videodev2.h       | 11 +++++++++++
 4 files changed, 27 insertions(+)
drivers/media/v4l2-core/v4l2-ctrls.c +0 -0
index cd1ae016706f..a3a0086c96ff 100644

drivers/media/v4l2-core/v4l2-ctrls.c const char *v4l2_ctrl_get_name(u32 id) · line 981

Quoted 4 lines
drivers/media/v4l2-core/v4l2-ctrls.c +1 -0
@@ -978,6 +978,7 @@ const char *v4l2_ctrl_get_name(u32 id)
      case V4L2_CID_AUTO_FOCUS_RANGE:         return "Auto Focus, Range";
      case V4L2_CID_PAN_SPEED:                return "Pan, Speed";
      case V4L2_CID_TILT_SPEED:               return "Tilt, Speed";
+     case V4L2_CID_PIXEL_SIZE:               return "Pixel Size";

Is this a camera class control or an image source one ?
Also, isn't pixel size a bit too generic? I would somehow specify this is the physical pixel size

drivers/media/v4l2-core/v4l2-ctrls.c void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, · line 1362

Quoted 9 lines
drivers/media/v4l2-core/v4l2-ctrls.c +2 -0
      /* FM Radio Modulator controls */
      /* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1357,6 +1358,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
      case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER:
              *type = V4L2_CTRL_TYPE_VP8_FRAME_HEADER;
              break;
+     case V4L2_CID_PIXEL_SIZE:
+             *type = V4L2_CTRL_TYPE_PIXEL_SIZE;

Isn't this a read-only control?

drivers/media/v4l2-core/v4l2-ctrls.c void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type, · line 1431

Quoted 10 lines
drivers/media/v4l2-core/v4l2-ctrls.c +2 -0
+             break;
      default:
              *type = V4L2_CTRL_TYPE_INTEGER;
              break;
@@ -1423,6 +1427,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
      case V4L2_CID_RDS_RX_TRAFFIC_ANNOUNCEMENT:
      case V4L2_CID_RDS_RX_TRAFFIC_PROGRAM:
      case V4L2_CID_RDS_RX_MUSIC_SPEECH:
+     case V4L2_CID_PIXEL_SIZE:
              *flags |= V4L2_CTRL_FLAG_READ_ONLY;

Ah yes, you set flags here... I don't get why we have two switches here.. Anyway, I would set both type and flags in a single case, otherwise one should jump back and forth...

Quoted 47 lines
drivers/media/v4l2-core/v4l2-ctrls.c +6 -0
              break;
      case V4L2_CID_RF_TUNER_PLL_LOCK:
@@ -1705,6 +1710,9 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
      case V4L2_CTRL_TYPE_FWHT_PARAMS:
              break;
 
+     case V4L2_CTRL_TYPE_PIXEL_SIZE:
+             break;
+
      case V4L2_CTRL_TYPE_H264_SPS:
      case V4L2_CTRL_TYPE_H264_PPS:
      case V4L2_CTRL_TYPE_H264_SCALING_MATRIX:
@@ -2403,6 +2411,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
      case V4L2_CTRL_TYPE_VP8_FRAME_HEADER:
              elem_size = sizeof(struct v4l2_ctrl_vp8_frame_header);
              break;
+     case V4L2_CTRL_TYPE_PIXEL_SIZE:
+             elem_size = sizeof(struct v4l2_pixel_size);
+             break;
      default:
              if (type < V4L2_CTRL_COMPOUND_TYPES)
                      elem_size = sizeof(s32);
include/media/v4l2-ctrls.h +2 -0
index 570ff4b0205a..63de780398b8 100644
@@ -50,6 +50,7 @@ struct poll_table_struct;
  * @p_h264_slice_params:     Pointer to a struct v4l2_ctrl_h264_slice_params.
  * @p_h264_decode_params:    Pointer to a struct v4l2_ctrl_h264_decode_params.
  * @p_vp8_frame_header:              Pointer to a VP8 frame header structure.
+ * @p_pixel_size:            Pointer to a pixel_size value.
  * @p:                               Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -68,6 +69,7 @@ union v4l2_ctrl_ptr {
      struct v4l2_ctrl_h264_slice_params *p_h264_slice_params;
      struct v4l2_ctrl_h264_decode_params *p_h264_decode_params;
      struct v4l2_ctrl_vp8_frame_header *p_vp8_frame_header;
+     struct v4l2_pixel_size *p_pixel_size;
      void *p;
 };
 
include/uapi/linux/v4l2-controls.h +0 -0
index a2669b79b294..13f0410df4c6 100644

include/uapi/linux/v4l2-controls.h enum v4l2_auto_focus_range { · line 917

Quoted 6 lines
include/uapi/linux/v4l2-controls.h +3 -0
@@ -912,6 +912,9 @@ enum v4l2_auto_focus_range {
 #define V4L2_CID_PAN_SPEED                   (V4L2_CID_CAMERA_CLASS_BASE+32)
 #define V4L2_CID_TILT_SPEED                  (V4L2_CID_CAMERA_CLASS_BASE+33)
 
+#define V4L2_CID_PIXEL_SIZE                  (V4L2_CID_CAMERA_CLASS_BASE+34)
+
+

Double empty line

include/uapi/linux/videodev2.h struct v4l2_fract { · line 425

Quoted 12 lines
include/uapi/linux/v4l2-controls.h +0 -0
 /* FM Modulator class control IDs */
 
 #define V4L2_CID_FM_TX_CLASS_BASE            (V4L2_CTRL_CLASS_FM_TX | 0x900)
include/uapi/linux/videodev2.h +1 -0
index 2427bc4d8eba..21f4846dca0b 100644
@@ -422,6 +422,11 @@ struct v4l2_fract {
      __u32   denominator;
 };
 
+struct v4l2_pixel_size {

I wonder if instead of defining a 'pixel size' this shouldn't be a more generic v4l2_size or something similar. Or even a v4l2_rect with left=top=0.

Quoted 8 lines
include/uapi/linux/videodev2.h +4 -0
+     __u32   width;
+     __u32   height;
+};
+
 /**
   * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
   *

include/uapi/linux/videodev2.h enum v4l2_ctrl_type { · line 1730

Quoted 8 lines
include/uapi/linux/videodev2.h +5 -0
@@ -1718,6 +1723,12 @@ enum v4l2_ctrl_type {
      V4L2_CTRL_TYPE_U8            = 0x0100,
      V4L2_CTRL_TYPE_U16           = 0x0101,
      V4L2_CTRL_TYPE_U32           = 0x0102,
+     /*
+      * V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
+      * V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
+      * V4L2_CTRL_TYPE_FWHT_PARAMS = 0x0105,
+      */

leftovers ?

Quoted 14 lines

include/uapi/linux/videodev2.h enum v4l2_ctrl_type { · line 1731

Quoted 1 line
include/uapi/linux/videodev2.h +1 -0
+     V4L2_CTRL_TYPE_PIXEL_SIZE    = 0x0106,

I don't see other compound controls with a custom payload adding their types here. What am I missing?

Thanks
j

Quoted 6 lines
include/uapi/linux/videodev2.h +0 -1
 };
 
 /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
--

2.23.0.rc1

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEtcQ9SICaIIqPWDjAcjQGjxahVjwFAl1dbUYACgkQcjQGjxah VjzMSg//T3Z0Pn/MM4rl0d9E08+UhovwymM92g/k2giBnTTDmLyZSanh4wJ5z95z CcySqDqSdsgk4oe7BSMR3+kaK/O0M9o7wkzNUIzpgT0H2zPIkhLo4rkYN97W2XCj eVIcUkdK0YSRpJLUYXP4PyTYMNhwwmP8ugCtfnhdMnVP5ly5H/jHcH9PVlBUVpaq qrZVRmfzX7N6jt5L3QE7EMXnH0OouQuZRE7c0RqcRYkFwtsyDtu7Wc9cfAxQGKW2 Udkx387YTsLGTvh3TgVyiR28fonYxfo48lj8YPM49O53stwsqgMeg667UJ6HPzko RY6QUwbz9Yo5ThnMhfIRqIP88wPTYyd+gls8Hs6PJ+LZx5AGYavfBvtWI15NuRSc +VKrN9s2pqrO4SJsZvtxR0P4he6SeGPNDjozLv2rgtrJDlWstPKe97fwoRWBhFwd xb8dHGD6LD5X6bDiJNWFNs5gMti3lMH92CfXC36S2OktV40EVAXWQCryQWohH3cm hvFbS8odj2AvXP+1ogErP5QgZe4iI+kDct1kUxt1TjE3iWjbvIENpjgTU9WUvM70 VY6YcW58J1vk+DZHORiEbB5lA/8cDeoWYSVTROKq9FPrfGxOybwzLzcVfo5N37QO JaPhMfm1TLiecZV2w8KdpdfHsEJF3QEGDy9pBztLV/B8FCw7xA0= =+EdP
-----END PGP SIGNATURE-----