<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" target="_blank">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">Previously it was possible to have AwbEnable set to false, yet have<br>
AwbMode on anything. This caused a confusion situation, so merge the two<br>
into AwbMode. While at it, pull in the android AWB modes.<br>
<br>
Adjust the previous users of AwbEnable accordingly.<br>
<br>
Signed-off-by: Paul Elder <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>><br>
---<br>
 include/libcamera/ipa/raspberrypi.h |  1 -<br>
 src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++----------------<br>
 src/libcamera/control_ids.yaml      | 32 +++++++++++++++--------------<br>
 test/controls/control_list.cpp      |  6 +++---<br>
 4 files changed, 29 insertions(+), 37 deletions(-)<br>
<br>
diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h<br>
index a8790000..63392a26 100644<br>
--- a/include/libcamera/ipa/raspberrypi.h<br>
+++ b/include/libcamera/ipa/raspberrypi.h<br>
@@ -35,7 +35,6 @@ static const ControlInfoMap Controls = {<br>
        { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },<br>
        { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },<br>
        { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },<br>
-       { &controls::AwbEnable, ControlInfo(false, true) },<br>
        { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },<br>
        { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },<br>
        { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },<br>
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
index ad0132c0..ed5f1250 100644<br>
--- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
+++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
@@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
                        break;<br>
                }<br>
<br>
-               case controls::AWB_ENABLE: {<br>
-                       RPiController::Algorithm *awb = controller_.GetAlgorithm("awb");<br>
-                       if (!awb) {<br>
-                               LOG(IPARPI, Warning)<br>
-                                       << "Could not set AWB_ENABLE - no AWB algorithm";<br>
-                               break;<br>
-                       }<br>
-<br>
-                       if (ctrl.second.get<bool>() == false)<br>
-                               awb->Pause();<br>
-                       else<br>
-                               awb->Resume();<br>
-<br>
-                       libcameraMetadata_.set(controls::AwbEnable,<br>
-                                              ctrl.second.get<bool>());<br>
-                       break;<br>
-               }<br>
-<br>
                case controls::AWB_MODE: {<br>
                        RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(<br>
                                controller_.GetAlgorithm("awb"));<br>
@@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
                        }<br>
<br>
                        int32_t idx = ctrl.second.get<int32_t>();<br>
+<br>
+                       if (idx == controls::AwbOff) {<br>
+                               awb->Pause();<br>
+                               break;<br>
+                       }<br>
+<br>
+                       if (idx == controls::AwbAuto)<br>
+                               awb->Resume();<br></blockquote><div><br></div><div>I think the logic here may need adjusting such that any state that is not</div><div>controls::AwbOff will need to call awb->Resume(), or conditionally call</div><div>resume if adb->IsPaused() returns true.</div><div><br></div><div>Thanks,</div><div>Naush</div><div><br></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">
                        if (AwbModeTable.count(idx)) {<br>
                                awb->SetMode(AwbModeTable.at(idx));<br>
                                libcameraMetadata_.set(controls::AwbMode, idx);<br>
diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml<br>
index 5717bc1f..2e62f61b 100644<br>
--- a/src/libcamera/control_ids.yaml<br>
+++ b/src/libcamera/control_ids.yaml<br>
@@ -229,13 +229,6 @@ controls:<br>
         Report an estimate of the current illuminance level in lux. The Lux<br>
         control can only be returned in metadata.<br>
<br>
-  - AwbEnable:<br>
-      type: bool<br>
-      description: |<br>
-        Enable or disable the AWB.<br>
-<br>
-        \sa ColourGains<br>
-<br>
   # AwbMode needs further attention:<br>
   # - Auto-generate max enum value.<br>
   # - Better handling of custom types.<br>
@@ -245,29 +238,38 @@ controls:<br>
         Specify the range of illuminants to use for the AWB algorithm. The modes<br>
         supported are platform specific, and not all modes may be supported.<br>
       enum:<br>
-        - name: AwbAuto<br>
+        - name: AwbOff<br>
           value: 0<br>
+          description: The AWB routune is disabled.<br>
+        - name: AwbAuto<br>
+          value: 1<br>
           description: Search over the whole colour temperature range.<br>
         - name: AwbIncandescent<br>
-          value: 1<br>
-          description: Incandescent AWB lamp mode.<br>
-        - name: AwbTungsten<br>
           value: 2<br>
-          description: Tungsten AWB lamp mode.<br>
+          description: Incandescent AWB lamp mode.<br>
         - name: AwbFluorescent<br>
           value: 3<br>
           description: Fluorescent AWB lamp mode.<br>
-        - name: AwbIndoor<br>
+        - name: AwbWarmFluorescent<br>
           value: 4<br>
-          description: Indoor AWB lighting mode.<br>
+          description: Warm fluorescent AWB lamp mode.<br>
         - name: AwbDaylight<br>
           value: 5<br>
           description: Daylight AWB lighting mode.<br>
         - name: AwbCloudy<br>
           value: 6<br>
           description: Cloudy AWB lighting mode.<br>
-        - name: AwbCustom<br>
+        - name: AwbTwilight<br>
           value: 7<br>
+          description: Twilight AWB lamp mode.<br>
+        - name: AwbTungsten<br>
+          value: 8<br>
+          description: Tungsten AWB lamp mode.<br>
+        - name: AwbIndoor<br>
+          value: 9<br>
+          description: Indoor AWB lighting mode.<br>
+        - name: AwbCustom<br>
+          value: 10<br>
           description: Custom AWB mode.<br>
<br>
   - AwbLocked:<br>
diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp<br>
index 70cf61b8..ce55d09b 100644<br>
--- a/test/controls/control_list.cpp<br>
+++ b/test/controls/control_list.cpp<br>
@@ -143,10 +143,10 @@ protected:<br>
                 * Attempt to set an invalid control and verify that the<br>
                 * operation failed.<br>
                 */<br>
-               list.set(controls::AwbEnable, true);<br>
+               list.set(controls::AwbMode, true);<br>
<br>
-               if (list.contains(controls::AwbEnable)) {<br>
-                       cout << "List shouldn't contain AwbEnable control" << endl;<br>
+               if (list.contains(controls::AwbMode)) {<br>
+                       cout << "List shouldn't contain AwbMode control" << endl;<br>
                        return TestFail;<br>
                }<br>
<br>
-- <br>
2.27.0<br>
<br>
</blockquote></div></div>