{"id":18901,"url":"https://patchwork.libcamera.org/api/patches/18901/?format=json","web_url":"https://patchwork.libcamera.org/patch/18901/","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":"<20230728133700.3713-4-david.plowman@raspberrypi.com>","date":"2023-07-28T13:37:00","name":"[libcamera-devel,3/3] ipa: rpi: agc: Split AgcStatus into AgcStatus and AgcPrepareStatus","commit_ref":"250565b5e8848c80443422d9e44c0693aa381c81","pull_url":null,"state":"accepted","archived":false,"hash":"d366fccb0a6973f6dd4eef58ae9884d7ed45a165","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/?format=json","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/18901/mbox/","series":[{"id":3993,"url":"https://patchwork.libcamera.org/api/series/3993/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3993","date":"2023-07-28T13:36:57","name":"Raspberry Pi AGC tidying","version":1,"mbox":"https://patchwork.libcamera.org/series/3993/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/18901/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/18901/checks/","tags":{},"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 8F7C5C32AA\n\tfor ;\n\tFri, 28 Jul 2023 13:37:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36AFA627F6;\n\tFri, 28 Jul 2023 15:37:08 +0200 (CEST)","from mail-wr1-x430.google.com (mail-wr1-x430.google.com\n\t[IPv6:2a00:1450:4864:20::430])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0132E627F0\n\tfor ;\n\tFri, 28 Jul 2023 15:37:05 +0200 (CEST)","by mail-wr1-x430.google.com with SMTP id\n\tffacd0b85a97d-31771a876b5so1973766f8f.3\n\tfor ;\n\tFri, 28 Jul 2023 06:37:04 -0700 (PDT)","from pi4-davidp.pitowers.org\n\t([2a00:1098:3142:14:2bce:64d6:1a5c:49a2])\n\tby smtp.gmail.com with ESMTPSA id\n\ti15-20020adffdcf000000b003145559a691sm4847758wrs.41.2023.07.28.06.37.03\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 28 Jul 2023 06:37:03 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1690551428;\n\tbh=qfswQo62xoNwkb8FaJzYW9d9N3UAYhmrrsRaMkii3Hc=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=Km62fjI74f88/WgMIqqKNaIfTIqRlbGJen8UY2Kx5fC4FllECtRJiT87cFfP3GGxi\n\tA9TcXU+s9p6vmtywaGG5xAS+K+PihDjJF/0+Of9Gcm4cBep+NzTS1twnqTcFvnw6yS\n\t3jrjnV4t7sJ14KASnIB+FyQYqe1vU1U6Jc4s0tVnjvgaSjT81n7vxVo7KL1+2Q40Wj\n\tPercazi0BQTcHKYBXwcF0N4ng22Y+owPg8SRFSbVL5k/QV3lk3Or7Dt8T7YHyElTiQ\n\t4dSoDeCPMmhKHZCGCC6+F4DcKCxSKfUzdLmST1kklnaeahGL5l25sBpLbTkkz9BKo3\n\tjb1W4tkUHlJFQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1690551424; x=1691156224;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=m9XmLKKQDUXgVbu1Hi4NS602c6ORw8D/if6U++CcbuI=;\n\tb=ef2YYTg41hVSAcvWRv2MZ+9KpLPD/lnp8bg9aNVMxcv3sItHnblPo1FcT4aeMm2jTn\n\tXg/dvVlvlIIV8kWmgRJE1O48F2/pbEoK99puX0MJKIbKEqLshE4Xllys6pvP8iGzzBTm\n\t+zlAlG9XPeKSbU9I7TNIiFAIe+EpMiGvDmRCswGjRSSD9Z7yqZFscM43XyEEy/cswZoC\n\toRu5AqABDbQ7jMiI2cFk3NSmT5Y9wD+FE+OX2K4gt5z9aenDStpD8whjDFRBOIkMHOuu\n\t8I1jkdf2JeOlkbUdmryYdmQI67MEp/FJICDKC7mmhyBI/7N/ZAz60fr9dFCBXEDX+zx1\n\tC36w=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"ef2YYTg4\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1690551424; x=1691156224;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=m9XmLKKQDUXgVbu1Hi4NS602c6ORw8D/if6U++CcbuI=;\n\tb=Z3o6G46T1q8Ut/XIJUsm1rb0p1JAQc6PPTQyRPwcQuqPtlY2D2kMOCMLYk092p0n93\n\tX1q5Zez5axMxHI+ssKab7VeWNdue2TXpyi1X0h5OqihS3Scq4ibcM9OuvHIygzGOQfeC\n\tff0o0ztsvK/+/0EnMn8FiV2DRXuDGTDlEgQyzo6XLP5e2/2wilDNF/scFRQ2i01t/2tp\n\tC/nxIOjDk7KdOBrpARf6hAhgZsrizAGVbbzD8z+39m5DuGTyg2PY5+0k7v2a+mq3mDR0\n\tPHEaqRCKk4FU+Zcl7le3+kQRfGgCslQjiuwHCvaS0bAvK1cI4MJiLGQpwuazmiOUJKTJ\n\t9fzA==","X-Gm-Message-State":"ABy/qLZZqwoQPgG0hjgsE5US1xd5fOsep8N/F9HlYYWIS+OmCQecF01h\n\t1zoKgLsdtlUuGMrmq7VYv85xag/neEScDH60XCs=","X-Google-Smtp-Source":"APBJJlEDwIh9/ebyKjrWblQ9tIcXz+ztQ9q7CH/0+m3bBNjKFrXk632bQyUQ6ZtZjtQf3KaG3Fru+g==","X-Received":"by 2002:a05:6000:48:b0:313:ef08:c83b with SMTP id\n\tk8-20020a056000004800b00313ef08c83bmr1751532wrx.56.1690551424308; \n\tFri, 28 Jul 2023 06:37:04 -0700 (PDT)","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 28 Jul 2023 14:37:00 +0100","Message-Id":"<20230728133700.3713-4-david.plowman@raspberrypi.com>","X-Mailer":"git-send-email 2.30.2","In-Reply-To":"<20230728133700.3713-1-david.plowman@raspberrypi.com>","References":"<20230728133700.3713-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 3/3] ipa: rpi: agc: Split AgcStatus into\n\tAgcStatus and AgcPrepareStatus","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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The Agc::process() function returns an AgcStatus object in the\nmetadata as before, but Agc::prepare() is changed to return the values\nit computes in a separate AgcPrepareStatus object (under the new tag\n\"agc.prepare_status\").\n\nThe \"digitalGain\" and \"locked\" fields are moved from AgcStatus to\nAgcPrepareStatus.\n\nThis will be useful going forward as we can be more flexible about the\norder in which prepare() and process() are called, without them\ntrampling on each other's results.\n\nSigned-off-by: David Plowman \n---\n src/ipa/rpi/common/ipa_base.cpp | 8 ++++----\n src/ipa/rpi/controller/agc_status.h | 9 +++++++--\n src/ipa/rpi/controller/rpi/agc.cpp | 20 +++++++++++---------\n src/ipa/rpi/controller/rpi/agc.h | 2 +-\n src/ipa/rpi/vc4/vc4.cpp | 6 +++---\n 5 files changed, 26 insertions(+), 19 deletions(-)","diff":"diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\nindex f40f2e71..6ae84cc6 100644\n--- a/src/ipa/rpi/common/ipa_base.cpp\n+++ b/src/ipa/rpi/common/ipa_base.cpp\n@@ -1151,10 +1151,10 @@ void IpaBase::reportMetadata(unsigned int ipaContext)\n \t\t\tlibcameraMetadata_.set(controls::LensPosition, *deviceStatus->lensPosition);\n \t}\n \n-\tAgcStatus *agcStatus = rpiMetadata.getLocked(\"agc.status\");\n-\tif (agcStatus) {\n-\t\tlibcameraMetadata_.set(controls::AeLocked, agcStatus->locked);\n-\t\tlibcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain);\n+\tAgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked(\"agc.prepare_status\");\n+\tif (agcPrepareStatus) {\n+\t\tlibcameraMetadata_.set(controls::AeLocked, agcPrepareStatus->locked);\n+\t\tlibcameraMetadata_.set(controls::DigitalGain, agcPrepareStatus->digitalGain);\n \t}\n \n \tLuxStatus *luxStatus = rpiMetadata.getLocked(\"lux.status\");\ndiff --git a/src/ipa/rpi/controller/agc_status.h b/src/ipa/rpi/controller/agc_status.h\nindex 6c112e76..597eddd7 100644\n--- a/src/ipa/rpi/controller/agc_status.h\n+++ b/src/ipa/rpi/controller/agc_status.h\n@@ -11,8 +11,10 @@\n #include \n \n /*\n- * The AGC algorithm should post the following structure into the image's\n- * \"agc.status\" metadata.\n+ * The AGC algorithm process method should post an AgcStatus into the image\n+ * metadata under the tag \"agc.status\".\n+ * The AGC algorithm prepare method should post an AgcPrepareStatus instead\n+ * under \"agc.prepare_status\".\n */\n \n /*\n@@ -34,6 +36,9 @@ struct AgcStatus {\n \tint floatingRegionEnable;\n \tlibcamera::utils::Duration fixedShutter;\n \tdouble fixedAnalogueGain;\n+};\n+\n+struct AgcPrepareStatus {\n \tdouble digitalGain;\n \tint locked;\n };\ndiff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\nindex 6087fc60..7b02972a 100644\n--- a/src/ipa/rpi/controller/rpi/agc.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc.cpp\n@@ -419,11 +419,13 @@ void Agc::prepare(Metadata *imageMetadata)\n {\n \tDuration totalExposureValue = status_.totalExposureValue;\n \tAgcStatus delayedStatus;\n+\tAgcPrepareStatus prepareStatus;\n \n \tif (!imageMetadata->get(\"agc.delayed_status\", delayedStatus))\n \t\ttotalExposureValue = delayedStatus.totalExposureValue;\n \n-\tstatus_.digitalGain = 1.0;\n+\tprepareStatus.digitalGain = 1.0;\n+\tprepareStatus.locked = false;\n \n \tif (status_.totalExposureValue) {\n \t\t/* Process has run, so we have meaningful values. */\n@@ -432,23 +434,23 @@ void Agc::prepare(Metadata *imageMetadata)\n \t\t\tDuration actualExposure = deviceStatus.shutterSpeed *\n \t\t\t\t\t\t deviceStatus.analogueGain;\n \t\t\tif (actualExposure) {\n-\t\t\t\tstatus_.digitalGain = totalExposureValue / actualExposure;\n+\t\t\t\tdouble digitalGain = totalExposureValue / actualExposure;\n \t\t\t\tLOG(RPiAgc, Debug) << \"Want total exposure \" << totalExposureValue;\n \t\t\t\t/*\n \t\t\t\t * Never ask for a gain < 1.0, and also impose\n \t\t\t\t * some upper limit. Make it customisable?\n \t\t\t\t */\n-\t\t\t\tstatus_.digitalGain = std::max(1.0, std::min(status_.digitalGain, 4.0));\n+\t\t\t\tprepareStatus.digitalGain = std::max(1.0, std::min(digitalGain, 4.0));\n \t\t\t\tLOG(RPiAgc, Debug) << \"Actual exposure \" << actualExposure;\n-\t\t\t\tLOG(RPiAgc, Debug) << \"Use digitalGain \" << status_.digitalGain;\n+\t\t\t\tLOG(RPiAgc, Debug) << \"Use digitalGain \" << prepareStatus.digitalGain;\n \t\t\t\tLOG(RPiAgc, Debug) << \"Effective exposure \"\n-\t\t\t\t\t\t << actualExposure * status_.digitalGain;\n+\t\t\t\t\t\t << actualExposure * prepareStatus.digitalGain;\n \t\t\t\t/* Decide whether AEC/AGC has converged. */\n-\t\t\t\tupdateLockStatus(deviceStatus);\n+\t\t\t\tprepareStatus.locked = updateLockStatus(deviceStatus);\n \t\t\t}\n \t\t} else\n \t\t\tLOG(RPiAgc, Warning) << name() << \": no device metadata\";\n-\t\timageMetadata->set(\"agc.status\", status_);\n+\t\timageMetadata->set(\"agc.prepare_status\", prepareStatus);\n \t}\n }\n \n@@ -486,7 +488,7 @@ void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata)\n \twriteAndFinish(imageMetadata, desaturate);\n }\n \n-void Agc::updateLockStatus(DeviceStatus const &deviceStatus)\n+bool Agc::updateLockStatus(DeviceStatus const &deviceStatus)\n {\n \tconst double errorFactor = 0.10; /* make these customisable? */\n \tconst int maxLockCount = 5;\n@@ -522,7 +524,7 @@ void Agc::updateLockStatus(DeviceStatus const &deviceStatus)\n \tlastTargetExposure_ = status_.targetExposureValue;\n \n \tLOG(RPiAgc, Debug) << \"Lock count updated to \" << lockCount_;\n-\tstatus_.locked = lockCount_ == maxLockCount;\n+\treturn lockCount_ == maxLockCount;\n }\n \n void Agc::housekeepConfig()\ndiff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h\nindex b7122de3..aaf77c8f 100644\n--- a/src/ipa/rpi/controller/rpi/agc.h\n+++ b/src/ipa/rpi/controller/rpi/agc.h\n@@ -85,7 +85,7 @@ public:\n \tvoid process(StatisticsPtr &stats, Metadata *imageMetadata) override;\n \n private:\n-\tvoid updateLockStatus(DeviceStatus const &deviceStatus);\n+\tbool updateLockStatus(DeviceStatus const &deviceStatus);\n \tAgcConfig config_;\n \tvoid housekeepConfig();\n \tvoid fetchCurrentExposure(Metadata *imageMetadata);\ndiff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp\nindex 3eea40a6..1de0d3cc 100644\n--- a/src/ipa/rpi/vc4/vc4.cpp\n+++ b/src/ipa/rpi/vc4/vc4.cpp\n@@ -60,7 +60,7 @@ private:\n \tbool validateIspControls();\n \n \tvoid applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);\n-\tvoid applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls);\n+\tvoid applyDG(const struct AgcPrepareStatus *dgStatus, ControlList &ctrls);\n \tvoid applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls);\n \tvoid applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls);\n \tvoid applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls);\n@@ -142,7 +142,7 @@ void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms,\n \tif (ccmStatus)\n \t\tapplyCCM(ccmStatus, ctrls);\n \n-\tAgcStatus *dgStatus = rpiMetadata.getLocked(\"agc.status\");\n+\tAgcPrepareStatus *dgStatus = rpiMetadata.getLocked(\"agc.prepare_status\");\n \tif (dgStatus)\n \t\tapplyDG(dgStatus, ctrls);\n \n@@ -284,7 +284,7 @@ void IpaVc4::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n \t\t static_cast(awbStatus->gainB * 1000));\n }\n \n-void IpaVc4::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)\n+void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus, ControlList &ctrls)\n {\n \tctrls.set(V4L2_CID_DIGITAL_GAIN,\n \t\t static_cast(dgStatus->digitalGain * 1000));\n","prefixes":["libcamera-devel","3/3"]}