[libcamera-devel] [PATCH v3] libcamera: controls: Add controls for HDR
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Oct 19 01:54:27 CEST 2023
Hello David,
Chiming in at last, sorry for the delay.
On Wed, Oct 11, 2023 at 11:39:26AM +0100, David Plowman via libcamera-devel wrote:
> On Wed, 11 Oct 2023 at 11:20, Kieran Bingham wrote:
> > Quoting David Plowman via libcamera-devel (2023-10-05 11:41:33)
> > > We add an HdrMode control (to enable and disable HDR processing)
> > > and an HdrChannel, which indicates what kind of HDR frame (short, long
> > > or otherwise) 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 | 63 ++++++++++++++++++++++++++++++++++
> > > 1 file changed, 63 insertions(+)
> > >
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index f2e542f4..e3699e74 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -774,6 +774,69 @@ 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.
Sounds quite non controversial :-) Maybe s/not enabled/disabled/
> > > + - 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. The expectation is that, if necessary,
> > > + the application can merge them to create HDR images for itself.
> > > + - name: HdrModeMultiExposure
> > > + value: 2
> > > + description: |
> > > + Multiple exposures will be generated and merged to create HDR
> > > + images.
> > > + - name: HdrModeSingleExposure
> > > + value: 3
> > > + description: |
> > > + Multiple frames all at a single exposure will be used to create HDR
> > > + images.
If a platform supports both the multi-exposure and single-exposure
modes, how will an application decide which one to use, what are the
pros and cons ?
> > > + - name: HdrModeNight
> > > + value: 4
> > > + description: |
> > > + Multiple frames will be combined to produce "night mode" images.
> >
> > MdrModeMultiExposureUnmerged has been documented to describe what that
> > is doing. HdrModeMultiExposure, and HdrModeSingleExposure are more self
> > explanatory
It feels a bit like we're mixing different concepts in the same control,
but I'm OK with it for now.
What does bother me though is that there's no definition of HDR
anywhere. I think that needs to be fixed, in order to clearly set the
boundaries of what falls in the category of HDR-related controls, and
what is entirely separate.
> > - but HdrModeNight might need more explaining.
> >
> > Does this just mean sequential images are stacked? Are there different
> > exposures? Are there any other expectations that a user may need or
> > assumptions they could get wrong?
> >
> > Or is it just 'Hey do anything you like to make pictures look better at
> > night time or in low light conditions'?.
>
> It means this last thing. I don't feel I can judge what any other
> vendor will do HDR-wise, so these modes are the most generic "hooks" I
> can think of that stand a fair chance of vendors at large being able
> to hang their HDR implementations off them.
>
> For night mode in particular - it's "do whatever it is that you do".
> Though there's an implicit "probably do it with some kind of frame
> merging or HDR-like procedure", because I think a number of vendors do
> indeed do this. But there's no knowing for sure. "Night modes" might
> be completely different things too,
If it's not HDR, then does it really belong to this control ?
> like we have this magic HDR mode
> in the imx708 sensor which just doesn't fit in here.
What is that magic HDR 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).
You mention short and long here, but there's also a medium value below.
If the implementation uses two exposures only, which ones will it be ?
This should be documented. Also, in the single exposure case, will the
channel be set to HdrChannelNone ? This should also be documented.
Similarly, in the HdrModeMultiExposure mode, what will be reported here,
given that all images will be the result of merging multiple exposures ?
> > > +
> > > + 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.
> > > + - name: HdrChannelNight
> > > + value: 4
> > > + description: |
> > > + This frame has been used to produce a "night mode" image.
This reminds me of the three-states boolean:
https://thedailywtf.com/articles/What_Is_Truth_0x3f_.
> > What does it mean if a 'single' frame has been used to produce a night
> > mode image. Does that mean it's blended with previous frames? Or is this
> > frame only 'one' of a set?
>
> It's hard to say what this really means, as I think different vendors
> will do all kinds of different things. Unfortunately I suspect the HDR
> or "night mode" is likely to be very vendor specific, and we may find
> they work quite differently.
>
> In our case, all it really means is that the exposure for the frame
> was calculated by AEC/AGC running with its "night mode" settings. It
> will have been combined with previous "night mode" frames (if any).
If I understand you correctly, this is single exposure HDR, but with AGC
running in night mode. Do you then need this value ? It seems to me that
you could report HdrChannelNone here, and applications would still know
that night mode is being used from the value of HdrMode.
> > "Frame has been used to produce" makes it sound like the result of using
> > this frame will be output later and maybe this frame should be discarded
> > because it's not the "final" night mode image ?
>
> To some extent this is me using vague language because I can't say for
> sure what anyone else will do. Actually it does so happen that in our
> "night mode" you will need to discard a few frames to give the image
> merging time to work (you could take the very first image - but it
> will be noisy). But other vendors may not do it like this. Or they
> may. But probably not. Who knows!
>
> But I'm definitely open to rewording any of this if we can agree on
> the level of vaguess/specificness that's required...
I want controls to be defined in an unambiguous way, for the benefit of
application developers who will then know what they get, and IPA
developers who will know what to implement. There's some leeway in the
sense that implementations can be given some latitude, but that latitude
needs to be documented with clear boundaries.
As guessing what other platforms would do, I would rather be more
precise here and document the use case(s) you want to target. When we
will implement support for other platforms, we'll revisit it.
> > > +
> > > # ----------------------------------------------------------------------------
> > > # Draft controls section
> > >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list