<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for the patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 25 Nov 2020 at 11:36, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">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">AE/AGC "disabled" is now implemented by leaving it running but fixing<br>
the exposure/gain to the last values we requested. This behaves better<br>
when, for example, a fixed shutter or gain is then specified - the<br>
other value remains fixed and doesn't start floating again.<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/raspberrypi.cpp | 48 ++++++++++++++++++-----------<br>
1 file changed, 30 insertions(+), 18 deletions(-)<br>
<br>
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
index 9853a343..30516665 100644<br>
--- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
+++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
@@ -67,7 +67,8 @@ public:<br>
IPARPi()<br>
: lastMode_({}), controller_(), controllerInit_(false),<br>
frameCount_(0), checkCount_(0), mistrustCount_(0),<br>
- lsTable_(nullptr)<br>
+ lsTable_(nullptr),<br>
+ lastShutter_(0), lastGain_(0)<br>
{<br>
}<br>
<br>
@@ -145,6 +146,10 @@ private:<br>
/* LS table allocation passed in from the pipeline handler. */<br>
FileDescriptor lsTableHandle_;<br>
void *lsTable_;<br>
+<br>
+ /* For enabling/disabling the AEC/AGC algorithm. */<br>
+ uint32_t lastShutter_;<br>
+ float lastGain_;<br>
};<br>
<br>
int IPARPi::init(const IPASettings &settings)<br>
@@ -493,12 +498,22 @@ 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>
ASSERT(agc);<br>
- if (ctrl.second.get<bool>() == false)<br>
- agc->Pause();<br>
- else<br>
- agc->Resume();<br>
+<br>
+ /*<br>
+ * We always run the AGC even if it's "disabled".<br>
+ * Both exposure and gain are "fixed" to the last<br>
+ * values it produced.<br>
+ */<br>
+ if (ctrl.second.get<bool>() == false) {<br>
+ agc->SetFixedShutter(lastShutter_);<br>
+ agc->SetFixedAnalogueGain(lastGain_);<br>
+ } else {<br>
+ agc->SetFixedShutter(0);<br>
+ agc->SetFixedAnalogueGain(0);<br>
+ }<br></blockquote><div><br></div><div>I wonder if we could do this within the AGC controller itself? So we keep the existing Pause()/Resume() logic in the IPA, and have the AGC do the above block on the appropriate API call. This way the IPA would not need to track the last used exposure/gain values that are already stored within the AGC block?</div><div><br></div><div>The rest of the changes would still be valid with this tweak.</div><div><br></div><div>Regards,</div><div>Naush</div><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">
<br>
libcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());<br>
break;<br>
@@ -510,13 +525,10 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
ASSERT(agc);<br>
<br>
/* This expects units of micro-seconds. */<br>
- agc->SetFixedShutter(ctrl.second.get<int32_t>());<br>
+ int32_t shutter = ctrl.second.get<int32_t>();<br>
+ agc->SetFixedShutter(shutter);<br>
<br>
- /* For the manual values to take effect, AGC must be unpaused. */<br>
- if (agc->IsPaused())<br>
- agc->Resume();<br>
-<br>
- libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());<br>
+ libcameraMetadata_.set(controls::ExposureTime, shutter);<br>
break;<br>
}<br>
<br>
@@ -524,14 +536,11 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(<br>
controller_.GetAlgorithm("agc"));<br>
ASSERT(agc);<br>
- agc->SetFixedAnalogueGain(ctrl.second.get<float>());<br>
<br>
- /* For the manual values to take effect, AGC must be unpaused. */<br>
- if (agc->IsPaused())<br>
- agc->Resume();<br>
+ float gain = ctrl.second.get<float>();<br>
+ agc->SetFixedAnalogueGain(gain);<br>
<br>
- libcameraMetadata_.set(controls::AnalogueGain,<br>
- ctrl.second.get<float>());<br>
+ libcameraMetadata_.set(controls::AnalogueGain, gain);<br>
break;<br>
}<br>
<br>
@@ -861,6 +870,9 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)<br>
<br>
void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)<br>
{<br>
+ lastShutter_ = agcStatus->shutter_time;<br>
+ lastGain_ = agcStatus->analogue_gain;<br>
+<br>
int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);<br>
int32_t exposureLines = helper_->ExposureLines(agcStatus->shutter_time);<br>
<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>