Message-ID: <8d452d63-b52b-4cda-acf0-14deeb81d65e@jjverkuil.nl>
Date: Fri, 25 Jul 2025 11:05:28 +0200
From: hans@...erkuil.nl
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Mirela Rabulea <mirela.rabulea@....com>
Cc: mchehab@...nel.org, sakari.ailus@...ux.intel.com,
 hverkuil-cisco@...all.nl, ribalda@...omium.org, jai.luthra@...asonboard.com,
 laurentiu.palcu@....com, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org, LnxRevLi@....com, julien.vuillaumier@....com,
 celine.laurencin@....com
Subject: Re: [RFC 2/2] Documentation: media: Describe exposure and gain
 controls for multiple captures

On 23/07/2025 15:49, Laurent Pinchart wrote:
> On Sun, Jul 20, 2025 at 10:02:13PM +0300, Mirela Rabulea wrote:
>> On 7/16/25 03:07, Laurent Pinchart wrote:
>>> On Fri, Jul 11, 2025 at 01:05:44AM +0300, Mirela Rabulea wrote:
>>>> The standard controls for exposure and gains allow a
>>>> single value, for a single capture. For sensors with HDR
>>>> capabilities or context switching, this is not enough, so
>>>> add new controls that allow multiple values, one for each
>>>> capture.
>>>
>>> One important question not addressed by this patch is how the new
>>> controls interact with the old ones. For instance, if a sensor
>>> implements 2-DOL, it should expose a V4L2_CID_EXPOSURE_MULTI control
>>> with 2 elements. Should it also expose the V4L2_CID_EXPOSURE control,
>>> when operating in SDR mode ? What should happen when both controls are
>>> set ?
>>
>> Yes, it's a good point. I experimented with the option of implementing 
>> both, at least for backward compatibility (libcamera requires them) and 
>> kept them consistent, I mean if V4L2_CID_EXPOSURE_MULTI values change, 
>> also change V4L2_CID_EXPOSURE and viceversa, so basically keep 
>> consistent the values from V4L2_CID_EXPOSURE with the values for the 
>> first exposure from V4L2_CID_EXPOSURE_MULTI. Also, I had to check if hdr 
>> mode is not enabled, do nothing in s_ctrl for V4L2_CID_EXPOSURE_MULTI 
>> (cannot return error, as it will make __v4l2_ctrl_handler_setup fail).
>>
>>> There are also sensors that implement multi-exposure with direct control
>>> of the long exposure, and indirect control of the short exposure through
>>> an exposure ratio. The sensors I'm working on support both, so we could
>>> just ignore the exposure ratio, but if I recall correctly CCS allows
>>> sensors to implement exposure ratio only without direct short exposure
>>> control. How should we deal with that ?
>>
>> I'm not sure I understand, but in case of indirect short exposure 
>> control I think we do not need these multiple exposure controls, we can 
>> use the existing ones, as only the value for the long exposure is 
>> needed, the driver can derive the value for the short exposure using the 
>> ratio.
> 
> I'm talking about sensors that implement the CCS exposure ratio, or a
> similar mechanism. With those sensors, the long exposure time is set
> directly, and the short exposure time is calculated by the sensor by
> dividing the long exposure time by a ratio. The ratio is programmed by
> the driver through a register. The ratio could be set to a fixed value,
> but I think there are use cases for controlling it from userspace.
> 
> Some sensors support both direct control of the short exposure and
> indirect control through a ratio, while other may support indirect
> control only. For the sensors that support both, we could decide to only
> expose the multi-exposure control with direct control of the short
> exposure. For sensors that support indirect control only, we need to
> define an API. We could possibly still use the same multi-exposure
> control and compute the ratio internally in the driver, but there may be
> a better option.
> 
>> In some cases, this may be enough, but when direct individual 
>> control is needed for both long and short exposure, then we need the 
>> multiple exposure controls. Do you have a specific sensor example in mind?
>> I think in the past we looked at imx708, and my understanding was that 
>> the exposure control affects only the long exposure and the sensor will 
>> automatically divide the medium and short one with the corresponding ratio:
>> https://github.com/raspberrypi/linux/blob/rpi-6.12.y/drivers/media/i2c/imx708.c
> 
> The ratio seems configurable. Register 0x0220 is programmed to 0x62,
> which selects ratio-based control of the exposure. I don't know if the
> sensor supports direct control of the short (and very short) exposure.
> 
>>> Finally, I was recently wondering if it would be possible to reuse the
>>> existing controls instead, allowing them to be either single values or
>>> arrays. The idea would be that setting the control to a single value
>>> (essentially ignoring it's an array) would provide the current
>>> behaviour, while setting values for multiple elements would control the
>>> separate exposures.
>>
>> You mean to divide the 32 bits value of the current controls between the 
>> multiple exposures?
>> Just one comment here, we have encountered the ox03c10 sensor with 4 
>> exposures (that will leave only 8 bits per exposure), and the ox05b1s 
>> sensor with context switching and the exposure on 24 bits (for 2 
>> contexts, 2x24=48). So reusing current 32 bit controls  might not be 
>> enough.
> 
> I'm not sure the controls here should be used in the context switching
> use case. It would be better to define a more generic mechanism that
> supports multiple contexts for all controls.
> 
>> Or do you mean changing the current controls type from 
>> V4L2_CTRL_TYPE_INTEGER to u32 array?
> 
> Yes, this is what I mean.
> 
>> Would that not cause issues with 
>> applications already using current controls?
> 
> That would only work if the kernel could handle some type of backward
> compatibility, doing the right thing when userspace sets the control to
> a single value (as opposed to an array of values). That's probably not
> very realistic, as the control would enumerate as a compound control,
> and that may break existing userspace.
> 
> Another option would be to change the control type at runtime based on
> whether or not HDR is enabled, but that also sounds like it will cause
> lots of issue.

While technically it is possible to change the type at initialization time
(although probably not dynamically), it is basically a uAPI change since it
was never documented that you had to check the control type first for these
controls before using them.

I think introducing these MULTI variants makes sense and is the right way to
go.

Regards,

	Hans

>>> I haven't checked if the control framework supports
>>> this, or if it could be supported with minimum changes. The advantage is
>>> that we wouldn't need to define how the new and old controls interact if
>>> we don't introduce new controls. 
>>
>> I think the same advantage will be achieved with stream-aware controls 
>> (no new controls, also the min/max/def semantics remain clear), but 
>> there is the issue we do not have streams if the sensor does internally 
>> the hdr merge. Does it sound any better to introduce some fake streams 
>> or pads that are not associated with any pixel stream, but just to allow 
>> multiple exposure control?
> 
> That also sounds like quite a bit of complexity for little gain. It
> seems that new controls are the best option. There are still a few
> issues to solve:
> 
> - Should sensors that support multi-exposure (or gains) implement
>   V4L2_CID_EXPOSURE for backward compatibility, or only
>   V4L2_CID_EXPOSURE_MULTI ? If both are implemented, how should the two
>   controls interact ?
> 
> - How do we handle ratio-based exposure control ?
> 
> - In which order are the exposures (and gains) stored in the array ?
> 
>> BTW, Jay, what are your plans around the stream-aware controls?
>>
>> Thanks again for feedback, Laurent!
>>
>>> Hans, what do you think ?
>>
>> Same question from me ;)
>>
>>>> Signed-off-by: Mirela Rabulea <mirela.rabulea@....com>
>>>> ---
>>>>   .../media/v4l/ext-ctrls-image-source.rst             | 12 ++++++++++++
>>>>   1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
>>>> index 71f23f131f97..6efdb58dacf5 100644
>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-source.rst
>>>> @@ -92,3 +92,15 @@ Image Source Control IDs
>>>>       representing a gain of exactly 1.0. For example, if this default value
>>>>       is reported as being (say) 128, then a value of 192 would represent
>>>>       a gain of exactly 1.5.
>>>> +
>>>> +``V4L2_CID_EXPOSURE_MULTI (__u32 array)``
>>>> +    Same as V4L2_CID_EXPOSURE, but for multiple exposure sensors. Each
>>>> +    element of the array holds the exposure value for one capture.
>>>> +
>>>> +``V4L2_CID_AGAIN_MULTI (__u32 array)``
>>>> +    Same as V4L2_CID_ANALOGUE_GAIN, but for multiple exposure sensors. Each
>>>> +    element of the array holds the analog gain value for one capture.
>>>> +
>>>> +``V4L2_CID_DGAIN_MULTI (__u32 array)``
>>>> +    Same as V4L2_CID_DIGITAL_GAIN, but for multiple exposure sensors. Each
>>>> +    element of the array holds the digital gain value for one capture.
> 

Powered by blists - more mailing lists