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

[Ricardo Ribalda]

On Mon, 9 Dec 2024 at 15:05, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxx>
> > ---
> >  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 ?

You are absolutely correct. Sorry about that.

Let's drop it for now.

Thanks!

>
> Regards,
>
> Hans
>
>


-- 
Ricardo Ribalda