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

[Stefan Klug]

Hi Kieran,

Thank you for the review.

On Tue, Mar 25, 2025 at 10:18:46AM +0000, Kieran Bingham wrote:
> 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 ?

The weights come from the tuning file. The libtuning agc module contains
hardcoded values for centre weighted, spot and matrix. If there are no
metering modes in the tuning file, a matrix mode is generated within
parseMeteringModes().  So yes, they are always available at this point.

Best regards,
Stefan

> 
> > +       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
> >