Message-ID: <c138e5dd-e1cd-4a38-8076-dd2691cef224@nxp.com>
Date: Mon, 28 Jul 2025 16:34:10 +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: [RFC 2/2] Documentation: media: Describe exposure and gain
 controls for multiple captures

Hi Laurent,

On 7/27/25 23:27, Laurent Pinchart wrote:

> 
> Hi Mirela,
> 
> On Thu, Jul 24, 2025 at 12:33:41PM +0300, Mirela Rabulea wrote:
>> 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?
> 
> If we have more than two exposures we would probably need an array,
> assuming the sensor can set different ratios between the long and short
> exposures compared to the short and very short exposures ratio. I
> haven't analyzed enough sensors to have a good enough view of the
> typical configuration parameters for exposure ratios.
> 
>> 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.
> 
> I agree. This is an area where the V4L2 control framework, with its
> extensive API and genericity, is counter-productive. When setting the
> V4L2_CID_EXPOSURE_RATIO ratio control, the driver would likely need to
> update the V4L2_CID_EXPOSURE_MULTI control. That's more work, for no
> gain as userspace would really not care.
> 
>>> 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.
> 
> Let's start with just direct exposure control then, and see where it
> leads us.

I will send a v2 for this RFC with documentation updates, according to 
the feedback received so far.

> 
>>>> 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.
> 
> It depends on the sensor, some sensors have multiple contexts but do not
> output images on different virtual channels (or at least not
> necessarily, it can also sometimes be configurable).

Do you have an example? In such case, what are the contexts used for?
> 
>>>> 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 don't think that's needed. Let's stick to the new MULTI controls, and
> see how they work in sensor drivers and in libcamera.

Ack.

> 
>>>>> 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?
> 
> The fake streams. Per-stream controls also add complexity, but are in my
> opinion still potentially worth the complexity. Adding fake streams and
> internal pads seems cumbersome. But maybe I'm worrying too much, if you
> think it's worth investigating, we could look at how it translates to
> patches (for both kernel and userspace).

If we'll have the MULTI controls, I think that's enough.

> 
>>> 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.
> 
> Give me a time machine please, there are so many things I'd like to
> change in V4L2 :-)
> 
>> 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.
> 
>  From a very selfish point of view, considering libcamera, I wouldn't
> mind camera sensors that only expose V4L2_CID_EXPOSURE_MULTI without
> V4L2_CID_EXPOSURE. They will work all fine (once support for
> V4L2_CID_EXPOSURE_MULTI will be merged in libcamera, of course), even in
> the non-HDR case.
> 
> On the other hand, some people may object to all the rest of userspace
> having to be updated to support new sensor drivers. If we want to
> support V4L2_CID_EXPOSURE, I think we can restrict the control to
> operating only when HDR is disabled (an HDR-unaware userspace should
> never enable HDR in the first place, and wouldn't work if it's enabled
> anyway), and define V4L2_CID_EXPOSURE_MULTI as having priority over
> V4L2_CID_EXPOSURE, and setting V4L2_CID_EXPOSURE to the first array
> value. We can also adapt this behaviour to ease the implementation.
> 
> Note that I think this should be implemented in a helper (possibly in the
> control handler core ?), HDR-aware drivers should deal with
> V4L2_CID_EXPOSURE_MULTI, and the V4L2_CID_EXPOSURE control should be
> created by the helper, and fully handled by it. Otherwise we'll make
> drivers more complex, and open the door to variations in behaviour
> between different drivers.

Should I try to include also a patch for this in v2? Do you have more 
details, or example helper to start with?

Thanks for feedback,
Mirela

> 
>>> - 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.
> 
> Ack.
> 
>>> - 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).
> 
> Ack.
> 
>> Well, these are my opinions, more or less justified, I would like to
>> hear your opinion further, as well as anyone's else.
>>
>>>> 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