[libcamera-devel,v3,6/8] android: Plumb all AE-related controls

15184 diff mbox series

With the AE-related controls reorganized in libcamera, with separate
value and lever controls for exposure and gain, plumb these through the
HAL layer (in order of appearence in the patch):
- static metadata: available AE modes, AE lock available
- manual template: add AE off (the other templates already have AE on)
- preview template: set exposure time to the minimum in the exposure
  time range
- request metadata: HAL -> libcamera controls conversion
- result metadata: libcamera -> HAL controls conversion
- result metadata: AE state

We add class variables to CameraDevice to save the last set android AE
controls, as that is how android controls function (no need to resubmit
controls if they are unchanged).

We also save libcamera's AE state in the request descriptor, as
otherwise there is insufficient information from libcamera's result
metadata alone to tell if the state is locked or manual (as they are an
internal state in libcamera).

Bug: https://bugs.libcamera.org/show_bug.cgi?id=42
Bug: https://bugs.libcamera.org/show_bug.cgi?id=43
Bug: https://bugs.libcamera.org/show_bug.cgi?id=47
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v3:
- rename AeMode to AutoMode as we'll also use it for awb later
- set the exposure time in the preview template
- a bunch of other big changes (mentioned by Jacopo, and discussed a
  bit) were not done, as more discussion is needed

New in v2
---
 src/android/camera_capabilities.cpp | 71 +++++++++++++++++++---
 src/android/camera_device.cpp       | 93 +++++++++++++++++++++++++++--
 src/android/camera_device.h         | 13 ++++
 src/android/camera_request.h        |  9 +++
 4 files changed, 174 insertions(+), 12 deletions(-)

Hi Paul, Thanks for the patch.

