[libcamera-devel] [PATCH v2 2/6] src: ipa: raspberrypi: agc: Add GetConvergenceFrames method to AGC base class

[Naushir Patuck]

Hi David,

Thank you for your patch.

On Mon, 7 Dec 2020 at 18:02, David Plowman <david.plowman at raspberrypi.com>
wrote:

> We add a GetConvergenceFrames method to the AgcAlgorithm class which
> can be called when the AGC is started from scratch. It suggests how
> many frames should be dropped before displaying any (while the AGC
> converges).
>
> The Raspberry Pi specific implementation makes this customisable from
> the tuning file.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/controller/agc_algorithm.hpp |  1 +
>  src/ipa/raspberrypi/controller/rpi/agc.cpp       | 13 +++++++++++++
>  src/ipa/raspberrypi/controller/rpi/agc.hpp       |  2 ++
>  3 files changed, 16 insertions(+)
>
> diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.hpp
> b/src/ipa/raspberrypi/controller/agc_algorithm.hpp
> index b4ea54fb..85fc6084 100644
> --- a/src/ipa/raspberrypi/controller/agc_algorithm.hpp
> +++ b/src/ipa/raspberrypi/controller/agc_algorithm.hpp
> @@ -15,6 +15,7 @@ class AgcAlgorithm : public Algorithm
>  public:
>         AgcAlgorithm(Controller *controller) : Algorithm(controller) {}
>         // An AGC algorithm must provide the following:
> +       virtual unsigned int GetConvergenceFrames(unsigned int
> mistrust_frames) const = 0;
>
        virtual void SetEv(double ev) = 0;
>         virtual void SetFlickerPeriod(double flicker_period) = 0;
>         virtual void SetFixedShutter(double fixed_shutter) = 0; //
> microseconds
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index 9da18c31..787918cc 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -142,6 +142,7 @@ void AgcConfig::Read(boost::property_tree::ptree const
> &params)
>         Y_target.Read(params.get_child("y_target"));
>         speed = params.get<double>("speed", 0.2);
>         startup_frames = params.get<uint16_t>("startup_frames", 10);
> +       convergence_frames = params.get<unsigned
> int>("convergence_frames", 6);
>         fast_reduce_threshold =
>                 params.get<double>("fast_reduce_threshold", 0.4);
>         base_ev = params.get<double>("base_ev", 1.0);
> @@ -206,6 +207,18 @@ void Agc::Resume()
>         fixed_analogue_gain_ = 0;
>  }
>
> +unsigned int Agc::GetConvergenceFrames(unsigned int mistrust_frames) const
> +{
> +       // If shutter and gain have been explicitly set, there is no
> +       // convergence to happen, so no need to drop any frames - return
> zero.
> +       // Otherwise, any frames for which we have been told not to trust
> the
> +       // statistics must be added to our own count.
> +       if (fixed_shutter_ && fixed_analogue_gain_)
> +               return 0;
> +       else
> +               return config_.convergence_frames + mistrust_frames;
> +}
> +
>

Minor nitpick, but why do you pass mistrust_frames into
GetConvergenceFrames?  Could you not leave it out and do the addition of
mistrust frames in the IPA? Either way,

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>


>  void Agc::SetEv(double ev)
>  {
>         ev_ = ev;
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> index 95db1812..7d41608a 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> @@ -52,6 +52,7 @@ struct AgcConfig {
>         Pwl Y_target;
>         double speed;
>         uint16_t startup_frames;
> +       unsigned int convergence_frames;
>         double max_change;
>         double min_change;
>         double fast_reduce_threshold;
> @@ -74,6 +75,7 @@ public:
>         bool IsPaused() const override;
>         void Pause() override;
>         void Resume() override;
> +       unsigned int GetConvergenceFrames(unsigned int mistrust_frames)
> const override;
>         void SetEv(double ev) override;
>         void SetFlickerPeriod(double flicker_period) override;
>         void SetFixedShutter(double fixed_shutter) override; //
> microseconds
> --
> 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/ea4e2c55/attachment-0001.htm>