| Message ID | 20210913145810.66515-11-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Mon, Sep 13, 2021 at 04:58:09PM +0200, Jean-Michel Hautbois wrote: > The Tonemapping algorithm calculates the gamma curve for every frame, s/Tonemapping/tone mapping/ > regardless of whether the gamma value has changed or not. This issue is > exasperated as we currently hardcode the gamma to a single value. > > Optimise the implementation to only recalculate the look up table when > the gamma setting is changed, and store the gamma setting of the LUT > curve as part of the IPA context. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/tone_mapping.cpp | 5 +++++ > src/ipa/ipu3/ipa_context.h | 1 + > src/ipa/ipu3/ipu3.cpp | 3 +++ > 3 files changed, 9 insertions(+) > > diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp > index 3af96261..40337f9d 100644 > --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp > +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp > @@ -43,6 +43,9 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context, > */ > gamma_ = 1.1; > > + if (context.frameContext.toneMapping.gamma == gamma_) > + return; We currently clear the context in IPAIPU3::configure() with context_ = {}; This ensures that the check above will never be true for the first frame, but it feels like it would probably be best to let algorithms perform the initial configuration. Should we implement ToneMapping::configure() and set context.frameContext.toneMapping.gamma to 0.0 there ? > + > struct ipu3_uapi_gamma_corr_lut &lut = > context.frameContext.toneMapping.gammaCorrection; > > @@ -53,6 +56,8 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context, > /* The output value is expressed on 13 bits. */ > lut.lut[i] = gamma * 8191; > } > + > + context.frameContext.toneMapping.gamma = gamma_; > } > > } /* namespace ipa::ipu3::algorithms */ > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index 9d9444dc..bffa7634 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -38,6 +38,7 @@ struct IPAFrameContext { > } awb; > > struct { > + double gamma; > struct ipu3_uapi_gamma_corr_lut gammaCorrection; > } toneMapping; > }; > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 5b487dcd..f76627f6 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -129,6 +129,9 @@ > * \struct IPAFrameContext::toneMapping > * \brief Context for ToneMapping and Gamma control > * > + * \var IPAFrameContext::toneMapping::gamma > + * \brief Gamma value for the LUT > + * > * \var IPAFrameContext::toneMapping::gammaCorrection > * \brief Per-pixel tone mapping implemented as a LUT > *
diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp index 3af96261..40337f9d 100644 --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp @@ -43,6 +43,9 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context, */ gamma_ = 1.1; + if (context.frameContext.toneMapping.gamma == gamma_) + return; + struct ipu3_uapi_gamma_corr_lut &lut = context.frameContext.toneMapping.gammaCorrection; @@ -53,6 +56,8 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context, /* The output value is expressed on 13 bits. */ lut.lut[i] = gamma * 8191; } + + context.frameContext.toneMapping.gamma = gamma_; } } /* namespace ipa::ipu3::algorithms */ diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 9d9444dc..bffa7634 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -38,6 +38,7 @@ struct IPAFrameContext { } awb; struct { + double gamma; struct ipu3_uapi_gamma_corr_lut gammaCorrection; } toneMapping; }; diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 5b487dcd..f76627f6 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -129,6 +129,9 @@ * \struct IPAFrameContext::toneMapping * \brief Context for ToneMapping and Gamma control * + * \var IPAFrameContext::toneMapping::gamma + * \brief Gamma value for the LUT + * * \var IPAFrameContext::toneMapping::gammaCorrection * \brief Per-pixel tone mapping implemented as a LUT *