| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPybu_1HEpqfXqQFrwxs7cwerrNb1YMf-hgg4WYmK3VC4wsbrA@mail.gmail.com>
Date: Mon, 17 Nov 2025 21:54:46 +0100
From: Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
To: Gergo Koteles <soyer@....hu>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>, Hans de Goede <hansg@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>, linux-media@...r.kernel.org,
linux-kernel@...r.kernel.org, John Bauer <johnebgood@...uritylive.com>
Subject: Re: [PATCH v5] media: uvcvideo: UVC minimum relative pan/tilt/zoom
speed fix.
Hi Gergo
Thanks for reposting the patch, Could you mention what device you have
used to test it? If you include a lsusb -v from the device that would
be extra awesome.
On Sat, Nov 15, 2025 at 2:30 AM Gergo Koteles <soyer@....hu> wrote:
>
> From: John Bauer <johnebgood@...uritylive.com>
>
> For relative PTZ controls, UVC_GET_MIN for b(Pan|Tilt|Zoom)Speed
> returns the minimum speed of the movement in direction specified
> in the sign field.
>
nit: What about this extra clarification in the commit message?
In Video4Linux, a negative value in V4L2_CID_ZOOM_CONTINUOUS,
V4L2_CID_PAN_SPEED and V4L2_CID_TILT_SPEED indicates a movement in the
"opposite" direction to the standard direction.
Currently, we were using -UVC_GET_MIN as the negative value, which
resulted in the camera moving in the slowest possible speed.
Quirk the driver to return -UVC_GET_MAX for the affected controls.
Note that the get/set function from the mapping cannot be used for
this, because we need to use the information from GET_MAX for GET_MIN
and hacking uvc_ctrl_populate_cache seems like a worse alternative.
> So in the negative direction, only the slowest speed can be used
> at the moment.
>
> For minimum value, use maximum speed but in negative direction.
>
> Signed-off-by: John Bauer <johnebgood@...uritylive.com>
> Signed-off-by: Gergo Koteles <soyer@....hu>
Reviewed-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
> Changes in v5:
> - Rebased on 6.18
> - Link to v4: https://lore.kernel.org/all/cover.1718835633.git.soyer@irl.hu/
>
> Changes in v4:
> - Based on Ricardo's suggestion, only query the min value in uvc_ctrl_set
> if necessary
> - Rename is_relative_ptz_ctrl function to uvc_ctrl_is_relative_ptz for
> consistency
> - Rename 'relative speed implementation' to 'relative PTZ controls' in
> comments
> - Fix indentation of comments
> - Reduce the length of the new lines to 80
> - Link to v3: https://lore.kernel.org/all/cover.1718726777.git.soyer@irl.hu/
>
> Changes in v3:
> - Based on Ricardo's suggestion, I squashed the two patches.
> - Link to v2: https://lore.kernel.org/all/20240405-uvc-fix-relative-ptz-speed-v1-0-c32cdb2a899d@securitylive.com/
>
> Changes in v2:
> - Made recommended changes, moved control check to helper function and removed dead code.
> - Link to v1: https://lore.kernel.org/all/20240326-uvc-relative-ptz-speed-fix-v1-1-453fd5ccfd37@securitylive.com/
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 52 +++++++++++++++++++++++++-------
> 1 file changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 2905505c240c..38a7d71526c2 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -526,8 +526,6 @@ static int uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping,
> *out = (sign == 0) ? 0 : (sign > 0 ? value : -value);
> return 0;
> case UVC_GET_MIN:
nit: Add comment. (and if you add it also to uvc_ctrl_get_zoom, it
would be even better)
/* Not used, we use -UVC_GET_MAX */
> - *out = -value;
> - return 0;
> case UVC_GET_MAX:
> case UVC_GET_RES:
> case UVC_GET_DEF:
> @@ -1517,6 +1515,17 @@ static u32 uvc_get_ctrl_bitmap(struct uvc_control *ctrl,
> return ~0;
> }
>
> +static bool uvc_ctrl_is_relative_ptz(__u32 ctrl_id)
> +{
> + switch (ctrl_id) {
> + case V4L2_CID_ZOOM_CONTINUOUS:
> + case V4L2_CID_PAN_SPEED:
> + case V4L2_CID_TILT_SPEED:
> + return true;
> + }
> + return false;
> +}
> +
> /*
> * Maximum retry count to avoid spurious errors with controls. Increasing this
> * value does no seem to produce better results in the tested hardware.
> @@ -1576,18 +1585,28 @@ static int __uvc_queryctrl_boundaries(struct uvc_video_chain *chain,
> break;
> }
>
> - if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)
> - v4l2_ctrl->minimum = uvc_mapping_get_s32(mapping, UVC_GET_MIN,
> - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> - else
> - v4l2_ctrl->minimum = 0;
> -
> if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX)
> v4l2_ctrl->maximum = uvc_mapping_get_s32(mapping, UVC_GET_MAX,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> else
> v4l2_ctrl->maximum = 0;
>
> + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) {
> + /*
> + * For relative PTZ controls, UVC_GET_MIN for
> + * b(Pan|Tilt|Zoom)Speed returns the minimum speed of the
> + * movement in direction specified in the sign field.
nit: mention 4.2.2.1.15 in the comment
> + * For minimum value, use maximum speed but in negative direction.
> + */
> + if (uvc_ctrl_is_relative_ptz(v4l2_ctrl->id))
> + v4l2_ctrl->minimum = -v4l2_ctrl->maximum;
> + else
> + v4l2_ctrl->minimum = uvc_mapping_get_s32(mapping,
> + UVC_GET_MIN, uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> + } else {
> + v4l2_ctrl->minimum = 0;
> + }
> +
> if (ctrl->info.flags & UVC_CTRL_FLAG_GET_RES)
> v4l2_ctrl->step = uvc_mapping_get_s32(mapping, UVC_GET_RES,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> @@ -2449,6 +2468,7 @@ int uvc_ctrl_get(struct uvc_video_chain *chain, u32 which,
>
> static int uvc_ctrl_clamp(struct uvc_video_chain *chain,
> struct uvc_control *ctrl,
> + u32 v4l2_id,
> struct uvc_control_mapping *mapping,
> s32 *value_in_out)
> {
> @@ -2466,10 +2486,20 @@ static int uvc_ctrl_clamp(struct uvc_video_chain *chain,
> return ret;
> }
>
> - min = uvc_mapping_get_s32(mapping, UVC_GET_MIN,
> - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> max = uvc_mapping_get_s32(mapping, UVC_GET_MAX,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> + /*
> + * For relative PTZ controls, UVC_GET_MIN for
> + * b(Pan|Tilt|Zoom)Speed returns the minimum speed of the
> + * movement in direction specified in the sign field.
> + * For minimum value, use maximum speed but in negative direction.
> + */
nit: Mention 4.2.2.1.15
> + if (uvc_ctrl_is_relative_ptz(v4l2_id))
> + min = -max;
> + else
> + min = uvc_mapping_get_s32(mapping, UVC_GET_MIN,
> + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> +
> step = uvc_mapping_get_s32(mapping, UVC_GET_RES,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> if (step == 0)
> @@ -2583,7 +2613,7 @@ int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl)
> if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR))
> return -EACCES;
>
> - ret = uvc_ctrl_clamp(chain, ctrl, mapping, &xctrl->value);
> + ret = uvc_ctrl_clamp(chain, ctrl, xctrl->id, mapping, &xctrl->value);
> if (ret)
> return ret;
> /*
> --
> 2.51.1
>
>
--
Ricardo Ribalda
Powered by blists - more mailing lists