[libcamera-devel] [PATCH v2 6/6] src: ipa: raspberrypi: Move initial frame drop decision to AGC/AWB
Naushir Patuck
naush at raspberrypi.com
Tue Dec 8 11:16:35 CET 2020
Hi David,
Thank you for your patch.
On Mon, 7 Dec 2020 at 18:02, David Plowman <david.plowman at raspberrypi.com>
wrote:
> Previously the CamHelper was returning the number of frames to drop
> (on account of AGC/AWB converging). This wasn't really appropriate,
> it's better for the algorithms to do it as they know how many frames
> they might need.
>
> The CamHelper::HideFramesStartup method should now just be returning
> the number of frames to hide because they're bad/invalid in some way,
> not worrying about the AGC/AWB. For many sensors, the correct value
> for this is zero. But the ov5647 needs updating as it must return 2.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
> src/ipa/raspberrypi/cam_helper.cpp | 6 +++---
> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 10 ++++++++++
> src/ipa/raspberrypi/raspberrypi.cpp | 16 ++++++++++++++++
> 3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp
> b/src/ipa/raspberrypi/cam_helper.cpp
> index c8ac3232..6efa0d7f 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -82,10 +82,10 @@ bool CamHelper::SensorEmbeddedDataPresent() const
> unsigned int CamHelper::HideFramesStartup() const
> {
> /*
> - * By default, hide 6 frames completely at start-up while AGC etc.
> sort
> - * themselves out (converge).
> + * The number of frames when a camera first starts that shouldn't
> be
> + * displayed as they are invalid in some way.
> */
> - return 6;
> + return 0;
> }
>
> unsigned int CamHelper::HideFramesModeSwitch() const
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index dc5d8275..0b841cd1 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> @@ -19,6 +19,7 @@ public:
> uint32_t GainCode(double gain) const override;
> double Gain(uint32_t gain_code) const override;
> void GetDelays(int &exposure_delay, int &gain_delay) const
> override;
> + unsigned int HideFramesStartup() const override;
> unsigned int HideFramesModeSwitch() const override;
> unsigned int MistrustFramesStartup() const override;
> unsigned int MistrustFramesModeSwitch() const override;
> @@ -54,6 +55,15 @@ void CamHelperOv5647::GetDelays(int &exposure_delay,
> int &gain_delay) const
> gain_delay = 2;
> }
>
> +unsigned int CamHelperOv5647::HideFramesStartup() const
> +{
> + /*
> + * On startup, we get a couple of under-exposed frames which
> + * we don't want shown.
> + */
> + return 2;
> +}
> +
> unsigned int CamHelperOv5647::HideFramesModeSwitch() const
> {
> /*
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index 0e89af00..da92e492 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -193,6 +193,22 @@ int IPARPi::start(const IPAOperationData &ipaConfig,
> IPAOperationData *result)
> if (firstStart_) {
> dropFrame = helper_->HideFramesStartup();
> mistrustCount_ = helper_->MistrustFramesStartup();
> +
> + /* The AGC and/or AWB algorithms may want us to drop more
> frames. */
> + unsigned int convergence_frames = 0;
>
Minor nit, but could you use camel case for convergence_frames to match the
rest of the variables?
> + RPiController::AgcAlgorithm *agc =
> dynamic_cast<RPiController::AgcAlgorithm *>(
> + controller_.GetAlgorithm("agc"));
> + if (agc)
> + convergence_frames =
> agc->GetConvergenceFrames(mistrustCount_);
> +
> + RPiController::AwbAlgorithm *awb =
> dynamic_cast<RPiController::AwbAlgorithm *>(
> + controller_.GetAlgorithm("awb"));
> + if (awb)
> + convergence_frames = std::max(convergence_frames,
> +
> awb->GetConvergenceFrames(mistrustCount_));
> +
> + dropFrame = std::max(dropFrame, convergence_frames);
> + LOG(IPARPI, Debug) << "Drop " << dropFrame << " frames on
> startup";
>
Apart from the above nit, and the earlier question of passing
convergence_frames into GetConvergenceFrames(),
Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> } else {
> dropFrame = helper_->HideFramesModeSwitch();
> mistrustCount_ = helper_->MistrustFramesModeSwitch();
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20201208/36c6bdb8/attachment.htm>
More information about the libcamera-devel
mailing list