<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 2 Dec 2020 at 11:53, David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.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">We add a GetDropFrames method to the AgcAlgorithm class which can be<br>
called when the AGC is started from scratch. It suggests how many<br>
frames should be dropped before displaying any (while the AGC<br>
converges).<br>
<br>
The Raspberry Pi specific implementation makes this customisable from<br>
the tuning file.<br>
<br>
Signed-off-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
---<br>
 src/ipa/raspberrypi/controller/agc_algorithm.hpp |  1 +<br>
 src/ipa/raspberrypi/controller/rpi/agc.cpp       | 11 +++++++++++<br>
 src/ipa/raspberrypi/controller/rpi/agc.hpp       |  2 ++<br>
 3 files changed, 14 insertions(+)<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.hpp b/src/ipa/raspberrypi/controller/agc_algorithm.hpp<br>
index b4ea54fb..bfc9743f 100644<br>
--- a/src/ipa/raspberrypi/controller/agc_algorithm.hpp<br>
+++ b/src/ipa/raspberrypi/controller/agc_algorithm.hpp<br>
@@ -15,6 +15,7 @@ class AgcAlgorithm : public Algorithm<br>
 public:<br>
        AgcAlgorithm(Controller *controller) : Algorithm(controller) {}<br>
        // An AGC algorithm must provide the following:<br>
+       virtual unsigned int GetDropFrames() const = 0;<br>
        virtual void SetEv(double ev) = 0;<br>
        virtual void SetFlickerPeriod(double flicker_period) = 0;<br>
        virtual void SetFixedShutter(double fixed_shutter) = 0; // microseconds<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
index 9da18c31..94c02d47 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
@@ -142,6 +142,7 @@ void AgcConfig::Read(boost::property_tree::ptree const &params)<br>
        Y_target.Read(params.get_child("y_target"));<br>
        speed = params.get<double>("speed", 0.2);<br>
        startup_frames = params.get<uint16_t>("startup_frames", 10);<br>
+       drop_frames = params.get<unsigned int>("drop_frames", 6);<br>
        fast_reduce_threshold =<br>
                params.get<double>("fast_reduce_threshold", 0.4);<br>
        base_ev = params.get<double>("base_ev", 1.0);<br>
@@ -206,6 +207,16 @@ void Agc::Resume()<br>
        fixed_analogue_gain_ = 0;<br>
 }<br>
<br>
+unsigned int Agc::GetDropFrames() const<br>
+{<br>
+       // If shutter and gain have been explicitly set, there is no<br>
+       // convergence to happen, so no need to drop any frames.<br>
+       if (fixed_shutter_ && fixed_analogue_gain_)<br>
+               return 0;<br>
+       else<br>
+               return config_.drop_frames;<br>
+}<br>
+<br></blockquote><div><br></div><div>  One little nit on this one.  The method name, 

GetDropFrames, doesn't sit right with me.  Could we rename this to something like, say, GetConvergenceFrames, or something.  I feel this would better convey what this number is.  What do you think?<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> void Agc::SetEv(double ev)<br>
 {<br>
        ev_ = ev;<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
index 95db1812..1de4d505 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
@@ -52,6 +52,7 @@ struct AgcConfig {<br>
        Pwl Y_target;<br>
        double speed;<br>
        uint16_t startup_frames;<br>
+       unsigned int drop_frames;<br></blockquote><div><br></div><div>Similarly, instead of calling it drop_frames, we could call it convergence_frames?</div><div><br></div><div><div>Regards,</div><div>Naush</div></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">
        double max_change;<br>
        double min_change;<br>
        double fast_reduce_threshold;<br>
@@ -74,6 +75,7 @@ public:<br>
        bool IsPaused() const override;<br>
        void Pause() override;<br>
        void Resume() override;<br>
+       unsigned int GetDropFrames() const override;<br>
        void SetEv(double ev) override;<br>
        void SetFlickerPeriod(double flicker_period) override;<br>
        void SetFixedShutter(double fixed_shutter) override; // microseconds<br>
-- <br>
2.20.1<br>
<br>
_______________________________________________<br>
libcamera-devel mailing list<br>
<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
<a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>