[PATCH 2/2] ipa: rkisp1: agc: Set measurement window to full frame

[Stefan Klug]

Hi Kieran,

Thank you for the review. 

On Tue, Mar 25, 2025 at 10:19:54AM +0000, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-03-19 16:01:37)
> > With the availability of metering modes and the corresponding weights,
> > there is a flexible way of defining the area that gets taken into
> > account when AEGC is calculated. There is no need to reduce that window
> > to an arbitrary region anymore. If need arises we can make this
> > parameter user configurable or add a control for it.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > index 101cad5d0893..b3ac9400b74f 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -193,14 +193,10 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
> >         context.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get<int64_t>());
> >         context.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get<int64_t>());
> >  
> > -       /*
> > -        * Define the measurement window for AGC as a centered rectangle
> > -        * covering 3/4 of the image width and height.
> > -        */
> 
> Do the existing weights already match this? Where are the weights
> currently coming from for this ? Are they filled in from the tuning file
> generator ? or is there a default somewhere?

Yes, the weights come from the tuning file generator. This code here
dates back to 2022 8917c9e7ba64 ("ipa: rkisp1: agc: Add a
histogram-based gain") and I believe the intention was to add a bit of a
spot measurement. We should have changed it when the metering modes were
added. You are right, the behavior of existing cameras with existing
tuning files change a bit. So we should strive for updated tuning files
in the near future. We could add the default metering modes to all
existing tuning files in a follow up patch.

Cheers,
Stefan

> 
> > -       context.configuration.agc.measureWindow.h_offs = configInfo.outputSize.width / 8;
> > -       context.configuration.agc.measureWindow.v_offs = configInfo.outputSize.height / 8;
> > -       context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;
> > -       context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;
> > +       context.configuration.agc.measureWindow.h_offs = 0;
> > +       context.configuration.agc.measureWindow.v_offs = 0;
> > +       context.configuration.agc.measureWindow.h_size = configInfo.outputSize.width;
> > +       context.configuration.agc.measureWindow.v_size = configInfo.outputSize.height;
> >  
> >         setLimits(context.configuration.sensor.minExposureTime,
> >                   context.configuration.sensor.maxExposureTime,
> > -- 
> > 2.43.0
> >