[libcamera-devel] [PATCH v4 29/32] ipa: rkisp1: awb: Prevent RGB means from being negative

[Jacopo Mondi]

Hi Laurent

On Thu, Sep 08, 2022 at 04:41:57AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Due to hardware rounding errors in the YCbCr means, the calculated RGB

Is it due to rounding or simply because
the values of yMean, cbMean and crMean

		double yMean = awb->awb_mean[0].mean_y_or_g;
		double cbMean = awb->awb_mean[0].mean_cb_or_b;
		double crMean = awb->awb_mean[0].mean_cr_or_r;

		/*
		 * 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;

are smaller than the above offsets ? I guess this can happen when the
image is particularly dark, right ?

Anyway, this certainly fixes an issue

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> means may be negative. This would lead to negative gains, messing up
> calculation. Prevent this by clamping the means to positive values.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index ed91e9277a16..de54c4d24650 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -242,6 +242,16 @@ void Awb::process(IPAContext &context,
>  		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;
> +
> +		/*
> +		 * Due to hardware rounding errors in the YCbCr means, the
> +		 * calculated RGB means may be negative. This would lead to
> +		 * 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);
>  	}
>
>  	/*
> --
> Regards,
>
> Laurent Pinchart
>