| Message ID | 20260805100502.1947139-2-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Naushir Patuck (2026-08-05 11:03:53) > 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 <naush@raspberrypi.com> > --- > 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 dfdcd167839a..9afcf4bb52cf 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 bd101895f23e..3bbc74310670 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 I note that the case of the other documentation differs here, but that's perhaps a separate patch, as this one now matches the implementation. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > * 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 @@ public: > 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 40d6b6d76452..f7d0515c8113 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 @@ public: > 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 dacafa578c98..74517503ebc2 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -351,12 +351,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. > */ > -- > 2.53.0 >
On Wed, 5 Aug 2026 at 13:15, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > Quoting Naushir Patuck (2026-08-05 11:03:53) > > 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 <naush@raspberrypi.com> > > --- > > 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 dfdcd167839a..9afcf4bb52cf 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 bd101895f23e..3bbc74310670 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 > > I note that the case of the other documentation differs here, but that's > perhaps a separate patch, as this one now matches the implementation. > Oops, that's a find/replace typo on my part. I'll fix this for v2. Regards, Naush > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > * 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 @@ public: > > 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 40d6b6d76452..f7d0515c8113 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 @@ public: > > 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 dacafa578c98..74517503ebc2 100644 > > --- a/src/ipa/rpi/common/ipa_base.cpp > > +++ b/src/ipa/rpi/common/ipa_base.cpp > > @@ -351,12 +351,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. > > */ > > -- > > 2.53.0 > > >
On Wed, 5 Aug 2026 at 13:17, Naushir Patuck <naush@raspberrypi.com> wrote: > > > > On Wed, 5 Aug 2026 at 13:15, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: >> >> Quoting Naushir Patuck (2026-08-05 11:03:53) >> > 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 <naush@raspberrypi.com> >> > --- >> > 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 dfdcd167839a..9afcf4bb52cf 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 bd101895f23e..3bbc74310670 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 >> >> I note that the case of the other documentation differs here, but that's >> perhaps a separate patch, as this one now matches the implementation. > > > Oops, that's a find/replace typo on my part. I'll fix this for v2. Sorry, let me withdraw that statement. I think this was wrong, but now correct! > > > Regards, > Naush > >> >> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >> > * 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 @@ public: >> > 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 40d6b6d76452..f7d0515c8113 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 @@ public: >> > 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 dacafa578c98..74517503ebc2 100644 >> > --- a/src/ipa/rpi/common/ipa_base.cpp >> > +++ b/src/ipa/rpi/common/ipa_base.cpp >> > @@ -351,12 +351,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. >> > */ >> > -- >> > 2.53.0 >> >
Hi Naush Thanks for the patch. On Wed, 5 Aug 2026 at 11:05, Naushir Patuck <naush@raspberrypi.com> wrote: > > 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 <naush@raspberrypi.com> > --- > 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 dfdcd167839a..9afcf4bb52cf 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 bd101895f23e..3bbc74310670 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 I think it was already commented on that the comment needs an update? > * which we should not run any control algorithms (for example, metadata > * may be invalid). > * MistrustFramesModeSwitch(): The number of frames, after a mode switch Do we need to worry about MistrustFramesModeSwitch() too? Happy with it otherwise, I just need persuading on this final point! Thanks David > @@ -96,7 +96,7 @@ public: > 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 40d6b6d76452..f7d0515c8113 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 @@ public: > 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 dacafa578c98..74517503ebc2 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -351,12 +351,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. > */ > -- > 2.53.0 >
Hi David, On Wed, 5 Aug 2026 at 14:22, David Plowman <david.plowman@raspberrypi.com> wrote: > > Hi Naush > > Thanks for the patch. > > On Wed, 5 Aug 2026 at 11:05, Naushir Patuck <naush@raspberrypi.com> wrote: > > > > 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 <naush@raspberrypi.com> > > --- > > 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 dfdcd167839a..9afcf4bb52cf 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 bd101895f23e..3bbc74310670 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 > > I think it was already commented on that the comment needs an update? > > > * which we should not run any control algorithms (for example, metadata > > * may be invalid). > > * MistrustFramesModeSwitch(): The number of frames, after a mode switch > > Do we need to worry about MistrustFramesModeSwitch() too? Yes we do! Only a rename, logic stays the same. I'll change it for v2. > > Happy with it otherwise, I just need persuading on this final point! > > Thanks > > David > > > @@ -96,7 +96,7 @@ public: > > 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 40d6b6d76452..f7d0515c8113 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 @@ public: > > 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 dacafa578c98..74517503ebc2 100644 > > --- a/src/ipa/rpi/common/ipa_base.cpp > > +++ b/src/ipa/rpi/common/ipa_base.cpp > > @@ -351,12 +351,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. > > */ > > -- > > 2.53.0 > >
diff --git a/src/ipa/rpi/cam_helper/cam_helper.cpp b/src/ipa/rpi/cam_helper/cam_helper.cpp index dfdcd167839a..9afcf4bb52cf 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 bd101895f23e..3bbc74310670 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 @@ public: 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 40d6b6d76452..f7d0515c8113 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 @@ public: 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 dacafa578c98..74517503ebc2 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -351,12 +351,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. */
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 <naush@raspberrypi.com> --- 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(-)