On Tue, Dec 21, 2021 at 12:36 PM Paul Elder <paul.elder@ideasonboard.com> wrote:
>
> With the AE-related controls reorganized in libcamera, with separate
> value and lever controls for exposure and gain, plumb these through the
> HAL layer (in order of appearence in the patch):
> - static metadata: available AE modes, AE lock available
> - manual template: add AE off (the other templates already have AE on)
> - preview template: set exposure time to the minimum in the exposure
>   time range
> - request metadata: HAL -> libcamera controls conversion
> - result metadata: libcamera -> HAL controls conversion
> - result metadata: AE state
>
> We add class variables to CameraDevice to save the last set android AE
> controls, as that is how android controls function (no need to resubmit
> controls if they are unchanged).
>
> We also save libcamera's AE state in the request descriptor, as
> otherwise there is insufficient information from libcamera's result
> metadata alone to tell if the state is locked or manual (as they are an
> internal state in libcamera).
>
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=43
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=47
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>
> ---
> Changes in v3:
> - rename AeMode to AutoMode as we'll also use it for awb later
> - set the exposure time in the preview template
> - a bunch of other big changes (mentioned by Jacopo, and discussed a
>   bit) were not done, as more discussion is needed
>
> New in v2
> ---
>  src/android/camera_capabilities.cpp | 71 +++++++++++++++++++---
>  src/android/camera_device.cpp       | 93 +++++++++++++++++++++++++++--
>  src/android/camera_device.h         | 13 ++++
>  src/android/camera_request.h        |  9 +++
>  4 files changed, 174 insertions(+), 12 deletions(-)
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index a2e42a5c..318ab666 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -922,12 +922,62 @@ int CameraCapabilities::initializeStaticMetadata()
>         staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
>                                   aeAvailableAntiBandingModes);
>
> -       std::vector<uint8_t> aeAvailableModes = {
> -               ANDROID_CONTROL_AE_MODE_ON,
> -       };
> +       std::vector<uint8_t> aeAvailableModes;
> +       /* This will cause problems if one supports only on and the other only off */
> +       bool aeAddedOn = false;
> +       bool aeAddedOff = false;
> +
> +       const auto &analogGainModeInfo = controlsInfo.find(&controls::AnalogueGainMode);
> +       if (analogGainModeInfo != controlsInfo.end()) {
> +               for (const auto &value : analogGainModeInfo->second.values()) {
> +                       switch (value.get<int32_t>()) {
> +                       case controls::AnalogueGainModeAuto:
> +                               if (!aeAddedOn)
> +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);
> +                               aeAddedOn = true;
> +                               break;
> +                       case controls::AnalogueGainModeDisabled:
> +                               if (!aeAddedOff)
> +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);
> +                               aeAddedOff = true;
> +                               break;
> +                       default:
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       const auto &exposureTimeModeInfo = controlsInfo.find(&controls::ExposureTimeMode);
> +       if (exposureTimeModeInfo != controlsInfo.end()) {
> +               for (const auto &value : exposureTimeModeInfo->second.values()) {
> +                       switch (value.get<int32_t>()) {
> +                       case controls::ExposureTimeModeAuto:
> +                               if (!aeAddedOn)
> +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);
> +                               aeAddedOn = true;
> +                               break;
> +                       case controls::ExposureTimeModeDisabled:
> +                               if (!aeAddedOff)
> +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);
> +                               aeAddedOff = true;
> +                               break;
> +                       default:
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       if (aeAvailableModes.empty())
> +               aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);
>         staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
>                                   aeAvailableModes);
>
> +       /* In libcamera, turning off AE is equivalient to locking. */
> +       uint8_t aeLockAvailable = aeAddedOff ? ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE
> +                                            : ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;
> +       staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> +                                 aeLockAvailable);
> +
>         std::vector<int32_t> aeCompensationRange = {
>                 0, 0,
>         };
> @@ -988,10 +1038,6 @@ int CameraCapabilities::initializeStaticMetadata()
>         staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,
>                                   sceneModesOverride);
>
> -       uint8_t aeLockAvailable = ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;
> -       staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> -                                 aeLockAvailable);
> -
>         uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE;
>         staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
>                                   awbLockAvailable);
> @@ -1468,6 +1514,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons
>                 manualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);
>         }
>
> +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> +                                                   ANDROID_CONTROL_AE_MODE_OFF)) {
> +               uint8_t aeMode = ANDROID_CONTROL_AE_MODE_OFF;
> +               manualTemplate->appendEntry(ANDROID_CONTROL_AE_MODE, aeMode);
> +       }
> +
>         return manualTemplate;
>  }
>
> @@ -1542,6 +1594,11 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
>         uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF;
>         requestTemplate->addEntry(ANDROID_CONTROL_AWB_LOCK, awbLock);
>
> +       if (availableRequestKeys_.count(ANDROID_SENSOR_EXPOSURE_TIME)) {
> +               staticMetadata_->getEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, &entry);
> +               requestTemplate->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, entry.data.i64[0]);
> +       }
> +
>         uint8_t flashMode = ANDROID_FLASH_MODE_OFF;
>         requestTemplate->addEntry(ANDROID_FLASH_MODE, flashMode);
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 83825736..3ade44c4 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -246,7 +246,9 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList)
>
>  CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)
>         : id_(id), state_(State::Stopped), camera_(std::move(camera)),
> -         facing_(CAMERA_FACING_FRONT), orientation_(0)
> +         facing_(CAMERA_FACING_FRONT), orientation_(0),
> +         aeOn_(true), aeLocked_(false), lastExposureTime_(0),
> +         lastAnalogueGain_(1.0f), lastDigitalGain_(1.0f)
>  {
>         camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>
> @@ -812,6 +814,59 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>                 controls.set(controls::draft::TestPatternMode, testPatternMode);
>         }
>
> +       /*
> +        * \todo Can we trust that we won't receive values that we didn't
> +        * report supporting?
> +        */
> +       if (settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry))
> +               aeLocked_ = *entry.data.u8 != ANDROID_CONTROL_AE_LOCK_OFF;
> +
> +       if (settings.getEntry(ANDROID_CONTROL_AE_MODE, &entry))
> +               aeOn_ = *entry.data.u8 != ANDROID_CONTROL_AE_MODE_OFF;
> +
> +       Camera3RequestDescriptor::AutoMode aeMode;
> +       if (aeLocked_ && aeOn_)
> +               aeMode = Camera3RequestDescriptor::AutoMode::Lock;
> +       else if (aeLocked_ && !aeOn_)
> +               aeMode = Camera3RequestDescriptor::AutoMode::Manual;
> +       else if (!aeLocked_ && aeOn_)
> +               aeMode = Camera3RequestDescriptor::AutoMode::Auto;
> +       else /* !aeLocked_ && !aeOn_ */
> +               aeMode = Camera3RequestDescriptor::AutoMode::Manual;
> +
aeLocked_ and aeOn_ seem to be only used locally. Could them be local
variables instead?
> +       /* Save this so that we can recover it in the result */
> +       descriptor->aeMode_ = aeMode;
> +
> +       const auto &eInfo = camera_->controls().find(&controls::ExposureTimeMode);
> +       if (eInfo != camera_->controls().end()) {
> +               controls.set(controls::ExposureTimeMode,
> +                            aeMode == Camera3RequestDescriptor::AutoMode::Auto ?
> +                            controls::ExposureTimeModeAuto :
> +                            controls::ExposureTimeModeDisabled);
> +       }
> +
> +       const auto &gInfo = camera_->controls().find(&controls::AnalogueGainMode);
> +       if (gInfo != camera_->controls().end()) {
> +               controls.set(controls::AnalogueGainMode,
> +                            aeMode == Camera3RequestDescriptor::AutoMode::Auto ?
> +                            controls::AnalogueGainModeAuto :
> +                            controls::AnalogueGainModeDisabled);
> +       }
> +
> +       if (settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry)) {
> +               const auto &info = camera_->controls().find(&controls::ExposureTime);
> +               if (info != camera_->controls().end()) {
> +                       lastExposureTime_ = (*entry.data.i64) / 1000;
> +               }
> +       }
> +
> +       /* Trigger libcamera's locked -> manual state change */
> +       if (aeMode == Camera3RequestDescriptor::AutoMode::Manual) {
> +               const auto &info = camera_->controls().find(&controls::ExposureTime);
> +               if (info != camera_->controls().end())
> +                       controls.set(controls::ExposureTime, lastExposureTime_);
> +       }
> +
>         return 0;
>  }
>
> @@ -1336,11 +1391,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>         resultMetadata->addEntry(ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,
>                                  value32);
>
> -       value = ANDROID_CONTROL_AE_LOCK_OFF;
> -       resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, value);
> +       uint8_t aeLock = (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)
> +                      ? ANDROID_CONTROL_AE_LOCK_ON
> +                      : ANDROID_CONTROL_AE_LOCK_OFF;
> +       resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, aeLock);
>
> -       value = ANDROID_CONTROL_AE_MODE_ON;
> -       resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, value);
> +       /* Locked means auto + locked in android */
> +       uint8_t aeMode = (descriptor.aeMode_ != Camera3RequestDescriptor::AutoMode::Manual)
> +                      ? ANDROID_CONTROL_AE_MODE_ON
> +                      : ANDROID_CONTROL_AE_MODE_OFF;
> +       resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);
>
>         if (settings.getEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, &entry))
>                 /*
> @@ -1357,6 +1417,29 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>         resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, value);
>
>         value = ANDROID_CONTROL_AE_STATE_CONVERGED;
> +       if (metadata.contains(controls::AeState)) {
> +               switch (metadata.get(controls::AeState)) {
> +               case controls::AeStateInactive:
> +                       value = ANDROID_CONTROL_AE_STATE_INACTIVE;
> +                       break;
> +               case controls::AeStateSearching:
> +                       value = ANDROID_CONTROL_AE_STATE_SEARCHING;
> +                       break;
> +               case controls::AeStateConverged:
> +                       value = ANDROID_CONTROL_AE_STATE_CONVERGED;
> +                       break;
> +               case controls::AeStateFlashRequired:
> +                       value = ANDROID_CONTROL_AE_STATE_FLASH_REQUIRED;
> +                       break;
> +               case controls::AeStatePrecapture:
> +                       value = ANDROID_CONTROL_AE_STATE_PRECAPTURE;
> +                       break;
> +               default:
> +                       LOG(HAL, Error) << "Invalid AeState, setting converged";
> +               }
> +       }
> +       if (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)
> +               value = ANDROID_CONTROL_AE_STATE_LOCKED;
>         resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, value);
>
>         value = ANDROID_CONTROL_AF_MODE_OFF;
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 64050416..a65f1670 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -126,5 +126,18 @@ private:
>         int facing_;
>         int orientation_;
>
> +       /*
> +        * Normally resubmission of controls would be handled on libcamera's
> +        * side, but these controls are not one-to-one between libcamera and
> +        * android, so we need to save them here
> +        */
> +
> +       /* Track the last-set android AE controls */
> +       bool aeOn_;
> +       bool aeLocked_;
> +       int32_t lastExposureTime_;
> +       float lastAnalogueGain_;
> +       float lastDigitalGain_;
> +
>         CameraMetadata lastSettings_;
>  };
> diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> index 37b6ae32..b2809179 100644
> --- a/src/android/camera_request.h
> +++ b/src/android/camera_request.h
> @@ -28,6 +28,12 @@ class CameraStream;
>  class Camera3RequestDescriptor
>  {
>  public:
> +       enum class AutoMode {
> +               Auto,
> +               Lock,
> +               Manual,
> +       };
> +
>         enum class Status {
>                 Success,
>                 Error,
> @@ -78,6 +84,9 @@ public:
>         bool complete_ = false;
>         Status status_ = Status::Success;
>
> +       /* The libcamera internal AE state for this request */
> +       AutoMode aeMode_ = AutoMode::Auto;
> +
>  private:
>         LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)
>  };
> --
> 2.27.0
>

