[libcamera-devel] [PATCH v3] libcamera: controls: Add controls for HDR
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Oct 20 11:55:35 CEST 2023
Hi David,
On Fri, Oct 20, 2023 at 09:37:28AM +0100, David Plowman wrote:
> On Thu, 19 Oct 2023 at 21:52, Laurent Pinchart wrote:
> > On Thu, Oct 19, 2023 at 12:52:20PM +0100, David Plowman via libcamera-devel wrote:
> > > We add an HdrMode control (to enable and disable HDR processing)
> > > and an HdrChannel, which indicates what kind of HDR frame (short, long
> > > or medium) has just arrived.
> > >
> > > Currently the HdrMode supports the following values:
> > >
> > > * Off - no HDR processing at all.
> > > * MultiExposureUnmerged - frames at multiple different exposures are
> > > produced, but not merged together. They are returned "as is".
> > > * MultiExposure - frames at multiple different exposures are merged
> > > to create HDR images.
> > > * SingleExposure - multiple frames all at the same exposure are
> > > merged to create HDR images.
> > > * Night - multiple frames will be combined to create "night mode"
> > > images.
> > >
> > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > > Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> > > ---
> > > src/libcamera/control_ids.yaml | 75 ++++++++++++++++++++++++++++++++++
> > > 1 file changed, 75 insertions(+)
> > >
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index f2e542f4..c3232abf 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -774,6 +774,81 @@ controls:
> > > Continuous AF is paused. No further state changes or lens movements
> > > will occur until the AfPauseResume control is sent.
> > >
> > > + - HdrMode:
> > > + type: int32_t
> > > + description: |
> > > + Control to set the mode to be used for High Dynamic Range (HDR)
> > > + imaging. HDR techniques typically include multiple exposure, image
> > > + fusion and tone mapping techniques to improve the dynamic range of the
> > > + resulting images.
> > > +
> > > + When using an HDR mode, images are tagged to indicate which HDR channel
> > > + (long, medium or short) they come from.
> > > +
> > > + \sa HdrChannel
> > > +
> > > + enum:
> > > + - name: HdrModeOff
> > > + value: 0
> > > + description: |
> > > + HDR is disabled. The HDR channel, if present, will report
> > > + HdrChannelNone.
> >
> > Stating what HDR channel is used is an improvement compared to the
> > previous version, but there's still an option left to implementors here:
> > reporting HdrChannelNone, or not reporting HdrChannel at all. Unless you
> > see a reason to allow both, I would pick the latter:
> >
> > HDR is disabled. Metadata for this frame will not include the
> > HdrChannel control.
>
> Yes, I think that's fine.
>
> > > + - name: HdrModeMultiExposureUnmerged
> > > + value: 1
> > > + description: |
> > > + Multiple exposures will be generated in an alternating fashion.
> > > + However, they will not be merged together and will be returned to
> > > + the application as they are. Each image will be tagged with the
> > > + correct HDR channel, indicating what kind of exposure (long, medium
> > > + or short) it is. The expectation is that, if necessary, the
> > > + application can merge them to create HDR images for itself.
> >
> > You mention here long, medium and short. Does this mean there will
> > always be three channels ?
>
> No - it's whatever the implementation wants to do. We don't use
> medium, for example. I think it's quite likely that some vendors would
> want other channels, such as "very short" and "very long". Maybe the
> description can avoid implying that all channels will appear.
Shouldn't this be something that the application should control ? I'm
increasingly thinking that this shouldn't be an HDR mode, but should
instead be controlled through a per-frame control mechanism. Is there
any reason this can't be done, not right now (I don't want to delay this
patch unnecessarily), but at some point in the future ?
> > > + - name: HdrModeMultiExposure
> > > + value: 2
> > > + description: |
> > > + Multiple exposures will be generated and merged to create HDR
> > > + images. Each image will be tagged with the HDR channel (long, medium
> > > + or short) that arrived and which caused this image to be output.
> > > + - name: HdrModeSingleExposure
> > > + value: 3
> > > + description: |
> > > + Multiple frames all at a single exposure will be used to create HDR
> > > + images. These images should be reported as all corresponding to the
> > > + HDR short channel.
> > > + - name: HdrModeNight
> > > + value: 4
> > > + description: |
> > > + Multiple frames will be combined to produce "night mode"
> > > + images. Images will be tagged as belonging either to the long,
> > > + medium or short HDR channel according to the implementation.
> >
> > Does this mean that night more will always use multi-exposure, or that
> > it is implementation-defined ?
>
> I really think that needs to be implementation defined. Our night mode
> is single-exposure, but I can't possibly predict what anyone else
> would want to do.
Won't it be problematic for applications if they don't know what
HdrChannel values they will get ? How do you expect HdrChannel to be
used in the HDR modes where the camera combines images (all but the
Unmerged mode) ?
> > > +
> > > + - HdrChannel:
> > > + type: int32_t
> > > + description: |
> > > + This value is reported back to the application so that it can discover
> > > + whether this capture corresponds to the short or long exposure image (or
> > > + any other image used by the HDR procedure). An application can monitor
> > > + the HDR channel to discover when the differently exposed images have
> > > + arrived.
> > > +
> > > + enum:
> > > + - name: HdrChannelNone
> > > + value: 0
> > > + description: |
> > > + This image does not correspond to any of the captures used to create
> > > + an HDR image.
> >
> > As indicated above, do we need this, or should we not report HdrChannel
> > when HDR is disabled ?
>
> Actually I'd quite like to keep this, even if it's not reported when HDR is off.
>
> One use case is multi-exposure HDR on a Pi 4. You can't merge images
> so how would you get a viewfinder? You could intersperse some
> "ordinary AGC" frames with your long/short/whatever frames. You might
> want to label these "HdrChannelNone". I suppose you could label them
> "medium", but then maybe you're using "medium" for other frames that
> your HDR implementation requires. Like everything else, it all falls a
> bit into the "who knows what anyone will do" category.
This makes me believe even more strongly that the unmerged mode
shouldn't be an HDR mode. We're trying to cram too many assumptions
about the application needs here.
If you want to keep HdrChannelNone for the time being until we drop
HdrMultiExposureUnmerged, then this needs better documentation. Until I
read your reply I had no idea that the unmerged mode would produce
images for the "None" channel. If this isn't documented properly, it
won't be usable for applications.
Depending on the priorities (I assume the Pi 5 HDR modes will be more
interesting than the unmerged operation on Pi 4), we could also drop the
unmerged mode for now, and design a solution better for application-side
HDR.
> I guess there's also an outside chance that some implementations can't
> flick exposures instantaneously and accurately like we can, so maybe
> it would suit them too. Or we could have an
> "HdrChannelCouldntMakeThisWorkProperly" value? Having wrestled with
> all this for a while I can see the attraction! :)
>
> I'll send out another version today with the changes in line with all
> the above. Obviously everyone please shout if we want to do anything
> different.
>
> > > + - name: HdrChannelShort
> > > + value: 1
> > > + description: |
> > > + This is a short exposure image.
> > > + - name: HdrChannelMedium
> > > + value: 2
> > > + description: |
> > > + This is a medium exposure image.
> > > + - name: HdrChannelLong
> > > + value: 3
> > > + description: |
> > > + This is a long exposure image.
> > > +
> > > # ----------------------------------------------------------------------------
> > > # Draft controls section
> > >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list