From 35764ed88dcbcbff678e9c6f035e76b8119999fa Mon Sep 17 00:00:00 2001 From: Naushir Patuck Date: Tue, 4 Aug 2026 16:08:04 +0100 Subject: [PATCH 1/2] ipa: rpi: Rename mistrustFramesStartup() to mistrustMetadataStartup() This matches what the CamHelper member function actually advertises. Rename this to avoid any confusion when reading the code. Also reword some of the comments for the same reason. No functional changes in this commit. Signed-off-by: Naushir Patuck --- src/ipa/rpi/cam_helper/cam_helper.cpp | 4 ++-- src/ipa/rpi/cam_helper/cam_helper.h | 4 ++-- src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp | 4 ++-- src/ipa/rpi/common/ipa_base.cpp | 5 ++--- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/ipa/rpi/cam_helper/cam_helper.cpp b/src/ipa/rpi/cam_helper/cam_helper.cpp index dfdcd1678..9afcf4bb5 100644 --- a/src/ipa/rpi/cam_helper/cam_helper.cpp +++ b/src/ipa/rpi/cam_helper/cam_helper.cpp @@ -192,9 +192,9 @@ unsigned int CamHelper::hideFramesModeSwitch() const return 0; } -unsigned int CamHelper::mistrustFramesStartup() const +unsigned int CamHelper::mistrustMetadataStartup() const { - /* Many sensors return a single bad frame on start-up. */ + /* Many sensors return bad metadata on the first frame after start-up. */ return 1; } diff --git a/src/ipa/rpi/cam_helper/cam_helper.h b/src/ipa/rpi/cam_helper/cam_helper.h index bd101895f..3bbc74310 100644 --- a/src/ipa/rpi/cam_helper/cam_helper.h +++ b/src/ipa/rpi/cam_helper/cam_helper.h @@ -56,7 +56,7 @@ namespace RPiController { * HideFramesModeSwitch(): Tell the pipeline handler not to return this * many frames after a mode switch (other than start-up). Some sensors * may produce innvalid frames after a mode switch; others may not. - * MistrustFramesStartup(): At start-up a sensor may return frames for + * mistrustMetadataStartup(): At start-up a sensor may return frames for * which we should not run any control algorithms (for example, metadata * may be invalid). * MistrustFramesModeSwitch(): The number of frames, after a mode switch @@ -96,7 +96,7 @@ class CamHelper virtual double getModeSensitivity(const CameraMode &mode) const; virtual unsigned int hideFramesStartup() const; virtual unsigned int hideFramesModeSwitch() const; - virtual unsigned int mistrustFramesStartup() const; + virtual unsigned int mistrustMetadataStartup() const; virtual unsigned int mistrustFramesModeSwitch() const; virtual unsigned int getMinDebinFactor() const; diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp index 40d6b6d76..f7d0515c8 100644 --- a/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp +++ b/src/ipa/rpi/cam_helper/cam_helper_ov5647.cpp @@ -19,7 +19,7 @@ class CamHelperOv5647 : public CamHelper double gain(uint32_t gainCode) const override; unsigned int hideFramesStartup() const override; unsigned int hideFramesModeSwitch() const override; - unsigned int mistrustFramesStartup() const override; + unsigned int mistrustMetadataStartup() const override; unsigned int mistrustFramesModeSwitch() const override; private: @@ -68,7 +68,7 @@ unsigned int CamHelperOv5647::hideFramesModeSwitch() const return 2; } -unsigned int CamHelperOv5647::mistrustFramesStartup() const +unsigned int CamHelperOv5647::mistrustMetadataStartup() const { /* * First couple of frames are under-exposed and are no good for control diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index f982e0bfd..d79d57695 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -359,12 +359,11 @@ void IpaBase::start(const ControlList &controls, StartResult *result) frameCount_ = 0; if (firstStart_) { invalidCount_ = helper_->hideFramesStartup(); - mistrustCount_ = helper_->mistrustFramesStartup(); - + mistrustCount_ = helper_->mistrustMetadataStartup(); /* * Query the AGC/AWB for how many frames they may take to * converge sufficiently. Where these numbers are non-zero - * we must allow for the frames with bad statistics + * we must allow for the frames with bad metadata * (mistrustCount_) that they won't see. But if zero (i.e. * no convergence necessary), no frames need to be dropped. */ From eaef2088165a6cd3e291f5ebe4a172eb2132c650 Mon Sep 17 00:00:00 2001 From: Naushir Patuck Date: Tue, 4 Aug 2026 16:10:54 +0100 Subject: [PATCH 2/2] ipa: rpi: Gate CamHelper::prepare() by the mistrust count The current logic in the IPA gates the controller/algorithms from running on a the frame if mistrustCount_ > 0. This logic is somwhat incorrect, as the image data is ineed valid, just the embedded data and other bits of metadata are possibly invalid. This commit changes the gating so that the controller/algorithms do run on frames where mistrustCount_ > 0, and CamHelper::prepare() gets gated CamHelper::prepare() is where the embedded data parsing occurs, and if it does not run, we simply use the DelayedControls values provided by the pipeline handler. Fixes: https://github.com/raspberrypi/libcamera/issues/345 Signed-off-by: Naushir Patuck --- src/ipa/rpi/common/ipa_base.cpp | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index d79d57695..9ce67b3f8 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -362,26 +362,17 @@ void IpaBase::start(const ControlList &controls, StartResult *result) mistrustCount_ = helper_->mistrustMetadataStartup(); /* * Query the AGC/AWB for how many frames they may take to - * converge sufficiently. Where these numbers are non-zero - * we must allow for the frames with bad metadata - * (mistrustCount_) that they won't see. But if zero (i.e. - * no convergence necessary), no frames need to be dropped. + * converge sufficiently. */ RPiController::AgcAlgorithm *agc = dynamic_cast( controller_.getAlgorithm("agc")); - if (agc) { + if (agc) agcConvergenceFrames = agc->getConvergenceFrames(); - if (agcConvergenceFrames) - agcConvergenceFrames += mistrustCount_; - } RPiController::AwbAlgorithm *awb = dynamic_cast( controller_.getAlgorithm("awb")); - if (awb) { + if (awb) awbConvergenceFrames = awb->getConvergenceFrames(); - if (awbConvergenceFrames) - awbConvergenceFrames += mistrustCount_; - } } else { invalidCount_ = helper_->hideFramesModeSwitch(); mistrustCount_ = helper_->mistrustFramesModeSwitch(); @@ -478,8 +469,11 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) /* * This may overwrite the DeviceStatus using values from the sensor * metadata, and may also do additional custom processing. + * + * Only call CamHelper::prepare() when we know the metadata can be trusted. */ - helper_->prepare(embeddedBuffer, rpiMetadata); + if (frameCount_ >= mistrustCount_) + helper_->prepare(embeddedBuffer, rpiMetadata); bool delayedRequestControls = false; delayedMetadata.get("ipa.request_controls", delayedRequestControls); @@ -536,7 +530,7 @@ void IpaBase::processStats(const ProcessParams ¶ms) RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext]; Duration offset(0s); - if (processPending_ && frameCount_ >= mistrustCount_) { + if (processPending_) { auto it = buffers_.find(params.buffers.stats); if (it == buffers_.end()) { LOG(IPARPI, Error) << "Could not find stats buffer!";