Hi Yunke,
On 9/29/22 06:11, Yunke Cao wrote:
> Removing priv_user_controls and its related checks.
>
> I suspect this is wrong because:
>
> 1. priv_user_controls == priv_user_controls_check is not always true.
>
> priv_user_controls counts the number of controls with
> id >= V4L2_CID_PRIVATE_BASE (0x08000000).
> priv_user_controls_check uses V4L2_CTRL_DRIVER_PRIV ((id) & 0xffff) >= 0x1000).
>
> The private controls defined in V4L2_CID_USER_BASE + 0x1000 will count towards
> priv_user_controls_check, but not priv_user_controls. For example,
> V4L2_CID_USER_MEYE_BASE (include/uapi/linux/v4l2-controls.h#n158).
>
> 2. Line 205 returns error for id >= V4L2_CID_PRIVATE_BASE. Counting
> priv_user_controls will not happen.
A long time ago all private controls in a driver started at ID V4L2_CID_PRIVATE_BASE.
When the control framework was created, all private controls were changed to start
at a control class base + 0x1000, and to stay compatible with old userspace the
control framework emulated enumerating such controls from V4L2_CID_PRIVATE_BASE.
These compliance tests verify that that emulation is still working correctly.
So this code is OK. If you have an example of where it fails, then that is likely
to be a bug elsewhere. I would need more information to see what could be the cause
in that case.
For the record:
Rejected-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
Regards,
Hans
>
> Signed-off-by: Yunke Cao <yunkec@xxxxxxxxxxxx>
> ---
> ---
> utils/v4l2-compliance/v4l2-test-controls.cpp | 22 +---------------------
> 1 file changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp b/utils/v4l2-compliance/v4l2-test-controls.cpp
> index 999dbcd7..18c9f638 100644
> --- a/utils/v4l2-compliance/v4l2-test-controls.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp
> @@ -182,7 +182,6 @@ int testQueryExtControls(struct node *node)
> __u32 which = 0;
> bool found_ctrl_class = false;
> unsigned user_controls = 0;
> - unsigned priv_user_controls = 0;
> unsigned user_controls_check = 0;
> unsigned priv_user_controls_check = 0;
> unsigned class_count = 0;
> @@ -299,30 +298,11 @@ int testQueryExtControls(struct node *node)
> user_controls++;
> }
>
> - for (id = V4L2_CID_PRIVATE_BASE; ; id++) {
> - memset(&qctrl, 0xff, sizeof(qctrl));
> - qctrl.id = id;
> - ret = doioctl(node, VIDIOC_QUERY_EXT_CTRL, &qctrl);
> - if (ret && ret != EINVAL)
> - return fail("invalid query_ext_ctrl return code (%d)\n", ret);
> - if (ret)
> - break;
> - if (qctrl.id != id)
> - return fail("qctrl.id (%08x) != id (%08x)\n",
> - qctrl.id, id);
> - if (checkQCtrl(node, qctrl))
> - return fail("invalid control %08x\n", qctrl.id);
> - priv_user_controls++;
> - }
> -
> - if (priv_user_controls + user_controls && node->controls.empty())
> + if (user_controls && node->controls.empty())
> return fail("does not support V4L2_CTRL_FLAG_NEXT_CTRL\n");
> if (user_controls != user_controls_check)
> return fail("expected %d user controls, got %d\n",
> user_controls_check, user_controls);
> - if (priv_user_controls != priv_user_controls_check)
> - return fail("expected %d private controls, got %d\n",
> - priv_user_controls_check, priv_user_controls);
> return result;
> }
>
>
> ---
> base-commit: 7f560aede797b659b585f063ed1f143f58b03df5
> change-id: 20220929-remove_private_control_check-ab8cc38a1b9e
>
> Best regards,
