[libcamera-devel] [PATCH] ipa: ipu3: agc: Fix -nan exception

[Kate Hsuan]

Hi Jean-Michel,

On Thu, Nov 11, 2021 at 4:25 PM Jean-Michel Hautbois
<jeanmichel.hautbois at ideasonboard.com> wrote:
>
> Hi Kate,
>
> Thanks for the patch !
>
> On 11/11/2021 09:10, Kate Hsuan wrote:
> > The user may equip a cam cover to make sure security.If they want to
> > disable the cam, the cam cover could be physically closed to ensure
> > the image will not be delivered. In this situation, the image will be
> > all black and it triggers the -nan exception for some values.
> > Eventually, the exposure will be locked. As a result, we only can get
> > a black image.
> >
> > evGain is calculated by kEvGainTarget * knumHistogramBins / iqMean_.
> > For a black image, a -nan of iqMean_ will lead evGain miss calculated.
> > Consequently, the exposure value can not be correctly estimated and be
> > locked at -nan as well. In this situation, evGain is set to 1.0 and try
> > to estimate again through the next frame. Also, an additional check
> > makes sure the exposure value is between minTotalExposure and
> > maxTotalExposure.
> >
> > Signed-off-by: Kate Hsuan <hpa at redhat.com>
> > ---
> >   src/ipa/ipu3/algorithms/agc.cpp | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > index b5d736c1..d965febe 100644
> > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > @@ -191,6 +191,8 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
> >
> >       /* Estimate the gain needed to have the proportion wanted */
> >       double evGain = kEvGainTarget * knumHistogramBins / iqMean_;
> > +     if(std::isnan(evGain))
> > +             evGain = 1.0;
>
> This has been found on our side too :-) but we solved it in a different
> way, improving the AGC behaviour. Maybe would you like to have a look to
> the "[PATCH v2 00/14] IPA: IPU3: Introduce per-frame controls" series ?
>
> To be honest, I hesitated between testing nan, patching the Histogram
> class and the third option you can find in "ipa: ipu3: agc: Limit the
> number of saturated cells".

Thank you for noticing me this :)

We are working on the same issue.

>
> >
> >       /* extracted from Rpi::Agc::computeTargetExposure */
> >       /* Calculate the shutter time in seconds */
> > @@ -210,7 +212,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
> >
> >       /* Clamp the exposure value to the min and max authorized */
> >       utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;
> > -     currentExposure_ = std::min(currentExposure_, maxTotalExposure);
> > +     utils::Duration minTotalExposure = minShutterSpeed * minAnalogueGain_;
> > +
> > +     currentExposure_ = std::clamp<utils::Duration>(currentExposure_, minTotalExposure, maxTotalExposure);
> >       LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
> >                           << ", maximum is " << maxTotalExposure;
> >
> > @@ -232,7 +236,6 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
> >       LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
> >                           << shutterTime << " and "
> >                           << stepGain;
> > -
> >       exposure = shutterTime / lineDuration_;
> >       analogueGain = stepGain;
> >
> >
>


-- 
BR,
Kate