[PATCH 11/12] ipa: rkisp1: agc: Correctly clamp maximum shutter speed

Paul Elder paul.elder at ideasonboard.com
Mon Jun 17 13:52:01 CEST 2024


On Mon, Jun 17, 2024 at 10:48:39AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-06-16 17:39:09)
> > The sensor's maximum shutter speed is clamped by the maximum frame
> > duration specified in requests. If the requested maximum frame duration
> > is lower than the sensor's minimum shutter speed, the Agc::process()
> > function will pass a minimum value higher than the maximum to the
> > setLimits() function, resulting in an assertion failure. Fix it by
> > clamping the value to both the lower and the upper bounds.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > index 5b917557b887..0018c43f18cf 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -420,8 +420,10 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> >                        [](uint32_t x) { return x >> 4; });
> >         expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };
> >  
> > -       utils::Duration maxShutterSpeed = std::min(context.configuration.sensor.maxShutterSpeed,
> > -                                                  frameContext.agc.maxFrameDuration);
> > +       utils::Duration maxShutterSpeed =
> > +               std::clamp(frameContext.agc.maxFrameDuration,
> > +                          context.configuration.sensor.minShutterSpeed,
> > +                          context.configuration.sensor.maxShutterSpeed);
> >         setLimits(context.configuration.sensor.minShutterSpeed,
> >                   maxShutterSpeed,
> 
> I'm somehow confused here, as we're passing sensor.minShutterSpeed in
> here as well. Does that now become redundant? Or could/should this
> clamping be done within setLimits? Does the other call site of setLimits
> experience the same issue?

setLimits() is to configure the ExposureModeHelper, so that you can use
it to split the exposure duration into gain and shutter time.
sensor.minShutterSpeed is the minimum shutter speed for the exposure
mode helper, and the maxShutterSpeed is, well, the maximum.

Looks fine to me.

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> 
> 
> 
> >                   context.configuration.sensor.minAnalogueGain,


More information about the libcamera-devel mailing list