Re: [PATCH v15 18/19] media: uvcvideo: implement UVC v1.5 ROI

[Gergo Koteles <soyer@irl.hu>]

Hi Ricardo,

On Thu, 2024-11-14 at 21:28 +0100, Ricardo Ribalda wrote:
> Hi
> 
> On Thu, 14 Nov 2024 at 21:16, Gergo Koteles <soyer@irl.hu> wrote:
> > 
> > Hi Ricardo,
> > 
> > On Thu, 2024-11-14 at 21:03 +0100, Ricardo Ribalda wrote:
> > > Hi Gergo
> > > 
> > > Sorry, I forgot to reply to your comment in v14.
> > > 
> > > On Thu, 14 Nov 2024 at 20:53, Gergo Koteles <soyer@irl.hu> wrote:
> > > > 
> > > > Hi Ricardo,
> > > > 
> > > > On Thu, 2024-11-14 at 19:10 +0000, Ricardo Ribalda wrote:
> > > > > 
> > > > > +     },
> > > > > +     {
> > > > > +             .id             = V4L2_CID_UVC_REGION_OF_INTEREST_AUTO,
> > > > > +             .entity         = UVC_GUID_UVC_CAMERA,
> > > > > +             .selector       = UVC_CT_REGION_OF_INTEREST_CONTROL,
> > > > > +             .size           = 16,
> > > > > +             .offset         = 64,
> > > > > +             .v4l2_type      = V4L2_CTRL_TYPE_BITMASK,
> > > > > +             .data_type      = UVC_CTRL_DATA_TYPE_BITMASK,
> > > > > +             .name           = "Region Of Interest Auto Controls",
> > > > > +     },
> > > > >  };
> > > > > 
> > > > 
> > > > Wouldn't be better to use 8 V4L2_CTRL_TYPE_BOOLEAN controls for this?
> > > 
> > > If I create 8 Booleans, they will always be shown in the device. And
> > > the user will not have a way to know which values are available and
> > > which are not.
> > > 
> > > We will also fail the v4l2-compliance test, because there will be up
> > > to 7 boolean controls that will not be able to be set to 1, eventhough
> > > they are writable.
> > > 
> > 
> > And can't it be that only those returned by GET_MAX be added?
> > 
> > ```
> > The bmAutoControls bitmask determines which, if any, on board features
> > should track to the region of interest. To detect if a device supports
> > a particular Auto Control, use GET_MAX which returns a mask indicating
> > all supported Auto Controls.
> > ```
> > 
> > Sorry for the misunderstanding, I just don't quite understand.
> 
> I guess we could, but we would have to make a big change in the way
> the controls are probed today. uvc does not use the control framework.
> 
> What will be the benefit of using 8 controls?
> - Applications still have to know what those controls do, they should
> not rely on the control name.

Applications like v4l2-ctl are not aware of every controls, work by
control type, and let the user decide what to do, based on the name.

To avoid having to know each bitmask type control, they need to be able
to query which bit means what and what to display to the user.

Could VIDIOC_QUERYMENU be supplemented with this?


> - Changing from lets say AUTO_EXPOSURE to AUTO_FOCUS, will require to
> send at least 2 controls via v4l2_s_ext_control... I think it is more
> practical and less prone to errrors to send just one control
> 

Yes, that could be a good reason.

Thanks,
Gergo