[libcamera-devel] [PATCH v3 09/14] ipa: ipu3: agc: Simplify division of exposure/gain
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Oct 22 06:08:01 CEST 2021
On Thu, Oct 21, 2021 at 10:08:37PM +0100, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-10-21 17:43:56)
> > Until now, the algorithm makes complex assumptions when dividing the
> > exposure and analogue gain values. Instead, use a simpler clamping of
> > the shutter speed first, and then of the analogue gain, based on the
> > limits configured.
> >
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/ipa/ipu3/algorithms/agc.cpp | 36 +++++++++++++++++----------------
> > 1 file changed, 19 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > index f5bb3328..6f5b6a45 100644
> > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > @@ -147,7 +147,9 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
> > << " Gain " << gain;
> >
> > currentExposure_ = currentExposureNoDg_ * newGain;
> > + utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;
> > utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;
> > +
> > utils::Duration maxTotalExposure = maxShutterSpeed * kMaxGain;
> > currentExposure_ = std::min(currentExposure_, maxTotalExposure);
> > LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
> > @@ -156,23 +158,23 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)
> > /* \todo: estimate if we need to desaturate */
> > filterExposure();
> >
> > - utils::Duration newExposure = 0.0s;
> > - if (currentShutter < maxShutterSpeed) {
> > - exposure = std::clamp<uint32_t>(exposure * filteredExposure_ / currentExposureNoDg_,
> > - minExposureLines_,
> > - maxExposureLines_);
> > - newExposure = filteredExposure_ / exposure;
> > - gain = std::clamp(gain * filteredExposure_ / newExposure,
> > - kMinGain, kMaxGain);
> > - } else {
> > - gain = std::clamp(gain * filteredExposure_ / currentExposureNoDg_,
> > - kMinGain, kMaxGain);
> > - newExposure = filteredExposure_ / gain;
> > - exposure = std::clamp<uint32_t>(exposure * filteredExposure_ / newExposure,
>
> Makes the previous patch a bit redundant, but at least we removed code
> that was more 'correct' ;-)
The two used to be combined in a single patch, and I've pointed out
twice that they were separate changes :-) The switch to
filteredExposure_ was easy to overlook in previous versions.
> > - minExposureLines_,
> > - maxExposureLines_);
> > - }
> > - LOG(IPU3Agc, Debug) << "Adjust exposure " << exposure * lineDuration_ << " and gain " << gain;
> > + utils::Duration exposureValue = filteredExposure_;
> > + utils::Duration shutterTime = minShutterSpeed;
> > +
> > + /*
> > + * Push the shutter time up to the maximum first, and only then
> > + * increase the gain.
> > + */
> > + shutterTime = std::clamp<utils::Duration>(exposureValue / kMinGain,
> > + minShutterSpeed, maxShutterSpeed);
> > + double stepGain = std::clamp(exposureValue / shutterTime,
> > + kMinGain, kMaxGain);
> > + LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
> > + << shutterTime << " and "
> > + << stepGain;
> > +
> > + exposure = shutterTime / lineDuration_;
> > + gain = stepGain;
>
> This looks a lot simpler indeed. I haven't gone through and fully
> validated all the equations, but as I believe I've seen this working,
> locally on my device, I think I can trust that it's functional.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > }
> > lastFrame_ = frameCount_;
> > }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list