[PATCH 09/12] ipa: rkisp1: agc: Use mode from frame context to calculate new EV

[Laurent Pinchart]

On Mon, Jun 17, 2024 at 09:52:21PM +0900, Paul Elder wrote:
> On Mon, Jun 17, 2024 at 12:32:44PM +0300, Laurent Pinchart wrote:
> > On Mon, Jun 17, 2024 at 10:29:18AM +0100, Kieran Bingham wrote:
> > > Quoting Laurent Pinchart (2024-06-16 17:39:07)
> > > > The effective exposure value for each frame is split into shutter time,
> > > > analog gain and digital gain based on the AGC constraint mode and
> > > > exposure mode. The algorithm uses the modes from the active state, which
> > > > tracks the latest queued request, instead of the frame context, which
> > > > tracks the value of the controls requested for that frame. Fix it by
> > > > using the correct modes.
> > > 
> > > The state tracking at different points of time is going to be a
> > > recurring tricky part isn't it.
> > 
> > It appears to be so. A recuring pain point, but at the same time, it
> > makes me happy to see a pattern emerging. Patterns mean we can try to
> > find structural solutions.
> 
> I can never remember which is the right one...

I'll take that as a vote for a structural solution :-)

> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> > > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > 
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > ---
> > > >  src/ipa/rkisp1/algorithms/agc.cpp | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > > > index a61201bb05c9..6a199b47c9ad 100644
> > > > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > > > @@ -439,8 +439,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > > >         utils::Duration shutterTime;
> > > >         double aGain, dGain;
> > > >         std::tie(shutterTime, aGain, dGain) =
> > > > -               calculateNewEv(context.activeState.agc.constraintMode,
> > > > -                              context.activeState.agc.exposureMode,
> > > > +               calculateNewEv(frameContext.agc.constraintMode,
> > > > +                              frameContext.agc.exposureMode,
> > > >                                hist, effectiveExposureValue);
> > > >  
> > > >         LOG(RkISP1Agc, Debug)

-- 
Regards,

Laurent Pinchart