[PATCH 1/2] ipa: rkisp1: agc: Fix metering modes

[Kieran Bingham]

Quoting Stefan Klug (2025-03-19 16:01:36)
> The weights for a given metering mode are applied to the histogram data
> inside the histogram statistics block. The AE statistics do not contain
> any weights. Therefore the weights are honored when AgcMeanLuminance
> calculates the upper or lower constraints, but ignored in the
> calculation of the frame luminance. Fix that by manually applying the
> weights in the luminance calculation.
> 
> Fixes: 4c5152843a2a ("ipa: rkisp1: Derive rkisp1::algorithms::Agc from AgcMeanLuminance")
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 13 ++++++++++---
>  src/ipa/rkisp1/algorithms/agc.h   |  1 +
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 5a3ba0131cf1..101cad5d0893 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -439,15 +439,20 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>   */
>  double Agc::estimateLuminance(double gain) const
>  {
> +       ASSERT(expMeans_.size() == weights_.size());
>         double ySum = 0.0;
> +       double wSum = 0.0;
>  
>         /* Sum the averages, saturated to 255. */
> -       for (uint8_t expMean : expMeans_)
> -               ySum += std::min(expMean * gain, 255.0);
> +       for (unsigned i = 0; i < expMeans_.size(); i++) {
> +               double w = weights_[i];
> +               ySum += std::min(expMeans_[i] * gain, 255.0) * w;
> +               wSum += w;
> +       }
>  
>         /* \todo Weight with the AWB gains */
>  
> -       return ySum / expMeans_.size() / 255;
> +       return ySum / wSum / 255;
>  }
>  
>  /**
> @@ -515,6 +520,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>         Histogram hist({ params->hist.hist_bins, context.hw->numHistogramBins },
>                        [](uint32_t x) { return x >> 4; });
>         expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };
> +       std::vector<uint8_t> &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);

Where do the weights come from? Are they always guaranteed to be
available at this point ?

> +       weights_ = { modeWeights.data(), modeWeights.size() };
>  
>         /*
>          * Set the AGC limits using the fixed exposure time and/or gain in
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index 62bcde999fe3..7867eed9c4e3 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -55,6 +55,7 @@ private:
>                                   utils::Duration frameDuration);
>  
>         Span<const uint8_t> expMeans_;
> +       Span<const uint8_t> weights_;
>  
>         std::map<int32_t, std::vector<uint8_t>> meteringModes_;
>  };
> -- 
> 2.43.0
>