[libcamera-devel] [PATCH v2] libcamera: v4l2_device: Workaround faulty control menus

[Kieran Bingham]

Quoting Jacopo Mondi (2022-11-03 10:21:54)
> Hi Kieran
> 
> On Thu, Nov 03, 2022 at 09:57:31AM +0000, Kieran Bingham wrote:
> > Quoting Marian Buschsieweke (2022-11-03 09:42:35)
> > > Hi,
> > >
> > > On Wed, 2 Nov 2022 21:44:41 +0100
> > > Marian Buschsieweke <marian.buschsieweke at ovgu.de> wrote:
> > >
> > > > I can test this tomorrow to confirm that exposure_auto cannot be set to any of
> > > > [0..3], or whether indeed just the labels are missing.
> > >
> > > at least for my K20 device it does indeed not support any of the menu items:
> > >
> > > ~ $ v4l2-ctl --list-devices
> > > K20 USB CAMERA: K20 USB CAMERA (usb-0000:00:14.0-1):
> > >         /dev/video2
> > >         /dev/video3
> > >         /dev/media1
> > >
> > > Integrated Camera: Integrated C (usb-0000:00:14.0-8):
> > >         /dev/video0
> > >         /dev/video1
> > >         /dev/media0
> > >
> > > ~ $ v4l2-ctl -d /dev/video2 -lL
> > >                      brightness 0x00980900 (int)    : min=-64 max=64 step=1 default=0 value=0
> > >                        contrast 0x00980901 (int)    : min=0 max=64 step=1 default=33 value=33
> > >                      saturation 0x00980902 (int)    : min=0 max=128 step=1 default=64 value=64
> > >                             hue 0x00980903 (int)    : min=-40 max=40 step=1 default=0 value=0
> > >  white_balance_temperature_auto 0x0098090c (bool)   : default=1 value=1
> > >                           gamma 0x00980910 (int)    : min=72 max=500 step=1 default=120 value=120
> > >                            gain 0x00980913 (int)    : min=0 max=100 step=1 default=0 value=0
> > >            power_line_frequency 0x00980918 (menu)   : min=0 max=2 default=1 value=1 (50 Hz)
> > >                                 0: Disabled
> > >                                 1: 50 Hz
> > >                                 2: 60 Hz
> > >       white_balance_temperature 0x0098091a (int)    : min=2800 max=6500 step=1 default=4600 value=4600 flags=inactive
> > >                       sharpness 0x0098091b (int)    : min=0 max=6 step=1 default=6 value=6
> > >          backlight_compensation 0x0098091c (int)    : min=0 max=2 step=1 default=1 value=1
> > >                   exposure_auto 0x009a0901 (menu)   : min=0 max=3 default=0 value=0
> > >               exposure_absolute 0x009a0902 (int)    : min=1 max=5000 step=1 default=2500 value=2500 flags=inactive
> > >          exposure_auto_priority 0x009a0903 (bool)   : default=0 value=1
> > >                  focus_absolute 0x009a090a (int)    : min=1 max=1023 step=1 default=1 value=230 flags=inactive
> > >                      focus_auto 0x009a090c (bool)   : default=1 value=1
> > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=0
> > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument
> > > Error setting controls: Invalid argument
> > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=1
> > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument
> > > Error setting controls: Invalid argument
> > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=2
> > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument
> > > Error setting controls: Invalid argument
> > > ~ $ v4l2-ctl -d /dev/video2 -c exposure_auto=3
> > > VIDIOC_S_EXT_CTRLS: failed: Invalid argument
> > > Error setting controls: Invalid argument
> >
> > Ok - so that makes me believe we're indeed better off returning
> > ControlInfo();
> 
> Do you have any clue why UVC reports the control as available even if
> it can't actually be set ? Is there any value in reporting it with a
> single fixed value, or the control as advertised by the kernel is
> completely useless and we can't even trust its default value ?

Ok - so digging into the kernel driver to see if I can see what
happened:

drivers/media/usb/uvc/uvc_ctrl.c has:
static const struct uvc_menu_info exposure_auto_controls[] = {
	{ 2, "Auto Mode" },
	{ 1, "Manual Mode" },
	{ 4, "Shutter Priority Mode" },
	{ 8, "Aperture Priority Mode" },
};

This is mapped/handled with the UVC_CT_AE_MODE_CONTROL selector. and
seems to be a bitmask.

So far so good.

4.2.2.1.2 Auto-Exposure Mode Control of the UVC 1.5 Class specification
states:

The Auto-Exposure Mode Control determines whether the device will
provide automatic adjustment of the Exposure Time and Iris controls.
Attempts to programmatically set the auto- adjusted controls shall
result in a protocol STALL and an error code of bRequestErrorCode =
“Wrong state”. A GET_RES request issued to this control will return a
bitmap of the modes supported by this control. A valid request to this
control would have only one bit set (a single mode selected). This
control must accept the GET_DEF request and return its default value.


So I would anticipate that the device in question has not returned a
valid bitmask from the GET_RES request.

We would need to get a debug log from a kernel probing the device in
question to look further, but for libcamera, as this problem 'exists' we
need to workaround / solve it here regardless.

I'll continue to update the code to be aware of menu controls that may
have zero menu items, and consider that control as invalid
(unsettable), and make sure that works in libcamera.

--
Kieran



> > That will require a bit more work to fix the upper layer so that it
> > correctly skips controls that are returned as empty - but actually
> > looking at the code - that's already something we need to do.
> >
> > Higher in the function there is the statement:
> >
> >          if (ctrl.minimum < 0)
> >                 return ControlInfo();
> >
> > Which I expect will crash in the caller right now, as we discovered.
> >
> > So lets drop this version of the patch and fix things up to be able to
> > return ControlInfo(); when indicies are empty - and make sure the higher
> > layers realise this and do not add the control.
> > --
> > Kieran
> >
> >
> > >
> > > Kind regards,
> > > Marian