[libcamera-devel] [PATCH v3 6/8] android: Plumb all AE-related controls
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Tue Dec 21 23:10:58 CET 2021
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 at 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 at 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
> >
More information about the libcamera-devel
mailing list