{"id":22592,"url":"https://patchwork.libcamera.org/api/patches/22592/?format=json","web_url":"https://patchwork.libcamera.org/patch/22592/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20250120204515.24096-6-laurent.pinchart@ideasonboard.com>","date":"2025-01-20T20:44:56","name":"[v9,05/12] ipa: raspberry: Port to the new AEGC controls","commit_ref":"c45a2682c3067ee92d0bdd4a5fbde3e941edc95a","pull_url":null,"state":"accepted","archived":false,"hash":"89f6385a39e493a6ec5bc2c7c6e9ad158fb78a20","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/22592/mbox/","series":[{"id":4958,"url":"https://patchwork.libcamera.org/api/series/4958/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4958","date":"2025-01-20T20:44:51","name":"AEGC controls","version":9,"mbox":"https://patchwork.libcamera.org/series/4958/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/22592/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/22592/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 39586C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jan 2025 20:45:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A621D68550;\n\tMon, 20 Jan 2025 21:45:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 80581684E7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jan 2025 21:45:29 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C4A181193;\n\tMon, 20 Jan 2025 21:44:27 +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=\"KMV4XEpB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737405868;\n\tbh=qUWcVP3vsd8OdZ5jzCUBougx8ggd3m0ScMroVL7eSwY=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=KMV4XEpBXMp3AL6y2fnIJ7B+KDk+9txLLC8L0YlaQNWiCEsQBXkgvVGPa19wq8q4U\n\tdLH0ULAGw7Iivtl5wJlSzfnQ9+bEgSTPa63pGkWeMZKCg+lCsMDL9PEpy9tahT+8jT\n\tJ6aLDgUT/60kbStmjLiqpR6HY0emonc2cRJenlQU=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Cc":"Naushir Patuck <naush@raspberrypi.com>,\n\tDavid Plowman <david.plowman@raspberrypi.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>,\n\tJacopo Mondi <jacopo@jmondi.org>","Subject":"[PATCH v9 05/12] ipa: raspberry: Port to the new AEGC controls","Date":"Mon, 20 Jan 2025 22:44:56 +0200","Message-ID":"<20250120204515.24096-6-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.45.2","In-Reply-To":"<20250120204515.24096-1-laurent.pinchart@ideasonboard.com>","References":"<20250120204515.24096-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"From: Jacopo Mondi <jacopo@jmondi.org>\n\nThe newly introduced controls to drive the AEGC algorithm allow to\ncontrol the computation of the exposure time and analogue gain\nseparately.\n\nThe RPi AEGC implementation already computes the shutter and gain values\nseparately but does not expose separate functions to control them.\n\nAugment the AgcAlgorithm interface to allow pausing/resuming the shutter\nand gain automatic computations separately and plumb them to the newly\nintroduced controls.\n\nAdd safety checks to ignore ExposureTime and AnalogueGain values if the\nalgorithms are not paused, and report the correct AeState value by\nchecking if both algorithms have been paused or if they have converged.\n\nSigned-off-by: Jacopo Mondi <jacopo@jmondi.org>\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>\nReviewed-by: Naushir Patuck <naush@raspberrypi.com>\nAcked-by: David Plowman <david.plowman@raspberrypi.com>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/rpi/common/ipa_base.cpp            | 102 +++++++++++++++++----\n src/ipa/rpi/controller/agc_algorithm.h     |   8 +-\n src/ipa/rpi/controller/rpi/agc.cpp         |  52 +++++++++--\n src/ipa/rpi/controller/rpi/agc.h           |   8 +-\n src/ipa/rpi/controller/rpi/agc_channel.cpp |  24 ++++-\n src/ipa/rpi/controller/rpi/agc_channel.h   |   8 +-\n 6 files changed, 167 insertions(+), 35 deletions(-)","diff":"diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\nindex 6ff1e22b15d4..5210a1f38efb 100644\n--- a/src/ipa/rpi/common/ipa_base.cpp\n+++ b/src/ipa/rpi/common/ipa_base.cpp\n@@ -55,8 +55,15 @@ constexpr Duration controllerMinFrameDuration = 1.0s / 30.0;\n \n /* List of controls handled by the Raspberry Pi IPA */\n const ControlInfoMap::Map ipaControls{\n-\t{ &controls::AeEnable, ControlInfo(false, true) },\n+\t{ &controls::ExposureTimeMode,\n+\t  ControlInfo(static_cast<int32_t>(controls::ExposureTimeModeAuto),\n+\t\t      static_cast<int32_t>(controls::ExposureTimeModeManual),\n+\t\t      static_cast<int32_t>(controls::ExposureTimeModeAuto)) },\n \t{ &controls::ExposureTime, ControlInfo(0, 66666) },\n+\t{ &controls::AnalogueGainMode,\n+\t  ControlInfo(static_cast<int32_t>(controls::AnalogueGainModeAuto),\n+\t\t      static_cast<int32_t>(controls::AnalogueGainModeManual),\n+\t\t      static_cast<int32_t>(controls::AnalogueGainModeAuto)) },\n \t{ &controls::AnalogueGain, ControlInfo(1.0f, 16.0f) },\n \t{ &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },\n \t{ &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },\n@@ -749,6 +756,42 @@ void IpaBase::applyControls(const ControlList &controls)\n \t\t\taf->setMode(mode->second);\n \t}\n \n+\t/*\n+\t * Because some AE controls are mode-specific, handle the AE-related\n+\t * mode changes first.\n+\t */\n+\tconst auto analogueGainMode = controls.get(controls::AnalogueGainMode);\n+\tconst auto exposureTimeMode = controls.get(controls::ExposureTimeMode);\n+\n+\tif (analogueGainMode || exposureTimeMode) {\n+\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n+\t\t\tcontroller_.getAlgorithm(\"agc\"));\n+\t\tif (agc) {\n+\t\t\tif (analogueGainMode) {\n+\t\t\t\tif (*analogueGainMode == controls::AnalogueGainModeManual)\n+\t\t\t\t\tagc->disableAutoGain();\n+\t\t\t\telse\n+\t\t\t\t\tagc->enableAutoGain();\n+\n+\t\t\t\tlibcameraMetadata_.set(controls::AnalogueGainMode,\n+\t\t\t\t\t\t       *analogueGainMode);\n+\t\t\t}\n+\n+\t\t\tif (exposureTimeMode) {\n+\t\t\t\tif (*exposureTimeMode == controls::ExposureTimeModeManual)\n+\t\t\t\t\tagc->disableAutoExposure();\n+\t\t\t\telse\n+\t\t\t\t\tagc->enableAutoExposure();\n+\n+\t\t\t\tlibcameraMetadata_.set(controls::ExposureTimeMode,\n+\t\t\t\t\t\t       *exposureTimeMode);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t<< \"Could not set AnalogueGainMode or ExposureTimeMode - no AGC algorithm\";\n+\t\t}\n+\t}\n+\n \t/* Iterate over controls */\n \tfor (auto const &ctrl : controls) {\n \t\tLOG(IPARPI, Debug) << \"Request ctrl: \"\n@@ -756,23 +799,8 @@ void IpaBase::applyControls(const ControlList &controls)\n \t\t\t\t   << \" = \" << ctrl.second.toString();\n \n \t\tswitch (ctrl.first) {\n-\t\tcase controls::AE_ENABLE: {\n-\t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n-\t\t\t\tcontroller_.getAlgorithm(\"agc\"));\n-\t\t\tif (!agc) {\n-\t\t\t\tLOG(IPARPI, Warning)\n-\t\t\t\t\t<< \"Could not set AE_ENABLE - no AGC algorithm\";\n-\t\t\t\tbreak;\n-\t\t\t}\n-\n-\t\t\tif (ctrl.second.get<bool>() == false)\n-\t\t\t\tagc->disableAuto();\n-\t\t\telse\n-\t\t\t\tagc->enableAuto();\n-\n-\t\t\tlibcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());\n-\t\t\tbreak;\n-\t\t}\n+\t\tcase controls::EXPOSURE_TIME_MODE:\n+\t\t\tbreak; /* We already handled this one above */\n \n \t\tcase controls::EXPOSURE_TIME: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n@@ -783,6 +811,13 @@ void IpaBase::applyControls(const ControlList &controls)\n \t\t\t\tbreak;\n \t\t\t}\n \n+\t\t\t/*\n+\t\t\t * Ignore manual exposure time when the auto exposure\n+\t\t\t * algorithm is running.\n+\t\t\t */\n+\t\t\tif (agc->autoExposureEnabled())\n+\t\t\t\tbreak;\n+\n \t\t\t/* The control provides units of microseconds. */\n \t\t\tagc->setFixedExposureTime(0, ctrl.second.get<int32_t>() * 1.0us);\n \n@@ -790,6 +825,9 @@ void IpaBase::applyControls(const ControlList &controls)\n \t\t\tbreak;\n \t\t}\n \n+\t\tcase controls::ANALOGUE_GAIN_MODE:\n+\t\t\tbreak; /* We already handled this one above */\n+\n \t\tcase controls::ANALOGUE_GAIN: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n \t\t\t\tcontroller_.getAlgorithm(\"agc\"));\n@@ -799,6 +837,13 @@ void IpaBase::applyControls(const ControlList &controls)\n \t\t\t\tbreak;\n \t\t\t}\n \n+\t\t\t/*\n+\t\t\t * Ignore manual analogue gain value when the auto gain\n+\t\t\t * algorithm is running.\n+\t\t\t */\n+\t\t\tif (agc->autoGainEnabled())\n+\t\t\t\tbreak;\n+\n \t\t\tagc->setFixedAnalogueGain(0, ctrl.second.get<float>());\n \n \t\t\tlibcameraMetadata_.set(controls::AnalogueGain,\n@@ -855,6 +900,13 @@ void IpaBase::applyControls(const ControlList &controls)\n \t\t\t\tbreak;\n \t\t\t}\n \n+\t\t\t/*\n+\t\t\t * Ignore AE_EXPOSURE_MODE if the shutter or the gain\n+\t\t\t * are in auto mode.\n+\t\t\t */\n+\t\t\tif (agc->autoExposureEnabled() || agc->autoGainEnabled())\n+\t\t\t\tbreak;\n+\n \t\t\tint32_t idx = ctrl.second.get<int32_t>();\n \t\t\tif (ExposureModeTable.count(idx)) {\n \t\t\t\tagc->setExposureMode(ExposureModeTable.at(idx));\n@@ -1334,9 +1386,19 @@ void IpaBase::reportMetadata(unsigned int ipaContext)\n \t}\n \n \tAgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked<AgcPrepareStatus>(\"agc.prepare_status\");\n-\tif (agcPrepareStatus) {\n-\t\tlibcameraMetadata_.set(controls::AeLocked, agcPrepareStatus->locked);\n+\tif (agcPrepareStatus)\n \t\tlibcameraMetadata_.set(controls::DigitalGain, agcPrepareStatus->digitalGain);\n+\n+\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n+\t\tcontroller_.getAlgorithm(\"agc\"));\n+\tif (agc) {\n+\t\tif (!agc->autoExposureEnabled() && !agc->autoGainEnabled())\n+\t\t\tlibcameraMetadata_.set(controls::AeState, controls::AeStateIdle);\n+\t\telse if (agcPrepareStatus)\n+\t\t\tlibcameraMetadata_.set(controls::AeState,\n+\t\t\t\t\t       agcPrepareStatus->locked\n+\t\t\t\t\t\t       ? controls::AeStateConverged\n+\t\t\t\t\t\t       : controls::AeStateSearching);\n \t}\n \n \tLuxStatus *luxStatus = rpiMetadata.getLocked<LuxStatus>(\"lux.status\");\ndiff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h\nindex c97828577602..fdaa10e6c176 100644\n--- a/src/ipa/rpi/controller/agc_algorithm.h\n+++ b/src/ipa/rpi/controller/agc_algorithm.h\n@@ -30,8 +30,12 @@ public:\n \tvirtual void setMeteringMode(std::string const &meteringModeName) = 0;\n \tvirtual void setExposureMode(std::string const &exposureModeName) = 0;\n \tvirtual void setConstraintMode(std::string const &contraintModeName) = 0;\n-\tvirtual void enableAuto() = 0;\n-\tvirtual void disableAuto() = 0;\n+\tvirtual void enableAutoExposure() = 0;\n+\tvirtual void disableAutoExposure() = 0;\n+\tvirtual bool autoExposureEnabled() const = 0;\n+\tvirtual void enableAutoGain() = 0;\n+\tvirtual void disableAutoGain() = 0;\n+\tvirtual bool autoGainEnabled() const = 0;\n \tvirtual void setActiveChannels(const std::vector<unsigned int> &activeChannels) = 0;\n };\n \ndiff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\nindex c48fdf156591..02bfdb4a5e22 100644\n--- a/src/ipa/rpi/controller/rpi/agc.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc.cpp\n@@ -74,22 +74,62 @@ int Agc::checkChannel(unsigned int channelIndex) const\n \treturn 0;\n }\n \n-void Agc::disableAuto()\n+void Agc::disableAutoExposure()\n {\n-\tLOG(RPiAgc, Debug) << \"disableAuto\";\n+\tLOG(RPiAgc, Debug) << \"disableAutoExposure\";\n \n \t/* All channels are enabled/disabled together. */\n \tfor (auto &data : channelData_)\n-\t\tdata.channel.disableAuto();\n+\t\tdata.channel.disableAutoExposure();\n }\n \n-void Agc::enableAuto()\n+void Agc::enableAutoExposure()\n {\n-\tLOG(RPiAgc, Debug) << \"enableAuto\";\n+\tLOG(RPiAgc, Debug) << \"enableAutoExposure\";\n \n \t/* All channels are enabled/disabled together. */\n \tfor (auto &data : channelData_)\n-\t\tdata.channel.enableAuto();\n+\t\tdata.channel.enableAutoExposure();\n+}\n+\n+bool Agc::autoExposureEnabled() const\n+{\n+\tLOG(RPiAgc, Debug) << \"autoExposureEnabled\";\n+\n+\t/*\n+\t * We always have at least one channel, and since all channels are\n+\t * enabled and disabled together we can simply check the first one.\n+\t */\n+\treturn channelData_[0].channel.autoExposureEnabled();\n+}\n+\n+void Agc::disableAutoGain()\n+{\n+\tLOG(RPiAgc, Debug) << \"disableAutoGain\";\n+\n+\t/* All channels are enabled/disabled together. */\n+\tfor (auto &data : channelData_)\n+\t\tdata.channel.disableAutoGain();\n+}\n+\n+void Agc::enableAutoGain()\n+{\n+\tLOG(RPiAgc, Debug) << \"enableAutoGain\";\n+\n+\t/* All channels are enabled/disabled together. */\n+\tfor (auto &data : channelData_)\n+\t\tdata.channel.enableAutoGain();\n+}\n+\n+bool Agc::autoGainEnabled() const\n+{\n+\tLOG(RPiAgc, Debug) << \"autoGainEnabled\";\n+\n+\t/*\n+\t * We always have at least one channel, and since all channels are\n+\t * enabled and disabled together we can simply check the first one.\n+\t */\n+\treturn channelData_[0].channel.autoGainEnabled();\n }\n \n unsigned int Agc::getConvergenceFrames() const\ndiff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h\nindex 3aca000bb4c2..c3a940bf697a 100644\n--- a/src/ipa/rpi/controller/rpi/agc.h\n+++ b/src/ipa/rpi/controller/rpi/agc.h\n@@ -40,8 +40,12 @@ public:\n \tvoid setMeteringMode(std::string const &meteringModeName) override;\n \tvoid setExposureMode(std::string const &exposureModeName) override;\n \tvoid setConstraintMode(std::string const &contraintModeName) override;\n-\tvoid enableAuto() override;\n-\tvoid disableAuto() override;\n+\tvoid enableAutoExposure() override;\n+\tvoid disableAutoExposure() override;\n+\tbool autoExposureEnabled() const override;\n+\tvoid enableAutoGain() override;\n+\tvoid disableAutoGain() override;\n+\tbool autoGainEnabled() const override;\n \tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata) override;\n \tvoid prepare(Metadata *imageMetadata) override;\n \tvoid process(StatisticsPtr &stats, Metadata *imageMetadata) override;\ndiff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp\nindex 79c459735dfd..e79184b7ac74 100644\n--- a/src/ipa/rpi/controller/rpi/agc_channel.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp\n@@ -319,18 +319,36 @@ int AgcChannel::read(const libcamera::YamlObject &params,\n \treturn 0;\n }\n \n-void AgcChannel::disableAuto()\n+void AgcChannel::disableAutoExposure()\n {\n \tfixedExposureTime_ = status_.exposureTime;\n+}\n+\n+void AgcChannel::enableAutoExposure()\n+{\n+\tfixedExposureTime_ = 0s;\n+}\n+\n+bool AgcChannel::autoExposureEnabled() const\n+{\n+\treturn fixedExposureTime_ == 0s;\n+}\n+\n+void AgcChannel::disableAutoGain()\n+{\n \tfixedAnalogueGain_ = status_.analogueGain;\n }\n \n-void AgcChannel::enableAuto()\n+void AgcChannel::enableAutoGain()\n {\n-\tfixedExposureTime_ = 0s;\n \tfixedAnalogueGain_ = 0;\n }\n \n+bool AgcChannel::autoGainEnabled() const\n+{\n+\treturn fixedAnalogueGain_ == 0;\n+}\n+\n unsigned int AgcChannel::getConvergenceFrames() const\n {\n \t/*\ndiff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h\nindex 734e5efd3266..fa697e6fa57d 100644\n--- a/src/ipa/rpi/controller/rpi/agc_channel.h\n+++ b/src/ipa/rpi/controller/rpi/agc_channel.h\n@@ -96,8 +96,12 @@ public:\n \tvoid setMeteringMode(std::string const &meteringModeName);\n \tvoid setExposureMode(std::string const &exposureModeName);\n \tvoid setConstraintMode(std::string const &contraintModeName);\n-\tvoid enableAuto();\n-\tvoid disableAuto();\n+\tvoid enableAutoExposure();\n+\tvoid disableAutoExposure();\n+\tbool autoExposureEnabled() const;\n+\tvoid enableAutoGain();\n+\tvoid disableAutoGain();\n+\tbool autoGainEnabled() const;\n \tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata);\n \tvoid prepare(Metadata *imageMetadata);\n \tvoid process(StatisticsPtr &stats, DeviceStatus const &deviceStatus, Metadata *imageMetadata,\n","prefixes":["v9","05/12"]}