Hi Hanlin,

Thanks for the review.

On Tue, Dec 21, 2021 at 07:23:07PM +0800, Hanlin Chen wrote:
> Hi Paul, Thanks for the patch.
> 
> On Tue, Dec 21, 2021 at 12:36 PM Paul Elder <paul.elder@ideasonboard.com> wrote:
> >
> > With the AE-related controls reorganized in libcamera, with separate
> > value and lever controls for exposure and gain, plumb these through the
> > HAL layer (in order of appearence in the patch):
> > - static metadata: available AE modes, AE lock available
> > - manual template: add AE off (the other templates already have AE on)
> > - preview template: set exposure time to the minimum in the exposure
> >   time range
> > - request metadata: HAL -> libcamera controls conversion
> > - result metadata: libcamera -> HAL controls conversion
> > - result metadata: AE state
> >
> > We add class variables to CameraDevice to save the last set android AE
> > controls, as that is how android controls function (no need to resubmit
> > controls if they are unchanged).
> >
> > We also save libcamera's AE state in the request descriptor, as
> > otherwise there is insufficient information from libcamera's result
> > metadata alone to tell if the state is locked or manual (as they are an
> > internal state in libcamera).
> >
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > ---
> > Changes in v3:
> > - rename AeMode to AutoMode as we'll also use it for awb later
> > - set the exposure time in the preview template
> > - a bunch of other big changes (mentioned by Jacopo, and discussed a
> >   bit) were not done, as more discussion is needed
> >
> > New in v2
> > ---
> >  src/android/camera_capabilities.cpp | 71 +++++++++++++++++++---
> >  src/android/camera_device.cpp       | 93 +++++++++++++++++++++++++++--
> >  src/android/camera_device.h         | 13 ++++
> >  src/android/camera_request.h        |  9 +++
> >  4 files changed, 174 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index a2e42a5c..318ab666 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -922,12 +922,62 @@ int CameraCapabilities::initializeStaticMetadata()
> >         staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
> >                                   aeAvailableAntiBandingModes);
> >
> > -       std::vector<uint8_t> aeAvailableModes = {
> > -               ANDROID_CONTROL_AE_MODE_ON,
> > -       };
> > +       std::vector<uint8_t> aeAvailableModes;
> > +       /* This will cause problems if one supports only on and the other only off */
> > +       bool aeAddedOn = false;
> > +       bool aeAddedOff = false;
> > +
> > +       const auto &analogGainModeInfo = controlsInfo.find(&controls::AnalogueGainMode);
> > +       if (analogGainModeInfo != controlsInfo.end()) {
> > +               for (const auto &value : analogGainModeInfo->second.values()) {
> > +                       switch (value.get<int32_t>()) {
> > +                       case controls::AnalogueGainModeAuto:
> > +                               if (!aeAddedOn)
> > +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);
> > +                               aeAddedOn = true;
> > +                               break;
> > +                       case controls::AnalogueGainModeDisabled:
> > +                               if (!aeAddedOff)
> > +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);
> > +                               aeAddedOff = true;
> > +                               break;
> > +                       default:
> > +                               break;
> > +                       }
> > +               }
> > +       }
> > +
> > +       const auto &exposureTimeModeInfo = controlsInfo.find(&controls::ExposureTimeMode);
> > +       if (exposureTimeModeInfo != controlsInfo.end()) {
> > +               for (const auto &value : exposureTimeModeInfo->second.values()) {
> > +                       switch (value.get<int32_t>()) {
> > +                       case controls::ExposureTimeModeAuto:
> > +                               if (!aeAddedOn)
> > +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);
> > +                               aeAddedOn = true;
> > +                               break;
> > +                       case controls::ExposureTimeModeDisabled:
> > +                               if (!aeAddedOff)
> > +                                       aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);
> > +                               aeAddedOff = true;
> > +                               break;
> > +                       default:
> > +                               break;
> > +                       }
> > +               }
> > +       }
> > +
> > +       if (aeAvailableModes.empty())
> > +               aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);
> >         staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> >                                   aeAvailableModes);
> >
> > +       /* In libcamera, turning off AE is equivalient to locking. */
> > +       uint8_t aeLockAvailable = aeAddedOff ? ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE
> > +                                            : ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;
> > +       staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > +                                 aeLockAvailable);
> > +
> >         std::vector<int32_t> aeCompensationRange = {
> >                 0, 0,
> >         };
> > @@ -988,10 +1038,6 @@ int CameraCapabilities::initializeStaticMetadata()
> >         staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,
> >                                   sceneModesOverride);
> >
> > -       uint8_t aeLockAvailable = ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;
> > -       staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > -                                 aeLockAvailable);
> > -
> >         uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE;
> >         staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> >                                   awbLockAvailable);
> > @@ -1468,6 +1514,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons
> >                 manualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);
> >         }
> >
> > +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > +                                                   ANDROID_CONTROL_AE_MODE_OFF)) {
> > +               uint8_t aeMode = ANDROID_CONTROL_AE_MODE_OFF;
> > +               manualTemplate->appendEntry(ANDROID_CONTROL_AE_MODE, aeMode);
> > +       }
> > +
> >         return manualTemplate;
> >  }
> >
> > @@ -1542,6 +1594,11 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
> >         uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF;
> >         requestTemplate->addEntry(ANDROID_CONTROL_AWB_LOCK, awbLock);
> >
> > +       if (availableRequestKeys_.count(ANDROID_SENSOR_EXPOSURE_TIME)) {
> > +               staticMetadata_->getEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, &entry);
> > +               requestTemplate->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, entry.data.i64[0]);
> > +       }
> > +
> >         uint8_t flashMode = ANDROID_FLASH_MODE_OFF;
> >         requestTemplate->addEntry(ANDROID_FLASH_MODE, flashMode);
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 83825736..3ade44c4 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -246,7 +246,9 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList)
> >
> >  CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)
> >         : id_(id), state_(State::Stopped), camera_(std::move(camera)),
> > -         facing_(CAMERA_FACING_FRONT), orientation_(0)
> > +         facing_(CAMERA_FACING_FRONT), orientation_(0),
> > +         aeOn_(true), aeLocked_(false), lastExposureTime_(0),
> > +         lastAnalogueGain_(1.0f), lastDigitalGain_(1.0f)
> >  {
> >         camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
> >
> > @@ -812,6 +814,59 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
> >                 controls.set(controls::draft::TestPatternMode, testPatternMode);
> >         }
> >
> > +       /*
> > +        * \todo Can we trust that we won't receive values that we didn't
> > +        * report supporting?
> > +        */
> > +       if (settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry))
> > +               aeLocked_ = *entry.data.u8 != ANDROID_CONTROL_AE_LOCK_OFF;
> > +
> > +       if (settings.getEntry(ANDROID_CONTROL_AE_MODE, &entry))
> > +               aeOn_ = *entry.data.u8 != ANDROID_CONTROL_AE_MODE_OFF;
> > +
> > +       Camera3RequestDescriptor::AutoMode aeMode;
> > +       if (aeLocked_ && aeOn_)
> > +               aeMode = Camera3RequestDescriptor::AutoMode::Lock;
> > +       else if (aeLocked_ && !aeOn_)
> > +               aeMode = Camera3RequestDescriptor::AutoMode::Manual;
> > +       else if (!aeLocked_ && aeOn_)
> > +               aeMode = Camera3RequestDescriptor::AutoMode::Auto;
> > +       else /* !aeLocked_ && !aeOn_ */
> > +               aeMode = Camera3RequestDescriptor::AutoMode::Manual;
> > +
> aeLocked_ and aeOn_ seem to be only used locally. Could them be local
> variables instead?

