<div dir="ltr"><div dir="ltr"><div>Hi Jacopo,</div><div><br></div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Thu, 5 Jun 2025 at 08:30, Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com">jacopo.mondi@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush<br>
<br>
On Thu, May 22, 2025 at 08:48:22AM +0100, Naushir Patuck wrote:<br>
> Rename dropFrameCount_ to startupCount_ to better reflect its use as<br>
> frames are no longer dropped by the pipeline handler.<br>
<br>
Makes sense<br>
<br>
><br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> ---<br>
>  src/ipa/rpi/common/ipa_base.cpp | 10 +++++-----<br>
>  src/ipa/rpi/common/ipa_base.h   |  4 ++--<br>
>  2 files changed, 7 insertions(+), 7 deletions(-)<br>
><br>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp<br>
> index c15f8a7bf71e..8d591faeceaa 100644<br>
> --- a/src/ipa/rpi/common/ipa_base.cpp<br>
> +++ b/src/ipa/rpi/common/ipa_base.cpp<br>
> @@ -327,7 +327,7 @@ void IpaBase::start(const ControlList &controls, StartResult *result)<br>
>       unsigned int agcConvergenceFrames = 0, awbConvergenceFrames = 0;<br>
>       frameCount_ = 0;<br>
>       if (firstStart_) {<br>
> -             dropFrameCount_ = helper_->hideFramesStartup();<br>
> +             startupCount_ = helper_->hideFramesStartup();<br>
>               mistrustCount_ = helper_->mistrustFramesStartup();<br>
><br>
>               /*<br>
> @@ -353,14 +353,14 @@ void IpaBase::start(const ControlList &controls, StartResult *result)<br>
>                               awbConvergenceFrames += mistrustCount_;<br>
>               }<br>
>       } else {<br>
> -             dropFrameCount_ = helper_->hideFramesModeSwitch();<br>
> +             startupCount_ = helper_->hideFramesModeSwitch();<br>
>               mistrustCount_ = helper_->mistrustFramesModeSwitch();<br>
>       }<br>
><br>
>       result->startupFrameCount = std::max({ agcConvergenceFrames, awbConvergenceFrames });<br>
> -     result->invalidFrameCount = dropFrameCount_;<br>
> +     result->invalidFrameCount = startupCount_;<br>
<br>
But here it makese<br>
<br>
        result->startupFrameCount = .. convergence frames;<br>
        result->invalidFrameCount = startupCount_;<br>
<br>
It's your IPA, so if it's fine with you, fine with me!<br>
<br>
Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com" target="_blank">jacopo.mondi@ideasonboard.com</a>><br></blockquote><div><br></div><div>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.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks<br>
  j<br>
<br>
><br>
> -     dropFrameCount_ = std::max({ dropFrameCount_, agcConvergenceFrames, awbConvergenceFrames });<br>
> +     startupCount_ = std::max({ startupCount_, agcConvergenceFrames, awbConvergenceFrames });<br>
><br>
>       LOG(IPARPI, Debug) << "Startup frames: " << result->startupFrameCount<br>
>                          << " Invalid frames: " << result->invalidFrameCount;<br>
> @@ -443,7 +443,7 @@ void IpaBase::prepareIsp(const PrepareParams &params)<br>
><br>
>       /* Allow a 10% margin on the comparison below. */<br>
>       Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;<br>
> -     if (lastRunTimestamp_ && frameCount_ > dropFrameCount_ &&<br>
> +     if (lastRunTimestamp_ && frameCount_ > startupCount_ &&<br>
>           delta < controllerMinFrameDuration * 0.9 && !hdrChange) {<br>
>               /*<br>
>                * Ensure we merge the previous frame's metadata with the current<br>
> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h<br>
> index 1a811beb31f2..a51afc156a8f 100644<br>
> --- a/src/ipa/rpi/common/ipa_base.h<br>
> +++ b/src/ipa/rpi/common/ipa_base.h<br>
> @@ -115,8 +115,8 @@ private:<br>
>       /* How many frames we should avoid running control algos on. */<br>
>       unsigned int mistrustCount_;<br>
><br>
> -     /* Number of frames that need to be dropped on startup. */<br>
> -     unsigned int dropFrameCount_;<br>
> +     /* Number of frames that need to be marked as dropped on startup. */<br>
> +     unsigned int startupCount_;<br>
><br>
>       /* Frame timestamp for the last run of the controller. */<br>
>       uint64_t lastRunTimestamp_;<br>
> --<br>
> 2.43.0<br>
><br>
</blockquote></div></div>