[libcamera-devel] [PATCH v3] libcamera: controls: Add controls for HDR

[David Plowman]

Hi Laurent

Thanks for the suggestions.

On Thu, 19 Oct 2023 at 21:52, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> 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.

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

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

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.

Thanks!
David

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