You're right, I'll do that.

They were leftover from when I moved the flags to the request wrapper :/


Paul

> > +       /* Save this so that we can recover it in the result */
> > +       descriptor->aeMode_ = aeMode;
> > +
> > +       const auto &eInfo = camera_->controls().find(&controls::ExposureTimeMode);
> > +       if (eInfo != camera_->controls().end()) {
> > +               controls.set(controls::ExposureTimeMode,
> > +                            aeMode == Camera3RequestDescriptor::AutoMode::Auto ?
> > +                            controls::ExposureTimeModeAuto :
> > +                            controls::ExposureTimeModeDisabled);
> > +       }
> > +
> > +       const auto &gInfo = camera_->controls().find(&controls::AnalogueGainMode);
> > +       if (gInfo != camera_->controls().end()) {
> > +               controls.set(controls::AnalogueGainMode,
> > +                            aeMode == Camera3RequestDescriptor::AutoMode::Auto ?
> > +                            controls::AnalogueGainModeAuto :
> > +                            controls::AnalogueGainModeDisabled);
> > +       }
> > +
> > +       if (settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry)) {
> > +               const auto &info = camera_->controls().find(&controls::ExposureTime);
> > +               if (info != camera_->controls().end()) {
> > +                       lastExposureTime_ = (*entry.data.i64) / 1000;
> > +               }
> > +       }
> > +
> > +       /* Trigger libcamera's locked -> manual state change */
> > +       if (aeMode == Camera3RequestDescriptor::AutoMode::Manual) {
> > +               const auto &info = camera_->controls().find(&controls::ExposureTime);
> > +               if (info != camera_->controls().end())
> > +                       controls.set(controls::ExposureTime, lastExposureTime_);
> > +       }
> > +
> >         return 0;
> >  }
> >
> > @@ -1336,11 +1391,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> >         resultMetadata->addEntry(ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,
> >                                  value32);
> >
> > -       value = ANDROID_CONTROL_AE_LOCK_OFF;
> > -       resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, value);
> > +       uint8_t aeLock = (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)
> > +                      ? ANDROID_CONTROL_AE_LOCK_ON
> > +                      : ANDROID_CONTROL_AE_LOCK_OFF;
> > +       resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, aeLock);
> >
> > -       value = ANDROID_CONTROL_AE_MODE_ON;
> > -       resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, value);
> > +       /* Locked means auto + locked in android */
> > +       uint8_t aeMode = (descriptor.aeMode_ != Camera3RequestDescriptor::AutoMode::Manual)
> > +                      ? ANDROID_CONTROL_AE_MODE_ON
> > +                      : ANDROID_CONTROL_AE_MODE_OFF;
> > +       resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);
> >
> >         if (settings.getEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, &entry))
> >                 /*
> > @@ -1357,6 +1417,29 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> >         resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, value);
> >
> >         value = ANDROID_CONTROL_AE_STATE_CONVERGED;
> > +       if (metadata.contains(controls::AeState)) {
> > +               switch (metadata.get(controls::AeState)) {
> > +               case controls::AeStateInactive:
> > +                       value = ANDROID_CONTROL_AE_STATE_INACTIVE;
> > +                       break;
> > +               case controls::AeStateSearching:
> > +                       value = ANDROID_CONTROL_AE_STATE_SEARCHING;
> > +                       break;
> > +               case controls::AeStateConverged:
> > +                       value = ANDROID_CONTROL_AE_STATE_CONVERGED;
> > +                       break;
> > +               case controls::AeStateFlashRequired:
> > +                       value = ANDROID_CONTROL_AE_STATE_FLASH_REQUIRED;
> > +                       break;
> > +               case controls::AeStatePrecapture:
> > +                       value = ANDROID_CONTROL_AE_STATE_PRECAPTURE;
> > +                       break;
> > +               default:
> > +                       LOG(HAL, Error) << "Invalid AeState, setting converged";
> > +               }
> > +       }
> > +       if (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)
> > +               value = ANDROID_CONTROL_AE_STATE_LOCKED;
> >         resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, value);
> >
> >         value = ANDROID_CONTROL_AF_MODE_OFF;
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 64050416..a65f1670 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -126,5 +126,18 @@ private:
> >         int facing_;
> >         int orientation_;
> >
> > +       /*
> > +        * Normally resubmission of controls would be handled on libcamera's
> > +        * side, but these controls are not one-to-one between libcamera and
> > +        * android, so we need to save them here
> > +        */
> > +
> > +       /* Track the last-set android AE controls */
> > +       bool aeOn_;
> > +       bool aeLocked_;
> > +       int32_t lastExposureTime_;
> > +       float lastAnalogueGain_;
> > +       float lastDigitalGain_;
> > +
> >         CameraMetadata lastSettings_;
> >  };
> > diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> > index 37b6ae32..b2809179 100644
> > --- a/src/android/camera_request.h
> > +++ b/src/android/camera_request.h
> > @@ -28,6 +28,12 @@ class CameraStream;
> >  class Camera3RequestDescriptor
> >  {
> >  public:
> > +       enum class AutoMode {
> > +               Auto,
> > +               Lock,
> > +               Manual,
> > +       };
> > +
> >         enum class Status {
> >                 Success,
> >                 Error,
> > @@ -78,6 +84,9 @@ public:
> >         bool complete_ = false;
> >         Status status_ = Status::Success;
> >
> > +       /* The libcamera internal AE state for this request */
> > +       AutoMode aeMode_ = AutoMode::Auto;
> > +
> >  private:
> >         LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)
> >  };
> > --
> > 2.27.0
> >