[libcamera-devel] Questions about tonemapping
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Wed Jun 16 11:46:44 CEST 2021
Hi Naush,
On Wed, Jun 16, 2021 at 10:11:57AM +0100, Naushir Patuck wrote:
> Hi Paul,
>
> Here are some initial thoughts from me. I'm sure David will have some views on
> this as well.
Thank you for the feedback.
>
> On Wed, 16 Jun 2021 at 09:47, <paul.elder at ideasonboard.com> wrote:
>
> Hello Naush and David,
>
> We are planning to add controls for tonemapping [1], and we were
> thinking to use the same controls that the android HAL uses.
>
> Something along the lines of:
>
> - TonemapMode
> type: int32_t
> draft: true
> description: |
> High-level global contrast/gamma/tonemapping control. Currently
> identical to ANDROID_TONEMAP_MODE.
> enum:
> - name: TonemapModeContrastCurve
> value: 0
> description: |
> Use the tonemapping curve specified in TonemapCurve* entries.
> All color enhancement and tonemapping must be disabled, except
> for applying the tonemapping curve specified by TonemapCurve*.
> - name: TonemapModeFast
> value: 1
> description: |
> Advanced gamma mapping and color enhancement may be applied,
> without reducing frame rate compared to raw sensor output.
> - name: TonemapModeHighQuality
> value: 2
> description: |
> High-quality gamma mapping and color enhancement will be
> applied, at the cost of possibly reduced frame rate compared to
> raw sensor output.
>
>
> I always wondered why Android had a Fast and HQ mode for tone mapping -
> assuming this
> is talking about global contrast enhancement, and not local which the
> description implies.
> All it is doing is an in -> out mapping, so I don't get why we need different
> quality settings.
Maybe, for example, the fast one has fewer points in the mapping while
the hq one has more, and the number of points affects the processing
speed...?
If you don't think that we need to separate them we could unify them
as letting the IPA decide on the curve, and in the HAL just map them
both to fast and hq.
> This may be related, but colour enhancement seems a bit vague, do you have any
> more
> details on that?
tbh I don't know about that; I just copy and pasted the description from
the android HAL documentation... :S
>
>
> - name: TonemapModeGammaValue
> value: 3
> description: |
> Use the gamma value specified in TonemapGamma to perform
> tonemapping. All color enhancement and tonemapping must be
> disabled, except for applying the tonemapping curve specified
> by TonemapGamma.
> - name: TonemapModePresetCurve
> value: 4
> description: |
> Use the present tonemapping curve specified in
> TonemapPresetCurve to perform tonemapping. All color
> enhancement and tonemapping must be disabled, except for
> applying the tonemapping curve specified by TonemapPresetCurve.
>
> - TonemapGamma
> type: float
> draft: true
> description: |
> Tonemapping curve to use when TonemapMode is TonemapModeGammaValue.
> The tonemap curve will be defined by the following formula: * OUT =
> pow(IN, 1.0 / gamma) where IN and OUT is the input pixel value
> scaled to range [0.0, 1.0], pow is the power function and gamma is
> the gamma value specified by this key. The same curve will be
> applied to all color channels. The camera device may clip the input
> gamma value to its supported range. The actual applied value will
> be returned in result metadata. The valid range of gamma value
> varies on different devices, but values within [1.0, 5.0] are
> guaranteed not to be clipped. Currently identical to
> ANDROID_TONEMAP_GAMMA.
>
> - TonemapPresetCurve
> type: int32_t
> draft: true
> description: |
> Tonemapping curve to use when TonemapMode is
> TonemapModePresetCurve.
> enum:
> - name: TonemapPresetCurveSRGB
> value: 0
> description: [2]
> - name: TonemapPresetCurveREC709
> value: 1
> description: [3]
>
>
> This bit is very related to the ColourSpace work that David has submitted for
> review. My feeling is that these presets do not make much sense in isolation
> without
> specifying things like colour primaries and ranges.
Maybe we could use this as an int alone and the IPA could have its own
preset curves instead?
Although then that won't map well to the HAL. Maybe that's okay, or we
could just map to sRGB anyway?
Fast/hq also use preset curves though, so we could ditch this control
entirely?
>
>
>
> - TonemapCurve{Red,Green,Blue}
> type: float
> draft: true
> description: |
> An array of [ [in, out], [in, out], ... ] format to describe a
> tonemapping curve to be applied when TonemapMode is set to
> TonemapModeContrastCurve.
> size: [64] (we need some way to specify the size)
>
>
> Generally speaking, do you expect different curves for R/G/B? There may be
android's TonemapCurve control describes the tonemap curve with a 2xMx3
array. (we don't necessarily have to do the same thing, that's just the
starting point of the brainstorm)
> a hardware limitation that only allows a single curve, so perhaps the
> description
> could be updated to reflect this?
The description reflects this imo, just that the control name doesn't.
Maybe we could have a separate control that would use a single curve for
all of R/G/B? Then I'm wondering how to handle the case if an android
application requests different curves; should we just average or take
only one curve? Or not allow the application to set curves at all if the
hardware doesn't support multicolor curves?
>
>
> What feedback do you have on these controls? How might you implement
> these? Is it something that could be done? Do you forsee any issues?
>
>
> Overall, I do find it quite strange that a user can take over the gamma/ce
> control
> from the AGC. I think the biggest challenge is how does this control play well
> with
> AGC? Perhaps there needs to be a Auto tone-mapping mode that allows the AGC
> to do what it needs to do?
fast, hq, and preset curve modes are auto, so that should be fine.
> Otherwise, this seems to be an easy way to mess
> up the
> quality of the AGC output, even though the intention may be the opposite :-)
Aha, yeah :)
Thanks,
Paul
>
> Another one is how this plays with Colourspace setup, where the tone mapping is
> just one
> part of the equation.
>
> [1] https://bugs.libcamera.org/show_bug.cgi?id=56
> [2] https://developer.android.com/reference/images/camera2/metadata/
> android.tonemap.curveRed/srgb_tonemap.png
> [3] https://developer.android.com/reference/images/camera2/metadata/
> android.tonemap.curveRed/rec709_tonemap.png
>
More information about the libcamera-devel
mailing list