Re: [PATCH v15 14/19] media: uvcvideo: Use the camera to clamp compound controls

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 14-Nov-24 8:10 PM, Ricardo Ribalda wrote:
> Compound controls cannot e reliable clamped. There is plenty of space
> for interpretation for the device manufacturer.
> 
> When we write a compound control, let the camera do the clamping and
> return back to the user the value used by the device.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 0dae5e8c3ca0..72ed7dc9cfc1 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2339,6 +2339,18 @@ int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl)
>  
>  	ctrl->dirty = 1;
>  	ctrl->modified = 1;
> +
> +	/*
> +	 * Compound controls cannot reliable clamp the value when they are
> +	 * written to the device. Let the device do the clamping and read back
> +	 * the value that the device is using. We do not need to return an
> +	 * error if this fails.
> +	 */
> +	if (uvc_ctrl_mapping_is_compound(mapping) &&
> +	    uvc_ctrl_is_readable(V4L2_CTRL_WHICH_CUR_VAL, ctrl, mapping))
> +		uvc_mapping_get_xctrl_compound(chain, ctrl, mapping,
> +					       V4L2_CTRL_WHICH_CUR_VAL, xctrl);
> +

I do not believe that this actually works / does what you want it to do.

At this point we have only updated in memory structures for the control
and not send anything to camera.

Querying the control to return the actual achieved values to userspace
only makes sense after uvc_ctrl_commit() has succeeded, unless I am
missing something ?

Regards,

Hans