mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mirela Rabulea <mirela.rabulea@nxp.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: mchehab@kernel.org, sakari.ailus@linux.intel.com,
	hverkuil-cisco@xs4all.nl, ribalda@chromium.org,
	jai.luthra@ideasonboard.com, laurentiu.palcu@nxp.com,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	LnxRevLi@nxp.com, julien.vuillaumier@nxp.com,
	celine.laurencin@nxp.com
Subject: Re: Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
Date: Sun, 20 Jul 2025 22:02:13 +0300	[thread overview]
Message-ID: <fcb87e2d-5ddf-4f33-b5f7-5af67c438af5@nxp.com> (raw)
In-Reply-To: <20250716000738.GF19299@pendragon.ideasonboard.com>

Hi Laurent,

On 7/16/25 03:07, Laurent Pinchart wrote:
> 
> 
> Hi Mirela,
> 
> Thank you for the patch.
> 
> 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. 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

> 
> 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.
Or do you mean changing the current controls type from 
V4L2_CTRL_TYPE_INTEGER to u32 array? Would that not cause issues with 
applications already using current controls?

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

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

Regards,
Mirela

> 
>> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.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


  reply	other threads:[~2025-07-20 19:02 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-10 22:05 [RFC 0/2] Add standard " Mirela Rabulea
2025-07-10 22:05 ` [RFC 1/2] media: Add " Mirela Rabulea
2025-07-10 22:05 ` [RFC 2/2] Documentation: media: Describe " Mirela Rabulea
2025-07-16  0:07   ` Laurent Pinchart
2025-07-20 19:02     ` Mirela Rabulea [this message]
2025-07-23 13:49       ` Laurent Pinchart
2025-07-24  9:33         ` Mirela Rabulea
2025-07-27 20:27           ` Laurent Pinchart
2025-07-28 13:34             ` Mirela Rabulea
2025-07-25  9:05         ` hans
2025-07-15 23:59 ` [RFC 0/2] Add standard " Laurent Pinchart
2025-07-16  0:12   ` Laurent Pinchart
2025-07-20 18:56     ` Mirela Rabulea
2025-07-22  9:53       ` Julien Vuillaumier
2025-07-23 15:02         ` Laurent Pinchart
2025-07-25  9:01           ` hans
2025-07-25  9:27             ` Laurent Pinchart
2025-07-22  8:46     ` Julien Vuillaumier
2025-07-23 14:00       ` Laurent Pinchart
2025-07-28 15:42         ` Mirela Rabulea

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fcb87e2d-5ddf-4f33-b5f7-5af67c438af5@nxp.com \
    --to=mirela.rabulea@nxp.com \
    --cc=LnxRevLi@nxp.com \
    --cc=celine.laurencin@nxp.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jai.luthra@ideasonboard.com \
    --cc=julien.vuillaumier@nxp.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=laurentiu.palcu@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=ribalda@chromium.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®