[libcamera-devel] [PATCH v4 30/32] ipa: rkisp1: awb: Clamp gains to prevent divisions by zero

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:58)
> The gain values are currently clamped to the range [0.0, 3.996] used by
> the hardware. A zero value makes little sense, as it would completely
> remove the contribution of the corresponding color channel from the AWB
> accumulators, but worse, would lead to divisions by zero when
> calculating the raw means in subsequent iterations. Prevent this by
> setting the minimum gain value to 1/256.
> 
> While at it, clamp the gain values before filtering them, to improve the
> stability of the control loop.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index de54c4d24650..5f2535688c93 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -265,21 +265,29 @@ void Awb::process(IPAContext &context,
>  
>         frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
>  
> -       /* Estimate the red and blue gains to apply in a grey world. */
> +       /*
> +        * Estimate the red and blue gains to apply in a grey world. The green
> +        * gain is hardcoded to 0.

0 or 1?

With that 

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

> +        */
>         double redGain = greenMean / (redMean + 1);
>         double blueGain = greenMean / (blueMean + 1);
>  
> +       /*
> +        * Clamp the gain values to the hardware, which expresses gains as Q2.8
> +        * unsigned integer values. Set the minimum just above zero to avoid
> +        * 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);
> +
>         /* 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;
>  
> -       /*
> -        * Gain values are unsigned integer value, range 0 to 4 with 8 bit
> -        * fractional part. Hardcode the green gain to 1.0.
> -        */
> -       activeState.awb.gains.automatic.red = std::clamp(redGain, 0.0, 1023.0 / 256);
> -       activeState.awb.gains.automatic.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);
> +       activeState.awb.gains.automatic.red = redGain;
> +       activeState.awb.gains.automatic.blue = blueGain;
>         activeState.awb.gains.automatic.green = 1.0;
>  
>         LOG(RkISP1Awb, Debug) << std::showpoint
> -- 
> Regards,
> 
> Laurent Pinchart
>