[PATCH v3 15/17] ipa: rkisp1: awb: Use RGB class to store colour gains

Milan Zamazal mzamazal at redhat.com
Tue Nov 19 11:51:09 CET 2024


Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> Replace the manual storage of gains in the IPA active state and frame
> context with usage of the RGB class. This simplifies the code thanks to
> usage of the arithmetic functions provided by the RGB class.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Milan Zamazal <mzamazal at redhat.com>

> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 71 +++++++++++--------------------
>  src/ipa/rkisp1/ipa_context.cpp    | 31 +-------------
>  src/ipa/rkisp1/ipa_context.h      | 20 ++-------
>  3 files changed, 32 insertions(+), 90 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index dbeaf81565ff..c330feff9f5d 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -47,12 +47,8 @@ Awb::Awb()
>  int Awb::configure(IPAContext &context,
>  		   const IPACameraSensorInfo &configInfo)
>  {
> -	context.activeState.awb.gains.manual.red = 1.0;
> -	context.activeState.awb.gains.manual.blue = 1.0;
> -	context.activeState.awb.gains.manual.green = 1.0;
> -	context.activeState.awb.gains.automatic.red = 1.0;
> -	context.activeState.awb.gains.automatic.blue = 1.0;
> -	context.activeState.awb.gains.automatic.green = 1.0;
> +	context.activeState.awb.gains.manual = RGB<double>{ 1.0 };
> +	context.activeState.awb.gains.automatic = RGB<double>{ 1.0 };
>  	context.activeState.awb.autoEnabled = true;
>  
>  	/*
> @@ -89,21 +85,17 @@ void Awb::queueRequest(IPAContext &context,
>  
>  	const auto &colourGains = controls.get(controls::ColourGains);
>  	if (colourGains && !awb.autoEnabled) {
> -		awb.gains.manual.red = (*colourGains)[0];
> -		awb.gains.manual.blue = (*colourGains)[1];
> +		awb.gains.manual.r() = (*colourGains)[0];
> +		awb.gains.manual.b() = (*colourGains)[1];
>  
>  		LOG(RkISP1Awb, Debug)
> -			<< "Set colour gains to red: " << awb.gains.manual.red
> -			<< ", blue: " << awb.gains.manual.blue;
> +			<< "Set colour gains to " << awb.gains.manual;
>  	}
>  
>  	frameContext.awb.autoEnabled = awb.autoEnabled;
>  
> -	if (!awb.autoEnabled) {
> -		frameContext.awb.gains.red = awb.gains.manual.red;
> -		frameContext.awb.gains.green = 1.0;
> -		frameContext.awb.gains.blue = awb.gains.manual.blue;
> -	}
> +	if (!awb.autoEnabled)
> +		frameContext.awb.gains = awb.gains.manual;
>  }
>  
>  /**
> @@ -116,19 +108,16 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,
>  	 * This is the latest time we can read the active state. This is the
>  	 * most up-to-date automatic values we can read.
>  	 */
> -	if (frameContext.awb.autoEnabled) {
> -		frameContext.awb.gains.red = context.activeState.awb.gains.automatic.red;
> -		frameContext.awb.gains.green = context.activeState.awb.gains.automatic.green;
> -		frameContext.awb.gains.blue = context.activeState.awb.gains.automatic.blue;
> -	}
> +	if (frameContext.awb.autoEnabled)
> +		frameContext.awb.gains = context.activeState.awb.gains.automatic;
>  
>  	auto gainConfig = params->block<BlockType::AwbGain>();
>  	gainConfig.setEnabled(true);
>  
> -	gainConfig->gain_green_b = std::clamp<int>(256 * frameContext.awb.gains.green, 0, 0x3ff);
> -	gainConfig->gain_blue = std::clamp<int>(256 * frameContext.awb.gains.blue, 0, 0x3ff);
> -	gainConfig->gain_red = std::clamp<int>(256 * frameContext.awb.gains.red, 0, 0x3ff);
> -	gainConfig->gain_green_r = std::clamp<int>(256 * frameContext.awb.gains.green, 0, 0x3ff);
> +	gainConfig->gain_green_b = std::clamp<int>(256 * frameContext.awb.gains.g(), 0, 0x3ff);
> +	gainConfig->gain_blue = std::clamp<int>(256 * frameContext.awb.gains.b(), 0, 0x3ff);
> +	gainConfig->gain_red = std::clamp<int>(256 * frameContext.awb.gains.r(), 0, 0x3ff);
> +	gainConfig->gain_green_r = std::clamp<int>(256 * frameContext.awb.gains.g(), 0, 0x3ff);
>  
>  	/* If we have already set the AWB measurement parameters, return. */
>  	if (frame > 0)
> @@ -196,8 +185,8 @@ void Awb::process(IPAContext &context,
>  
>  	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
>  	metadata.set(controls::ColourGains, {
> -			static_cast<float>(frameContext.awb.gains.red),
> -			static_cast<float>(frameContext.awb.gains.blue)
> +			static_cast<float>(frameContext.awb.gains.r()),
> +			static_cast<float>(frameContext.awb.gains.b())
>  		});
>  	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
>  
> @@ -253,12 +242,7 @@ void Awb::process(IPAContext &context,
>  	 * divide by the gains that were used to get the raw means from the
>  	 * sensor.
>  	 */
> -	RGB<double> gains{{
> -		frameContext.awb.gains.red,
> -		frameContext.awb.gains.green,
> -		frameContext.awb.gains.blue
> -	}};
> -	rgbMeans /= gains;
> +	rgbMeans /= frameContext.awb.gains;
>  
>  	/*
>  	 * If the means are too small we don't have enough information to
> @@ -278,8 +262,11 @@ void Awb::process(IPAContext &context,
>  	 * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
>  	 * divisor to a minimum value of 1.0.
>  	 */
> -	double redGain = rgbMeans.g() / std::max(rgbMeans.r(), 1.0);
> -	double blueGain = rgbMeans.g() / std::max(rgbMeans.b(), 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
> @@ -287,24 +274,18 @@ void Awb::process(IPAContext &context,
>  	 * divisions by zero when computing the raw means in subsequent
>  	 * iterations.
>  	 */
> -	redGain = std::clamp(redGain, 1.0 / 256, 1023.0 / 256);
> -	blueGain = std::clamp(blueGain, 1.0 / 256, 1023.0 / 256);
> +	gains = gains.max(1.0 / 256).min(1023.0 / 256);
>  
>  	/* Filter the values to avoid oscillations. */
>  	double speed = 0.2;
> -	redGain = speed * redGain + (1 - speed) * activeState.awb.gains.automatic.red;
> -	blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.automatic.blue;
> +	gains = gains * speed + activeState.awb.gains.automatic * (1 - speed);
>  
> -	activeState.awb.gains.automatic.red = redGain;
> -	activeState.awb.gains.automatic.blue = blueGain;
> -	activeState.awb.gains.automatic.green = 1.0;
> +	activeState.awb.gains.automatic = gains;
>  
>  	LOG(RkISP1Awb, Debug)
>  		<< std::showpoint
> -		<< "Means " << rgbMeans
> -		<< ", gains [" << activeState.awb.gains.automatic.red << ", "
> -		<< activeState.awb.gains.automatic.green << ", "
> -		<< activeState.awb.gains.automatic.blue << "], temp "
> +		<< "Means " << rgbMeans << ", gains "
> +		<< activeState.awb.gains.automatic << ", temp "
>  		<< activeState.awb.temperatureK << "K";
>  }
>  
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 14d0c02a2b32..8f545cd76d52 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -188,30 +188,12 @@ namespace libcamera::ipa::rkisp1 {
>   * \struct IPAActiveState::awb.gains
>   * \brief White balance gains
>   *
> - * \struct IPAActiveState::awb.gains.manual
> + * \var IPAActiveState::awb.gains.manual
>   * \brief Manual white balance gains (set through requests)
>   *
> - * \var IPAActiveState::awb.gains.manual.red
> - * \brief Manual white balance gain for R channel
> - *
> - * \var IPAActiveState::awb.gains.manual.green
> - * \brief Manual white balance gain for G channel
> - *
> - * \var IPAActiveState::awb.gains.manual.blue
> - * \brief Manual white balance gain for B channel
> - *
> - * \struct IPAActiveState::awb.gains.automatic
> + * \var IPAActiveState::awb.gains.automatic
>   * \brief Automatic white balance gains (computed by the algorithm)
>   *
> - * \var IPAActiveState::awb.gains.automatic.red
> - * \brief Automatic white balance gain for R channel
> - *
> - * \var IPAActiveState::awb.gains.automatic.green
> - * \brief Automatic white balance gain for G channel
> - *
> - * \var IPAActiveState::awb.gains.automatic.blue
> - * \brief Automatic white balance gain for B channel
> - *
>   * \var IPAActiveState::awb.temperatureK
>   * \brief Estimated color temperature
>   *
> @@ -333,15 +315,6 @@ namespace libcamera::ipa::rkisp1 {
>   * \struct IPAFrameContext::awb.gains
>   * \brief White balance gains
>   *
> - * \var IPAFrameContext::awb.gains.red
> - * \brief White balance gain for R channel
> - *
> - * \var IPAFrameContext::awb.gains.green
> - * \brief White balance gain for G channel
> - *
> - * \var IPAFrameContext::awb.gains.blue
> - * \brief White balance gain for B channel
> - *
>   * \var IPAFrameContext::awb.temperatureK
>   * \brief Estimated color temperature
>   *
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 7b93a9e9461d..b4dec0c3288d 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -25,6 +25,7 @@
>  #include <libipa/camera_sensor_helper.h>
>  #include <libipa/fc_queue.h>
>  #include <libipa/matrix.h>
> +#include <libipa/vector.h>
>  
>  namespace libcamera {
>  
> @@ -87,16 +88,8 @@ struct IPAActiveState {
>  
>  	struct {
>  		struct {
> -			struct {
> -				double red;
> -				double green;
> -				double blue;
> -			} manual;
> -			struct {
> -				double red;
> -				double green;
> -				double blue;
> -			} automatic;
> +			RGB<double> manual;
> +			RGB<double> automatic;
>  		} gains;
>  
>  		unsigned int temperatureK;
> @@ -140,12 +133,7 @@ struct IPAFrameContext : public FrameContext {
>  	} agc;
>  
>  	struct {
> -		struct {
> -			double red;
> -			double green;
> -			double blue;
> -		} gains;
> -
> +		RGB<double> gains;
>  		bool autoEnabled;
>  	} awb;



More information about the libcamera-devel mailing list