<div dir="ltr"><div dir="ltr">Hi Paul,<div><br></div><div>Thank you for your patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 18 Jun 2021 at 11:34, 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">AeLocked alone isn't sufficient for reporting the AE state, so replace<br>
it with AeState. Add an AeLock control for instructing the camera to<br>
lock the AE values.<br>
<br>
Update the current users of AeLocked accordingly.<br>
<br>
Signed-off-by: Paul Elder <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>><br>
---<br>
src/ipa/raspberrypi/raspberrypi.cpp | 5 +-<br>
src/ipa/rkisp1/rkisp1.cpp | 13 ++--<br>
src/libcamera/control_ids.yaml | 96 ++++++++++++++++++-----------<br>
3 files changed, 71 insertions(+), 43 deletions(-)<br>
<br>
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
index 1c1e802a..ad0132c0 100644<br>
--- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
+++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
@@ -468,7 +468,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::AeStateLocked :<br>
+ controls::AeStateSearching);<br></blockquote><div><br></div><div>I think, strictly speaking, if agcStatus->locked == true, we should return</div><div>control::AeStateConverged by the definition below. David, do you agree?<br></div><div><br></div><div>Thanks,</div><div>Naush</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">
libcameraMetadata_.set(controls::DigitalGain, agcStatus->digital_gain);<br>
}<br>
<br>
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp<br>
index b47ea324..88a562a8 100644<br>
--- a/src/ipa/rkisp1/rkisp1.cpp<br>
+++ b/src/ipa/rkisp1/rkisp1.cpp<br>
@@ -51,7 +51,7 @@ private:<br>
const rkisp1_stat_buffer *stats);<br>
<br>
void setControls(unsigned int frame);<br>
- void metadataReady(unsigned int frame, unsigned int aeState);<br>
+ void metadataReady(unsigned int frame, int aeState);<br>
<br>
std::map<unsigned int, FrameBuffer> buffers_;<br>
std::map<unsigned int, void *> buffersMemory_;<br>
@@ -227,7 +227,7 @@ void IPARkISP1::updateStatistics(unsigned int frame,<br>
const rkisp1_stat_buffer *stats)<br>
{<br>
const rkisp1_cif_isp_stat *params = &stats->params;<br>
- unsigned int aeState = 0;<br>
+ int aeState = controls::AeStateInactive;<br>
<br>
if (stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP) {<br>
const rkisp1_cif_isp_ae_stat *ae = ¶ms->ae;<br>
@@ -262,7 +262,9 @@ void IPARkISP1::updateStatistics(unsigned int frame,<br>
setControls(frame + 1);<br>
}<br>
<br>
- aeState = fabs(factor - 1.0f) < 0.05f ? 2 : 1;<br>
+ aeState = fabs(factor - 1.0f) < 0.05f ?<br>
+ controls::AeStateConverged :<br>
+ controls::AeStateSearching;<br>
}<br>
<br>
metadataReady(frame, aeState);<br>
@@ -281,12 +283,11 @@ void IPARkISP1::setControls(unsigned int frame)<br>
queueFrameAction.emit(frame, op);<br>
}<br>
<br>
-void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)<br>
+void IPARkISP1::metadataReady(unsigned int frame, int aeState)<br>
{<br>
ControlList ctrls(controls::controls);<br>
<br>
- if (aeState)<br>
- ctrls.set(controls::AeLocked, aeState == 2);<br>
+ ctrls.set(controls::AeState, aeState);<br>
<br>
RkISP1Action op;<br>
op.op = ActionMetadata;<br>
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml<br>
index 9d4638ae..5717bc1f 100644<br>
--- a/src/libcamera/control_ids.yaml<br>
+++ b/src/libcamera/control_ids.yaml<br>
@@ -14,16 +14,70 @@ controls:<br>
<br>
\sa ExposureTime AnalogueGain<br>
<br>
- - AeLocked:<br>
+ - AeLock:<br>
type: bool<br>
description: |<br>
- Report the lock status of a running AE algorithm.<br>
+ Control to lock the AE values.<br>
+ When set to true, the AE algorithm is locked to its latest parameters,<br>
+ and will not change exposure settings until set to false.<br>
+ \sa AeState<br>
<br>
- If the AE algorithm is locked the value shall be set to true, if it's<br>
- converging it shall be set to false. If the AE algorithm is not<br>
- running the control shall not be present in the metadata control list.<br>
+ - AeState:<br>
+ type: int32_t<br>
+ description: |<br>
+ Control to report the current AE algorithm state. Enabling or disabling<br>
+ AE (AeEnable) always resets the AeState to AeStateInactive. The camera<br>
+ device can do several state transitions between two results, if it is<br>
+ allowed by the state transition table. For example, AeStateInactive may<br>
+ never actually be seen in a result.<br>
<br>
- \sa AeEnable<br>
+ The state in the result is the state for this image (in sync with this<br>
+ image). If AE state becomes AeStateConverged, then the image data<br>
+ associated with the result should be good to use.<br>
+<br>
+ The state transitions mentioned below assume that AeEnable is on.<br>
+<br>
+ \sa AeLock<br>
+ enum:<br>
+ - name: AeStateInactive<br>
+ value: 0<br>
+ description: |<br>
+ The AE algorithm is inactive.<br>
+ If the camera initiates an AE scan, the state shall go to Searching.<br>
+ If AeLock is on, the state shall go to Locked.<br>
+ - name: AeStateSearching<br>
+ value: 1<br>
+ description: |<br>
+ The AE algorithm has not converged yet.<br>
+ If the camera finishes an AE scan, the state shall go to Converged.<br>
+ If the camera finishes an AE scan, but flash is required, the state<br>
+ shall go to FlashRequired.<br>
+ If AeLock is on, the state shall go to Locked.<br>
+ - name: AeStateConverged<br>
+ value: 2<br>
+ description: |<br>
+ The AE algorithm has converged.<br>
+ If the camera initiates an AE scan, the state shall go to Searching.<br>
+ If AeLock is on, the state shall go to Locked.<br>
+ - name: AeStateLocked<br>
+ value: 3<br>
+ description: |<br>
+ The AE algorithm is locked.<br>
+ If AeLock is off, the state can go to Searching, Converged, or<br>
+ FlashRequired.<br>
+ - name: AeStateFlashRequired<br>
+ value: 4<br>
+ description: |<br>
+ The AE algorithm would need a flash for good results<br>
+ If the camera initiates an AE scan, the state shall go to Searching.<br>
+ If AeLock is on, the state shall go to Locked.<br>
+ - name: AeStatePrecapture<br>
+ value: 5<br>
+ description: |<br>
+ The AE algorithm has started a pre-capture metering session.<br>
+ After the sequence is finished, the state shall go to Converged if<br>
+ AeLock is off, and Locked if it is on.<br>
+ \sa AePrecaptureTrigger<br>
<br>
# AeMeteringMode needs further attention:<br>
# - Auto-generate max enum value.<br>
@@ -477,36 +531,6 @@ controls:<br>
High quality aberration correction which might reduce the frame<br>
rate.<br>
<br>
- - AeState:<br>
- type: int32_t<br>
- draft: true<br>
- description: |<br>
- Control to report the current AE algorithm state. Currently identical to<br>
- ANDROID_CONTROL_AE_STATE.<br>
-<br>
- Current state of the AE algorithm.<br>
- enum:<br>
- - name: AeStateInactive<br>
- value: 0<br>
- description: The AE algorithm is inactive.<br>
- - name: AeStateSearching<br>
- value: 1<br>
- description: The AE algorithm has not converged yet.<br>
- - name: AeStateConverged<br>
- value: 2<br>
- description: The AE algorithm has converged.<br>
- - name: AeStateLocked<br>
- value: 3<br>
- description: The AE algorithm is locked.<br>
- - name: AeStateFlashRequired<br>
- value: 4<br>
- description: The AE algorithm would need a flash for good results<br>
- - name: AeStatePrecapture<br>
- value: 5<br>
- description: |<br>
- The AE algorithm has started a pre-capture metering session.<br>
- \sa AePrecaptureTrigger<br>
-<br>
- AfState:<br>
type: int32_t<br>
draft: true<br>
-- <br>
2.27.0<br>
<br>
</blockquote></div></div>