[PATCH v2 17/17] ipa: rkisp1: awb: Avoid division by zero

[Stefan Klug]

Hi Laurent,

Thank you for the review. 

On Tue, Apr 01, 2025 at 04:21:31AM +0300, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Wed, Mar 19, 2025 at 05:11:22PM +0100, Stefan Klug wrote:
> > As the gains can also be specified manually, the regulation can run into
> > numeric instabilities by dividing by near zero. Mitigate that by
> > applying a small minium value.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > ---
> > 
> > Changes in v2:
> > - Collected tag
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index e8b04d03748d..eb4545439ae3 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -421,9 +421,9 @@ RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rk
> >  	/*
> >  	 * The ISP computes the AWB means after applying the colour gains,
> >  	 * divide by the gains that were used to get the raw means from the
> > -	 * sensor.
> > +	 * sensor. Apply a minimum value to avoid divisions by near-zero.
> 
> Shouldn't we instead prevent the user from setting zero (or near-zero)
> gains ? 

That would also work, but afaik we have no mechanism to check if
controls adhere to the range provided in the ControlInfo. And there are
multiple code paths feeding into framContext.awb.gains. So I think it
is easier to move the check to where it might do harm instead of
ensuring every code path has similar checks.

Best regards,
Stefan

> 
> >  	 */
> > -	rgbMeans /= frameContext.awb.gains;
> > +	rgbMeans /= frameContext.awb.gains.max(0.01);
> >  
> >  	return rgbMeans;
> >  }
> 
> -- 
> Regards,
> 
> Laurent Pinchart