<div dir="ltr"><div dir="ltr">Hi Nick,<div><br></div><div>Thank you for your patch.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 21 Nov 2022 at 14:47, Nick Hollinghurst via libcamera-devel <<a href="mailto:libcamera-devel@lists.libcamera.org">libcamera-devel@lists.libcamera.org</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">No existing Algorithm used the base pause(), resume() functions<br>
or the paused_ flag, nor is there a need or a generic pause API.<br>
Remove these. The AGC and AWB algorithms now have methods named<br>
disableAuto(), enableAuto() which better describe their functionality.<br>
<br>
Signed-off-by: Nick Hollinghurst <<a href="mailto:nick.hollinghurst@raspberrypi.com" target="_blank">nick.hollinghurst@raspberrypi.com</a>><br></blockquote><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></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">
---<br>
src/ipa/raspberrypi/controller/agc_algorithm.h | 2 ++<br>
src/ipa/raspberrypi/controller/algorithm.h | 6 +-----<br>
src/ipa/raspberrypi/controller/awb_algorithm.h | 2 ++<br>
src/ipa/raspberrypi/controller/controller.cpp | 6 ++----<br>
src/ipa/raspberrypi/controller/rpi/agc.cpp | 13 ++++---------<br>
src/ipa/raspberrypi/controller/rpi/agc.h | 6 ++----<br>
src/ipa/raspberrypi/controller/rpi/awb.cpp | 11 +++--------<br>
src/ipa/raspberrypi/controller/rpi/awb.h | 6 ++----<br>
src/ipa/raspberrypi/raspberrypi.cpp | 14 ++++++++------<br>
9 files changed, 26 insertions(+), 40 deletions(-)<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.h b/src/ipa/raspberrypi/controller/agc_algorithm.h<br>
index 3a91444c..36e6c110 100644<br>
--- a/src/ipa/raspberrypi/controller/agc_algorithm.h<br>
+++ b/src/ipa/raspberrypi/controller/agc_algorithm.h<br>
@@ -26,6 +26,8 @@ public:<br>
virtual void setMeteringMode(std::string const &meteringModeName) = 0;<br>
virtual void setExposureMode(std::string const &exposureModeName) = 0;<br>
virtual void setConstraintMode(std::string const &contraintModeName) = 0;<br>
+ virtual void enableAuto() = 0;<br>
+ virtual void disableAuto() = 0;<br>
};<br>
<br>
} /* namespace RPiController */<br>
diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h<br>
index cbbb13ba..4f327598 100644<br>
--- a/src/ipa/raspberrypi/controller/algorithm.h<br>
+++ b/src/ipa/raspberrypi/controller/algorithm.h<br>
@@ -27,14 +27,11 @@ class Algorithm<br>
{<br>
public:<br>
Algorithm(Controller *controller)<br>
- : controller_(controller), paused_(false)<br>
+ : controller_(controller)<br>
{<br>
}<br>
virtual ~Algorithm() = default;<br>
virtual char const *name() const = 0;<br>
- virtual bool isPaused() const { return paused_; }<br>
- virtual void pause() { paused_ = true; }<br>
- virtual void resume() { paused_ = false; }<br>
virtual int read(const libcamera::YamlObject ¶ms);<br>
virtual void initialise();<br>
virtual void switchMode(CameraMode const &cameraMode, Metadata *metadata);<br>
@@ -47,7 +44,6 @@ public:<br>
<br>
private:<br>
Controller *controller_;<br>
- bool paused_;<br>
};<br>
<br>
/*<br>
diff --git a/src/ipa/raspberrypi/controller/awb_algorithm.h b/src/ipa/raspberrypi/controller/awb_algorithm.h<br>
index 48e08b60..8462c4db 100644<br>
--- a/src/ipa/raspberrypi/controller/awb_algorithm.h<br>
+++ b/src/ipa/raspberrypi/controller/awb_algorithm.h<br>
@@ -18,6 +18,8 @@ public:<br>
virtual unsigned int getConvergenceFrames() const = 0;<br>
virtual void setMode(std::string const &modeName) = 0;<br>
virtual void setManualGains(double manualR, double manualB) = 0;<br>
+ virtual void enableAuto() = 0;<br>
+ virtual void disableAuto() = 0;<br>
};<br>
<br>
} /* namespace RPiController */<br>
diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp<br>
index f4892786..e9156785 100644<br>
--- a/src/ipa/raspberrypi/controller/controller.cpp<br>
+++ b/src/ipa/raspberrypi/controller/controller.cpp<br>
@@ -108,16 +108,14 @@ void Controller::prepare(Metadata *imageMetadata)<br>
{<br>
assert(switchModeCalled_);<br>
for (auto &algo : algorithms_)<br>
- if (!algo->isPaused())<br>
- algo->prepare(imageMetadata);<br>
+ algo->prepare(imageMetadata);<br>
}<br>
<br>
void Controller::process(StatisticsPtr stats, Metadata *imageMetadata)<br>
{<br>
assert(switchModeCalled_);<br>
for (auto &algo : algorithms_)<br>
- if (!algo->isPaused())<br>
- algo->process(stats, imageMetadata);<br>
+ algo->process(stats, imageMetadata);<br>
}<br>
<br>
Metadata &Controller::getGlobalMetadata()<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
index bd54a639..a30e50c1 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
@@ -270,18 +270,13 @@ int Agc::read(const libcamera::YamlObject ¶ms)<br>
return 0;<br>
}<br>
<br>
-bool Agc::isPaused() const<br>
-{<br>
- return false;<br>
-}<br>
-<br>
-void Agc::pause()<br>
+void Agc::disableAuto()<br>
{<br>
fixedShutter_ = status_.shutterTime;<br>
fixedAnalogueGain_ = status_.analogueGain;<br>
}<br>
<br>
-void Agc::resume()<br>
+void Agc::enableAuto()<br>
{<br>
fixedShutter_ = 0s;<br>
fixedAnalogueGain_ = 0;<br>
@@ -317,14 +312,14 @@ void Agc::setMaxShutter(Duration maxShutter)<br>
void Agc::setFixedShutter(Duration fixedShutter)<br>
{<br>
fixedShutter_ = fixedShutter;<br>
- /* Set this in case someone calls Pause() straight after. */<br>
+ /* Set this in case someone calls disableAuto() straight after. */<br>
status_.shutterTime = clipShutter(fixedShutter_);<br>
}<br>
<br>
void Agc::setFixedAnalogueGain(double fixedAnalogueGain)<br>
{<br>
fixedAnalogueGain_ = fixedAnalogueGain;<br>
- /* Set this in case someone calls Pause() straight after. */<br>
+ /* Set this in case someone calls disableAuto() straight after. */<br>
status_.analogueGain = fixedAnalogueGain;<br>
}<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h<br>
index 6d6b0e5a..cf04da19 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.h<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.h<br>
@@ -75,10 +75,6 @@ public:<br>
Agc(Controller *controller);<br>
char const *name() const override;<br>
int read(const libcamera::YamlObject ¶ms) override;<br>
- /* AGC handles "pausing" for itself. */<br>
- bool isPaused() const override;<br>
- void pause() override;<br>
- void resume() override;<br>
unsigned int getConvergenceFrames() const override;<br>
void setEv(double ev) override;<br>
void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;<br>
@@ -88,6 +84,8 @@ public:<br>
void setMeteringMode(std::string const &meteringModeName) override;<br>
void setExposureMode(std::string const &exposureModeName) override;<br>
void setConstraintMode(std::string const &contraintModeName) override;<br>
+ void enableAuto() override;<br>
+ void disableAuto() override;<br>
void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;<br>
void prepare(Metadata *imageMetadata) override;<br>
void process(StatisticsPtr &stats, Metadata *imageMetadata) override;<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
index 8d8ddf09..4f6af4ba 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
@@ -222,21 +222,16 @@ void Awb::initialise()<br>
asyncResults_ = syncResults_;<br>
}<br>
<br>
-bool Awb::isPaused() const<br>
+void Awb::disableAuto()<br>
{<br>
- return false;<br>
-}<br>
-<br>
-void Awb::pause()<br>
-{<br>
- /* "Pause" by fixing everything to the most recent values. */<br>
+ /* Freeze the most recent values, and treat them as manual gains */<br>
manualR_ = syncResults_.gainR = prevSyncResults_.gainR;<br>
manualB_ = syncResults_.gainB = prevSyncResults_.gainB;<br>
syncResults_.gainG = prevSyncResults_.gainG;<br>
syncResults_.temperatureK = prevSyncResults_.temperatureK;<br>
}<br>
<br>
-void Awb::resume()<br>
+void Awb::enableAuto()<br>
{<br>
manualR_ = 0.0;<br>
manualB_ = 0.0;<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h b/src/ipa/raspberrypi/controller/rpi/awb.h<br>
index 30acd89d..2254c3ed 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/awb.h<br>
+++ b/src/ipa/raspberrypi/controller/rpi/awb.h<br>
@@ -93,13 +93,11 @@ public:<br>
char const *name() const override;<br>
void initialise() override;<br>
int read(const libcamera::YamlObject ¶ms) override;<br>
- /* AWB handles "pausing" for itself. */<br>
- bool isPaused() const override;<br>
- void pause() override;<br>
- void resume() override;<br>
unsigned int getConvergenceFrames() const override;<br>
void setMode(std::string const &name) override;<br>
void setManualGains(double manualR, double manualB) override;<br>
+ void enableAuto() override;<br>
+ void disableAuto() override;<br>
void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;<br>
void prepare(Metadata *imageMetadata) override;<br>
void process(StatisticsPtr &stats, Metadata *imageMetadata) override;<br>
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
index b74f1ecf..beb076dc 100644<br>
--- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
+++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
@@ -706,7 +706,8 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
<br>
switch (ctrl.first) {<br>
case controls::AE_ENABLE: {<br>
- RPiController::Algorithm *agc = controller_.getAlgorithm("agc");<br>
+ RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(<br>
+ controller_.getAlgorithm("agc"));<br>
if (!agc) {<br>
LOG(IPARPI, Warning)<br>
<< "Could not set AE_ENABLE - no AGC algorithm";<br>
@@ -714,9 +715,9 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
}<br>
<br>
if (ctrl.second.get<bool>() == false)<br>
- agc->pause();<br>
+ agc->disableAuto();<br>
else<br>
- agc->resume();<br>
+ agc->enableAuto();<br>
<br>
libcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());<br>
break;<br>
@@ -835,7 +836,8 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
}<br>
<br>
case controls::AWB_ENABLE: {<br>
- RPiController::Algorithm *awb = controller_.getAlgorithm("awb");<br>
+ RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(<br>
+ controller_.getAlgorithm("awb"));<br>
if (!awb) {<br>
LOG(IPARPI, Warning)<br>
<< "Could not set AWB_ENABLE - no AWB algorithm";<br>
@@ -843,9 +845,9 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
}<br>
<br>
if (ctrl.second.get<bool>() == false)<br>
- awb->pause();<br>
+ awb->disableAuto();<br>
else<br>
- awb->resume();<br>
+ awb->enableAuto();<br>
<br>
libcameraMetadata_.set(controls::AwbEnable,<br>
ctrl.second.get<bool>());<br>
-- <br>
2.20.1<br>
<br>
</blockquote></div></div>