Patch Detail
Show a patch.
GET /api/1.1/patches/18909/?format=api
{ "id": 18909, "url": "https://patchwork.libcamera.org/api/1.1/patches/18909/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18909/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20230731094641.73646-6-david.plowman@raspberrypi.com>", "date": "2023-07-31T09:46:41", "name": "[libcamera-devel,5/5] ipa: rpi: agc: Use channel constraints in the AGC algorithm", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "65449d8e8cf4cbfd400a0e2dc6d88a726455b444", "submitter": { "id": 42, "url": "https://patchwork.libcamera.org/api/1.1/people/42/?format=api", "name": "David Plowman", "email": "david.plowman@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18909/mbox/", "series": [ { "id": 3996, "url": "https://patchwork.libcamera.org/api/1.1/series/3996/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3996", "date": "2023-07-31T09:46:36", "name": "Multi-channel AGC", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3996/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18909/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18909/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 BDD3FBDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 31 Jul 2023 09:46:54 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59DFD627F5;\n\tMon, 31 Jul 2023 11:46:54 +0200 (CEST)", "from mail-wm1-x331.google.com (mail-wm1-x331.google.com\n\t[IPv6:2a00:1450:4864:20::331])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7FB4C627F4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 31 Jul 2023 11:46:48 +0200 (CEST)", "by mail-wm1-x331.google.com with SMTP id\n\t5b1f17b1804b1-3fbc63c2e84so48072575e9.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 31 Jul 2023 02:46:48 -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\t9-20020a05600c240900b003fa98908014sm13612838wmp.8.2023.07.31.02.46.47\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 31 Jul 2023 02:46:47 -0700 (PDT)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1690796814;\n\tbh=/uTwRHVyrFT2HTsBf8wu4vdEWf1dVb1Bcnb02Sh6A5o=;\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=yD74CfNRbAW6vXPAbdK6Cm0h1faX8gi7EmFkKU1x+h5Cn7VXL1EOzpp7MnIFS//jM\n\t+tIq/orQxiTB1cCTYUSgvefQqPZf20ZqXiEMq3p+HBehxLo3fQ6YOwjRr4QT8rdGpW\n\tWQGhBPfJUh3bmu84Rh/lDmluMiM9xB3O7R8TgNwBFW7HTGdBERPLgIcLLQ2TOxou+9\n\tOuXwMlUepwJ9FhJiThDusPqTNE1C5HNzuqtNncsGk5ujqNpOQTY2P8W/uHPugrBQWt\n\tkWyF1o/VptwO2Tp8rBnkgGH78gdI5NhDH9kD2KCL4syBxBGFAhEQR4muE3BZKC3oI/\n\tD0v5M4qWV82tA==", "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1690796808; x=1691401608;\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=4rXIM8Dmbucq8jQtDzcDP9vwhEVUIMbE0QaT2+4DNc4=;\n\tb=p3QU2A9KbeSr6QkEWyNc2YMZktFrGADj36Et2ZP9Mka7wPKvkhUWSaDzDH+bE14S4l\n\tQL6oaWwfPhWmH8w8SWW5dlmlXnkRFZ9g8TSnsuzqtpwcDBheW18S6k3Oj/KVTRBEgtUb\n\tZTxcvojbcs3xHg8o85EIqo6C7TvHA4eZrOLG1O9SfBnUwAURw+WtqcjIKeT3q+D47X42\n\tRdAh41XGZgolcDg4ev3yb3WzDsnfsUB11iC7fmTljTCmEWcmzfu7aF8vSJmZ0p6Q/B1M\n\to/qvQQAA6ZS8SBtxTC7Ni3kY5RDrt3Fim3D3iGV4PEsWRKKSIyTiYL27gunKFLaYiPUC\n\tAMsg==" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"p3QU2A9K\"; dkim-atps=neutral", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1690796808; x=1691401608;\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=4rXIM8Dmbucq8jQtDzcDP9vwhEVUIMbE0QaT2+4DNc4=;\n\tb=VsHeVO/3kyXodFgybR5zaY+UPtFV8eZxG+5YZLPTv6BVB1XFgN4kmEczkojusSUIWw\n\tly03/SonGZsU86VEBQmlCj7+mx922ZxXg3sozwp/10/MVq8UJm6EM3BhqSPEOPErO2AT\n\tqWkj0WEb/fYjBThLqBKGX/u3Udfx98SGhr0YdaSTFFVOFPquYmgRcbiUJ8E4TKcq60YG\n\tEl4c+ceW1qQD/e/zfzAUBtnN3I3ePLTGGRgg0TwmmrZOWCY0J5NV8WU92HOoS0MSHBAP\n\tGPF4UXgOb61WkFMCUwLjE2EjS/KV7Mbz6aEZ22NQmoQraxxlSGlOHmkTfmyA9MaFnm5U\n\t4y6g==", "X-Gm-Message-State": "ABy/qLabunypinXJicO0KAH/7WrwOU4hQ2yZPlFx944IKhYrOE4Vba3F\n\tdFksrW5ekKk6vdpdRYuefSRbdREv5HP109cirdk=", "X-Google-Smtp-Source": "APBJJlHgTehi+Q+xi1vtwdEQPJBcl1MwlPNr9OJfrlRNwva6Z0veOvlS0iFWhCgp5mMSorXMpjsRhA==", "X-Received": "by 2002:a1c:e915:0:b0:3fc:2e8:ea8b with SMTP id\n\tq21-20020a1ce915000000b003fc02e8ea8bmr7767293wmc.28.1690796807862; \n\tMon, 31 Jul 2023 02:46:47 -0700 (PDT)", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 31 Jul 2023 10:46:41 +0100", "Message-Id": "<20230731094641.73646-6-david.plowman@raspberrypi.com>", "X-Mailer": "git-send-email 2.30.2", "In-Reply-To": "<20230731094641.73646-1-david.plowman@raspberrypi.com>", "References": "<20230731094641.73646-1-david.plowman@raspberrypi.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 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>\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 | 76 ++++++++++++++++++----\n src/ipa/rpi/controller/rpi/agc_channel.h | 8 ++-\n src/ipa/rpi/vc4/data/imx477.json | 3 +-\n 5 files changed, 90 insertions(+), 19 deletions(-)", "diff": "diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\nindex 7e627bba..7077fbff 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 ¶ms)\n \t */\n \tif (!params.contains(\"channels\")) {\n \t\tLOG(RPiAgc, Debug) << \"Single channel only\";\n+\t\tchannelResults_.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 ¶ms)\n \t\treturn -1;\n \t}\n \n+\tchannelResults_.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, channelResults_);\n+\tauto dur = setChannelIndex(imageMetadata, \"process: no AGC status found\", channelIndex);\n+\tif (dur)\n+\t\tchannelResults_[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..acd6dc2f 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+\tAgcChannelResults channelResults_;\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 ed8a3b30..4e60802c 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 ¶ms)\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,15 +189,15 @@ int AgcChannelConstraint::read(const libcamera::YamlObject ¶ms)\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 \n-int readChannelConstraints(std::vector<AgcChannelConstraint> channelConstraints,\n+int readChannelConstraints(std::vector<AgcChannelConstraint> &channelConstraints,\n \t\t\t const libcamera::YamlObject ¶ms)\n {\n \tint ret = 0;\n@@ -231,6 +236,7 @@ int AgcConfig::read(const libcamera::YamlObject ¶ms)\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,8 @@ 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, const AgcChannelResults &channelResults)\n {\n \tframeCount_++;\n \t/*\n@@ -508,12 +515,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(channelResults);\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 +804,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 AgcChannelResults &channelResults)\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 >= channelResults.size() ||\n+\t\t !channelResults[constraint.channel]) {\n+\t\t\tLOG(RPiAgc, Debug) << \"no such channel - skipped\";\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (channelResults[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\tchannelResults[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 +866,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..7ce34360 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 AgcChannelResults = std::vector<libcamera::utils::Duration>;\n+\n struct AgcMeteringMode {\n \tstd::vector<double> weights;\n \tint read(const libcamera::YamlObject ¶ms);\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 AgcChannelResults &channelResults);\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 AgcChannelResults &channelResults);\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);\ndiff --git a/src/ipa/rpi/vc4/data/imx477.json b/src/ipa/rpi/vc4/data/imx477.json\nindex daffc268..bbe6da0d 100644\n--- a/src/ipa/rpi/vc4/data/imx477.json\n+++ b/src/ipa/rpi/vc4/data/imx477.json\n@@ -515,4 +515,5 @@\n \"rpi.sharpen\": { }\n }\n ]\n-}\n\\ No newline at end of file\n+}\n+\n", "prefixes": [ "libcamera-devel", "5/5" ] }