[libcamera-devel] [PATCH 10/13] ipa: raspberrypi: agc: Move weights out of AGC

[Laurent Pinchart]

On Wed, May 03, 2023 at 01:20:32PM +0100, Naushir Patuck via libcamera-devel wrote:
> From: David Plowman <david.plowman at raspberrypi.com>
> 
> The region weights for the the AGC zones are handled by the AGC
> algorithm. Apply them directly in the IPA (vc4.cpp) to the statistics
> that we pass to the AGC.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi>

There's a .com missing. I'll add it.

> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
>  src/ipa/rpi/controller/agc_algorithm.h |  3 +++
>  src/ipa/rpi/controller/rpi/agc.cpp     | 27 +++++++++++++++++---------
>  src/ipa/rpi/controller/rpi/agc.h       |  1 +
>  src/ipa/rpi/vc4/vc4.cpp                | 26 ++++++++++++++++++-------
>  4 files changed, 41 insertions(+), 16 deletions(-)
> 
> diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h
> index 36e6c11058ee..b6949daa7135 100644
> --- a/src/ipa/rpi/controller/agc_algorithm.h
> +++ b/src/ipa/rpi/controller/agc_algorithm.h
> @@ -6,6 +6,8 @@
>   */
>  #pragma once
>  
> +#include <vector>
> +
>  #include <libcamera/base/utils.h>
>  
>  #include "algorithm.h"
> @@ -18,6 +20,7 @@ public:
>  	AgcAlgorithm(Controller *controller) : Algorithm(controller) {}
>  	/* An AGC algorithm must provide the following: */
>  	virtual unsigned int getConvergenceFrames() const = 0;
> +	virtual std::vector<double> const &getWeights() const = 0;
>  	virtual void setEv(double ev) = 0;
>  	virtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0;
>  	virtual void setFixedShutter(libcamera::utils::Duration fixedShutter) = 0;
> diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp
> index e6fb7b8dbeb3..e79c82e2e65b 100644
> --- a/src/ipa/rpi/controller/rpi/agc.cpp
> +++ b/src/ipa/rpi/controller/rpi/agc.cpp
> @@ -292,6 +292,18 @@ unsigned int Agc::getConvergenceFrames() const
>  		return config_.convergenceFrames;
>  }
>  
> +std::vector<double> const &Agc::getWeights() const
> +{
> +	/*
> +	 * In case someone calls setMeteringMode and then this before the
> +	 * algorithm has run and updated the meteringMode_ pointer.
> +	 */
> +	auto it = config_.meteringModes.find(meteringModeName_);
> +	if (it == config_.meteringModes.end())
> +		return meteringMode_->weights;
> +	return it->second.weights;
> +}
> +
>  void Agc::setEv(double ev)
>  {
>  	ev_ = ev;
> @@ -595,19 +607,16 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,
>  	ASSERT(weights.size() == stats->agcRegions.numRegions());
>  
>  	/*
> -	 * Note how the calculation below means that equal weights give you
> -	 * "average" metering (i.e. all pixels equally important).
> +	 * Note that the weights are applied by the IPA to the statistics directly,
> +	 * before they are given to us here.
>  	 */
>  	double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;
>  	for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {
>  		auto &region = stats->agcRegions.get(i);
> -		double rAcc = std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);
> -		double gAcc = std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);
> -		double bAcc = std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);
> -		rSum += rAcc * weights[i];
> -		gSum += gAcc * weights[i];
> -		bSum += bAcc * weights[i];
> -		pixelSum += region.counted * weights[i];
> +		rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);
> +		gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);
> +		bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);
> +		pixelSum += region.counted;
>  	}
>  	if (pixelSum == 0.0) {
>  		LOG(RPiAgc, Warning) << "computeInitialY: pixelSum is zero";
> diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h
> index 4e5f272fac78..939f97295a02 100644
> --- a/src/ipa/rpi/controller/rpi/agc.h
> +++ b/src/ipa/rpi/controller/rpi/agc.h
> @@ -69,6 +69,7 @@ public:
>  	char const *name() const override;
>  	int read(const libcamera::YamlObject &params) override;
>  	unsigned int getConvergenceFrames() const override;
> +	std::vector<double> const &getWeights() const override;
>  	void setEv(double ev) override;
>  	void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;
>  	void setMaxShutter(libcamera::utils::Duration maxShutter) override;
> diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp
> index a32db9bcaf9a..e38024ac5418 100644
> --- a/src/ipa/rpi/vc4/vc4.cpp
> +++ b/src/ipa/rpi/vc4/vc4.cpp
> @@ -211,13 +211,25 @@ RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span<uint8_t> mem)
>  						stats->awb_stats[i].counted,
>  						stats->awb_stats[i].notcounted });
>  
> -	statistics->agcRegions.init(hw.agcRegions);
> -	for (i = 0; i < statistics->agcRegions.numRegions(); i++)
> -		statistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum << scale,
> -						  stats->agc_stats[i].g_sum << scale,
> -						  stats->agc_stats[i].b_sum << scale },
> -						stats->agc_stats[i].counted,
> -						stats->awb_stats[i].notcounted });
> +	RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
> +		controller_.getAlgorithm("agc"));
> +	if (!agc) {
> +		LOG(IPARPI, Debug) << "No AGC algorithm - not copying statistics";
> +		statistics->agcRegions.init(0);
> +	} else {
> +		statistics->agcRegions.init(hw.agcRegions);
> +		const std::vector<double> &weights = agc->getWeights();
> +		for (i = 0; i < statistics->agcRegions.numRegions(); i++) {
> +			uint64_t rSum = (stats->agc_stats[i].r_sum << scale) * weights[i];
> +			uint64_t gSum = (stats->agc_stats[i].g_sum << scale) * weights[i];
> +			uint64_t bSum = (stats->agc_stats[i].b_sum << scale) * weights[i];
> +			uint32_t counted = stats->agc_stats[i].counted * weights[i];
> +			uint32_t notcounted = stats->agc_stats[i].notcounted * weights[i];
> +			statistics->agcRegions.set(i, { { rSum, gSum, bSum },
> +							counted,
> +							notcounted });
> +		}
> +	}
>  
>  	statistics->focusRegions.init(hw.focusRegions);
>  	for (i = 0; i < statistics->focusRegions.numRegions(); i++)

-- 
Regards,

Laurent Pinchart