* [RFC 0/2] Add standard exposure and gain controls for multiple captures
@ 2025-07-10 22:05 Mirela Rabulea
2025-07-10 22:05 ` [RFC 1/2] media: Add " Mirela Rabulea
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-10 22:05 UTC (permalink / raw)
To: mchehab, sakari.ailus, hverkuil-cisco, laurent.pinchart+renesas,
ribalda, jai.luthra, laurentiu.palcu
Cc: linux-media, linux-kernel, LnxRevLi, julien.vuillaumier,
celine.laurencin
Add new standard controls as U32 arrays, for sensors with multiple
captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
that have multiple captures, but the HDR merge is done inside the sensor,
in the end exposing a single stream, but still requiring AEC control
for all captures.
All controls are in the same class, so they could all be set
atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
useful in case of sensors with context switching.
Each element of the array will hold an u32 value (exposure or gain)
for one capture. The size of the array is up to the sensor driver which
will implement the controls and initialize them via v4l2_ctrl_new_custom().
With this approach, the user-space will have to set valid values
for all the captures represented in the array.
The v4l2-core only supports one scalar min/max/step value for the
entire array, and each element is validated and adjusted to be within
these bounds in v4l2_ctrl_type_op_validate(). The significance for the
maximum value for the exposure control could be "the max value for the
long exposure" or "the max value for the sum of all exposures". If none
of these is ok, the sensor driver can adjust the values as supported and
the user space can use the TRY operation to query the sensor for the
minimum or maximum values.
Mirela Rabulea (2):
LF-15161-6: media: Add exposure and gain controls for multiple
captures
LF-15161-7: Documentation: media: Describe exposure and gain controls
for multiple captures
.../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
include/uapi/linux/v4l2-controls.h | 3 +++
3 files changed, 23 insertions(+)
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC 1/2] media: Add exposure and gain controls for multiple captures
2025-07-10 22:05 [RFC 0/2] Add standard exposure and gain controls for multiple captures Mirela Rabulea
@ 2025-07-10 22:05 ` Mirela Rabulea
2025-07-10 22:05 ` [RFC 2/2] Documentation: media: Describe " Mirela Rabulea
2025-07-15 23:59 ` [RFC 0/2] Add standard " Laurent Pinchart
2 siblings, 0 replies; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-10 22:05 UTC (permalink / raw)
To: mchehab, sakari.ailus, hverkuil-cisco, laurent.pinchart+renesas,
ribalda, jai.luthra, laurentiu.palcu
Cc: linux-media, linux-kernel, LnxRevLi, julien.vuillaumier,
celine.laurencin
Add V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
V4L2_CID_DGAIN_MULTI for exposure and gain control for
multiple exposure sensors.
Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
include/uapi/linux/v4l2-controls.h | 3 +++
2 files changed, 11 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 1ea52011247a..65c468a3b01c 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -1155,6 +1155,9 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_TEST_PATTERN_BLUE: return "Blue Pixel Value";
case V4L2_CID_TEST_PATTERN_GREENB: return "Green (Blue) Pixel Value";
case V4L2_CID_NOTIFY_GAINS: return "Notify Gains";
+ case V4L2_CID_EXPOSURE_MULTI: return "Exposure, Multiple Captures";
+ case V4L2_CID_AGAIN_MULTI: return "Analog Gain, Multiple Captures";
+ case V4L2_CID_DGAIN_MULTI: return "Digital Gain, Multiple Captures";
/* Image processing controls */
/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1607,6 +1610,11 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
*type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
break;
+ case V4L2_CID_EXPOSURE_MULTI:
+ case V4L2_CID_AGAIN_MULTI:
+ case V4L2_CID_DGAIN_MULTI:
+ *type = V4L2_CTRL_TYPE_U32;
+ break;
default:
*type = V4L2_CTRL_TYPE_INTEGER;
break;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 974fd254e573..0d5a34a6578e 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1213,6 +1213,9 @@ enum v4l2_jpeg_chroma_subsampling {
#define V4L2_CID_TEST_PATTERN_GREENB (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 7)
#define V4L2_CID_UNIT_CELL_SIZE (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 8)
#define V4L2_CID_NOTIFY_GAINS (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 9)
+#define V4L2_CID_EXPOSURE_MULTI (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 10)
+#define V4L2_CID_AGAIN_MULTI (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 11)
+#define V4L2_CID_DGAIN_MULTI (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 12)
/* Image processing controls */
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
2025-07-10 22:05 [RFC 0/2] Add standard exposure and gain controls for multiple captures Mirela Rabulea
2025-07-10 22:05 ` [RFC 1/2] media: Add " Mirela Rabulea
@ 2025-07-10 22:05 ` Mirela Rabulea
2025-07-16 0:07 ` Laurent Pinchart
2025-07-15 23:59 ` [RFC 0/2] Add standard " Laurent Pinchart
2 siblings, 1 reply; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-10 22:05 UTC (permalink / raw)
To: mchehab, sakari.ailus, hverkuil-cisco, laurent.pinchart+renesas,
ribalda, jai.luthra, laurentiu.palcu
Cc: linux-media, linux-kernel, LnxRevLi, julien.vuillaumier,
celine.laurencin
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.
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.
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-10 22:05 [RFC 0/2] Add standard exposure and gain controls for multiple captures 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-15 23:59 ` Laurent Pinchart
2025-07-16 0:12 ` Laurent Pinchart
2 siblings, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-15 23:59 UTC (permalink / raw)
To: Mirela Rabulea
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
Hi Mirela,
On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
> Add new standard controls as U32 arrays, for sensors with multiple
> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
> that have multiple captures, but the HDR merge is done inside the sensor,
> in the end exposing a single stream, but still requiring AEC control
> for all captures.
It's also useful for sensors supporting DOL or DCG with HDR merge being
performed outside of the sensor.
> All controls are in the same class, so they could all be set
> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
> useful in case of sensors with context switching.
Agreed, we should be able to set them all. Are we still unable to set
controls from multiple classes atomatically ? I thought that limitation
has been lifted.
> Each element of the array will hold an u32 value (exposure or gain)
> for one capture. The size of the array is up to the sensor driver which
> will implement the controls and initialize them via v4l2_ctrl_new_custom().
> With this approach, the user-space will have to set valid values
> for all the captures represented in the array.
I'll comment on the controls themselves in patch 2/2.
> The v4l2-core only supports one scalar min/max/step value for the
> entire array, and each element is validated and adjusted to be within
> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
> maximum value for the exposure control could be "the max value for the
> long exposure" or "the max value for the sum of all exposures". If none
> of these is ok, the sensor driver can adjust the values as supported and
> the user space can use the TRY operation to query the sensor for the
> minimum or maximum values.
Hmmmm... I wonder if we would need the ability to report different
limits for different array elements. There may be over-engineering
though, my experience with libcamera is that userspace really needs
detailed information about those controls, and attempting to convey the
precise information through the kernel-userspace API is bound to fail.
That's why we implement a sensor database in libcamera, with information
about how to convert control values to real gain and exposure time.
Exposing (close to) raw register values and letting userspace handle the
rest may be better.
> Mirela Rabulea (2):
> LF-15161-6: media: Add exposure and gain controls for multiple
> captures
> LF-15161-7: Documentation: media: Describe exposure and gain controls
> for multiple captures
Did you forget to remove the LF-* identifiers ? :-)
>
> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
> include/uapi/linux/v4l2-controls.h | 3 +++
> 3 files changed, 23 insertions(+)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
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
0 siblings, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-16 0:07 UTC (permalink / raw)
To: Mirela Rabulea
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
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 ?
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 ?
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. 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. Hans, what do you think ?
> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
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 8:46 ` Julien Vuillaumier
0 siblings, 2 replies; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-16 0:12 UTC (permalink / raw)
To: Mirela Rabulea
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
> > Add new standard controls as U32 arrays, for sensors with multiple
> > captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
> > V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
> > that have multiple captures, but the HDR merge is done inside the sensor,
> > in the end exposing a single stream, but still requiring AEC control
> > for all captures.
>
> It's also useful for sensors supporting DOL or DCG with HDR merge being
> performed outside of the sensor.
Regarless of where HDR merge is implemented, we will also need controls
to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
standardize the values, and that's not good enough. At least for DOL and
DCG with HDR merge implemented outside of the sensor, we need to
standardize the modes.
Can you tell which sensor(s) you're working with ?
> > All controls are in the same class, so they could all be set
> > atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
> > useful in case of sensors with context switching.
>
> Agreed, we should be able to set them all. Are we still unable to set
> controls from multiple classes atomatically ? I thought that limitation
> has been lifted.
>
> > Each element of the array will hold an u32 value (exposure or gain)
> > for one capture. The size of the array is up to the sensor driver which
> > will implement the controls and initialize them via v4l2_ctrl_new_custom().
> > With this approach, the user-space will have to set valid values
> > for all the captures represented in the array.
>
> I'll comment on the controls themselves in patch 2/2.
>
> > The v4l2-core only supports one scalar min/max/step value for the
> > entire array, and each element is validated and adjusted to be within
> > these bounds in v4l2_ctrl_type_op_validate(). The significance for the
> > maximum value for the exposure control could be "the max value for the
> > long exposure" or "the max value for the sum of all exposures". If none
> > of these is ok, the sensor driver can adjust the values as supported and
> > the user space can use the TRY operation to query the sensor for the
> > minimum or maximum values.
>
> Hmmmm... I wonder if we would need the ability to report different
> limits for different array elements. There may be over-engineering
> though, my experience with libcamera is that userspace really needs
> detailed information about those controls, and attempting to convey the
> precise information through the kernel-userspace API is bound to fail.
> That's why we implement a sensor database in libcamera, with information
> about how to convert control values to real gain and exposure time.
> Exposing (close to) raw register values and letting userspace handle the
> rest may be better.
>
> > Mirela Rabulea (2):
> > LF-15161-6: media: Add exposure and gain controls for multiple
> > captures
> > LF-15161-7: Documentation: media: Describe exposure and gain controls
> > for multiple captures
>
> Did you forget to remove the LF-* identifiers ? :-)
>
> >
> > .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
> > drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
> > include/uapi/linux/v4l2-controls.h | 3 +++
> > 3 files changed, 23 insertions(+)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-16 0:12 ` Laurent Pinchart
@ 2025-07-20 18:56 ` Mirela Rabulea
2025-07-22 9:53 ` Julien Vuillaumier
2025-07-22 8:46 ` Julien Vuillaumier
1 sibling, 1 reply; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-20 18:56 UTC (permalink / raw)
To: Laurent Pinchart
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
Hi Laurent,
On 7/16/25 03:12, Laurent Pinchart wrote:
>
>
> On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
>> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
>>> Add new standard controls as U32 arrays, for sensors with multiple
>>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
>>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
>>> that have multiple captures, but the HDR merge is done inside the sensor,
>>> in the end exposing a single stream, but still requiring AEC control
>>> for all captures.
>>
>> It's also useful for sensors supporting DOL or DCG with HDR merge being
>> performed outside of the sensor.
>
> Regarless of where HDR merge is implemented, we will also need controls
> to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
> standardize the values, and that's not good enough. At least for DOL and
> DCG with HDR merge implemented outside of the sensor, we need to
> standardize the modes.
>
> Can you tell which sensor(s) you're working with ?
We are working mostly with these 3:
Omnivision's os08a20 (2 exposures staggered hdr, each exposure on a
separate virtual channel, there are also other hdr modes which we do not
use)
Omnivision ox05b1s (RGB-Ir with context switching based on group holds,
1 context optimized for RGB, the other context optimized for Ir, each
context on a different virtual channel)
Omnivision ox03c10 (4 exposures, hdr merge in sensor).
>
>>> All controls are in the same class, so they could all be set
>>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
>>> useful in case of sensors with context switching.
>>
>> Agreed, we should be able to set them all. Are we still unable to set
>> controls from multiple classes atomatically ? I thought that limitation
>> has been lifted.
Maybe I need some background check on this, but looking at kernel tag
next-20250718, this comment still lies in the documentation:
"These ioctls allow the caller to get or set multiple controls
atomically. Control IDs are grouped into control classes (see
:ref:`ctrl-class`) and all controls in the control array must belong
to the same control class."
Maybe it needs to be updated, or not...since there is also this check in
check_ext_ctrls():
/* Check that all controls are from the same control class. */
for (i = 0; i < c->count; i++) {
if (V4L2_CTRL_ID2WHICH(c->controls[i].id) != c->which) {
c->error_idx = ioctl == VIDIOC_TRY_EXT_CTRLS ? i :
c->count;
return false;
}
}
There is also another inconvenient, the VIDIOC_S_EXT_CTRLS does not
reach the v4l2 subdevice driver, what we get in the sensor driver is a
set of .s_ctrl calls. I don't know about other sensors, but for the
Omivision sensors which I am familiar with, the group holds feature
could be used to get multiple registers to be applied atomically in the
same frame, but the sensor driver would need to know when to start and
when to end filling the group hold with the desired registers. If there
is some similar feature in other sensors, I think the VIDIOC_S_EXT_CTRLS
should have a corresponding v4l2-subdev operation, so that it can be
implemented in the sensor subdevice driver. This would probably require
some changes in the v4l2 core, as currently the subdev_do_ioctl()
function does not let the VIDIOC_S_EXT_CTRLS go to the subdevice.
Laurent, Hans, any thoughts on this?
>>
>>> Each element of the array will hold an u32 value (exposure or gain)
>>> for one capture. The size of the array is up to the sensor driver which
>>> will implement the controls and initialize them via v4l2_ctrl_new_custom().
>>> With this approach, the user-space will have to set valid values
>>> for all the captures represented in the array.
>>
>> I'll comment on the controls themselves in patch 2/2.
>>
>>> The v4l2-core only supports one scalar min/max/step value for the
>>> entire array, and each element is validated and adjusted to be within
>>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
>>> maximum value for the exposure control could be "the max value for the
>>> long exposure" or "the max value for the sum of all exposures". If none
>>> of these is ok, the sensor driver can adjust the values as supported and
>>> the user space can use the TRY operation to query the sensor for the
>>> minimum or maximum values.
>>
>> Hmmmm... I wonder if we would need the ability to report different
>> limits for different array elements. There may be over-engineering
>> though, my experience with libcamera is that userspace really needs
>> detailed information about those controls, and attempting to convey the
>> precise information through the kernel-userspace API is bound to fail.
>> That's why we implement a sensor database in libcamera, with information
>> about how to convert control values to real gain and exposure time.
>> Exposing (close to) raw register values and letting userspace handle the
>> rest may be better.
Julien, any thoughts on this?
If we don't need to report different limits for different array
elements, we are fine, just we need to document better what those limits
stand for in case of arrays.
>>
>>> Mirela Rabulea (2):
>>> LF-15161-6: media: Add exposure and gain controls for multiple
>>> captures
>>> LF-15161-7: Documentation: media: Describe exposure and gain controls
>>> for multiple captures
>>
>> Did you forget to remove the LF-* identifiers ? :-)
Yes, at least in the cover-letter, my bad :(
Thanks for feedback.
Regards,
Mirela
>>
>>>
>>> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
>>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
>>> include/uapi/linux/v4l2-controls.h | 3 +++
>>> 3 files changed, 23 insertions(+)
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
2025-07-16 0:07 ` Laurent Pinchart
@ 2025-07-20 19:02 ` Mirela Rabulea
2025-07-23 13:49 ` Laurent Pinchart
0 siblings, 1 reply; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-20 19:02 UTC (permalink / raw)
To: Laurent Pinchart
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-16 0:12 ` Laurent Pinchart
2025-07-20 18:56 ` Mirela Rabulea
@ 2025-07-22 8:46 ` Julien Vuillaumier
2025-07-23 14:00 ` Laurent Pinchart
1 sibling, 1 reply; 20+ messages in thread
From: Julien Vuillaumier @ 2025-07-22 8:46 UTC (permalink / raw)
To: Laurent Pinchart, Mirela Rabulea
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
celine.laurencin
Hi Laurent,
On 16/07/2025 02:12, Laurent Pinchart wrote:
> On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
>> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
>>> Add new standard controls as U32 arrays, for sensors with multiple
>>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
>>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
>>> that have multiple captures, but the HDR merge is done inside the sensor,
>>> in the end exposing a single stream, but still requiring AEC control
>>> for all captures.
>>
>> It's also useful for sensors supporting DOL or DCG with HDR merge being
>> performed outside of the sensor.
>
> Regarless of where HDR merge is implemented, we will also need controls
> to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
> standardize the values, and that's not good enough. At least for DOL and
> DCG with HDR merge implemented outside of the sensor, we need to
> standardize the modes.
For the HDR-capable sensors with the HDR merge implemented outside, the
short capture(s) are likely implemented as separate streams, in order to
match the raw camera sensor model.
In that case, the SDR/HDR mode switch, when supported, can be done by
configuring the sensor device internal route for the short capture stream.
You mentioned the need to be able to select the HDR mode in a standard
way. Could you elaborate on the foreseen usage: would it be to select
SDR/HDR operation, to select between different HDR sub-modes, to inform
user space about HDR capability... ?
>
> Can you tell which sensor(s) you're working with ?
>
>>> All controls are in the same class, so they could all be set
>>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
>>> useful in case of sensors with context switching.
>>
>> Agreed, we should be able to set them all. Are we still unable to set
>> controls from multiple classes atomatically ? I thought that limitation
>> has been lifted.
>>
>>> Each element of the array will hold an u32 value (exposure or gain)
>>> for one capture. The size of the array is up to the sensor driver which
>>> will implement the controls and initialize them via v4l2_ctrl_new_custom().
>>> With this approach, the user-space will have to set valid values
>>> for all the captures represented in the array.
>>
>> I'll comment on the controls themselves in patch 2/2.
>>
>>> The v4l2-core only supports one scalar min/max/step value for the
>>> entire array, and each element is validated and adjusted to be within
>>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
>>> maximum value for the exposure control could be "the max value for the
>>> long exposure" or "the max value for the sum of all exposures". If none
>>> of these is ok, the sensor driver can adjust the values as supported and
>>> the user space can use the TRY operation to query the sensor for the
>>> minimum or maximum values.
>>
>> Hmmmm... I wonder if we would need the ability to report different
>> limits for different array elements. There may be over-engineering
>> though, my experience with libcamera is that userspace really needs
>> detailed information about those controls, and attempting to convey the
>> precise information through the kernel-userspace API is bound to fail.
>> That's why we implement a sensor database in libcamera, with information
>> about how to convert control values to real gain and exposure time.
>> Exposing (close to) raw register values and letting userspace handle the
>> rest may be better.
>>
>>> Mirela Rabulea (2):
>>> LF-15161-6: media: Add exposure and gain controls for multiple
>>> captures
>>> LF-15161-7: Documentation: media: Describe exposure and gain controls
>>> for multiple captures
>>
>> Did you forget to remove the LF-* identifiers ? :-)
>>
>>>
>>> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
>>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
>>> include/uapi/linux/v4l2-controls.h | 3 +++
>>> 3 files changed, 23 insertions(+)
>
> --
> Regards,
>
> Laurent Pinchart
Thanks,
Julien
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-20 18:56 ` Mirela Rabulea
@ 2025-07-22 9:53 ` Julien Vuillaumier
2025-07-23 15:02 ` Laurent Pinchart
0 siblings, 1 reply; 20+ messages in thread
From: Julien Vuillaumier @ 2025-07-22 9:53 UTC (permalink / raw)
To: Mirela Rabulea, Laurent Pinchart
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
celine.laurencin
Hi Mirela,
On 20/07/2025 20:56, Mirela Rabulea wrote:
> Hi Laurent,
>
> On 7/16/25 03:12, Laurent Pinchart wrote:
>>
>>
>> On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
>>> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
>>>> Add new standard controls as U32 arrays, for sensors with multiple
>>>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
>>>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
>>>> that have multiple captures, but the HDR merge is done inside the
>>>> sensor,
>>>> in the end exposing a single stream, but still requiring AEC control
>>>> for all captures.
>>>
>>> It's also useful for sensors supporting DOL or DCG with HDR merge being
>>> performed outside of the sensor.
>>
>> Regarless of where HDR merge is implemented, we will also need controls
>> to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
>> standardize the values, and that's not good enough. At least for DOL and
>> DCG with HDR merge implemented outside of the sensor, we need to
>> standardize the modes.
>>
>> Can you tell which sensor(s) you're working with ?
>
> We are working mostly with these 3:
> Omnivision's os08a20 (2 exposures staggered hdr, each exposure on a
> separate virtual channel, there are also other hdr modes which we do not
> use)
> Omnivision ox05b1s (RGB-Ir with context switching based on group holds,
> 1 context optimized for RGB, the other context optimized for Ir, each
> context on a different virtual channel)
> Omnivision ox03c10 (4 exposures, hdr merge in sensor).
>
>>
>>>> All controls are in the same class, so they could all be set
>>>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
>>>> useful in case of sensors with context switching.
>>>
>>> Agreed, we should be able to set them all. Are we still unable to set
>>> controls from multiple classes atomatically ? I thought that limitation
>>> has been lifted.
>
>
> Maybe I need some background check on this, but looking at kernel tag
> next-20250718, this comment still lies in the documentation:
> "These ioctls allow the caller to get or set multiple controls
> atomically. Control IDs are grouped into control classes (see
> :ref:`ctrl-class`) and all controls in the control array must belong
> to the same control class."
>
> Maybe it needs to be updated, or not...since there is also this check in
> check_ext_ctrls():
> /* Check that all controls are from the same control class. */
> for (i = 0; i < c->count; i++) {
> if (V4L2_CTRL_ID2WHICH(c->controls[i].id) != c->which) {
> c->error_idx = ioctl == VIDIOC_TRY_EXT_CTRLS ? i :
> c->count;
> return false;
> }
> }
>
> There is also another inconvenient, the VIDIOC_S_EXT_CTRLS does not
> reach the v4l2 subdevice driver, what we get in the sensor driver is a
> set of .s_ctrl calls. I don't know about other sensors, but for the
> Omivision sensors which I am familiar with, the group holds feature
> could be used to get multiple registers to be applied atomically in the
> same frame, but the sensor driver would need to know when to start and
> when to end filling the group hold with the desired registers. If there
> is some similar feature in other sensors, I think the VIDIOC_S_EXT_CTRLS
> should have a corresponding v4l2-subdev operation, so that it can be
> implemented in the sensor subdevice driver. This would probably require
> some changes in the v4l2 core, as currently the subdev_do_ioctl()
> function does not let the VIDIOC_S_EXT_CTRLS go to the subdevice.
>
> Laurent, Hans, any thoughts on this?
>
>
>>>
>>>> Each element of the array will hold an u32 value (exposure or gain)
>>>> for one capture. The size of the array is up to the sensor driver which
>>>> will implement the controls and initialize them via
>>>> v4l2_ctrl_new_custom().
>>>> With this approach, the user-space will have to set valid values
>>>> for all the captures represented in the array.
>>>
>>> I'll comment on the controls themselves in patch 2/2.
>>>
>>>> The v4l2-core only supports one scalar min/max/step value for the
>>>> entire array, and each element is validated and adjusted to be within
>>>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
>>>> maximum value for the exposure control could be "the max value for the
>>>> long exposure" or "the max value for the sum of all exposures". If none
>>>> of these is ok, the sensor driver can adjust the values as supported
>>>> and
>>>> the user space can use the TRY operation to query the sensor for the
>>>> minimum or maximum values.
>>>
>>> Hmmmm... I wonder if we would need the ability to report different
>>> limits for different array elements. There may be over-engineering
>>> though, my experience with libcamera is that userspace really needs
>>> detailed information about those controls, and attempting to convey the
>>> precise information through the kernel-userspace API is bound to fail.
>>> That's why we implement a sensor database in libcamera, with information
>>> about how to convert control values to real gain and exposure time.
>>> Exposing (close to) raw register values and letting userspace handle the
>>> rest may be better.
>
> Julien, any thoughts on this?
Reporting min/max value per array element could have made sense for some
controls. For instance we have a HDR sensor whose long capture analog
gain range is different from the shorter captures gain. Conversely, it
may not work well for the multi-capture exposure control where the
constraint can be more about the sum of the exposures for each capture
rather than the individual exposure values. In that case, exposing
min/max values per array element does not really help the user space.
Thus, having the user space to have the necessary insight into each
sensor specifics for its AEC control seems to be the versatile option.
>
> If we don't need to report different limits for different array
> elements, we are fine, just we need to document better what those limits
> stand for in case of arrays.
>
>>>
>>>> Mirela Rabulea (2):
>>>> LF-15161-6: media: Add exposure and gain controls for multiple
>>>> captures
>>>> LF-15161-7: Documentation: media: Describe exposure and gain
>>>> controls
>>>> for multiple captures
>>>
>>> Did you forget to remove the LF-* identifiers ? :-)
>
> Yes, at least in the cover-letter, my bad :(
>
> Thanks for feedback.
>
> Regards,
> Mirela
>>>
>>>>
>>>> .../media/v4l/ext-ctrls-image-source.rst | 12
>>>> ++++++++++++
>>>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
>>>> include/uapi/linux/v4l2-controls.h | 3 +++
>>>> 3 files changed, 23 insertions(+)
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
>
Thanks,
Julien
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
2025-07-20 19:02 ` Mirela Rabulea
@ 2025-07-23 13:49 ` Laurent Pinchart
2025-07-24 9:33 ` Mirela Rabulea
2025-07-25 9:05 ` hans
0 siblings, 2 replies; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-23 13:49 UTC (permalink / raw)
To: Mirela Rabulea
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
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.
> > 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@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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-22 8:46 ` Julien Vuillaumier
@ 2025-07-23 14:00 ` Laurent Pinchart
2025-07-28 15:42 ` Mirela Rabulea
0 siblings, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-23 14:00 UTC (permalink / raw)
To: Julien Vuillaumier
Cc: Mirela Rabulea, mchehab, sakari.ailus, hverkuil-cisco, ribalda,
jai.luthra, laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
celine.laurencin
On Tue, Jul 22, 2025 at 10:46:16AM +0200, Julien Vuillaumier wrote:
> On 16/07/2025 02:12, Laurent Pinchart wrote:
> > On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
> >> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
> >>> Add new standard controls as U32 arrays, for sensors with multiple
> >>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
> >>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
> >>> that have multiple captures, but the HDR merge is done inside the sensor,
> >>> in the end exposing a single stream, but still requiring AEC control
> >>> for all captures.
> >>
> >> It's also useful for sensors supporting DOL or DCG with HDR merge being
> >> performed outside of the sensor.
> >
> > Regarless of where HDR merge is implemented, we will also need controls
> > to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
> > standardize the values, and that's not good enough. At least for DOL and
> > DCG with HDR merge implemented outside of the sensor, we need to
> > standardize the modes.
>
> For the HDR-capable sensors with the HDR merge implemented outside, the
> short capture(s) are likely implemented as separate streams, in order to
> match the raw camera sensor model.
Yes, that's my expectation. They should use a different data type or a
different virtual channel (I expect most sensors to support both
options).
> In that case, the SDR/HDR mode switch, when supported, can be done by
> configuring the sensor device internal route for the short capture stream.
That's an option too, but it won't allow us to select between different
HDR modes. For instance, the AR0830 supports both DOL (2 exposures) and
DCG (2 gains). We would need a way to select between those two modes.
> You mentioned the need to be able to select the HDR mode in a standard
> way. Could you elaborate on the foreseen usage: would it be to select
> SDR/HDR operation, to select between different HDR sub-modes, to inform
> user space about HDR capability... ?
Both. From a libcamera perspective, I want standardized controls for
this, to avoid sensor-specific code as much as possible.
> > Can you tell which sensor(s) you're working with ?
> >
> >>> All controls are in the same class, so they could all be set
> >>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
> >>> useful in case of sensors with context switching.
> >>
> >> Agreed, we should be able to set them all. Are we still unable to set
> >> controls from multiple classes atomatically ? I thought that limitation
> >> has been lifted.
> >>
> >>> Each element of the array will hold an u32 value (exposure or gain)
> >>> for one capture. The size of the array is up to the sensor driver which
> >>> will implement the controls and initialize them via v4l2_ctrl_new_custom().
> >>> With this approach, the user-space will have to set valid values
> >>> for all the captures represented in the array.
> >>
> >> I'll comment on the controls themselves in patch 2/2.
> >>
> >>> The v4l2-core only supports one scalar min/max/step value for the
> >>> entire array, and each element is validated and adjusted to be within
> >>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
> >>> maximum value for the exposure control could be "the max value for the
> >>> long exposure" or "the max value for the sum of all exposures". If none
> >>> of these is ok, the sensor driver can adjust the values as supported and
> >>> the user space can use the TRY operation to query the sensor for the
> >>> minimum or maximum values.
> >>
> >> Hmmmm... I wonder if we would need the ability to report different
> >> limits for different array elements. There may be over-engineering
> >> though, my experience with libcamera is that userspace really needs
> >> detailed information about those controls, and attempting to convey the
> >> precise information through the kernel-userspace API is bound to fail.
> >> That's why we implement a sensor database in libcamera, with information
> >> about how to convert control values to real gain and exposure time.
> >> Exposing (close to) raw register values and letting userspace handle the
> >> rest may be better.
> >>
> >>> Mirela Rabulea (2):
> >>> LF-15161-6: media: Add exposure and gain controls for multiple
> >>> captures
> >>> LF-15161-7: Documentation: media: Describe exposure and gain controls
> >>> for multiple captures
> >>
> >> Did you forget to remove the LF-* identifiers ? :-)
> >>
> >>>
> >>> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
> >>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
> >>> include/uapi/linux/v4l2-controls.h | 3 +++
> >>> 3 files changed, 23 insertions(+)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-22 9:53 ` Julien Vuillaumier
@ 2025-07-23 15:02 ` Laurent Pinchart
2025-07-25 9:01 ` hans
0 siblings, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-23 15:02 UTC (permalink / raw)
To: Julien Vuillaumier
Cc: Mirela Rabulea, mchehab, sakari.ailus, hverkuil-cisco, ribalda,
jai.luthra, laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
celine.laurencin
On Tue, Jul 22, 2025 at 11:53:53AM +0200, Julien Vuillaumier wrote:
> On 20/07/2025 20:56, Mirela Rabulea wrote:
> > On 7/16/25 03:12, Laurent Pinchart wrote:
> >> On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
> >>> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
> >>>> Add new standard controls as U32 arrays, for sensors with multiple
> >>>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
> >>>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
> >>>> that have multiple captures, but the HDR merge is done inside the
> >>>> sensor,
> >>>> in the end exposing a single stream, but still requiring AEC control
> >>>> for all captures.
> >>>
> >>> It's also useful for sensors supporting DOL or DCG with HDR merge being
> >>> performed outside of the sensor.
> >>
> >> Regarless of where HDR merge is implemented, we will also need controls
> >> to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
> >> standardize the values, and that's not good enough. At least for DOL and
> >> DCG with HDR merge implemented outside of the sensor, we need to
> >> standardize the modes.
> >>
> >> Can you tell which sensor(s) you're working with ?
> >
> > We are working mostly with these 3:
> > Omnivision's os08a20 (2 exposures staggered hdr, each exposure on a
> > separate virtual channel, there are also other hdr modes which we do not
> > use)
> > Omnivision ox05b1s (RGB-Ir with context switching based on group holds,
> > 1 context optimized for RGB, the other context optimized for Ir, each
> > context on a different virtual channel)
> > Omnivision ox03c10 (4 exposures, hdr merge in sensor).
> >
> >>>> All controls are in the same class, so they could all be set
> >>>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
> >>>> useful in case of sensors with context switching.
> >>>
> >>> Agreed, we should be able to set them all. Are we still unable to set
> >>> controls from multiple classes atomatically ? I thought that limitation
> >>> has been lifted.
> >
> > Maybe I need some background check on this, but looking at kernel tag
> > next-20250718, this comment still lies in the documentation:
> > "These ioctls allow the caller to get or set multiple controls
> > atomically. Control IDs are grouped into control classes (see
> > :ref:`ctrl-class`) and all controls in the control array must belong
> > to the same control class."
> >
> > Maybe it needs to be updated, or not...since there is also this check in
> > check_ext_ctrls():
> > /* Check that all controls are from the same control class. */
> > for (i = 0; i < c->count; i++) {
> > if (V4L2_CTRL_ID2WHICH(c->controls[i].id) != c->which) {
> > c->error_idx = ioctl == VIDIOC_TRY_EXT_CTRLS ? i :
> > c->count;
> > return false;
> > }
> > }
This only when c->which is set to a control class. If you set it to
V4L2_CTRL_WHICH_CUR_VAL (equal to 0) then you can set (or get) controls
from multiple classes in one go.
> > There is also another inconvenient, the VIDIOC_S_EXT_CTRLS does not
> > reach the v4l2 subdevice driver, what we get in the sensor driver is a
> > set of .s_ctrl calls. I don't know about other sensors, but for the
> > Omivision sensors which I am familiar with, the group holds feature
> > could be used to get multiple registers to be applied atomically in the
> > same frame, but the sensor driver would need to know when to start and
> > when to end filling the group hold with the desired registers. If there
> > is some similar feature in other sensors, I think the VIDIOC_S_EXT_CTRLS
> > should have a corresponding v4l2-subdev operation, so that it can be
> > implemented in the sensor subdevice driver. This would probably require
> > some changes in the v4l2 core, as currently the subdev_do_ioctl()
> > function does not let the VIDIOC_S_EXT_CTRLS go to the subdevice.
> >
> > Laurent, Hans, any thoughts on this?
I can think of at least 3 ways to handle this.
The first method would be to group all controls in a cluster. That way
you will get a single .s_ctrl() call per VIDIOC_S_EXT_CTRLS. You will
have to iterate over the controls to see which ones have changed, and
configure the sensor accordingly. This short-circuits the logic in the
control framework that dispatches individual controls to separate
.s_ctrl() calls (or rather still goes through that logic, but doesn't
make use of it), and requires reimplementing it manually in the
.s_ctrl() handler. It's not ideal.
The second method would be to add new .begin() and .end() (name to be
bikeshedded) control operations. I experimented with this a while ago to
expose group hold to userspace, but never upstreamed the patches as I
didn't really need them in the end. Alternatively, the VIDIOC_S_EXT_CTRL
could be exposed to drivers, allowing them to implement begin/end
operations before and after calling the control framework. I don't have
a strong preference (maybe Hans would).
I increasingly think that the control framework doesn't provide the best
value for subdevs. It has been developed for video devices, and for
subdevs in video-centric devices where subdevs are hidden behind a video
device, but not for MC-centric use cases where subdevs are exposed to
userspace. The third option would be to implement something better,
dropping the useless features and adding support for the needs of modern
devices, but that would be much more work.
> >>>> Each element of the array will hold an u32 value (exposure or gain)
> >>>> for one capture. The size of the array is up to the sensor driver which
> >>>> will implement the controls and initialize them via
> >>>> v4l2_ctrl_new_custom().
> >>>> With this approach, the user-space will have to set valid values
> >>>> for all the captures represented in the array.
> >>>
> >>> I'll comment on the controls themselves in patch 2/2.
> >>>
> >>>> The v4l2-core only supports one scalar min/max/step value for the
> >>>> entire array, and each element is validated and adjusted to be within
> >>>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
> >>>> maximum value for the exposure control could be "the max value for the
> >>>> long exposure" or "the max value for the sum of all exposures". If none
> >>>> of these is ok, the sensor driver can adjust the values as supported and
> >>>> the user space can use the TRY operation to query the sensor for the
> >>>> minimum or maximum values.
> >>>
> >>> Hmmmm... I wonder if we would need the ability to report different
> >>> limits for different array elements. There may be over-engineering
> >>> though, my experience with libcamera is that userspace really needs
> >>> detailed information about those controls, and attempting to convey the
> >>> precise information through the kernel-userspace API is bound to fail.
> >>> That's why we implement a sensor database in libcamera, with information
> >>> about how to convert control values to real gain and exposure time.
> >>> Exposing (close to) raw register values and letting userspace handle the
> >>> rest may be better.
> >
> > Julien, any thoughts on this?
>
> Reporting min/max value per array element could have made sense for some
> controls. For instance we have a HDR sensor whose long capture analog
> gain range is different from the shorter captures gain. Conversely, it
> may not work well for the multi-capture exposure control where the
> constraint can be more about the sum of the exposures for each capture
> rather than the individual exposure values. In that case, exposing
> min/max values per array element does not really help the user space.
>
> Thus, having the user space to have the necessary insight into each
> sensor specifics for its AEC control seems to be the versatile option.
Then I think we should look at a libcamera implementation alongside with
this patch series, and review them together.
> > If we don't need to report different limits for different array
> > elements, we are fine, just we need to document better what those limits
> > stand for in case of arrays.
> >
> >>>> Mirela Rabulea (2):
> >>>> LF-15161-6: media: Add exposure and gain controls for multiple
> >>>> captures
> >>>> LF-15161-7: Documentation: media: Describe exposure and gain
> >>>> controls
> >>>> for multiple captures
> >>>
> >>> Did you forget to remove the LF-* identifiers ? :-)
> >
> > Yes, at least in the cover-letter, my bad :(
> >
> > Thanks for feedback.
> >
> >>>>
> >>>> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
> >>>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
> >>>> include/uapi/linux/v4l2-controls.h | 3 +++
> >>>> 3 files changed, 23 insertions(+)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re: Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
2025-07-23 13:49 ` Laurent Pinchart
@ 2025-07-24 9:33 ` Mirela Rabulea
2025-07-27 20:27 ` Laurent Pinchart
2025-07-25 9:05 ` hans
1 sibling, 1 reply; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-24 9:33 UTC (permalink / raw)
To: Laurent Pinchart
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
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@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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-23 15:02 ` Laurent Pinchart
@ 2025-07-25 9:01 ` hans
2025-07-25 9:27 ` Laurent Pinchart
0 siblings, 1 reply; 20+ messages in thread
From: hans @ 2025-07-25 9:01 UTC (permalink / raw)
To: Laurent Pinchart, Julien Vuillaumier
Cc: Mirela Rabulea, mchehab, sakari.ailus, hverkuil-cisco, ribalda,
jai.luthra, laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
celine.laurencin
On 23/07/2025 17:02, Laurent Pinchart wrote:
> On Tue, Jul 22, 2025 at 11:53:53AM +0200, Julien Vuillaumier wrote:
>> On 20/07/2025 20:56, Mirela Rabulea wrote:
>>> On 7/16/25 03:12, Laurent Pinchart wrote:
>>>> On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
>>>>> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
>>>>>> Add new standard controls as U32 arrays, for sensors with multiple
>>>>>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
>>>>>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
>>>>>> that have multiple captures, but the HDR merge is done inside the
>>>>>> sensor,
>>>>>> in the end exposing a single stream, but still requiring AEC control
>>>>>> for all captures.
>>>>>
>>>>> It's also useful for sensors supporting DOL or DCG with HDR merge being
>>>>> performed outside of the sensor.
>>>>
>>>> Regarless of where HDR merge is implemented, we will also need controls
>>>> to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
>>>> standardize the values, and that's not good enough. At least for DOL and
>>>> DCG with HDR merge implemented outside of the sensor, we need to
>>>> standardize the modes.
>>>>
>>>> Can you tell which sensor(s) you're working with ?
>>>
>>> We are working mostly with these 3:
>>> Omnivision's os08a20 (2 exposures staggered hdr, each exposure on a
>>> separate virtual channel, there are also other hdr modes which we do not
>>> use)
>>> Omnivision ox05b1s (RGB-Ir with context switching based on group holds,
>>> 1 context optimized for RGB, the other context optimized for Ir, each
>>> context on a different virtual channel)
>>> Omnivision ox03c10 (4 exposures, hdr merge in sensor).
>>>
>>>>>> All controls are in the same class, so they could all be set
>>>>>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
>>>>>> useful in case of sensors with context switching.
>>>>>
>>>>> Agreed, we should be able to set them all. Are we still unable to set
>>>>> controls from multiple classes atomatically ? I thought that limitation
>>>>> has been lifted.
>>>
>>> Maybe I need some background check on this, but looking at kernel tag
>>> next-20250718, this comment still lies in the documentation:
>>> "These ioctls allow the caller to get or set multiple controls
>>> atomically. Control IDs are grouped into control classes (see
>>> :ref:`ctrl-class`) and all controls in the control array must belong
>>> to the same control class."
>>>
>>> Maybe it needs to be updated, or not...since there is also this check in
>>> check_ext_ctrls():
>>> /* Check that all controls are from the same control class. */
>>> for (i = 0; i < c->count; i++) {
>>> if (V4L2_CTRL_ID2WHICH(c->controls[i].id) != c->which) {
>>> c->error_idx = ioctl == VIDIOC_TRY_EXT_CTRLS ? i :
>>> c->count;
>>> return false;
>>> }
>>> }
>
> This only when c->which is set to a control class. If you set it to
> V4L2_CTRL_WHICH_CUR_VAL (equal to 0) then you can set (or get) controls
> from multiple classes in one go.
That's correct. Early implementations of the control framework required
that all controls were from the same control class, but later I dropped
that requirement and you can just set 'which' to 0 and it no longer matters.
>
>>> There is also another inconvenient, the VIDIOC_S_EXT_CTRLS does not
>>> reach the v4l2 subdevice driver, what we get in the sensor driver is a
>>> set of .s_ctrl calls. I don't know about other sensors, but for the
>>> Omivision sensors which I am familiar with, the group holds feature
>>> could be used to get multiple registers to be applied atomically in the
>>> same frame, but the sensor driver would need to know when to start and
>>> when to end filling the group hold with the desired registers. If there
>>> is some similar feature in other sensors, I think the VIDIOC_S_EXT_CTRLS
>>> should have a corresponding v4l2-subdev operation, so that it can be
>>> implemented in the sensor subdevice driver. This would probably require
>>> some changes in the v4l2 core, as currently the subdev_do_ioctl()
>>> function does not let the VIDIOC_S_EXT_CTRLS go to the subdevice.
>>>
>>> Laurent, Hans, any thoughts on this?
>
> I can think of at least 3 ways to handle this.
>
> The first method would be to group all controls in a cluster. That way
> you will get a single .s_ctrl() call per VIDIOC_S_EXT_CTRLS. You will
> have to iterate over the controls to see which ones have changed, and
> configure the sensor accordingly. This short-circuits the logic in the
> control framework that dispatches individual controls to separate
> .s_ctrl() calls (or rather still goes through that logic, but doesn't
> make use of it), and requires reimplementing it manually in the
> .s_ctrl() handler. It's not ideal.
This should work out-of-the-box.
>
> The second method would be to add new .begin() and .end() (name to be
> bikeshedded) control operations. I experimented with this a while ago to
> expose group hold to userspace, but never upstreamed the patches as I
> didn't really need them in the end. Alternatively, the VIDIOC_S_EXT_CTRL
> could be exposed to drivers, allowing them to implement begin/end
> operations before and after calling the control framework. I don't have
> a strong preference (maybe Hans would).
I wonder if you could make 'HOLD_BEGIN' and 'HOLD_END' button controls, and
start and end the control array in VIDIOC_S_EXT_CTRLS with it. There are
some issues that need to be figured out, but I think this is doable.
Regards,
Hans
>
> I increasingly think that the control framework doesn't provide the best
> value for subdevs. It has been developed for video devices, and for
> subdevs in video-centric devices where subdevs are hidden behind a video
> device, but not for MC-centric use cases where subdevs are exposed to
> userspace. The third option would be to implement something better,
> dropping the useless features and adding support for the needs of modern
> devices, but that would be much more work.
>
>>>>>> Each element of the array will hold an u32 value (exposure or gain)
>>>>>> for one capture. The size of the array is up to the sensor driver which
>>>>>> will implement the controls and initialize them via
>>>>>> v4l2_ctrl_new_custom().
>>>>>> With this approach, the user-space will have to set valid values
>>>>>> for all the captures represented in the array.
>>>>>
>>>>> I'll comment on the controls themselves in patch 2/2.
>>>>>
>>>>>> The v4l2-core only supports one scalar min/max/step value for the
>>>>>> entire array, and each element is validated and adjusted to be within
>>>>>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
>>>>>> maximum value for the exposure control could be "the max value for the
>>>>>> long exposure" or "the max value for the sum of all exposures". If none
>>>>>> of these is ok, the sensor driver can adjust the values as supported and
>>>>>> the user space can use the TRY operation to query the sensor for the
>>>>>> minimum or maximum values.
>>>>>
>>>>> Hmmmm... I wonder if we would need the ability to report different
>>>>> limits for different array elements. There may be over-engineering
>>>>> though, my experience with libcamera is that userspace really needs
>>>>> detailed information about those controls, and attempting to convey the
>>>>> precise information through the kernel-userspace API is bound to fail.
>>>>> That's why we implement a sensor database in libcamera, with information
>>>>> about how to convert control values to real gain and exposure time.
>>>>> Exposing (close to) raw register values and letting userspace handle the
>>>>> rest may be better.
>>>
>>> Julien, any thoughts on this?
>>
>> Reporting min/max value per array element could have made sense for some
>> controls. For instance we have a HDR sensor whose long capture analog
Actually, support for this exists. See the VIDIOC_G_EXT_CTRLS documentation
and look for V4L2_CTRL_WHICH_DEF_VAL/V4L2_CTRL_WHICH_MIN_VAL/V4L2_CTRL_WHICH_MAX_VAL.
>> gain range is different from the shorter captures gain. Conversely, it
>> may not work well for the multi-capture exposure control where the
>> constraint can be more about the sum of the exposures for each capture
>> rather than the individual exposure values. In that case, exposing
>> min/max values per array element does not really help the user space.
>>
>> Thus, having the user space to have the necessary insight into each
>> sensor specifics for its AEC control seems to be the versatile option.
>
> Then I think we should look at a libcamera implementation alongside with
> this patch series, and review them together.
>
>>> If we don't need to report different limits for different array
>>> elements, we are fine, just we need to document better what those limits
>>> stand for in case of arrays.
>>>
>>>>>> Mirela Rabulea (2):
>>>>>> LF-15161-6: media: Add exposure and gain controls for multiple
>>>>>> captures
>>>>>> LF-15161-7: Documentation: media: Describe exposure and gain
>>>>>> controls
>>>>>> for multiple captures
>>>>>
>>>>> Did you forget to remove the LF-* identifiers ? :-)
>>>
>>> Yes, at least in the cover-letter, my bad :(
>>>
>>> Thanks for feedback.
>>>
>>>>>>
>>>>>> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
>>>>>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
>>>>>> include/uapi/linux/v4l2-controls.h | 3 +++
>>>>>> 3 files changed, 23 insertions(+)
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
2025-07-23 13:49 ` Laurent Pinchart
2025-07-24 9:33 ` Mirela Rabulea
@ 2025-07-25 9:05 ` hans
1 sibling, 0 replies; 20+ messages in thread
From: hans @ 2025-07-25 9:05 UTC (permalink / raw)
To: Laurent Pinchart, Mirela Rabulea
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
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@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.
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-25 9:01 ` hans
@ 2025-07-25 9:27 ` Laurent Pinchart
0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-25 9:27 UTC (permalink / raw)
To: hans
Cc: Julien Vuillaumier, Mirela Rabulea, mchehab, sakari.ailus,
hverkuil-cisco, ribalda, jai.luthra, laurentiu.palcu, linux-media,
linux-kernel, LnxRevLi, celine.laurencin
Hi Hans,
On Fri, Jul 25, 2025 at 11:01:11AM +0200, Hans Verkuil wrote:
> On 23/07/2025 17:02, Laurent Pinchart wrote:
> > On Tue, Jul 22, 2025 at 11:53:53AM +0200, Julien Vuillaumier wrote:
> >> On 20/07/2025 20:56, Mirela Rabulea wrote:
> >>> On 7/16/25 03:12, Laurent Pinchart wrote:
> >>>> On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
> >>>>> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
> >>>>>> Add new standard controls as U32 arrays, for sensors with multiple
> >>>>>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
> >>>>>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
> >>>>>> that have multiple captures, but the HDR merge is done inside the
> >>>>>> sensor,
> >>>>>> in the end exposing a single stream, but still requiring AEC control
> >>>>>> for all captures.
> >>>>>
> >>>>> It's also useful for sensors supporting DOL or DCG with HDR merge being
> >>>>> performed outside of the sensor.
> >>>>
> >>>> Regarless of where HDR merge is implemented, we will also need controls
> >>>> to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
> >>>> standardize the values, and that's not good enough. At least for DOL and
> >>>> DCG with HDR merge implemented outside of the sensor, we need to
> >>>> standardize the modes.
> >>>>
> >>>> Can you tell which sensor(s) you're working with ?
> >>>
> >>> We are working mostly with these 3:
> >>> Omnivision's os08a20 (2 exposures staggered hdr, each exposure on a
> >>> separate virtual channel, there are also other hdr modes which we do not
> >>> use)
> >>> Omnivision ox05b1s (RGB-Ir with context switching based on group holds,
> >>> 1 context optimized for RGB, the other context optimized for Ir, each
> >>> context on a different virtual channel)
> >>> Omnivision ox03c10 (4 exposures, hdr merge in sensor).
> >>>
> >>>>>> All controls are in the same class, so they could all be set
> >>>>>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
> >>>>>> useful in case of sensors with context switching.
> >>>>>
> >>>>> Agreed, we should be able to set them all. Are we still unable to set
> >>>>> controls from multiple classes atomatically ? I thought that limitation
> >>>>> has been lifted.
> >>>
> >>> Maybe I need some background check on this, but looking at kernel tag
> >>> next-20250718, this comment still lies in the documentation:
> >>> "These ioctls allow the caller to get or set multiple controls
> >>> atomically. Control IDs are grouped into control classes (see
> >>> :ref:`ctrl-class`) and all controls in the control array must belong
> >>> to the same control class."
> >>>
> >>> Maybe it needs to be updated, or not...since there is also this check in
> >>> check_ext_ctrls():
> >>> /* Check that all controls are from the same control class. */
> >>> for (i = 0; i < c->count; i++) {
> >>> if (V4L2_CTRL_ID2WHICH(c->controls[i].id) != c->which) {
> >>> c->error_idx = ioctl == VIDIOC_TRY_EXT_CTRLS ? i :
> >>> c->count;
> >>> return false;
> >>> }
> >>> }
> >
> > This only when c->which is set to a control class. If you set it to
> > V4L2_CTRL_WHICH_CUR_VAL (equal to 0) then you can set (or get) controls
> > from multiple classes in one go.
>
> That's correct. Early implementations of the control framework required
> that all controls were from the same control class, but later I dropped
> that requirement and you can just set 'which' to 0 and it no longer matters.
>
> >>> There is also another inconvenient, the VIDIOC_S_EXT_CTRLS does not
> >>> reach the v4l2 subdevice driver, what we get in the sensor driver is a
> >>> set of .s_ctrl calls. I don't know about other sensors, but for the
> >>> Omivision sensors which I am familiar with, the group holds feature
> >>> could be used to get multiple registers to be applied atomically in the
> >>> same frame, but the sensor driver would need to know when to start and
> >>> when to end filling the group hold with the desired registers. If there
> >>> is some similar feature in other sensors, I think the VIDIOC_S_EXT_CTRLS
> >>> should have a corresponding v4l2-subdev operation, so that it can be
> >>> implemented in the sensor subdevice driver. This would probably require
> >>> some changes in the v4l2 core, as currently the subdev_do_ioctl()
> >>> function does not let the VIDIOC_S_EXT_CTRLS go to the subdevice.
> >>>
> >>> Laurent, Hans, any thoughts on this?
> >
> > I can think of at least 3 ways to handle this.
> >
> > The first method would be to group all controls in a cluster. That way
> > you will get a single .s_ctrl() call per VIDIOC_S_EXT_CTRLS. You will
> > have to iterate over the controls to see which ones have changed, and
> > configure the sensor accordingly. This short-circuits the logic in the
> > control framework that dispatches individual controls to separate
> > .s_ctrl() calls (or rather still goes through that logic, but doesn't
> > make use of it), and requires reimplementing it manually in the
> > .s_ctrl() handler. It's not ideal.
>
> This should work out-of-the-box.
>
> > The second method would be to add new .begin() and .end() (name to be
> > bikeshedded) control operations. I experimented with this a while ago to
> > expose group hold to userspace, but never upstreamed the patches as I
> > didn't really need them in the end. Alternatively, the VIDIOC_S_EXT_CTRL
> > could be exposed to drivers, allowing them to implement begin/end
> > operations before and after calling the control framework. I don't have
> > a strong preference (maybe Hans would).
>
> I wonder if you could make 'HOLD_BEGIN' and 'HOLD_END' button controls, and
> start and end the control array in VIDIOC_S_EXT_CTRLS with it. There are
> some issues that need to be figured out, but I think this is doable.
That seems needlessly complicated for userspace. Ultimately, what we
want in userspace is to set several controls in an atomic way. What
mechanism the sensor and driver use for this should be internal. As we
already have a way to set multiple controls in the V4L2 API, I don't see
a reason to introduce more userspace complexity.
> > I increasingly think that the control framework doesn't provide the best
> > value for subdevs. It has been developed for video devices, and for
> > subdevs in video-centric devices where subdevs are hidden behind a video
> > device, but not for MC-centric use cases where subdevs are exposed to
> > userspace. The third option would be to implement something better,
> > dropping the useless features and adding support for the needs of modern
> > devices, but that would be much more work.
> >
> >>>>>> Each element of the array will hold an u32 value (exposure or gain)
> >>>>>> for one capture. The size of the array is up to the sensor driver which
> >>>>>> will implement the controls and initialize them via
> >>>>>> v4l2_ctrl_new_custom().
> >>>>>> With this approach, the user-space will have to set valid values
> >>>>>> for all the captures represented in the array.
> >>>>>
> >>>>> I'll comment on the controls themselves in patch 2/2.
> >>>>>
> >>>>>> The v4l2-core only supports one scalar min/max/step value for the
> >>>>>> entire array, and each element is validated and adjusted to be within
> >>>>>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
> >>>>>> maximum value for the exposure control could be "the max value for the
> >>>>>> long exposure" or "the max value for the sum of all exposures". If none
> >>>>>> of these is ok, the sensor driver can adjust the values as supported and
> >>>>>> the user space can use the TRY operation to query the sensor for the
> >>>>>> minimum or maximum values.
> >>>>>
> >>>>> Hmmmm... I wonder if we would need the ability to report different
> >>>>> limits for different array elements. There may be over-engineering
> >>>>> though, my experience with libcamera is that userspace really needs
> >>>>> detailed information about those controls, and attempting to convey the
> >>>>> precise information through the kernel-userspace API is bound to fail.
> >>>>> That's why we implement a sensor database in libcamera, with information
> >>>>> about how to convert control values to real gain and exposure time.
> >>>>> Exposing (close to) raw register values and letting userspace handle the
> >>>>> rest may be better.
> >>>
> >>> Julien, any thoughts on this?
> >>
> >> Reporting min/max value per array element could have made sense for some
> >> controls. For instance we have a HDR sensor whose long capture analog
>
> Actually, support for this exists. See the VIDIOC_G_EXT_CTRLS documentation
> and look for V4L2_CTRL_WHICH_DEF_VAL/V4L2_CTRL_WHICH_MIN_VAL/V4L2_CTRL_WHICH_MAX_VAL.
Ah thanks.
> >> gain range is different from the shorter captures gain. Conversely, it
> >> may not work well for the multi-capture exposure control where the
> >> constraint can be more about the sum of the exposures for each capture
> >> rather than the individual exposure values. In that case, exposing
> >> min/max values per array element does not really help the user space.
> >>
> >> Thus, having the user space to have the necessary insight into each
> >> sensor specifics for its AEC control seems to be the versatile option.
> >
> > Then I think we should look at a libcamera implementation alongside with
> > this patch series, and review them together.
> >
> >>> If we don't need to report different limits for different array
> >>> elements, we are fine, just we need to document better what those limits
> >>> stand for in case of arrays.
> >>>
> >>>>>> Mirela Rabulea (2):
> >>>>>> LF-15161-6: media: Add exposure and gain controls for multiple
> >>>>>> captures
> >>>>>> LF-15161-7: Documentation: media: Describe exposure and gain
> >>>>>> controls
> >>>>>> for multiple captures
> >>>>>
> >>>>> Did you forget to remove the LF-* identifiers ? :-)
> >>>
> >>> Yes, at least in the cover-letter, my bad :(
> >>>
> >>> Thanks for feedback.
> >>>
> >>>>>> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
> >>>>>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
> >>>>>> include/uapi/linux/v4l2-controls.h | 3 +++
> >>>>>> 3 files changed, 23 insertions(+)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re: Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
2025-07-24 9:33 ` Mirela Rabulea
@ 2025-07-27 20:27 ` Laurent Pinchart
2025-07-28 13:34 ` Mirela Rabulea
0 siblings, 1 reply; 20+ messages in thread
From: Laurent Pinchart @ 2025-07-27 20:27 UTC (permalink / raw)
To: Mirela Rabulea
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
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.
> >> 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).
> >> 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.
> >>> 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).
> > 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.
> > - 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@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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC 2/2] Documentation: media: Describe exposure and gain controls for multiple captures
2025-07-27 20:27 ` Laurent Pinchart
@ 2025-07-28 13:34 ` Mirela Rabulea
0 siblings, 0 replies; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-28 13:34 UTC (permalink / raw)
To: Laurent Pinchart
Cc: mchehab, sakari.ailus, hverkuil-cisco, ribalda, jai.luthra,
laurentiu.palcu, linux-media, linux-kernel, LnxRevLi,
julien.vuillaumier, celine.laurencin
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@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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re: [RFC 0/2] Add standard exposure and gain controls for multiple captures
2025-07-23 14:00 ` Laurent Pinchart
@ 2025-07-28 15:42 ` Mirela Rabulea
0 siblings, 0 replies; 20+ messages in thread
From: Mirela Rabulea @ 2025-07-28 15:42 UTC (permalink / raw)
To: Laurent Pinchart, Julien Vuillaumier, sakari.ailus,
hverkuil-cisco
Cc: mchehab, ribalda, jai.luthra, laurentiu.palcu, linux-media,
linux-kernel, LnxRevLi, celine.laurencin
Hi Laurent and all,
On 7/23/25 17:00, Laurent Pinchart wrote:
>
> On Tue, Jul 22, 2025 at 10:46:16AM +0200, Julien Vuillaumier wrote:
>> On 16/07/2025 02:12, Laurent Pinchart wrote:
>>> On Wed, Jul 16, 2025 at 02:59:54AM +0300, Laurent Pinchart wrote:
>>>> On Fri, Jul 11, 2025 at 01:05:42AM +0300, Mirela Rabulea wrote:
>>>>> Add new standard controls as U32 arrays, for sensors with multiple
>>>>> captures: V4L2_CID_EXPOSURE_MULTI, V4L2_CID_AGAIN_MULTI and
>>>>> V4L2_CID_DGAIN_MULTI. These will be particularly useful for sensors
>>>>> that have multiple captures, but the HDR merge is done inside the sensor,
>>>>> in the end exposing a single stream, but still requiring AEC control
>>>>> for all captures.
>>>>
>>>> It's also useful for sensors supporting DOL or DCG with HDR merge being
>>>> performed outside of the sensor.
>>>
>>> Regarless of where HDR merge is implemented, we will also need controls
>>> to select the HDR mode. We have V4L2_CID_HDR_SENSOR_MODE, which doesn't
>>> standardize the values, and that's not good enough. At least for DOL and
>>> DCG with HDR merge implemented outside of the sensor, we need to
>>> standardize the modes.
>>
>> For the HDR-capable sensors with the HDR merge implemented outside, the
>> short capture(s) are likely implemented as separate streams, in order to
>> match the raw camera sensor model.
>
> Yes, that's my expectation. They should use a different data type or a
> different virtual channel (I expect most sensors to support both
> options).
>
>> In that case, the SDR/HDR mode switch, when supported, can be done by
>> configuring the sensor device internal route for the short capture stream.
>
> That's an option too, but it won't allow us to select between different
> HDR modes. For instance, the AR0830 supports both DOL (2 exposures) and
> DCG (2 gains). We would need a way to select between those two modes.
>
>> You mentioned the need to be able to select the HDR mode in a standard
>> way. Could you elaborate on the foreseen usage: would it be to select
>> SDR/HDR operation, to select between different HDR sub-modes, to inform
>> user space about HDR capability... ?
>
> Both. From a libcamera perspective, I want standardized controls for
> this, to avoid sensor-specific code as much as possible.
This sounds complicated to achieve, at least with the one existing
V4L2_CID_HDR_SENSOR_MODE control. There are a multitude of modes and
technologies used around HDR sensors.
A few things we can handle with existing API's:
- number of exposures (the ones that have a separated stream)
- bitdepth (by mbus code, can give a hint on compression type)
There are other aspects we need to offer user-space some controls on, in
order to be able to select the desired mode:
-number of exposures, even when not exposed as a separate stream
-number of captures (which in some cases may be different than the
number of exposures)
-type of gain, or number of gains (DCG, LCG, HCG)
-type: sequential/ staggered / interleaved
-type of companding (none, PWL, other types?)
-LFM indication
-LFM replacement enable/disabled
-SPD data present (Split-Pixel Detection, as LFM enhancement)
For ox03c10, the sensor hdr mode can be determined by these factors:
number of exposures (dual/triple), staggered/unstaggered, companded (pwl
20/16/14/12)/uncompanded, LFM/LFM+SPD/none, with all combinations
possible. HDR data is on VC0, LFM/SPD if enabled on VC1.
For os08a20 there is staggered hdr mode or sequential hdr mode (via
context switching, up to 4 set each group having different exposure/gain
sets). For staggered HDR mode there are 2 possible output modes: on
separate 2 virtual channels for long/short exposure, or on single VC
with dummy lines.
For ov2775, besides no hdr(either HCG or LCG), there is single exposure
hdr (DCG) or dual exposure hdr (DCG + VS). The DCG data may be combined
or not (HCG+LCG). Compression may apply.
I'm sure there are a lot of other fancy HDR related technologies around.
Do you think this can be standardized? Up to what level of detail? I
think most sensor drivers will only support a limited number of hdr
modes, out of the multitude supported by the hardware. Parameters that
may be relevant for one sensor may have no relevance for others.
What exactly is disturbing with the current approach, where each driver
defines the hdr modes it supports, and what do you expect to have for
libcamera?
Is this standardization talk something you would like to conclude in the
context of multi-controls, or can it go as a separate topic? I propose
divide-et-impera, conquer one by one ;)
Thanks,
Mirela
>
>>> Can you tell which sensor(s) you're working with ?
>>>
>>>>> All controls are in the same class, so they could all be set
>>>>> atomically via VIDIOC_S_EXT_CTRLS, this could turn out to be
>>>>> useful in case of sensors with context switching.
>>>>
>>>> Agreed, we should be able to set them all. Are we still unable to set
>>>> controls from multiple classes atomatically ? I thought that limitation
>>>> has been lifted.
>>>>
>>>>> Each element of the array will hold an u32 value (exposure or gain)
>>>>> for one capture. The size of the array is up to the sensor driver which
>>>>> will implement the controls and initialize them via v4l2_ctrl_new_custom().
>>>>> With this approach, the user-space will have to set valid values
>>>>> for all the captures represented in the array.
>>>>
>>>> I'll comment on the controls themselves in patch 2/2.
>>>>
>>>>> The v4l2-core only supports one scalar min/max/step value for the
>>>>> entire array, and each element is validated and adjusted to be within
>>>>> these bounds in v4l2_ctrl_type_op_validate(). The significance for the
>>>>> maximum value for the exposure control could be "the max value for the
>>>>> long exposure" or "the max value for the sum of all exposures". If none
>>>>> of these is ok, the sensor driver can adjust the values as supported and
>>>>> the user space can use the TRY operation to query the sensor for the
>>>>> minimum or maximum values.
>>>>
>>>> Hmmmm... I wonder if we would need the ability to report different
>>>> limits for different array elements. There may be over-engineering
>>>> though, my experience with libcamera is that userspace really needs
>>>> detailed information about those controls, and attempting to convey the
>>>> precise information through the kernel-userspace API is bound to fail.
>>>> That's why we implement a sensor database in libcamera, with information
>>>> about how to convert control values to real gain and exposure time.
>>>> Exposing (close to) raw register values and letting userspace handle the
>>>> rest may be better.
>>>>
>>>>> Mirela Rabulea (2):
>>>>> LF-15161-6: media: Add exposure and gain controls for multiple
>>>>> captures
>>>>> LF-15161-7: Documentation: media: Describe exposure and gain controls
>>>>> for multiple captures
>>>>
>>>> Did you forget to remove the LF-* identifiers ? :-)
>>>>
>>>>>
>>>>> .../media/v4l/ext-ctrls-image-source.rst | 12 ++++++++++++
>>>>> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 8 ++++++++
>>>>> include/uapi/linux/v4l2-controls.h | 3 +++
>>>>> 3 files changed, 23 insertions(+)
>
> --
> Regards,
>
> Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-07-28 15:42 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-10 22:05 [RFC 0/2] Add standard exposure and gain controls for multiple captures 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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).