[libcamera-devel] [RFC PATCH 1/2] ipa: rkisp1: awb: add support for RGB means

[Laurent Pinchart]

Hi Quentin,

Thank you for the patch.

On Thu, May 12, 2022 at 10:42:43AM +0200, Quentin Schulz via libcamera-devel wrote:
> From: Quentin Schulz <quentin.schulz at theobroma-systems.com>
> 
> RkISP actually supports two modes for color means, RGB and YCbCr. The
> variables where the means are stored are identically named regardless of
> the color means mode that's been selected.
> 
> Since the gains are computed in RGB mode, a conversion needs to be done
> when the mode is YCbCr, which is unnecessary when RGB mode is selected.
> 
> This adds support for RGB means mode too, by checking at runtime which
> mode is selected at a given time.
> 
> Cc: Quentin Schulz <foss+libcamera at 0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz at theobroma-systems.com>
> ---
> 
> To be decided if we want to keep supporting both modes or only one?
> (Given that at the moment we harcode which mode is used at compile time,
>  I'm not sure it's worth keeping dead code around)

I'd like to test this on RK3399 and on i.MX8MP (I assume you've tested
the PX30). If all tests pass, then I think we can just drop YUV support
completely.

Jean-Michel, I don't have RK3399 hardware with me, could you give this
series a try ?

>  src/ipa/rkisp1/algorithms/awb.cpp | 57 ++++++++++++++++++-------------
>  1 file changed, 33 insertions(+), 24 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index be4585c6..df749b9b 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -124,30 +124,39 @@ void Awb::process([[maybe_unused]] IPAContext &context, const rkisp1_stat_buffer
>  	const rkisp1_cif_isp_stat *params = &stats->params;
>  	const rkisp1_cif_isp_awb_stat *awb = &params->awb;
>  	IPAFrameContext &frameContext = context.frameContext;
> -
> -	/* Get the YCbCr mean values */
> -	double yMean = awb->awb_mean[0].mean_y_or_g;
> -	double crMean = awb->awb_mean[0].mean_cr_or_r;
> -	double cbMean = awb->awb_mean[0].mean_cb_or_b;
> -
> -	/*
> -	 * Convert from YCbCr to RGB.
> -	 * The hardware uses the following formulas:
> -	 * Y = 16 + 0.2500 R + 0.5000 G + 0.1094 B
> -	 * Cb = 128 - 0.1406 R - 0.2969 G + 0.4375 B
> -	 * Cr = 128 + 0.4375 R - 0.3750 G - 0.0625 B
> -	 *
> -	 * The inverse matrix is thus:
> -	 * [[1,1636, -0,0623,  1,6008]
> -	 *  [1,1636, -0,4045, -0,7949]
> -	 *  [1,1636,  1,9912, -0,0250]]
> -	 */
> -	yMean -= 16;
> -	cbMean -= 128;
> -	crMean -= 128;
> -	double redMean = 1.1636 * yMean - 0.0623 * cbMean + 1.6008 * crMean;
> -	double greenMean = 1.1636 * yMean - 0.4045 * cbMean - 0.7949 * crMean;
> -	double blueMean = 1.1636 * yMean + 1.9912 * cbMean - 0.0250 * crMean;
> +	double greenMean;
> +	double redMean;
> +	double blueMean;
> +
> +	if (params->meas.awb_meas_config.awb_mode == RKISP1_CIF_ISP_AWB_MODE_RGB) {
> +		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;
> +	} else {
> +		/* Get the YCbCr mean values */
> +		double yMean = awb->awb_mean[0].mean_y_or_g;
> +		double crMean = awb->awb_mean[0].mean_cr_or_r;
> +		double cbMean = awb->awb_mean[0].mean_cb_or_b;
> +
> +		/*
> +		 * Convert from YCbCr to RGB.
> +		 * The hardware uses the following formulas:
> +		 * Y = 16 + 0.2500 R + 0.5000 G + 0.1094 B
> +		 * Cb = 128 - 0.1406 R - 0.2969 G + 0.4375 B
> +		 * Cr = 128 + 0.4375 R - 0.3750 G - 0.0625 B
> +		 *
> +		 * The inverse matrix is thus:
> +		 * [[1,1636, -0,0623,  1,6008]
> +		 *  [1,1636, -0,4045, -0,7949]
> +		 *  [1,1636,  1,9912, -0,0250]]
> +		 */
> +		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;
> +	}
>  
>  	/* Estimate the red and blue gains to apply in a grey world. */
>  	double redGain = greenMean / (redMean + 1);

-- 
Regards,

Laurent Pinchart