<div dir="ltr"><div dir="ltr">Hi Paul,<div><br></div><div>Thank you for your work.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 1 Oct 2021 at 11:33, Paul Elder <<a href="mailto:paul.elder@ideasonboard.com">paul.elder@ideasonboard.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">Add support for the new AE controls in the raspberrypi pipeline handler,<br>
and in the IPA.<br>
<br>
Bug: <a href="https://bugs.libcamera.org/show_bug.cgi?id=42" rel="noreferrer" target="_blank">https://bugs.libcamera.org/show_bug.cgi?id=42</a><br>
Bug: <a href="https://bugs.libcamera.org/show_bug.cgi?id=43" rel="noreferrer" target="_blank">https://bugs.libcamera.org/show_bug.cgi?id=43</a><br>
Signed-off-by: Paul Elder <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>><br>
<br>
---<br>
Changes in v2:<br>
- fix the rebase error where some uvc stuff was in rasberrypi<br>
- i haven't yet taken in the comments to move the new Pause/Resume<br>
functions<br>
<br>
Initial versoin:<br>
This is very hacky. I wasn't sure what the best way was to plumb it into<br>
the raspberrypi IPA as it was a bit hairy...<br>
---<br>
include/libcamera/ipa/raspberrypi.h | 3 +-<br>
src/ipa/raspberrypi/controller/rpi/agc.cpp | 18 +++++++++-<br>
src/ipa/raspberrypi/controller/rpi/agc.hpp | 5 +++<br>
src/ipa/raspberrypi/raspberrypi.cpp | 42 +++++++++++++++++-----<br>
4 files changed, 58 insertions(+), 10 deletions(-)<br>
<br>
diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h<br>
index 521eaecd..363ea038 100644<br>
--- a/include/libcamera/ipa/raspberrypi.h<br>
+++ b/include/libcamera/ipa/raspberrypi.h<br>
@@ -28,8 +28,9 @@ namespace RPi {<br>
* unsupported control is encountered.<br>
*/<br>
static const ControlInfoMap Controls({<br>
- { &controls::AeEnable, ControlInfo(false, true) },<br>
+ { &controls::ExposureTimeMode, ControlInfo(controls::ExposureTimeModeValues) },<br>
{ &controls::ExposureTime, ControlInfo(0, 999999) },<br>
+ { &controls::AnalogueGainMode, ControlInfo(controls::AnalogueGainModeValues) },<br>
{ &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },<br>
{ &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },<br>
{ &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
index 289c1fce..b45ea454 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
@@ -203,14 +203,30 @@ bool Agc::IsPaused() const<br>
}<br>
<br>
void Agc::Pause()<br>
+{<br>
+}<br>
+<br>
+void Agc::Resume()<br>
+{<br>
+}<br>
+<br>
+void Agc::PauseExposure()<br></blockquote><div><br></div><div>One small suggestion, could this be called PauseShutter() instead? Helps clarify what part</div><div>of the exposure is paused.</div><div><br></div><div>Thanks,</div><div>Naush</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">
{<br>
fixed_shutter_ = status_.shutter_time;<br>
+}<br>
+<br>
+void Agc::PauseGain()<br>
+{<br>
fixed_analogue_gain_ = status_.analogue_gain;<br>
}<br>
<br>
-void Agc::Resume()<br>
+void Agc::ResumeExposure()<br>
{<br>
fixed_shutter_ = 0s;<br>
+}<br>
+<br>
+void Agc::ResumeGain()<br>
+{<br>
fixed_analogue_gain_ = 0;<br>
}<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
index 82063636..7ca3ca2f 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
@@ -92,6 +92,11 @@ public:<br>
void Prepare(Metadata *image_metadata) override;<br>
void Process(StatisticsPtr &stats, Metadata *image_metadata) override;<br>
<br>
+ void PauseExposure();<br>
+ void PauseGain();<br>
+ void ResumeExposure();<br>
+ void ResumeGain();<br>
+<br>
private:<br>
void updateLockStatus(DeviceStatus const &device_status);<br>
AgcConfig config_;<br>
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
index 047123ab..99935515 100644<br>
--- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
+++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
@@ -53,6 +53,8 @@<br>
#include "sharpen_algorithm.hpp"<br>
#include "sharpen_status.h"<br>
<br>
+#include "controller/rpi/agc.hpp"<br>
+<br>
namespace libcamera {<br>
<br>
using namespace std::literals::chrono_literals;<br>
@@ -478,7 +480,10 @@ void IPARPi::reportMetadata()<br>
<br>
AgcStatus *agcStatus = rpiMetadata_.GetLocked<AgcStatus>("agc.status");<br>
if (agcStatus) {<br>
- libcameraMetadata_.set(controls::AeLocked, agcStatus->locked);<br>
+ libcameraMetadata_.set(controls::AeState,<br>
+ agcStatus->locked ?<br>
+ controls::AeStateConverged :<br>
+ controls::AeStateSearching);<br>
libcameraMetadata_.set(controls::DigitalGain, agcStatus->digital_gain);<br>
}<br>
<br>
@@ -623,20 +628,22 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
<< " = " << ctrl.second.toString();<br>
<br>
switch (ctrl.first) {<br>
- case controls::AE_ENABLE: {<br>
- RPiController::Algorithm *agc = controller_.GetAlgorithm("agc");<br>
+ case controls::EXPOSURE_TIME_MODE: {<br>
+ RPiController::Algorithm *algo = controller_.GetAlgorithm("agc");<br>
+ RPiController::Agc *agc = reinterpret_cast<RPiController::Agc *>(algo);<br>
if (!agc) {<br>
LOG(IPARPI, Warning)<br>
- << "Could not set AE_ENABLE - no AGC algorithm";<br>
+ << "Could not set EXPOSURE_TIME_MODE - no AGC algorithm";<br>
break;<br>
}<br>
<br>
- if (ctrl.second.get<bool>() == false)<br>
- agc->Pause();<br>
+ if (ctrl.second.get<int32_t>() == controls::ExposureTimeModeDisabled)<br>
+ agc->PauseExposure();<br>
else<br>
- agc->Resume();<br>
+ agc->ResumeExposure();<br>
<br>
- libcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());<br>
+ libcameraMetadata_.set(controls::ExposureTimeMode,<br>
+ ctrl.second.get<int32_t>());<br>
break;<br>
}<br>
<br>
@@ -656,6 +663,25 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
break;<br>
}<br>
<br>
+ case controls::ANALOGUE_GAIN_MODE: {<br>
+ RPiController::Algorithm *algo = controller_.GetAlgorithm("agc");<br>
+ RPiController::Agc *agc = reinterpret_cast<RPiController::Agc *>(algo);<br>
+ if (!agc) {<br>
+ LOG(IPARPI, Warning)<br>
+ << "Could not set ANALOGUE_GAIN_MODE - no AGC algorithm";<br>
+ break;<br>
+ }<br>
+<br>
+ if (ctrl.second.get<int32_t>() == controls::AnalogueGainModeDisabled)<br>
+ agc->PauseGain();<br>
+ else<br>
+ agc->ResumeGain();<br>
+<br>
+ libcameraMetadata_.set(controls::AnalogueGainMode,<br>
+ ctrl.second.get<int32_t>());<br>
+ break;<br>
+ }<br>
+<br>
case controls::ANALOGUE_GAIN: {<br>
RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(<br>
controller_.GetAlgorithm("agc"));<br>
-- <br>
2.27.0<br>
<br>
</blockquote></div></div>