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

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:57)
> 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.
> 
> 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);

We don't need to clamp to a value greater than zero for any reason here
do we?

I was trying to conjour up in my head if perhaps this needs to clamp to
a max with 1.0 for any reason, but these are not the gains - but the
values used later I believe.

So I think it sounds fine...


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>         }
>  
>         /*
> -- 
> Regards,
> 
> Laurent Pinchart
>