{"id":18955,"url":"https://patchwork.libcamera.org/api/1.1/patches/18955/?format=json","web_url":"https://patchwork.libcamera.org/patch/18955/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20230823120915.18621-6-david.plowman@raspberrypi.com>","date":"2023-08-23T12:09:15","name":"[libcamera-devel,v2,5/5] ipa: rpi: agc: Use channel constraints in the AGC algorithm","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"d6bc7687fe0154b43f9ba32d60221f85bd9f5633","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/1.1/people/42/?format=json","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/18955/mbox/","series":[{"id":4006,"url":"https://patchwork.libcamera.org/api/1.1/series/4006/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4006","date":"2023-08-23T12:09:10","name":"Multi-channel AGC","version":2,"mbox":"https://patchwork.libcamera.org/series/4006/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/18955/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/18955/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 1EDD5BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Aug 2023 12:09:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 795A6628D3;\n\tWed, 23 Aug 2023 14:09:28 +0200 (CEST)","from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com\n\t[IPv6:2a00:1450:4864:20::12c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF094627E0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Aug 2023 14:09:23 +0200 (CEST)","by mail-lf1-x12c.google.com with SMTP id\n\t2adb3069b0e04-4fe15bfb1adso8368046e87.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Aug 2023 05:09:23 -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\tu1-20020a7bcb01000000b003fefcbe7fa8sm3800589wmj.28.2023.08.23.05.09.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 23 Aug 2023 05:09:21 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1692792568;\n\tbh=GXR53e9ZMMN7FEeQLxwMvMJw0aKoGYRWt1/n+HsBzS4=;\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=W1kNP57zjmybdPjGl90EghtcscMZ1+I2OjWNLD/AEOxelgG9jDyQOH6qIjRphaJAA\n\tlevcfdipczEKFx2caWXof3KCOQ+l0c0j9z7+nY1NLz1Ox7L3yF4O1Y8xj805llryFa\n\tHkNOhWX+e9rbm1voqGNgUsdkgm7J+85SeStauyj8W76jUPrztEjKgPtLdV5nnhRDc6\n\tlBbFhF3AIxdRZGCykIW1g7sBk+dFY/7NYKeKoHma17Vnv1puydKp43NTRB05X90+2s\n\tZIjolHfHMlA+ZcD4SP9HxeA7pCjhnTLbKJtgaQs8DE3Z54pXXWJBfpt/G3dL4uiO9A\n\twFcVsGVe1r5tA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1692792563; x=1693397363;\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=WVYw2x9Nser63k+jpeDj6sKU4/vs2CXhB/RQyQOHhlA=;\n\tb=tf+TFdtTYq9S8TlGtS8llERvtXTHkGQBoGTcg+lbngxx5+dz5AVOz7S5e+gujRIYRo\n\tEHCRAi2Dqvl+Q8uDgZ+F47HHjTdI7pzcOOSs9RgYVwltkTENFhAKOXbSRlusW9lPB/Jc\n\tetYeUFAlh4n0I2Rd3R1vmlto2KMoktXet9p/sP9/R7KsM2RYz2/61ryFJsOX/kFctQU9\n\tBYRxhb39pnaaDsFyLTO5HrBEw0cwSm9oQofuOM9667jQrv+XXRtsiZhyeT8ar8Kp5rWH\n\tmvHpBW1Jb3AxPgOAaO2ezBUMIwg2gGWbVhCsODwnLqTAHfw0x+ltt4AkAAwPKJix7zgD\n\t9Xzg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"tf+TFdtT\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1692792563; x=1693397363;\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=WVYw2x9Nser63k+jpeDj6sKU4/vs2CXhB/RQyQOHhlA=;\n\tb=PP2YAnkcUhruAAr+PhyqCrdQ5Oahh4w+im/8l6gGXkvEUX/pPNVIRLs75HBMuSYDk4\n\tte1FvisQhjdfNQA1PAJ3bCq1vyj4+lfh3grMHUwOV6cYjT3RqXw6WMtMZiBQHb6PrmHO\n\tfYdi4ihsRuyFSBBIRtA0YYz+6JE+8X0dxq/CKZfZwNGl33n4VTDtx46kCgk7i1yOoZ4Y\n\tfzQk7Dmixk/8MGFHB4ACrKULOJ3lemXqK2P7F+x7/adL5Z7TVWWIlG9bIpRSQg3eZCBD\n\tHjttalnlOyjqZ83IL16V/Y0eHsb1IQEZvzQA+3Ba2MupXNCzvZbn5UgiQAeUDVIIjGS+\n\tlIeA==","X-Gm-Message-State":"AOJu0YwbwF199QmHMAZ5zL+aWr6UsFBysGjvL97j9qyEyyHfi4x/zsRy\n\tJNHI0J1FJ2BfO60InG9yPA6m7bW4p8TWBoxDnrc=","X-Google-Smtp-Source":"AGHT+IENdKcJs+SMKhq+egJGF/BWKb845VBkMQqlk9zULgo1tzAnt4k8vlS/HpiWYY8sXIEcjhpb6g==","X-Received":"by 2002:a05:6512:3d1a:b0:4fe:18be:ef37 with SMTP id\n\td26-20020a0565123d1a00b004fe18beef37mr11332691lfv.61.1692792562662; \n\tWed, 23 Aug 2023 05:09:22 -0700 (PDT)","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 23 Aug 2023 13:09:15 +0100","Message-Id":"<20230823120915.18621-6-david.plowman@raspberrypi.com>","X-Mailer":"git-send-email 2.30.2","In-Reply-To":"<20230823120915.18621-1-david.plowman@raspberrypi.com>","References":"<20230823120915.18621-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 5/5] ipa: rpi: agc: Use channel\n\tconstraints in the AGC algorithm","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>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"Whenever we run Agc::process(), we store the most recent total\nexposure requested for each channel.\n\nWith these values we can apply the channel constraints after\ntime-filtering the requested total exposure, but before working out\nhow much digital gain is needed.\n\nSigned-off-by: David Plowman <david.plowman@raspberrypi.com>\nReviewed-by: Naushir Patuck <naush@raspberrypi.com>\n---\n src/ipa/rpi/controller/rpi/agc.cpp         | 21 ++++--\n src/ipa/rpi/controller/rpi/agc.h           |  1 +\n src/ipa/rpi/controller/rpi/agc_channel.cpp | 75 +++++++++++++++++++---\n src/ipa/rpi/controller/rpi/agc_channel.h   |  8 ++-\n 4 files changed, 88 insertions(+), 17 deletions(-)","diff":"diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\nindex 7e627bba..3c730d4c 100644\n--- a/src/ipa/rpi/controller/rpi/agc.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc.cpp\n@@ -40,6 +40,7 @@ int Agc::read(const libcamera::YamlObject &params)\n \t */\n \tif (!params.contains(\"channels\")) {\n \t\tLOG(RPiAgc, Debug) << \"Single channel only\";\n+\t\tchannelTotalExposures_.resize(1, 0s);\n \t\tchannelData_.emplace_back();\n \t\treturn channelData_.back().channel.read(params, getHardwareConfig());\n \t}\n@@ -59,6 +60,8 @@ int Agc::read(const libcamera::YamlObject &params)\n \t\treturn -1;\n \t}\n \n+\tchannelTotalExposures_.resize(channelData_.size(), 0s);\n+\n \treturn 0;\n }\n \n@@ -234,15 +237,21 @@ static void getChannelIndex(Metadata *metadata, const char *message, unsigned in\n \t\tLOG(RPiAgc, Debug) << message;\n }\n \n-static void setChannelIndex(Metadata *metadata, const char *message, unsigned int channelIndex)\n+static libcamera::utils::Duration\n+setChannelIndex(Metadata *metadata, const char *message, unsigned int channelIndex)\n {\n \tstd::unique_lock<RPiController::Metadata> lock(*metadata);\n \tAgcStatus *status = metadata->getLocked<AgcStatus>(\"agc.status\");\n-\tif (status)\n+\tlibcamera::utils::Duration dur = 0s;\n+\n+\tif (status) {\n \t\tstatus->channel = channelIndex;\n-\telse\n+\t\tdur = status->totalExposureValue;\n+\t} else\n \t\t/* This does happen at startup, otherwise it would be a Warning or Error. */\n \t\tLOG(RPiAgc, Debug) << message;\n+\n+\treturn dur;\n }\n \n void Agc::prepare(Metadata *imageMetadata)\n@@ -306,8 +315,10 @@ void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata)\n \t\t/* Can also happen when new channels start. */\n \t\tLOG(RPiAgc, Debug) << \"process: channel \" << channelIndex << \" not seen yet\";\n \n-\tchannelData.channel.process(stats, &deviceStatus, imageMetadata);\n-\tsetChannelIndex(imageMetadata, \"process: no AGC status found\", channelIndex);\n+\tchannelData.channel.process(stats, &deviceStatus, imageMetadata, channelTotalExposures_);\n+\tauto dur = setChannelIndex(imageMetadata, \"process: no AGC status found\", channelIndex);\n+\tif (dur)\n+\t\tchannelTotalExposures_[channelIndex] = dur;\n \n \t/* And onto the next channel for the next call. */\n \tindex_ = (index_ + 1) % activeChannels_.size();\ndiff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h\nindex 2eed2bab..e301c5cb 100644\n--- a/src/ipa/rpi/controller/rpi/agc.h\n+++ b/src/ipa/rpi/controller/rpi/agc.h\n@@ -54,6 +54,7 @@ private:\n \tstd::vector<AgcChannelData> channelData_;\n \tstd::vector<unsigned int> activeChannels_;\n \tunsigned int index_; /* index into the activeChannels_ */\n+\tAgcChannelTotalExposures channelTotalExposures_;\n };\n \n } /* namespace RPiController */\ndiff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp\nindex 44198c2c..54fd08e8 100644\n--- a/src/ipa/rpi/controller/rpi/agc_channel.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp\n@@ -175,6 +175,11 @@ readConstraintModes(std::map<std::string, AgcConstraintMode> &constraintModes,\n \n int AgcChannelConstraint::read(const libcamera::YamlObject &params)\n {\n+\tauto channelValue = params[\"channel\"].get<unsigned int>();\n+\tif (!channelValue)\n+\t\treturn -EINVAL;\n+\tchannel = *channelValue;\n+\n \tstd::string boundString = params[\"bound\"].get<std::string>(\"\");\n \ttransform(boundString.begin(), boundString.end(),\n \t\t  boundString.begin(), ::toupper);\n@@ -184,10 +189,10 @@ int AgcChannelConstraint::read(const libcamera::YamlObject &params)\n \t}\n \tbound = boundString == \"UPPER\" ? Bound::UPPER : Bound::LOWER;\n \n-\tauto value = params[\"factor\"].get<double>();\n-\tif (!value)\n+\tauto factorValue = params[\"factor\"].get<double>();\n+\tif (!factorValue)\n \t\treturn -EINVAL;\n-\tfactor = *value;\n+\tfactor = *factorValue;\n \n \treturn 0;\n }\n@@ -231,6 +236,7 @@ int AgcConfig::read(const libcamera::YamlObject &params)\n \t\tret = readChannelConstraints(channelConstraints, params[\"channel_constraints\"]);\n \t\tif (ret)\n \t\t\treturn ret;\n+\t\tLOG(RPiAgc, Info) << \"Read \" << channelConstraints.size() << \" channel constraints\";\n \t}\n \n \tret = yTarget.read(params[\"y_target\"]);\n@@ -489,7 +495,9 @@ void AgcChannel::prepare(Metadata *imageMetadata)\n \t}\n }\n \n-void AgcChannel::process(StatisticsPtr &stats, const DeviceStatus *deviceStatus, Metadata *imageMetadata)\n+void AgcChannel::process(StatisticsPtr &stats, const DeviceStatus *deviceStatus,\n+\t\t\t Metadata *imageMetadata,\n+\t\t\t const AgcChannelTotalExposures &channelTotalExposures)\n {\n \tframeCount_++;\n \t/*\n@@ -508,12 +516,17 @@ void AgcChannel::process(StatisticsPtr &stats, const DeviceStatus *deviceStatus,\n \tcomputeTargetExposure(gain);\n \t/* The results have to be filtered so as not to change too rapidly. */\n \tfilterExposure();\n+\t/*\n+\t * We may be asked to limit the exposure using other channels. If another channel\n+\t * determines our upper bound we may want to know this later.\n+\t */\n+\tbool channelBound = applyChannelConstraints(channelTotalExposures);\n \t/*\n \t * Some of the exposure has to be applied as digital gain, so work out\n-\t * what that is. This function also tells us whether it's decided to\n-\t * \"desaturate\" the image more quickly.\n+\t * what that is. It also tells us whether it's trying to desaturate the image\n+\t * more quickly, which can only happen when another channel is not limiting us.\n \t */\n-\tbool desaturate = applyDigitalGain(gain, targetY);\n+\tbool desaturate = applyDigitalGain(gain, targetY, channelBound);\n \t/*\n \t * The last thing is to divide up the exposure value into a shutter time\n \t * and analogue gain, according to the current exposure mode.\n@@ -792,7 +805,49 @@ void AgcChannel::computeTargetExposure(double gain)\n \tLOG(RPiAgc, Debug) << \"Target totalExposure \" << target_.totalExposure;\n }\n \n-bool AgcChannel::applyDigitalGain(double gain, double targetY)\n+bool AgcChannel::applyChannelConstraints(const AgcChannelTotalExposures &channelTotalExposures)\n+{\n+\tbool channelBound = false;\n+\tLOG(RPiAgc, Debug)\n+\t\t<< \"Total exposure before channel constraints \" << filtered_.totalExposure;\n+\n+\tfor (const auto &constraint : config_.channelConstraints) {\n+\t\tLOG(RPiAgc, Debug)\n+\t\t\t<< \"Check constraint: channel \" << constraint.channel << \" bound \"\n+\t\t\t<< (constraint.bound == AgcChannelConstraint::Bound::UPPER ? \"UPPER\" : \"LOWER\")\n+\t\t\t<< \" factor \" << constraint.factor;\n+\t\tif (constraint.channel >= channelTotalExposures.size() ||\n+\t\t    !channelTotalExposures[constraint.channel]) {\n+\t\t\tLOG(RPiAgc, Debug) << \"no such channel - skipped\";\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (channelTotalExposures[constraint.channel] == 0s) {\n+\t\t\tLOG(RPiAgc, Debug) << \"zero exposure - skipped\";\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tlibcamera::utils::Duration limitExposure =\n+\t\t\tchannelTotalExposures[constraint.channel] * constraint.factor;\n+\t\tLOG(RPiAgc, Debug) << \"Limit exposure \" << limitExposure;\n+\t\tif ((constraint.bound == AgcChannelConstraint::Bound::UPPER &&\n+\t\t     filtered_.totalExposure > limitExposure) ||\n+\t\t    (constraint.bound == AgcChannelConstraint::Bound::LOWER &&\n+\t\t     filtered_.totalExposure < limitExposure)) {\n+\t\t\tfiltered_.totalExposure = limitExposure;\n+\t\t\tLOG(RPiAgc, Debug) << \"Applies\";\n+\t\t\tchannelBound = true;\n+\t\t} else\n+\t\t\tLOG(RPiAgc, Debug) << \"Does not apply\";\n+\t}\n+\n+\tLOG(RPiAgc, Debug)\n+\t\t<< \"Total exposure after channel constraints \" << filtered_.totalExposure;\n+\n+\treturn channelBound;\n+}\n+\n+bool AgcChannel::applyDigitalGain(double gain, double targetY, bool channelBound)\n {\n \tdouble minColourGain = std::min({ awb_.gainR, awb_.gainG, awb_.gainB, 1.0 });\n \tASSERT(minColourGain != 0.0);\n@@ -812,8 +867,8 @@ bool AgcChannel::applyDigitalGain(double gain, double targetY)\n \t * quickly (and we then approach the correct value more quickly from\n \t * below).\n \t */\n-\tbool desaturate = targetY > config_.fastReduceThreshold &&\n-\t\t\t  gain < sqrt(targetY);\n+\tbool desaturate = !channelBound &&\n+\t\t\t  targetY > config_.fastReduceThreshold && gain < sqrt(targetY);\n \tif (desaturate)\n \t\tdg /= config_.fastReduceThreshold;\n \tLOG(RPiAgc, Debug) << \"Digital gain \" << dg << \" desaturate? \" << desaturate;\ndiff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h\nindex 446125ef..c3f701eb 100644\n--- a/src/ipa/rpi/controller/rpi/agc_channel.h\n+++ b/src/ipa/rpi/controller/rpi/agc_channel.h\n@@ -19,6 +19,8 @@\n \n namespace RPiController {\n \n+using AgcChannelTotalExposures = std::vector<libcamera::utils::Duration>;\n+\n struct AgcMeteringMode {\n \tstd::vector<double> weights;\n \tint read(const libcamera::YamlObject &params);\n@@ -93,7 +95,8 @@ public:\n \tvoid disableAuto();\n \tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata);\n \tvoid prepare(Metadata *imageMetadata);\n-\tvoid process(StatisticsPtr &stats, const DeviceStatus *deviceStatus, Metadata *imageMetadata);\n+\tvoid process(StatisticsPtr &stats, const DeviceStatus *deviceStatus, Metadata *imageMetadata,\n+\t\t     const AgcChannelTotalExposures &channelTotalExposures);\n \n private:\n \tbool updateLockStatus(DeviceStatus const &deviceStatus);\n@@ -105,7 +108,8 @@ private:\n \t\t\t double &gain, double &targetY);\n \tvoid computeTargetExposure(double gain);\n \tvoid filterExposure();\n-\tbool applyDigitalGain(double gain, double targetY);\n+\tbool applyChannelConstraints(const AgcChannelTotalExposures &channelTotalExposures);\n+\tbool applyDigitalGain(double gain, double targetY, bool channelBound);\n \tvoid divideUpExposure();\n \tvoid writeAndFinish(Metadata *imageMetadata, bool desaturate);\n \tlibcamera::utils::Duration limitShutter(libcamera::utils::Duration shutter);\n","prefixes":["libcamera-devel","v2","5/5"]}