Re: [PATCH] Remove priv_user_controls in v4l2-test-controls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



nit: mention in the subject that the change is for v4l2-compliance


On Thu, 29 Sept 2022 at 06:12, Yunke Cao <yunkec@xxxxxxxxxxxx> 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.
>
> Signed-off-by: Yunke Cao <yunkec@xxxxxxxxxxxx>
Reviewed-by: Ricardo Ribalda <ribalda@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,
> --
> Yunke Cao <yunkec@xxxxxxxxxxxx>


Thanks!
-- 
Ricardo Ribalda



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux