[libcamera-devel] [PATCH 13/22] ipa: ipu3: Do not access IPAFrameContext in configure

[Kieran Bingham]

Quoting Jean-Michel Hautbois (2021-11-08 13:13:41)
> The IPAContext contains a SessionConfiguration and a FrameContext. The
> configure call can populate the SessionConfiguration, but there is not
> yet a frame.
> 
> Do not use the IPAFrameContext to prepare the next patches which will
> introduce per-frame controls.

"""
The IPAContext contains a SessionConfiguration and a FrameContext. The
configure call can populate the SessionConfiguration, but the
FrameContext stores frame specific data, which should not be managed at
configuration time.

Update the algorithm configure calls for AGC and ToneMapping to remove
accesses to the FrameContext data.
"""


But ... can this patch standalone? Are other parts of the algorithm
relying upon those values being initialised? Or are they safe to remove
like this?



> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp          | 8 --------
>  src/ipa/ipu3/algorithms/tone_mapping.cpp | 5 +----
>  2 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 3a15f5d9..52cf2753 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -96,14 +96,6 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>         minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
>         maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>  
> -       /* Configure the default exposure and gain. */
> -       context.frameContext.agc.gain = minAnalogueGain_;
> -       context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
> -
> -       prevExposureValue_ = context.frameContext.agc.gain
> -                          * context.frameContext.agc.exposure
> -                          * lineDuration_;
> -
>         return 0;
>  }
>  
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> index 2040eda5..5d74c552 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -38,12 +38,9 @@ ToneMapping::ToneMapping()
>   *
>   * \return 0
>   */
> -int ToneMapping::configure(IPAContext &context,
> +int ToneMapping::configure([[maybe_unused]] IPAContext &context,
>                            [[maybe_unused]] const IPAConfigInfo &configInfo)
>  {
> -       /* Initialise tone mapping gamma value. */
> -       context.frameContext.toneMapping.gamma = 0.0;
> -
>         return 0;
>  }
>  
> -- 
> 2.32.0
>