[libcamera-devel,1/1] libcamera: controls: Add controls for HDR
diff mbox series

Message ID 20230728122758.2411-2-david.plowman@raspberrypi.com
State Accepted
Headers show
Series
  • HDR controls
Related show

Commit Message

David Plowman July 28, 2023, 12:27 p.m. UTC
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@raspberrypi.com>
---
 src/libcamera/control_ids.yaml | 47 ++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Comments

David Plowman Aug. 16, 2023, 9:59 a.m. UTC | #1
Hi everyone

Just a little prod to see if anyone has any thoughts on this?

Thanks!
David

On Fri, 28 Jul 2023 at 13:28, David Plowman
<david.plowman@raspberrypi.com> 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@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).
> +
> +      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
>
Naushir Patuck Aug. 21, 2023, 8:41 a.m. UTC | #2
Hi David,

Thank you for your patch.

On Fri, 28 Jul 2023 at 13:28, David Plowman via libcamera-devel
<libcamera-devel@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@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?

I'm not too bothered either way so,

Reviewed-by: Naushir Patuck <naush@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
>
David Plowman Aug. 21, 2023, 9:06 a.m. UTC | #3
Hi Naush

Thanks for the comments.

On Mon, 21 Aug 2023 at 09:41, Naushir Patuck <naush@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@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@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@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
> >
Jacopo Mondi Sept. 16, 2023, 1:11 p.m. UTC | #4
Hi David

On Mon, Aug 21, 2023 at 10:06:46AM +0100, David Plowman via libcamera-devel wrote:
> Hi Naush
>
> Thanks for the comments.
>
> On Mon, 21 Aug 2023 at 09:41, Naushir Patuck <naush@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@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@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.

Related to the discussion on "[PATCH v2 2/2] ipa: rpi: vc4: data:
Update tuning files for HDR ", the association between a channel index
(defined by their declaration order) and the menomic name is done in
the config file, using the "channel_map" property.

To know what "short" corresponds to, an application has to look at the
config file and reverse the association. Is this fine in your opinion ?


>
> 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@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
> > >
David Plowman Sept. 18, 2023, 8:45 a.m. UTC | #5
Hi Jacopo

Thanks for the feedback.

On Sat, 16 Sept 2023 at 14:11, Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi David
>
> On Mon, Aug 21, 2023 at 10:06:46AM +0100, David Plowman via libcamera-devel wrote:
> > Hi Naush
> >
> > Thanks for the comments.
> >
> > On Mon, 21 Aug 2023 at 09:41, Naushir Patuck <naush@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@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@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.
>
> Related to the discussion on "[PATCH v2 2/2] ipa: rpi: vc4: data:
> Update tuning files for HDR ", the association between a channel index
> (defined by their declaration order) and the menomic name is done in
> the config file, using the "channel_map" property.
>
> To know what "short" corresponds to, an application has to look at the
> config file and reverse the association. Is this fine in your opinion ?

For the Pi platform, yes that's correct. Other platforms can of course
do completely different things.

I think there does need to be some kind of mapping somewhere, because
we never pass libcamera control values directly into our algorithms.
Though we could certainly look again at how the mapping works.

I'll talk more about that in that other patch as I don't think it's
actually relevant to this HDR control.

Thanks!
David

>
>
> >
> > 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@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
> > > >
David Plowman Sept. 21, 2023, 2 p.m. UTC | #6
Hi again

Actually I'm wondering about adding another option to this list. I
think a "night" mode might be useful. It might be very similar to some
of the other HDR modes, but I could imagine placing less emphasis on
expanding the dynamic range and rather more on using the image
stacking to increase the amount of denoise. It's clearly very similar
to the other HDR techniques, but it would be left to the
implementation to decide exactly how to do the "night" version (if
indeed it wants to support that).

 Any thoughts? I could submit another version of this patch...

David

On Mon, 18 Sept 2023 at 09:45, David Plowman
<david.plowman@raspberrypi.com> wrote:
>
> Hi Jacopo
>
> Thanks for the feedback.
>
> On Sat, 16 Sept 2023 at 14:11, Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hi David
> >
> > On Mon, Aug 21, 2023 at 10:06:46AM +0100, David Plowman via libcamera-devel wrote:
> > > Hi Naush
> > >
> > > Thanks for the comments.
> > >
> > > On Mon, 21 Aug 2023 at 09:41, Naushir Patuck <naush@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@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@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.
> >
> > Related to the discussion on "[PATCH v2 2/2] ipa: rpi: vc4: data:
> > Update tuning files for HDR ", the association between a channel index
> > (defined by their declaration order) and the menomic name is done in
> > the config file, using the "channel_map" property.
> >
> > To know what "short" corresponds to, an application has to look at the
> > config file and reverse the association. Is this fine in your opinion ?
>
> For the Pi platform, yes that's correct. Other platforms can of course
> do completely different things.
>
> I think there does need to be some kind of mapping somewhere, because
> we never pass libcamera control values directly into our algorithms.
> Though we could certainly look again at how the mapping works.
>
> I'll talk more about that in that other patch as I don't think it's
> actually relevant to this HDR control.
>
> Thanks!
> David
>
> >
> >
> > >
> > > 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@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
> > > > >

Patch
diff mbox series

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