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

[Laurent Pinchart]

On Wed, Apr 02, 2025 at 05:26:55PM +0200, Stefan Klug wrote:
> On Tue, Apr 01, 2025 at 04:21:31AM +0300, Laurent Pinchart wrote:
> > 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.

As a short term fix that's fine with me, but I think we should still
address the overall issue, and clamp/adjust control values at queue
request time. That's the entry point for controls, if we handle it
there, it should be safe everywhere else in the IPA module.

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

-- 
Regards,

Laurent Pinchart