Hi John,
Thanks for your patch. If we confirm that there are no devices with
ranges [-A,B], with A!=B, then it looks like the way to go
I would have removed the
return -data[first+1];
from uvc_ctrl_get_rel_speed(), as it becomes now dead code.
And I would have done it a little bit different with the switch, but I
have no idea what Laurent prefers ;):
@@ -1334,6 +1333,18 @@ static int __uvc_query_v4l2_ctrl(struct
uvc_video_chain *chain,
v4l2_ctrl->step = mapping->get(mapping, UVC_GET_RES,
uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
+ switch (v4l2_ctrl->id) {
+ case V4L2_CID_ZOOM_CONTINUOUS:
+ case V4L2_CID_PAN_SPEED:
+ case V4L2_CID_TILT_SPEED:
+ /*
+ * For the relative speed implementation the minimum
+ * value cannot be probed so it becomes the additive
+ * inverse of maximum.
+ */
+ v4l2_ctrl->minimum = -v4l2_ctrl->maximum;
+ }
+
return 0;
}
@@ -1919,6 +1930,18 @@ int uvc_ctrl_set(struct uvc_fh *handle,
if (step == 0)
step = 1;
+ switch (xctrl->id) {
+ case V4L2_CID_ZOOM_CONTINUOUS:
+ case V4L2_CID_PAN_SPEED:
+ case V4L2_CID_TILT_SPEED:
+ /*
+ * For the relative speed implementation the minimum
+ * value cannot be probed so it becomes the additive
+ * inverse of maximum.
+ */
+ min = -max;
+ }
+
xctrl->value = min +
DIV_ROUND_CLOSEST((u32)(xctrl->value - min),
step) * step;
if (mapping->data_type == UVC_CTRL_DATA_TYPE_SIGNED)
On Tue, 26 Mar 2024 at 10:02, John Bauer via B4 Relay
<devnull+johnebgood.securitylive.com@xxxxxxxxxx> wrote:
>
> From: John Bauer <johnebgood@xxxxxxxxxxxxxxxx>
>
> The minimum UVC control value for the relative pan/tilt/zoom speeds
> cannot be probed as the implementation condenses the pan and tilt
> direction and speed into two 16 bit values. The minimum cannot be
> set at probe time because it is probed first and the maximum is not
> yet known. With this fix if a relative speed control is queried
> or set the minimum is set and checked based on the additive inverse of
> the maximum at that time.
>
> Signed-off-by: John Bauer <johnebgood@xxxxxxxxxxxxxxxx>
> ---
> Gergo noticed that a quirk fix would not be needed and the just
> the minimum value needed to be set. Thanks Ricardo, Linh and Gergo
> for helping me along here. The reason the problem presented is that
> the minimum probe is done before the maximum however the way the
> driver has condensed direction and speed controls
> (with great simplification benefit to the user) the minimum
> value must be derived from the maximum and negated. This
> fix gets/checks/sets the correct relative minimum value when
> needed instead of of at probe time. This minimum value does not
> reflect the UVC 1.5 spec but the simplified condensed
> implementation of the driver; it's beautiful.
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 37 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index e59a463c2761..b389ab3ee05d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1322,9 +1322,25 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> break;
> }
>
> - if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)
> - v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN,
> - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) {
> + switch (v4l2_ctrl->id) {
> + case V4L2_CID_ZOOM_CONTINUOUS:
> + case V4L2_CID_PAN_SPEED:
> + case V4L2_CID_TILT_SPEED:
> + /*
> + * For the relative speed implementation the minimum
> + * value cannot be probed so it becomes the additive
> + * inverse of maximum.
> + */
> + v4l2_ctrl->minimum = -1 * mapping->get(mapping, UVC_GET_MAX,
> + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> + break;
> + default:
> + v4l2_ctrl->minimum = mapping->get(mapping, UVC_GET_MIN,
> + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> + break;
> + }
> + }
>
> if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX)
> v4l2_ctrl->maximum = mapping->get(mapping, UVC_GET_MAX,
> @@ -1914,6 +1930,21 @@ int uvc_ctrl_set(struct uvc_fh *handle,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MIN));
> max = mapping->get(mapping, UVC_GET_MAX,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
> +
> + /*
> + * For the relative speed implementation the minimum
> + * value cannot be probed so it becomes the additive
> + * inverse of maximum.
> + */
> + switch (xctrl->id) {
> + case V4L2_CID_ZOOM_CONTINUOUS:
> + case V4L2_CID_PAN_SPEED:
> + case V4L2_CID_TILT_SPEED:
> + min = max * -1;
> + default:
> + break;
> + }
> +
> step = mapping->get(mapping, UVC_GET_RES,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> if (step == 0)
>
> ---
> base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
> change-id: 20240325-uvc-relative-ptz-speed-fix-2011aea68b5f
>
> Best regards,
> --
> John Bauer <johnebgood@xxxxxxxxxxxxxxxx>
>
>
--
Ricardo Ribalda
