Hi Gergo
Thanks for your your patch.
On Tue, 18 Jun 2024 at 18:33, Gergo Koteles <soyer@xxxxxx> 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>
> Signed-off-by: Gergo Koteles <soyer@xxxxxx>
Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 38 +++++++++++++++++++++++++++-----
> 1 file changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 4b685f883e4d..93ed2462e90b 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -441,7 +441,6 @@ static s32 uvc_ctrl_get_rel_speed(struct uvc_control_mapping *mapping,
> return (rel == 0) ? 0 : (rel > 0 ? data[first+1]
> : -data[first+1]);
> case UVC_GET_MIN:
> - return -data[first+1];
> case UVC_GET_MAX:
> case UVC_GET_RES:
> case UVC_GET_DEF:
> @@ -1233,6 +1232,17 @@ static u32 uvc_get_ctrl_bitmap(struct uvc_control *ctrl,
> return ~0;
> }
>
> +static bool is_relative_ptz_ctrl(__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;
> +}
> +
> static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain,
> struct uvc_control *ctrl,
> struct uvc_control_mapping *mapping,
> @@ -1322,14 +1332,23 @@ 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_MAX)
> v4l2_ctrl->maximum = mapping->get(mapping, UVC_GET_MAX,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX));
>
> + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN) {
> + /*
> + * For the relative speed implementation the minimum
> + * value cannot be probed so it becomes the additive
> + * inverse of maximum.
> + */
> + if (is_relative_ptz_ctrl(v4l2_ctrl->id))
> + v4l2_ctrl->minimum = -v4l2_ctrl->maximum;
> + else
> + 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_RES)
> v4l2_ctrl->step = mapping->get(mapping, UVC_GET_RES,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> @@ -1916,6 +1935,15 @@ 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.
> + */
> + if (is_relative_ptz_ctrl(xctrl->id))
> + min = -max;
> +
nit: The following would probably be more correct but less clear:
if (is_relative_ptz_ctrl(xctrl->id))
min = -max;
else
min = mapping->get(mapping, UVC_GET_MIN,...)
So up to you what do you/Laurent what is better ;)
> step = mapping->get(mapping, UVC_GET_RES,
> uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES));
> if (step == 0)
> --
> 2.45.2
>
--
Ricardo Ribalda
