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

[Dan Scally]

Hi Stefan

On 19/03/2025 16:01, Stefan Klug wrote:
> 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>
> ---

Looks good to me:


Reviewed-by: Daniel Scally <dan.scally 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);
> +	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_;
>   };