Re: [PATCHv2 2/3] media: uvcvideo: add ROI auto controls

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (21/03/17 18:18), Sergey Senozhatsky wrote:
> On (21/02/08 14:17), Sergey Senozhatsky wrote:
> > This patch adds support for Region of Interest bmAutoControls.
> > 
> > ROI control is a compound data type:
> >   Control Selector     CT_REGION_OF_INTEREST_CONTROL
> >   Mandatory Requests   SET_CUR, GET_CUR, GET_MIN, GET_MAX, GET_DEF
> >   wLength 10
> >   Offset   Field            Size
> >   0        wROI_Top         2
> >   2        wROI_Left        2
> >   4        wROI_Bottom      2
> >   6        wROI_Right       2
> >   8        bmAutoControls   2       (Bitmap)
> > 
> > uvc_control_mapping, however, can handle only s32 data type at the
> > moment: ->get() returns s32 value, ->set() accepts s32 value; while
> > v4l2_ctrl maximum/minimum/default_value can hold only s64 values.
> > 
> > Hence ROI control handling is split into two patches:
> > a) bmAutoControls is handled via uvc_control_mapping as V4L2_CTRL_TYPE_MENU
> > b) ROI rectangle (SET_CUR, GET_CUR, GET_DEF) handling is implemented
> >    separately, by the means of selection API.
> 
> This approach is "no go".
> 
> I just figured out (am still debugging tho) that this patch set works on
> some devices and doesn't work on other. The root cause seems to be the
> fact that some firmwares error out all ROI requests when sizeof() of the
> ROI data is not 5 * __u16.
> 
> So those devices are not happy if we set/get ROI rectangle 4 * __u16
> and auto-controls __u16 separately; they want to set/get rect and
> rectanles in one shot.
> 
> This fixes ROI on those devices.
> 
> ---
> 
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -1190,6 +1190,7 @@ struct uvc_roi_rect {
>         __u16                   left;
>         __u16                   bottom;
>         __u16                   right;
> +       __u16                   auto_controls;
>  };

Maybe I can drop the separate auto-control and just use v4l2_selection::flags
to set the roi auto-controls value.