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

[Jacopo Mondi]

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 ?

>
> 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