Message-ID: <e86c8543-b883-4bd3-ae21-87e7c9443957@nxp.com>
Date: Thu, 24 Jul 2025 12:33:41 +0300
From: Mirela Rabulea <mirela.rabulea@....com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.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: Re: Re: [RFC 2/2] Documentation: media: Describe exposure and
 gain controls for multiple captures

Hi Laurent & all,

On 7/23/25 16: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.

Sounds like we could use another control to allow userspace to control 
the exposure ratio, let's hypothetically call it 
V4L2_CID_EXPOSURE_RATIO? Would the ratio be a scalar number, or do you 
think we need an array?
While a combination of the existing V4L2_CID_EXPOSURE + a new 
V4L2_CID_EXPOSURE_RATIO control could make an API for sensors with 
indirect exposure control only, I am concerned that if we were to add 
such a control, we would also need to define it's interaction with 
V4L2_CID_EXPOSURE/V4L2_CID_EXPOSURE_MULTI, I think the logic here can 
get complicated, especially if we begin to think also for sensors that 
support both direct and indirect short exposure control.

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

I think I like better the idea of using the multi-exposure control and 
compute the ratio internally in the driver, it sounds more flexible, in 
case different ratios are needed, maybe for sensors with more than 2 
exposures, it saves us the trouble of adding a new ratio control 
(possibly array) and defining it's interaction with the other controls.

For the sensors that support both direct and indirect short exposure 
control, I like the idea of exposing only the multi controls, and let 
the driver use what it needs from the array, depending on what routes 
are active. But, if needed for backward compatibility with userspace 
applications, we can have both.

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

Stream-aware controls could also do it, in case of context switching we 
have a stream/vc per context.

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

Let me know if you think it is worth investigating any of these paths 
(control as single&array or change control type at runtime).

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

What sounds like complexity, stream-aware controls or fake streams/pads?

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

I think sensor developer's life would be simpler with only 
V4L2_CID_EXPOSURE_MULTI, it would have been ideal if V4L2_CID_EXPOSURE 
was an array in the first place.

For backward compatibility though, which is an important practical 
aspect, we can allow both V4L2_CID_EXPOSURE and V4L2_CID_EXPOSURE_MULTI, 
with the mention that V4L2_CID_EXPOSURE, when used,it has clear effects 
on the first (longest?) exposure, but may have undefined behavior for 
the other exposures (a default ratio could be applied by the driver, or 
a default or previous exposure could be set). On the other hand, 
V4L2_CID_EXPOSURE_MULTI has clear effects on all exposures and would 
recommended it to be used in case of multiple captures.


> 
> - How do we handle ratio-based exposure control ?

For ratio-based exposure control, I'm thinking it is better to use 
V4L2_CID_EXPOSURE_MULTI for both direct and indirect short exposure 
control. Let the driver calculate the ratio, and there can be n-1 ratios 
(n=number of exposures). This would save us the troubles to define and 
manage the interaction of a ratio control with the exposure controls.


> 
> - In which order are the exposures (and gains) stored in the array ?

With the os08a20 in mind, I would propose from the longest to the 
shortest (when the sensor operates in non-hdr mode, only the long 
exposure registers are used, that is the first and only exposure).

Well, these are my opinions, more or less justified, I would like to 
hear your opinion further, as well as anyone's else.

Thanks,
Mirela

> 
>> 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.
> 
> --
> Regards,
> 
> Laurent Pinchart

Powered by blists - more mailing lists