[libcamera-devel] [PATCH 1/1] libcamera: controls: Add controls for HDR

[David Plowman]

Hi Naush

Thanks for the comments.

On Mon, 21 Aug 2023 at 09:41, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi David,
>
> Thank you for your patch.
>
> On Fri, 28 Jul 2023 at 13:28, David Plowman via libcamera-devel
> <libcamera-devel at lists.libcamera.org> wrote:
> >
> > We add an HdrMode control (to enable and disable HDR processing)
> > and an HdrChannel, which indicates what kind of HDR frame (short, long
> > medium) has just arrived.
> >
> > Currently the HdrMode supports three values:
> >
> > * Off - no HDR processing at all.
> > * MultiExposure - frames at multiple different exposures are combined
> >   to create HDR images.
> > * SingleExposure - multiple frames all at the same exposure are
> >   combined to create HDR images.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/libcamera/control_ids.yaml | 47 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 056886e6..34df7adb 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -701,6 +701,53 @@ 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.
> > +
> > +      enum:
> > +        - name: HdrModeOff
> > +          value: 0
> > +          description: |
> > +            HDR is not enabled.
> > +        - name: HdrModeMultiExposure
> > +          value: 1
> > +          description: |
> > +            Multiple exposures will be used to create HDR images.
> > +        - name: HdrModeSingleExposure
> > +          value: 2
> > +          description: |
> > +            Multiple frames all at a single exposure will be used to create HDR
> > +            images.
> > +
> > +  - 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).
>
> Only a minor comment about this control.  Instead of an enum, should we use an
> explicit integer to determine channel number?  Then we are not limited to only
> short/medium/long channels as below?

This slightly gets us into the portable-applications vs.
platform-specific-capabilities debate...

I'd quite like to have "AgcChannel" metadata that tells you exactly
this - namely which AGC channel this frame is from. But other
platforms might have no such concept even if they have some kind of
HDR support.

So I'm slightly inclined to have a more generic notion of long/short
frame for HDR, whilst leaving the door open to adding "AgcChannel"
when it's needed.

But I agree that there's no clear answer here, and other people's
opinions would be interesting.

Thanks!
David

>
> I'm not too bothered either way so,
>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
>
> > +      enum:
> > +        - name: HdrChannelNone
> > +          value: 0
> > +          description: |
> > +            This image does not correspond to any of the captures used to create
> > +            an HDR image.
> > +        - 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
> >
> > --
> > 2.30.2
> >