[PATCH 1/2] libcamera: software_isp: Initialize exposure+gain before agc calculations

[Kieran Bingham]

Hi Robert,

Quoting Robert Mader (2024-11-12 11:38:07)
> Thanks for the patch!
> 
> Tested-by: Robert Mader <robert.mader at collabora.com>
> 
> Would love to see it land so 
> https://patchwork.libcamera.org/patch/21865/ can be rebased and land on top

Have you tested https://patchwork.libcamera.org/patch/21865/ ? Could you
send a tag (RB or TB) for that one please?

Regards
--
Kieran



> 
> On 29.10.24 12:25, Stanislaw Gruszka wrote:
> > On my setup, since commit fb8ad13d ("libcamera: software_isp: Move exposure+gain
> > to an algorithm module"), at start camera output stays very dark for dozen
> > of seconds, and then later slowly gets to normal. This is because existing
> > sensor exposure+gain settings are not used at start. We save initial
> > values in frameContext but in the agc algorithm we use IPA context.
> >
> > Fix the problem by using in frameContext sensor values, since we already
> > use those in blc algorithm and change exposure type to int32_t to
> > unnecessary castings.
> >
> > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
> > ---
> >   src/ipa/simple/algorithms/agc.cpp | 8 ++++----
> >   src/ipa/simple/algorithms/agc.h   | 2 +-
> >   src/ipa/simple/algorithms/blc.h   | 2 +-
> >   src/ipa/simple/ipa_context.h      | 2 +-
> >   src/ipa/simple/soft_simple.cpp    | 4 ++--
> >   5 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp
> > index df92edd7..72aade14 100644
> > --- a/src/ipa/simple/algorithms/agc.cpp
> > +++ b/src/ipa/simple/algorithms/agc.cpp
> > @@ -39,7 +39,7 @@ Agc::Agc()
> >   {
> >   }
> >   
> > -void Agc::updateExposure(IPAContext &context, double exposureMSV)
> > +void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)
> >   {
> >       /*
> >        * kExpDenominator of 10 gives ~10% increment/decrement;
> > @@ -50,8 +50,8 @@ void Agc::updateExposure(IPAContext &context, double exposureMSV)
> >       static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;
> >   
> >       double next;
> > -     int32_t &exposure = context.activeState.agc.exposure;
> > -     double &again = context.activeState.agc.again;
> > +     int32_t &exposure = frameContext.sensor.exposure;
> > +     double &again = frameContext.sensor.gain;
> >   
> >       if (exposureMSV < kExposureOptimal - kExposureSatisfactory) {
> >               next = exposure * kExpNumeratorUp / kExpDenominator;
> > @@ -129,7 +129,7 @@ void Agc::process(IPAContext &context,
> >       }
> >   
> >       float exposureMSV = (denom == 0 ? 0 : static_cast<float>(num) / denom);
> > -     updateExposure(context, exposureMSV);
> > +     updateExposure(context, frameContext, exposureMSV);
> >   }
> >   
> >   REGISTER_IPA_ALGORITHM(Agc, "Agc")
> > diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h
> > index ad5fca9f..112d9f5a 100644
> > --- a/src/ipa/simple/algorithms/agc.h
> > +++ b/src/ipa/simple/algorithms/agc.h
> > @@ -25,7 +25,7 @@ public:
> >                    ControlList &metadata) override;
> >   
> >   private:
> > -     void updateExposure(IPAContext &context, double exposureMSV);
> > +     void updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV);
> >   };
> >   
> >   } /* namespace ipa::soft::algorithms */
> > diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h
> > index 2cf2a877..67c688ae 100644
> > --- a/src/ipa/simple/algorithms/blc.h
> > +++ b/src/ipa/simple/algorithms/blc.h
> > @@ -27,7 +27,7 @@ public:
> >                    ControlList &metadata) override;
> >   
> >   private:
> > -     uint32_t exposure_;
> > +     int32_t exposure_;
> >       double gain_;
> >   };
> >   
> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> > index fd121eeb..c510c436 100644
> > --- a/src/ipa/simple/ipa_context.h
> > +++ b/src/ipa/simple/ipa_context.h
> > @@ -53,7 +53,7 @@ struct IPAActiveState {
> >   
> >   struct IPAFrameContext : public FrameContext {
> >       struct {
> > -             uint32_t exposure;
> > +             int32_t exposure;
> >               double gain;
> >       } sensor;
> >   };
> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> > index c8ad55a2..dfacd6aa 100644
> > --- a/src/ipa/simple/soft_simple.cpp
> > +++ b/src/ipa/simple/soft_simple.cpp
> > @@ -310,8 +310,8 @@ void IPASoftSimple::processStats(const uint32_t frame,
> >   
> >       ControlList ctrls(sensorInfoMap_);
> >   
> > -     auto &againNew = context_.activeState.agc.again;
> > -     ctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);
> > +     auto &againNew = frameContext.sensor.gain;
> > +     ctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure);
> >       ctrls.set(V4L2_CID_ANALOGUE_GAIN,
> >                 static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(againNew) : againNew));
> >   
> 
> -- 
> Robert Mader
> Consultant Software Developer
> 
> Collabora Ltd.
> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
> Registered in England & Wales, no. 5513718
>