[libcamera-devel] Problems with getControls() and listControls()

[Kieran Bingham]

Hi David,

On 05/03/2021 11:06, Kieran Bingham wrote:
> Hi David,
> 
> On 05/03/2021 10:47, David Plowman wrote:
>> To fill in the details on some of this, the controls that can have
>> their ranges altered (looking at current drivers) are:
>>
>> Exposure: this range can change whenever the vblanking is updated, so
>> it can happen every frame. Luckily I don't think anyone cares about
>> the ranges here so everything seems to work as expected...
>>
>> The remaining ones normally get updated only when the camera mode is changed.
>>
>> Pixel Rate
>> Hblanking
>> Vblanking
>>
>> As far as I'm aware, only vblanking is a problem as we use those
>> limits to calculate possible frame timings. For the pixel rate and
>> hblanking we use the value, not the limits, so I believe we get the
>> right numbers back there.
>>
>> I'm inclined to agree with Kieran that watching for changes to values
>> probably won't work. A modification of the range (I take it there's no
>> event for that directly..?) doesn't necessarily cause a change in the
>> value. Pragmatically speaking, regenerating the ControlInfo list after
>> a setFormat doesn't sound like a terrible idea. Alternatively, and
>> 100% safe, perhaps we should give up caching max/min values and
>> re-fetch them every time they're wanted? Do we query these limits
>> often?
> 
> I'd also be a bit weary of some sort of race between when the kernel
> updates us, to when we process the control event update, vs when some
> other code would read it.

So I've just been told there is an event V4L2_EVENT_CTRL_CH_RANGE which
we can subscribe to, to know when a controls range is changed.

So we can handle these updates 'automagically' ...

The next question is ... is that the right way ;-) or will that be racy...

--
KB


> 
> What about marking some controls as 'volatile' such that it must always
> get the full control info, (or whatever it needs), or would 'volatile'
> be too much?
> 
> I don't know if it's something we need to update all values once per
> frame, or if we only need 'some' ?
> 
> 
> Otherwise we could have an explicit function to manually reload min/max
> values on a per-control basis ?
>  (if there's only 'one' that needs to be reloaded?)
> 
> 
> --
> Kieran
> 
> 
>> David
>>
>> On Fri, 5 Mar 2021 at 09:11, Naushir Patuck <naush at raspberrypi.com> wrote:
>>>
>>> Hi Kieran,
>>>
>>> On Thu, 4 Mar 2021 at 20:05, Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:
>>>>
>>>> Hi David,
>>>>
>>>> On 03/03/2021 15:57, David Plowman wrote:
>>>>> Hi everyone
>>>>>
>>>>> I've been chasing down problems with a camera driver (sometimes) not
>>>>> running at the correct framerates. It seems that I've tracked it down
>>>>> to libcamera's control list mechanism. Please correct me if I've got
>>>>> anything wrong in my explanation below.
>>>>>
>>>>> V4L2 devices produce a list of all their controls, including min and
>>>>> max values, when they are opened - as the V4L2Device constructor calls
>>>>> the private listControls() method.
>>>>>
>>>>> The CameraInfo::sensorInfo() method calculates the minimum and maximum
>>>>> frame lengths using the min and max vblank values. These values were
>>>>> read and stored when the device was opened. Since then, however, the
>>>>> format requested from the sensor may well have changed which may mean
>>>>> these values are now wrong.
>>>>
>>>> This sounds troublesome indeed.
>>>>
>>>>> So firstly, have I understood this correctly? And secondly, what do
>>>>> you think would be the preferred way to fix it? Perhaps the
>>>>> listControls() method should be made "protected" and called at the end
>>>>> of V4L2Subdevice::setFormat? Or should getControls() always update the
>>>>> ranges..?
>>>>
>>>> I suspect the 'right' way to do this is to get the kernel to tell us if
>>>> the control has changed. V4L2 devices can expose events:
>>>>
>>>> https://www.kernel.org/doc/html/latest/driver-api/media/v4l2-event.html
>>>>
>>>> We should be able to receive events for control changes through
>>>> V4L2_EVENT_CTRL:
>>>>
>>>> https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/vidioc-dqevent.html#event-type
>>>>
>>>> We already have a means to subscribe to events in our V4L2Device and
>>>> V4L2VideoDevice classes, so handling control updates there should be a
>>>> case of extending from there.
>>>>
>>>> However, I have one concern on your reports above.
>>>>
>>>> It seems it's not the value that has changed, but the minimum and
>>>> maximums? It looks like the events are reported when the 'value'
>>>> changes, but I don't know if the min/max can be changed? I thought those
>>>> were initialised at control creation time. But perhaps I've just never
>>>> done it ;-)
>>>
>>>
>>> This is correct.  The VBLANK min/max limits change when we change
>>> sensor modes.  When we call V4L2Subdevice::getControl(VBLANK), it
>>> does return the correct "value", but the min/max remain unchanged.
>>> In the current codebase, the min/max values are only updated on a
>>> V4L2Device:listControl() call, which only happens when you open the device.
>>>
>>> Regards,
>>> Naush
>>>
>>>>
>>>>
>>>> Hope this helps (or at least bumps the discussions).
>>>>
>>>> Kieran
>>>>
>>>>
>>>>> Thanks!
>>>>> David
>>>>> _______________________________________________
>>>>> libcamera-devel mailing list
>>>>> libcamera-devel at lists.libcamera.org
>>>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>>>>
>>>>
>>>> --
>>>> Regards
>>>> --
>>>> Kieran
>>>> _______________________________________________
>>>> libcamera-devel mailing list
>>>> libcamera-devel at lists.libcamera.org
>>>> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran