[PATCH v3 13/17] ipa: libipa: colour: Use the RGB class to model RGB values

Milan Zamazal mzamazal at redhat.com
Tue Nov 19 11:46:59 CET 2024


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

> The rec601LuminanceFromRGB() and estimateCCT() functions take RGB
> triplets as three variables. Replace them with instances of the RGB
> class and adapt the users accordingly. Only variables passed directly to
> these functions are converted to RGB instances, further conversion of
> IPA moduels to the RGB class will be performed separately.

... modules ...

> While at it, fix a typo in the documentation of the estimateCCT()
> function.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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

> ---
>  src/ipa/ipu3/algorithms/agc.cpp            | 14 +++----
>  src/ipa/ipu3/algorithms/awb.cpp            |  2 +-
>  src/ipa/libipa/colours.cpp                 | 22 +++++-----
>  src/ipa/libipa/colours.h                   |  6 ++-
>  src/ipa/rkisp1/algorithms/awb.cpp          | 47 ++++++++++++----------
>  src/ipa/rpi/controller/rpi/agc_channel.cpp | 21 +++++-----
>  6 files changed, 55 insertions(+), 57 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 466b3fb31a6c..fda4daa6306c 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -178,18 +178,16 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
>   */
>  double Agc::estimateLuminance(double gain) const
>  {
> -	double redSum = 0, greenSum = 0, blueSum = 0;
> +	RGB<double> sum{ 0.0 };
>  
>  	for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
> -		redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
> -		greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
> -		blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
> +		sum.r() += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
> +		sum.g() += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
> +		sum.b() += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
>  	}
>  
> -	double ySum = rec601LuminanceFromRGB(redSum * rGain_,
> -					     greenSum * gGain_,
> -					     blueSum * bGain_);
> -
> +	RGB<double> gains{{ rGain_, gGain_, bGain_ }};
> +	double ySum = rec601LuminanceFromRGB(sum * gains);
>  	return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
>  }
>  
> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> index 7c6bff09147c..55de05d9e39f 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -412,7 +412,7 @@ void Awb::awbGreyWorld()
>  	       blueGain = sumBlue.g() / (sumBlue.b() + 1);
>  
>  	/* Color temperature is not relevant in Grey world but still useful to estimate it :-) */
> -	asyncResults_.temperatureK = estimateCCT(sumRed.r(), sumRed.g(), sumBlue.b());
> +	asyncResults_.temperatureK = estimateCCT({{ sumRed.r(), sumRed.g(), sumBlue.b() }});
>  
>  	/*
>  	 * Gain values are unsigned integer value ranging [0, 8) with 13 bit
> diff --git a/src/ipa/libipa/colours.cpp b/src/ipa/libipa/colours.cpp
> index 9fcb53b08ffb..6c597093ddf8 100644
> --- a/src/ipa/libipa/colours.cpp
> +++ b/src/ipa/libipa/colours.cpp
> @@ -21,9 +21,7 @@ namespace ipa {
>  
>  /**
>   * \brief Estimate luminance from RGB values following ITU-R BT.601
> - * \param[in] r The red value
> - * \param[in] g The green value
> - * \param[in] b The blue value
> + * \param[in] rgb The RGB value
>   *
>   * This function estimates a luminance value from a triplet of Red, Green and
>   * Blue values, following the formula defined by ITU-R Recommendation BT.601-7
> @@ -31,21 +29,19 @@ namespace ipa {
>   *
>   * \return The estimated luminance value
>   */
> -double rec601LuminanceFromRGB(double r, double g, double b)
> +double rec601LuminanceFromRGB(const RGB<double> &rgb)
>  {
> -	return (r * .299) + (g * .587) + (b * .114);
> +	return (rgb.r() * .299) + (rgb.g() * .587) + (rgb.b() * .114);
>  }
>  
>  /**
>   * \brief Estimate correlated colour temperature from RGB color space input
> - * \param[in] red The input red value
> - * \param[in] green The input green value
> - * \param[in] blue The input blue value
> + * \param[in] rgb The RGB value
>   *
>   * This function estimates the correlated color temperature RGB color space
>   * input. In physics and color science, the Planckian locus or black body locus
>   * is the path or locus that the color of an incandescent black body would take
> - * in a particular chromaticity space as the blackbody temperature changes.
> + * in a particular chromaticity space as the black body temperature changes.
>   *
>   * If a narrow range of color temperatures is considered (those encapsulating
>   * daylight being the most practical case) one can approximate the Planckian
> @@ -56,12 +52,12 @@ double rec601LuminanceFromRGB(double r, double g, double b)
>   *
>   * \return The estimated color temperature
>   */
> -uint32_t estimateCCT(double red, double green, double blue)
> +uint32_t estimateCCT(const RGB<double> &rgb)
>  {
>  	/* Convert the RGB values to CIE tristimulus values (XYZ) */
> -	double X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);
> -	double Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);
> -	double Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);
> +	double X = (-0.14282) * rgb.r() + (1.54924) * rgb.g() + (-0.95641) * rgb.b();
> +	double Y = (-0.32466) * rgb.r() + (1.57837) * rgb.g() + (-0.73191) * rgb.b();
> +	double Z = (-0.68202) * rgb.r() + (0.77073) * rgb.g() + (0.56332) * rgb.b();
>  
>  	/* Calculate the normalized chromaticity values */
>  	double x = X / (X + Y + Z);
> diff --git a/src/ipa/libipa/colours.h b/src/ipa/libipa/colours.h
> index b42ed0ac1612..fa6a8b575cc7 100644
> --- a/src/ipa/libipa/colours.h
> +++ b/src/ipa/libipa/colours.h
> @@ -9,12 +9,14 @@
>  
>  #include <stdint.h>
>  
> +#include "vector.h"
> +
>  namespace libcamera {
>  
>  namespace ipa {
>  
> -double rec601LuminanceFromRGB(double r, double g, double b);
> -uint32_t estimateCCT(double red, double green, double blue);
> +double rec601LuminanceFromRGB(const RGB<double> &rgb);
> +uint32_t estimateCCT(const RGB<double> &rgb);
>  
>  } /* namespace ipa */
>  
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 5c1d9511ce8b..dbeaf81565ff 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -192,9 +192,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;
> -	double greenMean;
> -	double redMean;
> -	double blueMean;
> +	RGB<double> rgbMeans;
>  
>  	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
>  	metadata.set(controls::ColourGains, {
> @@ -209,9 +207,11 @@ void Awb::process(IPAContext &context,
>  	}
>  
>  	if (rgbMode_) {
> -		greenMean = awb->awb_mean[0].mean_y_or_g;
> -		redMean = awb->awb_mean[0].mean_cr_or_r;
> -		blueMean = awb->awb_mean[0].mean_cb_or_b;
> +		rgbMeans = {{
> +			static_cast<double>(awb->awb_mean[0].mean_y_or_g),
> +			static_cast<double>(awb->awb_mean[0].mean_cr_or_r),
> +			static_cast<double>(awb->awb_mean[0].mean_cb_or_b)
> +		}};
>  	} else {
>  		/* Get the YCbCr mean values */
>  		double yMean = awb->awb_mean[0].mean_y_or_g;
> @@ -233,9 +233,11 @@ void Awb::process(IPAContext &context,
>  		yMean -= 16;
>  		cbMean -= 128;
>  		crMean -= 128;
> -		redMean = 1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean;
> -		greenMean = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean;
> -		blueMean = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean;
> +		rgbMeans = {{
> +			1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean,
> +			1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean,
> +			1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean
> +		}};
>  
>  		/*
>  		 * Due to hardware rounding errors in the YCbCr means, the
> @@ -243,9 +245,7 @@ void Awb::process(IPAContext &context,
>  		 * negative gains, messing up calculation. Prevent this by
>  		 * clamping the means to positive values.
>  		 */
> -		redMean = std::max(redMean, 0.0);
> -		greenMean = std::max(greenMean, 0.0);
> -		blueMean = std::max(blueMean, 0.0);
> +		rgbMeans = rgbMeans.max(0.0);
>  	}
>  
>  	/*
> @@ -253,19 +253,22 @@ void Awb::process(IPAContext &context,
>  	 * divide by the gains that were used to get the raw means from the
>  	 * sensor.
>  	 */
> -	redMean /= frameContext.awb.gains.red;
> -	greenMean /= frameContext.awb.gains.green;
> -	blueMean /= frameContext.awb.gains.blue;
> +	RGB<double> gains{{
> +		frameContext.awb.gains.red,
> +		frameContext.awb.gains.green,
> +		frameContext.awb.gains.blue
> +	}};
> +	rgbMeans /= gains;
>  
>  	/*
>  	 * If the means are too small we don't have enough information to
>  	 * meaningfully calculate gains. Freeze the algorithm in that case.
>  	 */
> -	if (redMean < kMeanMinThreshold && greenMean < kMeanMinThreshold &&
> -	    blueMean < kMeanMinThreshold)
> +	if (rgbMeans.r() < kMeanMinThreshold && rgbMeans.g() < kMeanMinThreshold &&
> +	    rgbMeans.b() < kMeanMinThreshold)
>  		return;
>  
> -	activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
> +	activeState.awb.temperatureK = estimateCCT(rgbMeans);
>  
>  	/* Metadata shall contain the up to date measurement */
>  	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> @@ -275,8 +278,8 @@ 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 = greenMean / std::max(redMean, 1.0);
> -	double blueGain = greenMean / std::max(blueMean, 1.0);
> +	double redGain = rgbMeans.g() / std::max(rgbMeans.r(), 1.0);
> +	double blueGain = rgbMeans.g() / std::max(rgbMeans.b(), 1.0);
>  
>  	/*
>  	 * Clamp the gain values to the hardware, which expresses gains as Q2.8
> @@ -298,8 +301,8 @@ void Awb::process(IPAContext &context,
>  
>  	LOG(RkISP1Awb, Debug)
>  		<< std::showpoint
> -		<< "Means [" << redMean << ", " << greenMean << ", " << blueMean
> -		<< "], gains [" << activeState.awb.gains.automatic.red << ", "
> +		<< "Means " << rgbMeans
> +		<< ", gains [" << activeState.awb.gains.automatic.red << ", "
>  		<< activeState.awb.gains.automatic.green << ", "
>  		<< activeState.awb.gains.automatic.blue << "], temp "
>  		<< activeState.awb.temperatureK << "K";
> diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp
> index 8583f4f31907..a99beb70b89a 100644
> --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp
> +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp
> @@ -13,6 +13,7 @@
>  #include <libcamera/base/log.h>
>  
>  #include "libipa/colours.h"
> +#include "libipa/vector.h"
>  
>  #include "../awb_status.h"
>  #include "../device_status.h"
> @@ -680,12 +681,13 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,
>  	 * 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;
> +	ipa::RGB<double> sum{ 0.0 };
> +	double pixelSum = 0;
>  	for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {
>  		auto &region = stats->agcRegions.get(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);
> +		sum.r() += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);
> +		sum.g() += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);
> +		sum.b() += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);
>  		pixelSum += region.counted;
>  	}
>  	if (pixelSum == 0.0) {
> @@ -693,14 +695,11 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,
>  		return 0;
>  	}
>  
> -	double ySum;
>  	/* Factor in the AWB correction if needed. */
> -	if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {
> -		ySum = ipa::rec601LuminanceFromRGB(rSum * awb.gainR,
> -						   gSum * awb.gainG,
> -						   bSum * awb.gainB);
> -	} else
> -		ySum = ipa::rec601LuminanceFromRGB(rSum, gSum, bSum);
> +	if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb)
> +		sum *= ipa::RGB<double>{{ awb.gainR, awb.gainR, awb.gainB }};
> +
> +	double ySum = ipa::rec601LuminanceFromRGB(sum);
>  
>  	return ySum / pixelSum / (1 << 16);
>  }



More information about the libcamera-devel mailing list