v4l2-ctrls: fix NEXT_COMPOUND support
Subsystems (2)
RFC PATCH 1/1 v4l2-ctrls: fix NEXT_COMPOUND support
Ricardo reported a problem in v4l2-compliance if an integer control was used in an array. It turned out to be a problem in the implementation of NEXT_COMPOUND that didn't match arrays as being compound controls.
I also did some DocBook updates. The final version of this patch will split off the docbook changes in a separate patch (or patches).
Ricardo, can you test this?
In order to be able to use integer controls in an array we also need a new field in the union in struct v4l2_ext_control: __s32 __user *p_s32.
Ricardo, please test this thoroughly. I've never tested INTEGER arrays before, so I'm not sure if there are no hidden surprises somewhere.
Regards,
Hans
Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml +7 -0
@@ -200,6 +200,13 @@ Valid if this control is of type <constant>V4L2_CTRL_TYPE_U16</constant>.</entry </row> <row> <entry></entry> + <entry>__u32 *</entry> + <entry><structfield>p_u32</structfield></entry> + <entry>A pointer to a matrix control of unsigned 32-bit values. +Valid if this control is of type <constant>V4L2_CTRL_TYPE_U32</constant>.</entry> + </row> + <row> + <entry></entry> <entry>void *</entry> <entry><structfield>ptr</structfield></entry> <entry>A pointer to a compound type which can be an N-dimensional array and/or a
Documentation/DocBook/media/v4l/vidioc-queryctrl.xml +16 -5
@@ -101,8 +101,9 @@ prematurely end the enumeration).</para></footnote></para> next supported non-compound control, or <errorcode>EINVAL</errorcode> if there is none. In addition, the <constant>V4L2_CTRL_FLAG_NEXT_COMPOUND</constant> flag can be specified to enumerate all compound controls (i.e. controls -with type ≥ <constant>V4L2_CTRL_COMPOUND_TYPES</constant>). Specify both -<constant>V4L2_CTRL_FLAG_NEXT_CTRL</constant> and +with type ≥ <constant>V4L2_CTRL_COMPOUND_TYPES</constant> and/or array +control, in other words controls that contain more than one value).i +Specify both <constant>V4L2_CTRL_FLAG_NEXT_CTRL</constant> and <constant>V4L2_CTRL_FLAG_NEXT_COMPOUND</constant> in order to enumerate all controls, compound or not. Drivers which do not support these flags yet always return <errorcode>EINVAL</errorcode>.</para>
@@ -422,7 +423,7 @@ the array to zero.</entry> <entry>any</entry> <entry>An integer-valued control ranging from minimum to maximum inclusive. The step value indicates the increment between -values which are actually different on the hardware.</entry> +values.</entry> </row> <row> <entry><constant>V4L2_CTRL_TYPE_BOOLEAN</constant></entry>
@@ -518,7 +519,7 @@ Older drivers which do not support this feature return an <entry>any</entry> <entry>An unsigned 8-bit valued control ranging from minimum to maximum inclusive. The step value indicates the increment between -values which are actually different on the hardware. +values. </entry> </row> <row>
@@ -528,7 +529,17 @@ values which are actually different on the hardware. <entry>any</entry> <entry>An unsigned 16-bit valued control ranging from minimum to maximum inclusive. The step value indicates the increment between -values which are actually different on the hardware. +values. +</entry> + </row> + <row> + <entry><constant>V4L2_CTRL_TYPE_U32</constant></entry> + <entry>any</entry> + <entry>any</entry> + <entry>any</entry> + <entry>An unsigned 32-bit valued control ranging from minimum to +maximum inclusive. The step value indicates the increment between +values. </entry> </row> </tbody>
drivers/media/v4l2-core/v4l2-ctrls.c +2 -2
@@ -2498,7 +2498,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr/* We found a control with the given ID, so just get the next valid one in the list. */ list_for_each_entry_continue (ref, &hdl->ctrl_refs, node) { - is_compound = + is_compound = ref->ctrl->is_array || ref->ctrl->type >=V4L2_CTRL_COMPOUND_TYPES ;if (id < ref->ctrl->id && (is_compound & mask) == match)
@@ -2512,7 +2512,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctris one, otherwise the first 'if' above would have been true. */ list_for_each_entry (ref, &hdl->ctrl_refs, node) { - is_compound = + is_compound = ref->ctrl->is_array || ref->ctrl->type >=V4L2_CTRL_COMPOUND_TYPES ;if (id < ref->ctrl->id && (is_compound & mask) == match)
Hello Hans
I tried this patch on my platform and v4l2compliance now works ok.
I would suggest to add this definition to the userland header for clarity
#define V4L2_CTRL_FLAG_NEXT_COMPOUND_OR_ARRAY V4L2_CTRL_FLAG_NEXT_COMPOUND
Thanks!
Quoted 115 lines · Hans Verkuil
Ricardo reported a problem in v4l2-compliance if an integer control was used in an array. It turned out to be a problem in the implementation of
NEXT_COMPOUNDthat didn't match arrays as being compound controls.I also did some DocBook updates. The final version of this patch will split off the docbook changes in a separate patch (or patches).
Ricardo, can you test this?
In order to be able to use integer controls in an array we also need a new field in the union in struct
v4l2_ext_control:__s32__user*p_s32.Ricardo, please test this thoroughly. I've never tested INTEGER arrays before, so I'm not sure if there are no hidden surprises somewhere.
Regards,
Hans
Signed-off-by Hans Verkuil <[email protected]>Reported-by Ricardo Ribalda Delgado <[email protected]>Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml +7
-0@@ -200,6 +200,13 @@ Valid if this control is of type <constant>V4L2_CTRL_TYPE_U16</constant>.</entry </row> <row> <entry></entry> + <entry>__u32 *</entry> + <entry><structfield>p_u32</structfield></entry> + <entry>A pointer to a matrix control of unsigned 32-bit values. +Valid if this control is of type <constant>V4L2_CTRL_TYPE_U32</constant>.</entry> + </row> + <row> + <entry></entry> <entry>void *</entry> <entry><structfield>ptr</structfield></entry> <entry>A pointer to a compound type which can be an N-dimensional array and/or aDocumentation/DocBook/media/v4l/vidioc-queryctrl.xml +16
-5@@ -101,8 +101,9 @@ prematurely end the enumeration).</para></footnote></para> next supported non-compound control, or <errorcode>EINVAL</errorcode> if there is none. In addition, the <constant>V4L2_CTRL_FLAG_NEXT_COMPOUND</constant> flag can be specified to enumerate all compound controls (i.e. controls -with type ≥ <constant>V4L2_CTRL_COMPOUND_TYPES</constant>). Specify both -<constant>V4L2_CTRL_FLAG_NEXT_CTRL</constant> and +with type ≥ <constant>V4L2_CTRL_COMPOUND_TYPES</constant> and/or array +control, in other words controls that contain more than one value).i +Specify both <constant>V4L2_CTRL_FLAG_NEXT_CTRL</constant> and <constant>V4L2_CTRL_FLAG_NEXT_COMPOUND</constant> in order to enumerate all controls, compound or not. Drivers which do not support these flags yet always return <errorcode>EINVAL</errorcode>.</para>@@ -422,7 +423,7 @@ the array to zero.</entry> <entry>any</entry> <entry>An integer-valued control ranging from minimum to maximum inclusive. The step value indicates the increment between -values which are actually different on the hardware.</entry> +values.</entry> </row> <row> <entry><constant>V4L2_CTRL_TYPE_BOOLEAN</constant></entry>@@ -518,7 +519,7 @@ Older drivers which do not support this feature return an <entry>any</entry> <entry>An unsigned 8-bit valued control ranging from minimum to maximum inclusive. The step value indicates the increment between -values which are actually different on the hardware. +values. </entry> </row> <row>@@ -528,7 +529,17 @@ values which are actually different on the hardware. <entry>any</entry> <entry>An unsigned 16-bit valued control ranging from minimum to maximum inclusive. The step value indicates the increment between -values which are actually different on the hardware. +values. +</entry> + </row> + <row> + <entry><constant>V4L2_CTRL_TYPE_U32</constant></entry> + <entry>any</entry> + <entry>any</entry> + <entry>any</entry> + <entry>An unsigned 32-bit valued control ranging from minimum to +maximum inclusive. The step value indicates the increment between +values. </entry> </row> </tbody>drivers/media/v4l2-core/v4l2-ctrls.c +2
-2@@ -2498,7 +2498,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr/* We found a control with the given ID, so just get the next valid one in the list. */ list_for_each_entry_continue (ref, &hdl->ctrl_refs, node) { - is_compound = + is_compound = ref->ctrl->is_array || ref->ctrl->type >=V4L2_CTRL_COMPOUND_TYPES ;if (id < ref->ctrl->id && (is_compound & mask) == match)@@ -2512,7 +2512,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctris one, otherwise the first 'if' above would have been true. */ list_for_each_entry (ref, &hdl->ctrl_refs, node) { - is_compound = + is_compound = ref->ctrl->is_array || ref->ctrl->type >=V4L2_CTRL_COMPOUND_TYPES ;if (id < ref->ctrl->id && (is_compound & mask) == match)
Signature · 1 line
Ricardo Ribalda