[PATCH v1 06/11] ipa: rkisp1: Move calculation of RGB means into own function

[Dan Scally]

Morning Stefan

On 09/01/2025 11:53, Stefan Klug wrote:
> Move the calculation of the RGB means into an own function for better
> code clarity. This commit doesn't contain any functional changes.
>
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---

Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
>   src/ipa/rkisp1/algorithms/awb.cpp | 90 +++++++++++++++++--------------
>   src/ipa/rkisp1/algorithms/awb.h   |  3 ++
>   2 files changed, 52 insertions(+), 41 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index cffaa06a22c1..f6449565834b 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -229,7 +229,7 @@ void Awb::process(IPAContext &context,
>   	const rkisp1_cif_isp_stat *params = &stats->params;
>   	const rkisp1_cif_isp_awb_stat *awb = &params->awb;
>   	IPAActiveState &activeState = context.activeState;
> -	RGB<double> rgbMeans;
> +	RGB<double> rgbMeans = calculateRgbMeans(frameContext, awb);
>   
>   	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
>   	metadata.set(controls::ColourGains, {
> @@ -243,6 +243,53 @@ void Awb::process(IPAContext &context,
>   		return;
>   	}
>   
> +	/*
> +	 * If the means are too small we don't have enough information to
> +	 * meaningfully calculate gains. Freeze the algorithm in that case.
> +	 */
> +	if (rgbMeans.r() < kMeanMinThreshold && rgbMeans.g() < kMeanMinThreshold &&
> +	    rgbMeans.b() < kMeanMinThreshold)
> +		return;
> +
> +	activeState.awb.temperatureK = estimateCCT(rgbMeans);
> +
> +	/* Metadata shall contain the up to date measurement */
> +	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> +
> +	/*
> +	 * Estimate the red and blue gains to apply in a grey world. The green
> +	 * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
> +	 * divisor to a minimum value of 1.0.
> +	 */
> +	RGB<double> gains({ rgbMeans.g() / std::max(rgbMeans.r(), 1.0),
> +			    1.0,
> +			    rgbMeans.g() / std::max(rgbMeans.b(), 1.0) });
> +
> +	/*
> +	 * Clamp the gain values to the hardware, which expresses gains as Q2.8
> +	 * unsigned integer values. Set the minimum just above zero to avoid
> +	 * divisions by zero when computing the raw means in subsequent
> +	 * iterations.
> +	 */
> +	gains = gains.max(1.0 / 256).min(1023.0 / 256);
> +
> +	/* Filter the values to avoid oscillations. */
> +	double speed = 0.2;
> +	gains = gains * speed + activeState.awb.gains.automatic * (1 - speed);
> +
> +	activeState.awb.gains.automatic = gains;
> +
> +	LOG(RkISP1Awb, Debug)
> +		<< std::showpoint
> +		<< "Means " << rgbMeans << ", gains "
> +		<< activeState.awb.gains.automatic << ", temp "
> +		<< activeState.awb.temperatureK << "K";
> +}
> +
> +RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rkisp1_cif_isp_awb_stat *awb) const
> +{
> +	Vector<double, 3> rgbMeans;
> +
>   	if (rgbMode_) {
>   		rgbMeans = {{
>   			static_cast<double>(awb->awb_mean[0].mean_y_or_g),
> @@ -301,46 +348,7 @@ void Awb::process(IPAContext &context,
>   	 */
>   	rgbMeans /= frameContext.awb.gains;
>   
> -	/*
> -	 * If the means are too small we don't have enough information to
> -	 * meaningfully calculate gains. Freeze the algorithm in that case.
> -	 */
> -	if (rgbMeans.r() < kMeanMinThreshold && rgbMeans.g() < kMeanMinThreshold &&
> -	    rgbMeans.b() < kMeanMinThreshold)
> -		return;
> -
> -	activeState.awb.temperatureK = estimateCCT(rgbMeans);
> -
> -	/*
> -	 * Estimate the red and blue gains to apply in a grey world. The green
> -	 * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
> -	 * divisor to a minimum value of 1.0.
> -	 */
> -	RGB<double> gains({
> -		rgbMeans.g() / std::max(rgbMeans.r(), 1.0),
> -		1.0,
> -		rgbMeans.g() / std::max(rgbMeans.b(), 1.0)
> -	});
> -
> -	/*
> -	 * Clamp the gain values to the hardware, which expresses gains as Q2.8
> -	 * unsigned integer values. Set the minimum just above zero to avoid
> -	 * divisions by zero when computing the raw means in subsequent
> -	 * iterations.
> -	 */
> -	gains = gains.max(1.0 / 256).min(1023.0 / 256);
> -
> -	/* Filter the values to avoid oscillations. */
> -	double speed = 0.2;
> -	gains = gains * speed + activeState.awb.gains.automatic * (1 - speed);
> -
> -	activeState.awb.gains.automatic = gains;
> -
> -	LOG(RkISP1Awb, Debug)
> -		<< std::showpoint
> -		<< "Means " << rgbMeans << ", gains "
> -		<< activeState.awb.gains.automatic << ", temp "
> -		<< activeState.awb.temperatureK << "K";
> +	return rgbMeans;
>   }
>   
>   REGISTER_IPA_ALGORITHM(Awb, "Awb")
> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h
> index e424804861a6..2a64cb60d5f4 100644
> --- a/src/ipa/rkisp1/algorithms/awb.h
> +++ b/src/ipa/rkisp1/algorithms/awb.h
> @@ -38,6 +38,9 @@ public:
>   		     ControlList &metadata) override;
>   
>   private:
> +	RGB<double> calculateRgbMeans(const IPAFrameContext &frameContext,
> +				      const rkisp1_cif_isp_awb_stat *awb) const;
> +
>   	std::optional<Interpolator<Vector<double, 2>>> colourGainCurve_;
>   	bool rgbMode_;
>   };