[{"id":21848,"web_url":"https://patchwork.libcamera.org/comment/21848/","msgid":"","date":"2021-12-21T11:23:07","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: Plumb all AE-related\n\tcontrols","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi Paul, Thanks for the patch.\n\nOn Tue, Dec 21, 2021 at 12:36 PM Paul Elder wrote:\n>\n> With the AE-related controls reorganized in libcamera, with separate\n> value and lever controls for exposure and gain, plumb these through the\n> HAL layer (in order of appearence in the patch):\n> - static metadata: available AE modes, AE lock available\n> - manual template: add AE off (the other templates already have AE on)\n> - preview template: set exposure time to the minimum in the exposure\n> time range\n> - request metadata: HAL -> libcamera controls conversion\n> - result metadata: libcamera -> HAL controls conversion\n> - result metadata: AE state\n>\n> We add class variables to CameraDevice to save the last set android AE\n> controls, as that is how android controls function (no need to resubmit\n> controls if they are unchanged).\n>\n> We also save libcamera's AE state in the request descriptor, as\n> otherwise there is insufficient information from libcamera's result\n> metadata alone to tell if the state is locked or manual (as they are an\n> internal state in libcamera).\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v3:\n> - rename AeMode to AutoMode as we'll also use it for awb later\n> - set the exposure time in the preview template\n> - a bunch of other big changes (mentioned by Jacopo, and discussed a\n> bit) were not done, as more discussion is needed\n>\n> New in v2\n> ---\n> src/android/camera_capabilities.cpp | 71 +++++++++++++++++++---\n> src/android/camera_device.cpp | 93 +++++++++++++++++++++++++++--\n> src/android/camera_device.h | 13 ++++\n> src/android/camera_request.h | 9 +++\n> 4 files changed, 174 insertions(+), 12 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index a2e42a5c..318ab666 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -922,12 +922,62 @@ int CameraCapabilities::initializeStaticMetadata()\n> staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,\n> aeAvailableAntiBandingModes);\n>\n> - std::vector aeAvailableModes = {\n> - ANDROID_CONTROL_AE_MODE_ON,\n> - };\n> + std::vector aeAvailableModes;\n> + /* This will cause problems if one supports only on and the other only off */\n> + bool aeAddedOn = false;\n> + bool aeAddedOff = false;\n> +\n> + const auto &analogGainModeInfo = controlsInfo.find(&controls::AnalogueGainMode);\n> + if (analogGainModeInfo != controlsInfo.end()) {\n> + for (const auto &value : analogGainModeInfo->second.values()) {\n> + switch (value.get()) {\n> + case controls::AnalogueGainModeAuto:\n> + if (!aeAddedOn)\n> + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);\n> + aeAddedOn = true;\n> + break;\n> + case controls::AnalogueGainModeDisabled:\n> + if (!aeAddedOff)\n> + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);\n> + aeAddedOff = true;\n> + break;\n> + default:\n> + break;\n> + }\n> + }\n> + }\n> +\n> + const auto &exposureTimeModeInfo = controlsInfo.find(&controls::ExposureTimeMode);\n> + if (exposureTimeModeInfo != controlsInfo.end()) {\n> + for (const auto &value : exposureTimeModeInfo->second.values()) {\n> + switch (value.get()) {\n> + case controls::ExposureTimeModeAuto:\n> + if (!aeAddedOn)\n> + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);\n> + aeAddedOn = true;\n> + break;\n> + case controls::ExposureTimeModeDisabled:\n> + if (!aeAddedOff)\n> + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);\n> + aeAddedOff = true;\n> + break;\n> + default:\n> + break;\n> + }\n> + }\n> + }\n> +\n> + if (aeAvailableModes.empty())\n> + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);\n> staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> aeAvailableModes);\n>\n> + /* In libcamera, turning off AE is equivalient to locking. */\n> + uint8_t aeLockAvailable = aeAddedOff ? ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE\n> + : ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;\n> + staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> + aeLockAvailable);\n> +\n> std::vector aeCompensationRange = {\n> 0, 0,\n> };\n> @@ -988,10 +1038,6 @@ int CameraCapabilities::initializeStaticMetadata()\n> staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,\n> sceneModesOverride);\n>\n> - uint8_t aeLockAvailable = ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;\n> - staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> - aeLockAvailable);\n> -\n> uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE;\n> staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> awbLockAvailable);\n> @@ -1468,6 +1514,12 @@ std::unique_ptr CameraCapabilities::requestTemplateManual() cons\n> manualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);\n> }\n>\n> + if (staticMetadata_->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> + ANDROID_CONTROL_AE_MODE_OFF)) {\n> + uint8_t aeMode = ANDROID_CONTROL_AE_MODE_OFF;\n> + manualTemplate->appendEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n> + }\n> +\n> return manualTemplate;\n> }\n>\n> @@ -1542,6 +1594,11 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF;\n> requestTemplate->addEntry(ANDROID_CONTROL_AWB_LOCK, awbLock);\n>\n> + if (availableRequestKeys_.count(ANDROID_SENSOR_EXPOSURE_TIME)) {\n> + staticMetadata_->getEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, &entry);\n> + requestTemplate->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, entry.data.i64[0]);\n> + }\n> +\n> uint8_t flashMode = ANDROID_FLASH_MODE_OFF;\n> requestTemplate->addEntry(ANDROID_FLASH_MODE, flashMode);\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 83825736..3ade44c4 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -246,7 +246,9 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList)\n>\n> CameraDevice::CameraDevice(unsigned int id, std::shared_ptr camera)\n> : id_(id), state_(State::Stopped), camera_(std::move(camera)),\n> - facing_(CAMERA_FACING_FRONT), orientation_(0)\n> + facing_(CAMERA_FACING_FRONT), orientation_(0),\n> + aeOn_(true), aeLocked_(false), lastExposureTime_(0),\n> + lastAnalogueGain_(1.0f), lastDigitalGain_(1.0f)\n> {\n> camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);\n>\n> @@ -812,6 +814,59 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> controls.set(controls::draft::TestPatternMode, testPatternMode);\n> }\n>\n> + /*\n> + * \\todo Can we trust that we won't receive values that we didn't\n> + * report supporting?\n> + */\n> + if (settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry))\n> + aeLocked_ = *entry.data.u8 != ANDROID_CONTROL_AE_LOCK_OFF;\n> +\n> + if (settings.getEntry(ANDROID_CONTROL_AE_MODE, &entry))\n> + aeOn_ = *entry.data.u8 != ANDROID_CONTROL_AE_MODE_OFF;\n> +\n> + Camera3RequestDescriptor::AutoMode aeMode;\n> + if (aeLocked_ && aeOn_)\n> + aeMode = Camera3RequestDescriptor::AutoMode::Lock;\n> + else if (aeLocked_ && !aeOn_)\n> + aeMode = Camera3RequestDescriptor::AutoMode::Manual;\n> + else if (!aeLocked_ && aeOn_)\n> + aeMode = Camera3RequestDescriptor::AutoMode::Auto;\n> + else /* !aeLocked_ && !aeOn_ */\n> + aeMode = Camera3RequestDescriptor::AutoMode::Manual;\n> +\naeLocked_ and aeOn_ seem to be only used locally. Could them be local\nvariables instead?\n> + /* Save this so that we can recover it in the result */\n> + descriptor->aeMode_ = aeMode;\n> +\n> + const auto &eInfo = camera_->controls().find(&controls::ExposureTimeMode);\n> + if (eInfo != camera_->controls().end()) {\n> + controls.set(controls::ExposureTimeMode,\n> + aeMode == Camera3RequestDescriptor::AutoMode::Auto ?\n> + controls::ExposureTimeModeAuto :\n> + controls::ExposureTimeModeDisabled);\n> + }\n> +\n> + const auto &gInfo = camera_->controls().find(&controls::AnalogueGainMode);\n> + if (gInfo != camera_->controls().end()) {\n> + controls.set(controls::AnalogueGainMode,\n> + aeMode == Camera3RequestDescriptor::AutoMode::Auto ?\n> + controls::AnalogueGainModeAuto :\n> + controls::AnalogueGainModeDisabled);\n> + }\n> +\n> + if (settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry)) {\n> + const auto &info = camera_->controls().find(&controls::ExposureTime);\n> + if (info != camera_->controls().end()) {\n> + lastExposureTime_ = (*entry.data.i64) / 1000;\n> + }\n> + }\n> +\n> + /* Trigger libcamera's locked -> manual state change */\n> + if (aeMode == Camera3RequestDescriptor::AutoMode::Manual) {\n> + const auto &info = camera_->controls().find(&controls::ExposureTime);\n> + if (info != camera_->controls().end())\n> + controls.set(controls::ExposureTime, lastExposureTime_);\n> + }\n> +\n> return 0;\n> }\n>\n> @@ -1336,11 +1391,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> resultMetadata->addEntry(ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,\n> value32);\n>\n> - value = ANDROID_CONTROL_AE_LOCK_OFF;\n> - resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, value);\n> + uint8_t aeLock = (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)\n> + ? ANDROID_CONTROL_AE_LOCK_ON\n> + : ANDROID_CONTROL_AE_LOCK_OFF;\n> + resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, aeLock);\n>\n> - value = ANDROID_CONTROL_AE_MODE_ON;\n> - resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, value);\n> + /* Locked means auto + locked in android */\n> + uint8_t aeMode = (descriptor.aeMode_ != Camera3RequestDescriptor::AutoMode::Manual)\n> + ? ANDROID_CONTROL_AE_MODE_ON\n> + : ANDROID_CONTROL_AE_MODE_OFF;\n> + resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n>\n> if (settings.getEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, &entry))\n> /*\n> @@ -1357,6 +1417,29 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, value);\n>\n> value = ANDROID_CONTROL_AE_STATE_CONVERGED;\n> + if (metadata.contains(controls::AeState)) {\n> + switch (metadata.get(controls::AeState)) {\n> + case controls::AeStateInactive:\n> + value = ANDROID_CONTROL_AE_STATE_INACTIVE;\n> + break;\n> + case controls::AeStateSearching:\n> + value = ANDROID_CONTROL_AE_STATE_SEARCHING;\n> + break;\n> + case controls::AeStateConverged:\n> + value = ANDROID_CONTROL_AE_STATE_CONVERGED;\n> + break;\n> + case controls::AeStateFlashRequired:\n> + value = ANDROID_CONTROL_AE_STATE_FLASH_REQUIRED;\n> + break;\n> + case controls::AeStatePrecapture:\n> + value = ANDROID_CONTROL_AE_STATE_PRECAPTURE;\n> + break;\n> + default:\n> + LOG(HAL, Error) << \"Invalid AeState, setting converged\";\n> + }\n> + }\n> + if (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)\n> + value = ANDROID_CONTROL_AE_STATE_LOCKED;\n> resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, value);\n>\n> value = ANDROID_CONTROL_AF_MODE_OFF;\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 64050416..a65f1670 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -126,5 +126,18 @@ private:\n> int facing_;\n> int orientation_;\n>\n> + /*\n> + * Normally resubmission of controls would be handled on libcamera's\n> + * side, but these controls are not one-to-one between libcamera and\n> + * android, so we need to save them here\n> + */\n> +\n> + /* Track the last-set android AE controls */\n> + bool aeOn_;\n> + bool aeLocked_;\n> + int32_t lastExposureTime_;\n> + float lastAnalogueGain_;\n> + float lastDigitalGain_;\n> +\n> CameraMetadata lastSettings_;\n> };\n> diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> index 37b6ae32..b2809179 100644\n> --- a/src/android/camera_request.h\n> +++ b/src/android/camera_request.h\n> @@ -28,6 +28,12 @@ class CameraStream;\n> class Camera3RequestDescriptor\n> {\n> public:\n> + enum class AutoMode {\n> + Auto,\n> + Lock,\n> + Manual,\n> + };\n> +\n> enum class Status {\n> Success,\n> Error,\n> @@ -78,6 +84,9 @@ public:\n> bool complete_ = false;\n> Status status_ = Status::Success;\n>\n> + /* The libcamera internal AE state for this request */\n> + AutoMode aeMode_ = AutoMode::Auto;\n> +\n> private:\n> LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)\n> };\n> --\n> 2.27.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 70F68BE080\n\tfor ;\n\tTue, 21 Dec 2021 11:23:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 97FCD608A2;\n\tTue, 21 Dec 2021 12:23:22 +0100 (CET)","from mail-ot1-x32c.google.com (mail-ot1-x32c.google.com\n\t[IPv6:2607:f8b0:4864:20::32c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A393960115\n\tfor ;\n\tTue, 21 Dec 2021 12:23:20 +0100 (CET)","by mail-ot1-x32c.google.com with SMTP id\n\th19-20020a9d3e53000000b0056547b797b2so16237932otg.4\n\tfor ;\n\tTue, 21 Dec 2021 03:23:20 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"OYLydImK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=wFNOtT9sOR0gnApKNlA73Ik3pDjSYo3BCpW5kjKHb58=;\n\tb=OYLydImK9ykKQkC1jA4+ZjWGz77kFlbeTaJpwIm/dn0ciPiHKzSZDKKGf17RDVgpE5\n\tM0QgmqAKtAYosNNlyYy/Vr4CCtj8CpGcLSyuC2lccCVeYP/Eaj7W8+P0MbLDGyeTKaqH\n\tnEpFgf0ph8ZEtgEU3oVG1CaO8MkU5JoE3Bam0=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=wFNOtT9sOR0gnApKNlA73Ik3pDjSYo3BCpW5kjKHb58=;\n\tb=Q4uv/9HfM5UELO3zFHBY2zMEZ/g7JqbbDby2rCg17nkVn3esRz+r7+ZRErz8FbEuLv\n\tuIUt4SlCLjjjIjRnZxcNgvOQK4ITVaA1F3rJmQi7IfoOc4VHSB+yzYVeIwWUHQjmdSaY\n\tYg3RcM3OufgRYmALNqb1EppfsdykiuA8X+Ft0/SOPArRRFjabQuqTBhL10E2/FykIYN+\n\tGUJN71kV0mnXndE65AoTiYtXuxu4T7dNz23H/gOwyiVAqqiHJp/MpjXyblyUW5S1fH4+\n\tQo1DfuPbckVENoPt4Nfy98B77NrAX7nmx8qNzAID0bUednGHqpJSsGVj+Yg7n8PfSip5\n\tZE2w==","X-Gm-Message-State":"AOAM533oPplyjKZ3uBa1dzo+Gt3b4vOIHQII0yMRBE4dNEdi3U4ltA0Y\n\tMauYXAPvVrA1SxlNJZuHIKePvkPoHrSEK7YVlAqt39iAZtPGYg==","X-Google-Smtp-Source":"ABdhPJxm0xgzFYMLcDq8AwcQLseJM8ZE0ErPQHFqMZujb/f4dTbTZOwGBND66k/7077cG2FTBTEIzuodaPx7Svd5j5M=","X-Received":"by 2002:a9d:1726:: with SMTP id\n\ti38mr1787716ota.176.1640085798049; \n\tTue, 21 Dec 2021 03:23:18 -0800 (PST)","MIME-Version":"1.0","References":"<20211221043610.2512334-1-paul.elder@ideasonboard.com>\n\t<20211221043610.2512334-7-paul.elder@ideasonboard.com>","In-Reply-To":"<20211221043610.2512334-7-paul.elder@ideasonboard.com>","From":"Hanlin Chen ","Date":"Tue, 21 Dec 2021 19:23:07 +0800","Message-ID":"","To":"Paul Elder ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: Plumb all AE-related\n\tcontrols","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21861,"web_url":"https://patchwork.libcamera.org/comment/21861/","msgid":"<20211221221058.GI2742@pyrite.rasen.tech>","date":"2021-12-21T22:10:58","subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: Plumb all AE-related\n\tcontrols","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Hanlin,\n\nThanks for the review.\n\nOn Tue, Dec 21, 2021 at 07:23:07PM +0800, Hanlin Chen wrote:\n> Hi Paul, Thanks for the patch.\n> \n> On Tue, Dec 21, 2021 at 12:36 PM Paul Elder wrote:\n> >\n> > With the AE-related controls reorganized in libcamera, with separate\n> > value and lever controls for exposure and gain, plumb these through the\n> > HAL layer (in order of appearence in the patch):\n> > - static metadata: available AE modes, AE lock available\n> > - manual template: add AE off (the other templates already have AE on)\n> > - preview template: set exposure time to the minimum in the exposure\n> > time range\n> > - request metadata: HAL -> libcamera controls conversion\n> > - result metadata: libcamera -> HAL controls conversion\n> > - result metadata: AE state\n> >\n> > We add class variables to CameraDevice to save the last set android AE\n> > controls, as that is how android controls function (no need to resubmit\n> > controls if they are unchanged).\n> >\n> > We also save libcamera's AE state in the request descriptor, as\n> > otherwise there is insufficient information from libcamera's result\n> > metadata alone to tell if the state is locked or manual (as they are an\n> > internal state in libcamera).\n> >\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=42\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=43\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=47\n> > Signed-off-by: Paul Elder \n> >\n> > ---\n> > Changes in v3:\n> > - rename AeMode to AutoMode as we'll also use it for awb later\n> > - set the exposure time in the preview template\n> > - a bunch of other big changes (mentioned by Jacopo, and discussed a\n> > bit) were not done, as more discussion is needed\n> >\n> > New in v2\n> > ---\n> > src/android/camera_capabilities.cpp | 71 +++++++++++++++++++---\n> > src/android/camera_device.cpp | 93 +++++++++++++++++++++++++++--\n> > src/android/camera_device.h | 13 ++++\n> > src/android/camera_request.h | 9 +++\n> > 4 files changed, 174 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index a2e42a5c..318ab666 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -922,12 +922,62 @@ int CameraCapabilities::initializeStaticMetadata()\n> > staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,\n> > aeAvailableAntiBandingModes);\n> >\n> > - std::vector aeAvailableModes = {\n> > - ANDROID_CONTROL_AE_MODE_ON,\n> > - };\n> > + std::vector aeAvailableModes;\n> > + /* This will cause problems if one supports only on and the other only off */\n> > + bool aeAddedOn = false;\n> > + bool aeAddedOff = false;\n> > +\n> > + const auto &analogGainModeInfo = controlsInfo.find(&controls::AnalogueGainMode);\n> > + if (analogGainModeInfo != controlsInfo.end()) {\n> > + for (const auto &value : analogGainModeInfo->second.values()) {\n> > + switch (value.get()) {\n> > + case controls::AnalogueGainModeAuto:\n> > + if (!aeAddedOn)\n> > + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);\n> > + aeAddedOn = true;\n> > + break;\n> > + case controls::AnalogueGainModeDisabled:\n> > + if (!aeAddedOff)\n> > + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);\n> > + aeAddedOff = true;\n> > + break;\n> > + default:\n> > + break;\n> > + }\n> > + }\n> > + }\n> > +\n> > + const auto &exposureTimeModeInfo = controlsInfo.find(&controls::ExposureTimeMode);\n> > + if (exposureTimeModeInfo != controlsInfo.end()) {\n> > + for (const auto &value : exposureTimeModeInfo->second.values()) {\n> > + switch (value.get()) {\n> > + case controls::ExposureTimeModeAuto:\n> > + if (!aeAddedOn)\n> > + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);\n> > + aeAddedOn = true;\n> > + break;\n> > + case controls::ExposureTimeModeDisabled:\n> > + if (!aeAddedOff)\n> > + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_OFF);\n> > + aeAddedOff = true;\n> > + break;\n> > + default:\n> > + break;\n> > + }\n> > + }\n> > + }\n> > +\n> > + if (aeAvailableModes.empty())\n> > + aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);\n> > staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> > aeAvailableModes);\n> >\n> > + /* In libcamera, turning off AE is equivalient to locking. */\n> > + uint8_t aeLockAvailable = aeAddedOff ? ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE\n> > + : ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;\n> > + staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> > + aeLockAvailable);\n> > +\n> > std::vector aeCompensationRange = {\n> > 0, 0,\n> > };\n> > @@ -988,10 +1038,6 @@ int CameraCapabilities::initializeStaticMetadata()\n> > staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,\n> > sceneModesOverride);\n> >\n> > - uint8_t aeLockAvailable = ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE;\n> > - staticMetadata_->addEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> > - aeLockAvailable);\n> > -\n> > uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE;\n> > staticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> > awbLockAvailable);\n> > @@ -1468,6 +1514,12 @@ std::unique_ptr CameraCapabilities::requestTemplateManual() cons\n> > manualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);\n> > }\n> >\n> > + if (staticMetadata_->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> > + ANDROID_CONTROL_AE_MODE_OFF)) {\n> > + uint8_t aeMode = ANDROID_CONTROL_AE_MODE_OFF;\n> > + manualTemplate->appendEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n> > + }\n> > +\n> > return manualTemplate;\n> > }\n> >\n> > @@ -1542,6 +1594,11 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> > uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF;\n> > requestTemplate->addEntry(ANDROID_CONTROL_AWB_LOCK, awbLock);\n> >\n> > + if (availableRequestKeys_.count(ANDROID_SENSOR_EXPOSURE_TIME)) {\n> > + staticMetadata_->getEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, &entry);\n> > + requestTemplate->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, entry.data.i64[0]);\n> > + }\n> > +\n> > uint8_t flashMode = ANDROID_FLASH_MODE_OFF;\n> > requestTemplate->addEntry(ANDROID_FLASH_MODE, flashMode);\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 83825736..3ade44c4 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -246,7 +246,9 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList)\n> >\n> > CameraDevice::CameraDevice(unsigned int id, std::shared_ptr camera)\n> > : id_(id), state_(State::Stopped), camera_(std::move(camera)),\n> > - facing_(CAMERA_FACING_FRONT), orientation_(0)\n> > + facing_(CAMERA_FACING_FRONT), orientation_(0),\n> > + aeOn_(true), aeLocked_(false), lastExposureTime_(0),\n> > + lastAnalogueGain_(1.0f), lastDigitalGain_(1.0f)\n> > {\n> > camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);\n> >\n> > @@ -812,6 +814,59 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> > controls.set(controls::draft::TestPatternMode, testPatternMode);\n> > }\n> >\n> > + /*\n> > + * \\todo Can we trust that we won't receive values that we didn't\n> > + * report supporting?\n> > + */\n> > + if (settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry))\n> > + aeLocked_ = *entry.data.u8 != ANDROID_CONTROL_AE_LOCK_OFF;\n> > +\n> > + if (settings.getEntry(ANDROID_CONTROL_AE_MODE, &entry))\n> > + aeOn_ = *entry.data.u8 != ANDROID_CONTROL_AE_MODE_OFF;\n> > +\n> > + Camera3RequestDescriptor::AutoMode aeMode;\n> > + if (aeLocked_ && aeOn_)\n> > + aeMode = Camera3RequestDescriptor::AutoMode::Lock;\n> > + else if (aeLocked_ && !aeOn_)\n> > + aeMode = Camera3RequestDescriptor::AutoMode::Manual;\n> > + else if (!aeLocked_ && aeOn_)\n> > + aeMode = Camera3RequestDescriptor::AutoMode::Auto;\n> > + else /* !aeLocked_ && !aeOn_ */\n> > + aeMode = Camera3RequestDescriptor::AutoMode::Manual;\n> > +\n> aeLocked_ and aeOn_ seem to be only used locally. Could them be local\n> variables instead?\n\nYou're right, I'll do that.\n\nThey were leftover from when I moved the flags to the request wrapper :/\n\n\nPaul\n\n> > + /* Save this so that we can recover it in the result */\n> > + descriptor->aeMode_ = aeMode;\n> > +\n> > + const auto &eInfo = camera_->controls().find(&controls::ExposureTimeMode);\n> > + if (eInfo != camera_->controls().end()) {\n> > + controls.set(controls::ExposureTimeMode,\n> > + aeMode == Camera3RequestDescriptor::AutoMode::Auto ?\n> > + controls::ExposureTimeModeAuto :\n> > + controls::ExposureTimeModeDisabled);\n> > + }\n> > +\n> > + const auto &gInfo = camera_->controls().find(&controls::AnalogueGainMode);\n> > + if (gInfo != camera_->controls().end()) {\n> > + controls.set(controls::AnalogueGainMode,\n> > + aeMode == Camera3RequestDescriptor::AutoMode::Auto ?\n> > + controls::AnalogueGainModeAuto :\n> > + controls::AnalogueGainModeDisabled);\n> > + }\n> > +\n> > + if (settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry)) {\n> > + const auto &info = camera_->controls().find(&controls::ExposureTime);\n> > + if (info != camera_->controls().end()) {\n> > + lastExposureTime_ = (*entry.data.i64) / 1000;\n> > + }\n> > + }\n> > +\n> > + /* Trigger libcamera's locked -> manual state change */\n> > + if (aeMode == Camera3RequestDescriptor::AutoMode::Manual) {\n> > + const auto &info = camera_->controls().find(&controls::ExposureTime);\n> > + if (info != camera_->controls().end())\n> > + controls.set(controls::ExposureTime, lastExposureTime_);\n> > + }\n> > +\n> > return 0;\n> > }\n> >\n> > @@ -1336,11 +1391,16 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > resultMetadata->addEntry(ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,\n> > value32);\n> >\n> > - value = ANDROID_CONTROL_AE_LOCK_OFF;\n> > - resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, value);\n> > + uint8_t aeLock = (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)\n> > + ? ANDROID_CONTROL_AE_LOCK_ON\n> > + : ANDROID_CONTROL_AE_LOCK_OFF;\n> > + resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, aeLock);\n> >\n> > - value = ANDROID_CONTROL_AE_MODE_ON;\n> > - resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, value);\n> > + /* Locked means auto + locked in android */\n> > + uint8_t aeMode = (descriptor.aeMode_ != Camera3RequestDescriptor::AutoMode::Manual)\n> > + ? ANDROID_CONTROL_AE_MODE_ON\n> > + : ANDROID_CONTROL_AE_MODE_OFF;\n> > + resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n> >\n> > if (settings.getEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, &entry))\n> > /*\n> > @@ -1357,6 +1417,29 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, value);\n> >\n> > value = ANDROID_CONTROL_AE_STATE_CONVERGED;\n> > + if (metadata.contains(controls::AeState)) {\n> > + switch (metadata.get(controls::AeState)) {\n> > + case controls::AeStateInactive:\n> > + value = ANDROID_CONTROL_AE_STATE_INACTIVE;\n> > + break;\n> > + case controls::AeStateSearching:\n> > + value = ANDROID_CONTROL_AE_STATE_SEARCHING;\n> > + break;\n> > + case controls::AeStateConverged:\n> > + value = ANDROID_CONTROL_AE_STATE_CONVERGED;\n> > + break;\n> > + case controls::AeStateFlashRequired:\n> > + value = ANDROID_CONTROL_AE_STATE_FLASH_REQUIRED;\n> > + break;\n> > + case controls::AeStatePrecapture:\n> > + value = ANDROID_CONTROL_AE_STATE_PRECAPTURE;\n> > + break;\n> > + default:\n> > + LOG(HAL, Error) << \"Invalid AeState, setting converged\";\n> > + }\n> > + }\n> > + if (descriptor.aeMode_ == Camera3RequestDescriptor::AutoMode::Lock)\n> > + value = ANDROID_CONTROL_AE_STATE_LOCKED;\n> > resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, value);\n> >\n> > value = ANDROID_CONTROL_AF_MODE_OFF;\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 64050416..a65f1670 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -126,5 +126,18 @@ private:\n> > int facing_;\n> > int orientation_;\n> >\n> > + /*\n> > + * Normally resubmission of controls would be handled on libcamera's\n> > + * side, but these controls are not one-to-one between libcamera and\n> > + * android, so we need to save them here\n> > + */\n> > +\n> > + /* Track the last-set android AE controls */\n> > + bool aeOn_;\n> > + bool aeLocked_;\n> > + int32_t lastExposureTime_;\n> > + float lastAnalogueGain_;\n> > + float lastDigitalGain_;\n> > +\n> > CameraMetadata lastSettings_;\n> > };\n> > diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> > index 37b6ae32..b2809179 100644\n> > --- a/src/android/camera_request.h\n> > +++ b/src/android/camera_request.h\n> > @@ -28,6 +28,12 @@ class CameraStream;\n> > class Camera3RequestDescriptor\n> > {\n> > public:\n> > + enum class AutoMode {\n> > + Auto,\n> > + Lock,\n> > + Manual,\n> > + };\n> > +\n> > enum class Status {\n> > Success,\n> > Error,\n> > @@ -78,6 +84,9 @@ public:\n> > bool complete_ = false;\n> > Status status_ = Status::Success;\n> >\n> > + /* The libcamera internal AE state for this request */\n> > + AutoMode aeMode_ = AutoMode::Auto;\n> > +\n> > private:\n> > LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)\n> > };\n> > --\n> > 2.27.0\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 32E9FBE080\n\tfor ;\n\tTue, 21 Dec 2021 22:11:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 253DE60900;\n\tTue, 21 Dec 2021 23:11:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29DC9605A8\n\tfor ;\n\tTue, 21 Dec 2021 23:11:05 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2604:2d80:ad90:fb00:96fd:8874:873:6c16])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0ECA9881;\n\tTue, 21 Dec 2021 23:11:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Q+JAtk0k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1640124664;\n\tbh=F+FFhNN4qX1jkwJcX2BK57bYlY4JeOO8UepWWR7Rekw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Q+JAtk0kk7gyqA5KW3zLCqoVZ6c0tq2UFWGKwKrWOU0CmnTs16e60dzDfWuflZ+V4\n\tghFRrGM+kyT2Agh2MaICtOumTd7xfw71+SZet0CROL2yWFdM070IwSLSbcah1qhz++\n\tNNebAiN15C0CeKocnjjsHN0vstS1cFyTGMffty7I=","Date":"Tue, 21 Dec 2021 16:10:58 -0600","From":"paul.elder@ideasonboard.com","To":"Hanlin Chen ","Message-ID":"<20211221221058.GI2742@pyrite.rasen.tech>","References":"<20211221043610.2512334-1-paul.elder@ideasonboard.com>\n\t<20211221043610.2512334-7-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 6/8] android: Plumb all AE-related\n\tcontrols","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]