[PATCH v2 6/6] ipa: rpi: Rename dropFrameCount_ to startupCount_

Naushir Patuck naush at raspberrypi.com
Thu Jun 5 13:34:32 CEST 2025


Hi Jacopo,


On Thu, 5 Jun 2025 at 08:30, Jacopo Mondi <jacopo.mondi at ideasonboard.com>
wrote:

> Hi Naush
>
> On Thu, May 22, 2025 at 08:48:22AM +0100, Naushir Patuck wrote:
> > Rename dropFrameCount_ to startupCount_ to better reflect its use as
> > frames are no longer dropped by the pipeline handler.
>
> Makes sense
>
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/ipa/rpi/common/ipa_base.cpp | 10 +++++-----
> >  src/ipa/rpi/common/ipa_base.h   |  4 ++--
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/ipa/rpi/common/ipa_base.cpp
> b/src/ipa/rpi/common/ipa_base.cpp
> > index c15f8a7bf71e..8d591faeceaa 100644
> > --- a/src/ipa/rpi/common/ipa_base.cpp
> > +++ b/src/ipa/rpi/common/ipa_base.cpp
> > @@ -327,7 +327,7 @@ void IpaBase::start(const ControlList &controls,
> StartResult *result)
> >       unsigned int agcConvergenceFrames = 0, awbConvergenceFrames = 0;
> >       frameCount_ = 0;
> >       if (firstStart_) {
> > -             dropFrameCount_ = helper_->hideFramesStartup();
> > +             startupCount_ = helper_->hideFramesStartup();
> >               mistrustCount_ = helper_->mistrustFramesStartup();
> >
> >               /*
> > @@ -353,14 +353,14 @@ void IpaBase::start(const ControlList &controls,
> StartResult *result)
> >                               awbConvergenceFrames += mistrustCount_;
> >               }
> >       } else {
> > -             dropFrameCount_ = helper_->hideFramesModeSwitch();
> > +             startupCount_ = helper_->hideFramesModeSwitch();
> >               mistrustCount_ = helper_->mistrustFramesModeSwitch();
> >       }
> >
> >       result->startupFrameCount = std::max({ agcConvergenceFrames,
> awbConvergenceFrames });
> > -     result->invalidFrameCount = dropFrameCount_;
> > +     result->invalidFrameCount = startupCount_;
>
> But here it makese
>
>         result->startupFrameCount = .. convergence frames;
>         result->invalidFrameCount = startupCount_;
>
> It's your IPA, so if it's fine with you, fine with me!
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>

It took me a while to realise what was wrong here, and indeed the naming is
completely opposite between the IPA and PH.  I will rename to make these
variables consistent.

Regards,
Naush


>
> Thanks
>   j
>
> >
> > -     dropFrameCount_ = std::max({ dropFrameCount_,
> agcConvergenceFrames, awbConvergenceFrames });
> > +     startupCount_ = std::max({ startupCount_, agcConvergenceFrames,
> awbConvergenceFrames });
> >
> >       LOG(IPARPI, Debug) << "Startup frames: " <<
> result->startupFrameCount
> >                          << " Invalid frames: " <<
> result->invalidFrameCount;
> > @@ -443,7 +443,7 @@ void IpaBase::prepareIsp(const PrepareParams &params)
> >
> >       /* Allow a 10% margin on the comparison below. */
> >       Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;
> > -     if (lastRunTimestamp_ && frameCount_ > dropFrameCount_ &&
> > +     if (lastRunTimestamp_ && frameCount_ > startupCount_ &&
> >           delta < controllerMinFrameDuration * 0.9 && !hdrChange) {
> >               /*
> >                * Ensure we merge the previous frame's metadata with the
> current
> > diff --git a/src/ipa/rpi/common/ipa_base.h
> b/src/ipa/rpi/common/ipa_base.h
> > index 1a811beb31f2..a51afc156a8f 100644
> > --- a/src/ipa/rpi/common/ipa_base.h
> > +++ b/src/ipa/rpi/common/ipa_base.h
> > @@ -115,8 +115,8 @@ private:
> >       /* How many frames we should avoid running control algos on. */
> >       unsigned int mistrustCount_;
> >
> > -     /* Number of frames that need to be dropped on startup. */
> > -     unsigned int dropFrameCount_;
> > +     /* Number of frames that need to be marked as dropped on startup.
> */
> > +     unsigned int startupCount_;
> >
> >       /* Frame timestamp for the last run of the controller. */
> >       uint64_t lastRunTimestamp_;
> > --
> > 2.43.0
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20250605/1e25e605/attachment.htm>


More information about the libcamera-devel mailing list