[libcamera-devel] [PATCH 12/13] ipa: ipu3: agc: Increase IIR filter speed

[Laurent Pinchart]

On Thu, Oct 14, 2021 at 12:38:15PM +0100, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-10-13 16:41:24)
> > The filter used is an infinite response filter which is controlled by
> > the speed variable. It is used to limit the gap between two exposure
> > values, and avoid nasty oscillations. The higher the speed, the less the
> > result oscillates.
> 
> Does that also mean it will take longer to obtain a good exposure?
> 
> > As we are only calculating the exposure every 6 frames, as controled by
> > the 'kFrameSkipCount' constant, we can increase the speed, to avoid a
> > slow response (it can be more than 1 second to get a stable enough
> > output).
> 
> Are we able to move towards calculating on every frame? or would that
> cause more oscillations? If so that implies to me that we're using the
> wrong inputs to perform the calculations.
> 
> But ... although we might need to do further investigations on the
> inputs if we hit oscillations, I don't think this is a problem if it's a
> short term gain.
> 
> But if we do need to do more work to improve the calculatoins to allow
> us to run the calculations faster, then we might need to record a todo
> somewhere.

I think this patch tries to hide a problem instead of solving it, I'm
not sure I like that :-S (I'm actually pretty sure I dislike it)

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > ---
> >  src/ipa/ipu3/algorithms/agc.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > index b922bcdf..81eaf436 100644
> > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > @@ -92,7 +92,7 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,
> >  
> >  void Agc::filterExposure()
> >  {
> > -       double speed = 0.2;
> > +       double speed = 0.9;
> >         if (filteredExposure_ == 0s) {
> >                 /* DG stands for digital gain.*/
> >                 filteredExposure_ = currentExposure_;

-- 
Regards,

Laurent Pinchart