[PATCH v5] media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix.

Gergo Koteles posted 1 patch 2 months, 3 weeks ago
There is a newer version of this series
drivers/media/usb/uvc/uvc_ctrl.c | 52 +++++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 11 deletions(-)
[PATCH v5] media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix.
Posted by Gergo Koteles 2 months, 3 weeks ago
From: John Bauer <johnebgood@securitylive.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.

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@securitylive.com>
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
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:
-		*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.
+		 * 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.
+		 */
+		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
Re: [PATCH v5] media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix.
Posted by Ricardo Ribalda Delgado 2 months, 3 weeks ago
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@irl.hu> wrote:
>
> From: John Bauer <johnebgood@securitylive.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@securitylive.com>
> Signed-off-by: Gergo Koteles <soyer@irl.hu>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.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
Re: [PATCH v5] media: uvcvideo: UVC minimum relative pan/tilt/zoom speed fix.
Posted by Gergo Koteles 2 months, 3 weeks ago
Hi Ricardo,

On Mon, 2025-11-17 at 21:54 +0100, Ricardo Ribalda Delgado wrote:
> 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.
> 

This is an OBSbot Tiny 2.

> On Sat, Nov 15, 2025 at 2:30 AM Gergo Koteles <soyer@irl.hu> wrote:
> > 
> > From: John Bauer <johnebgood@securitylive.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?
> 

Thank you, it's much better. I'll send a v6 with the improved commit
message, comments and the lsusb output.


Gergo