{"id":18991,"url":"https://patchwork.libcamera.org/api/1.1/patches/18991/?format=json","web_url":"https://patchwork.libcamera.org/patch/18991/","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":"<20230912102442.169001-4-david.plowman@raspberrypi.com>","date":"2023-09-12T10:24:40","name":"[libcamera-devel,v3,3/5] ipa: rpi: agc: Implementation of multi-channel AGC","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"6c9112b90687ea14fd2260f37fb63e3dc8e6af7c","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/18991/mbox/","series":[{"id":4017,"url":"https://patchwork.libcamera.org/api/1.1/series/4017/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4017","date":"2023-09-12T10:24:37","name":"Multi-channel AGC","version":3,"mbox":"https://patchwork.libcamera.org/series/4017/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/18991/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/18991/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 B747AC32B0\n\tfor ;\n\tTue, 12 Sep 2023 10:24:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2EE1D61DF5;\n\tTue, 12 Sep 2023 12:24:54 +0200 (CEST)","from mail-wm1-x329.google.com (mail-wm1-x329.google.com\n\t[IPv6:2a00:1450:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B324161DEF\n\tfor ;\n\tTue, 12 Sep 2023 12:24:51 +0200 (CEST)","by mail-wm1-x329.google.com with SMTP id\n\t5b1f17b1804b1-401187f8071so34649785e9.0\n\tfor ;\n\tTue, 12 Sep 2023 03:24:51 -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\tn10-20020a05600c294a00b003fefaf299b6sm12426575wmd.38.2023.09.12.03.24.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 12 Sep 2023 03:24:50 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694514294;\n\tbh=ZrC22mwcY2qu9W3m0mC6XuqqMhOb/z5Rywsrm3wJERA=;\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=b8IYiKHt54OVHsdV+JHN6/2tw3cy+vqKQzEbruLihCCrUciUT+koeAAhqTvKpbe+k\n\tDw6dEsHquvldapLaik1gnXLvYMNeVWL+gJ8oY1qGQi2gsR3SytE8VGWbsrd2HCVJv1\n\tvk5SVdEExoGhPLkRbHvuz25VZ/vAPUyykCZ6xd9Rdo42sqtH+SbYnKBDYNJtEWZUHy\n\tJdI9U1ClI3key1VhznOYxl4A8Q/rp9k21z9/ClYOnbHuA4xY0uMVpvoSQm2kLs2JFN\n\tbKDNn+IQKHeBv6SgwOelfkISN7EkcC5PdwsbQ4fY91gEYRVT+oQ9sL5uuUoKqAjAeR\n\ttJrW+u20067JQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1694514291; x=1695119091;\n\tdarn=lists.libcamera.org; \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=7w6S6uug9rm57PbKJUHS2RejDYdEJSRa7Jw03vZTY+I=;\n\tb=psGCP0cTjI1LanYpeGmwk6+aFwRoeoV6rx8WZS6GpPcr0HjC9LViBX/C90LZcIk6S6\n\tV3idC2BzIvvb/s+0n11PjBWWQXoPU0qjmg3elwfK8AQ2qstDhNvw5leEb3IlmWpUB3iT\n\tWWSrn2i7ewgGXjG9QcZZwPZuCDeNEkfGd+3FBbSpQcZ5SFIj3/eh/wes5schoR5TwhKt\n\te9j73wtspXQUfqYA8rKgskl9ocSrdFRZx2UHxNamKQ7SIWBObaxK87h4zAmz0PEeRgIC\n\tx6c+iEAIvoT0PmkWUDhKk4jWq//DIYrAcN7XubuK3SvhAtYdcGDNYdagkF10kgs5QEC5\n\t3rvg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"psGCP0cT\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1694514291; x=1695119091;\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=7w6S6uug9rm57PbKJUHS2RejDYdEJSRa7Jw03vZTY+I=;\n\tb=endGCEpoYZBbnOTRnLKogvWDsAUJo+NBm1y2wu6O468ps4MCCIU6Vpu+fiSlWCpkaX\n\tAPgWqHp1HV0mlGPKQbLRdVC+RlVdMWf79q0pUi1lerjWXcQqd2181AzkJQih+bPQkdA9\n\tsIED2dyw+OqpOC7zQOfrmOahZXxDQggS+NEGO5UxaSFOaOnjd0nkzXnZPWUPSDFBtC8S\n\tk0cEh9/J3zeBS+U8X2rjWPrafy1Sy9eya9JcvIQe/N9wWGhLZpyoEzGSUiAIBqV5Loap\n\tlIpsif7lIm7H0N4iHjpvon+YXQgJJD/8Vf12miaGa0AnSeCXyZOcUtE8yoarbAMA64wI\n\t/S9w==","X-Gm-Message-State":"AOJu0Yzy9shXPIFav3+qOt4l53DFdx4JomoYr8blhVEG6zjh6ABFclRV\n\tvo4vV/VJtVEoSP/pKEOCDgsj67JM+H5hHYH9zK4=","X-Google-Smtp-Source":"AGHT+IHtNEq2JvzEDzqtrNH0zrDVvlD0VOtyRbw2PUFjOtzj2s72zuLr5NW7N/D+plWBLjE/uF68Vw==","X-Received":"by 2002:a7b:c849:0:b0:402:ea5c:1845 with SMTP id\n\tc9-20020a7bc849000000b00402ea5c1845mr1744344wml.5.1694514290992; \n\tTue, 12 Sep 2023 03:24:50 -0700 (PDT)","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 12 Sep 2023 11:24:40 +0100","Message-Id":"<20230912102442.169001-4-david.plowman@raspberrypi.com>","X-Mailer":"git-send-email 2.30.2","In-Reply-To":"<20230912102442.169001-1-david.plowman@raspberrypi.com>","References":"<20230912102442.169001-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 3/5] ipa: rpi: agc: Implementation of\n\tmulti-channel AGC","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 switchMode, prepare and process methods are updated to implement\nmulti-channel AGC correctly:\n\n* switchMode now invokes switchMode on all the channels (whether\n active or not).\n\n* prepare must find what channel the current frame is, and run on\n behalf of that channel.\n\n* process updates the most recent DeviceStatus and statistics for the\n channel of the frame that has just arrived, but generates updated\n values working through the active channels in round-robin fashion.\n\nOne minor detail in process is that we don't want to change the\nDeviceStatus metadata of the current frame, so we now pass this to the\nAgcChannel's process method, rather than letting it find the\nDeviceStatus in the metadata.\n\nSigned-off-by: David Plowman \nReviewed-by: Naushir Patuck \n---\n src/ipa/rpi/controller/agc_status.h | 1 +\n src/ipa/rpi/controller/rpi/agc.cpp | 108 +++++++++++++++++++--\n src/ipa/rpi/controller/rpi/agc.h | 3 +\n src/ipa/rpi/controller/rpi/agc_channel.cpp | 13 +--\n src/ipa/rpi/controller/rpi/agc_channel.h | 4 +-\n 5 files changed, 111 insertions(+), 18 deletions(-)","diff":"diff --git a/src/ipa/rpi/controller/agc_status.h b/src/ipa/rpi/controller/agc_status.h\nindex 597eddd7..e5c4ee22 100644\n--- a/src/ipa/rpi/controller/agc_status.h\n+++ b/src/ipa/rpi/controller/agc_status.h\n@@ -36,6 +36,7 @@ struct AgcStatus {\n \tint floatingRegionEnable;\n \tlibcamera::utils::Duration fixedShutter;\n \tdouble fixedAnalogueGain;\n+\tunsigned int channel;\n };\n \n struct AgcPrepareStatus {\ndiff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\nindex 598fc890..58ba8839 100644\n--- a/src/ipa/rpi/controller/rpi/agc.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc.cpp\n@@ -22,7 +22,8 @@ LOG_DEFINE_CATEGORY(RPiAgc)\n \n Agc::Agc(Controller *controller)\n \t: AgcAlgorithm(controller),\n-\t activeChannels_({ 0 })\n+\t activeChannels_({ 0 }),\n+\t index_(0)\n {\n }\n \n@@ -205,20 +206,113 @@ void Agc::setActiveChannels(const std::vector &activeChannels)\n void Agc::switchMode(CameraMode const &cameraMode,\n \t\t Metadata *metadata)\n {\n-\tLOG(RPiAgc, Debug) << \"switchMode for channel 0\";\n-\tchannelData_[0].channel.switchMode(cameraMode, metadata);\n+\t/*\n+\t * We run switchMode on every channel, and then we're going to start over\n+\t * with the first active channel again which means that this channel's\n+\t * status needs to be the one we leave in the metadata.\n+\t */\n+\tAgcStatus status;\n+\n+\tfor (unsigned int channelIndex = 0; channelIndex < channelData_.size(); channelIndex++) {\n+\t\tLOG(RPiAgc, Debug) << \"switchMode for channel \" << channelIndex;\n+\t\tchannelData_[channelIndex].channel.switchMode(cameraMode, metadata);\n+\t\tif (channelIndex == activeChannels_[0])\n+\t\t\tmetadata->get(\"agc.status\", status);\n+\t}\n+\n+\tstatus.channel = activeChannels_[0];\n+\tmetadata->set(\"agc.status\", status);\n+\tindex_ = 0;\n+}\n+\n+static void getChannelIndex(Metadata *metadata, const char *message, unsigned int &channelIndex)\n+{\n+\tstd::unique_lock lock(*metadata);\n+\tAgcStatus *status = metadata->getLocked(\"agc.delayed_status\");\n+\tif (status)\n+\t\tchannelIndex = status->channel;\n+\telse\n+\t\t/* This does happen at startup, otherwise it would be a Warning or Error. */\n+\t\tLOG(RPiAgc, Debug) << message;\n+}\n+\n+static void setChannelIndex(Metadata *metadata, const char *message, unsigned int channelIndex)\n+{\n+\tstd::unique_lock lock(*metadata);\n+\tAgcStatus *status = metadata->getLocked(\"agc.status\");\n+\tif (status)\n+\t\tstatus->channel = channelIndex;\n+\telse\n+\t\t/* This does happen at startup, otherwise it would be a Warning or Error. */\n+\t\tLOG(RPiAgc, Debug) << message;\n }\n \n void Agc::prepare(Metadata *imageMetadata)\n {\n-\tLOG(RPiAgc, Debug) << \"prepare for channel 0\";\n-\tchannelData_[0].channel.prepare(imageMetadata);\n+\t/*\n+\t * The DeviceStatus in the metadata should be correct for the image we\n+\t * are processing. THe delayed status should tell us what channel this frame\n+\t * was from, so we will use that channel's prepare method.\n+\t *\n+\t * \\todo To be honest, there's not much that's stateful in the prepare methods\n+\t * so we should perhaps re-evaluate whether prepare even needs to be done\n+\t * \"per channel\".\n+\t */\n+\tunsigned int channelIndex = activeChannels_[0];\n+\tgetChannelIndex(imageMetadata, \"prepare: no delayed status\", channelIndex);\n+\n+\tLOG(RPiAgc, Debug) << \"prepare for channel \" << channelIndex;\n+\tchannelData_[channelIndex].channel.prepare(imageMetadata);\n }\n \n void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata)\n {\n-\tLOG(RPiAgc, Debug) << \"process for channel 0\";\n-\tchannelData_[0].channel.process(stats, imageMetadata);\n+\t/*\n+\t * We want to generate values for the next channel in round robin fashion\n+\t * (i.e. the channel at location index_ in the activeChannel list), even though\n+\t * the statistics we have will be for a different channel (which we find\n+\t * again from the delayed status).\n+\t */\n+\n+\t/* Generate updated AGC values for this channel: */\n+\tunsigned int channelIndex = activeChannels_[index_];\n+\tAgcChannelData &channelData = channelData_[channelIndex];\n+\t/* Stats are from this channel: */\n+\tunsigned int statsIndex = 0;\n+\tgetChannelIndex(imageMetadata, \"process: no delayed status for stats\", statsIndex);\n+\tLOG(RPiAgc, Debug) << \"process for channel \" << channelIndex;\n+\n+\t/*\n+\t * We keep a cache of the most recent DeviceStatus and stats for each channel,\n+\t * so that we can invoke the next channel's process method with the most up to date\n+\t * values.\n+\t */\n+\tLOG(RPiAgc, Debug) << \"Save DeviceStatus and stats for channel \" << statsIndex;\n+\tDeviceStatus deviceStatus;\n+\tif (imageMetadata->get(\"device.status\", deviceStatus) == 0)\n+\t\tchannelData_[statsIndex].deviceStatus = deviceStatus;\n+\telse\n+\t\t/* Every frame should have a DeviceStatus. */\n+\t\tLOG(RPiAgc, Error) << \"process: no device status found\";\n+\tchannelData_[statsIndex].statistics = stats;\n+\n+\t/*\n+\t * Finally fetch the most recent DeviceStatus and stats for this channel, if both\n+\t * exist, and call process(). We must make the agc.status metadata record correctly\n+\t * which channel this is.\n+\t */\n+\tif (channelData.statistics && channelData.deviceStatus) {\n+\t\tdeviceStatus = *channelData.deviceStatus;\n+\t\tstats = channelData.statistics;\n+\t} else\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+\n+\t/* And onto the next channel for the next call. */\n+\tindex_ = (index_ + 1) % activeChannels_.size();\n }\n \n /* Register algorithm with the system. */\ndiff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h\nindex 24f0a271..ee85c693 100644\n--- a/src/ipa/rpi/controller/rpi/agc.h\n+++ b/src/ipa/rpi/controller/rpi/agc.h\n@@ -18,6 +18,8 @@ namespace RPiController {\n \n struct AgcChannelData {\n \tAgcChannel channel;\n+\tstd::optional deviceStatus;\n+\tStatisticsPtr statistics;\n };\n \n class Agc : public AgcAlgorithm\n@@ -52,6 +54,7 @@ private:\n \tint checkChannel(unsigned int channel) const;\n \tstd::vector channelData_;\n \tstd::vector activeChannels_;\n+\tunsigned int index_; /* index into the activeChannels_ */\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 7c1aba81..f1f19598 100644\n--- a/src/ipa/rpi/controller/rpi/agc_channel.cpp\n+++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp\n@@ -444,7 +444,7 @@ void AgcChannel::prepare(Metadata *imageMetadata)\n \t}\n }\n \n-void AgcChannel::process(StatisticsPtr &stats, Metadata *imageMetadata)\n+void AgcChannel::process(StatisticsPtr &stats, const DeviceStatus *deviceStatus, Metadata *imageMetadata)\n {\n \tframeCount_++;\n \t/*\n@@ -455,7 +455,7 @@ void AgcChannel::process(StatisticsPtr &stats, Metadata *imageMetadata)\n \t/* Fetch the AWB status immediately, so that we can assume it's there. */\n \tfetchAwbStatus(imageMetadata);\n \t/* Get the current exposure values for the frame that's just arrived. */\n-\tfetchCurrentExposure(imageMetadata);\n+\tfetchCurrentExposure(deviceStatus);\n \t/* Compute the total gain we require relative to the current exposure. */\n \tdouble gain, targetY;\n \tcomputeGain(stats, imageMetadata, gain, targetY);\n@@ -567,18 +567,13 @@ void AgcChannel::housekeepConfig()\n \t\t\t << meteringModeName_;\n }\n \n-void AgcChannel::fetchCurrentExposure(Metadata *imageMetadata)\n+void AgcChannel::fetchCurrentExposure(const DeviceStatus *deviceStatus)\n {\n-\tstd::unique_lock lock(*imageMetadata);\n-\tDeviceStatus *deviceStatus =\n-\t\timageMetadata->getLocked(\"device.status\");\n \tif (!deviceStatus)\n \t\tLOG(RPiAgc, Fatal) << \"No device metadata\";\n \tcurrent_.shutter = deviceStatus->shutterSpeed;\n \tcurrent_.analogueGain = deviceStatus->analogueGain;\n-\tAgcStatus *agcStatus =\n-\t\timageMetadata->getLocked(\"agc.status\");\n-\tcurrent_.totalExposure = agcStatus ? agcStatus->totalExposureValue : 0s;\n+\tcurrent_.totalExposure = 0s; /* this value is unused */\n \tcurrent_.totalExposureNoDG = current_.shutter * current_.analogueGain;\n }\n \ndiff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h\nindex d5a5cf3a..24ee3491 100644\n--- a/src/ipa/rpi/controller/rpi/agc_channel.h\n+++ b/src/ipa/rpi/controller/rpi/agc_channel.h\n@@ -85,13 +85,13 @@ public:\n \tvoid disableAuto();\n \tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata);\n \tvoid prepare(Metadata *imageMetadata);\n-\tvoid process(StatisticsPtr &stats, Metadata *imageMetadata);\n+\tvoid process(StatisticsPtr &stats, const DeviceStatus *deviceStatus, Metadata *imageMetadata);\n \n private:\n \tbool updateLockStatus(DeviceStatus const &deviceStatus);\n \tAgcConfig config_;\n \tvoid housekeepConfig();\n-\tvoid fetchCurrentExposure(Metadata *imageMetadata);\n+\tvoid fetchCurrentExposure(const DeviceStatus *deviceStatus);\n \tvoid fetchAwbStatus(Metadata *imageMetadata);\n \tvoid computeGain(StatisticsPtr &statistics, Metadata *imageMetadata,\n \t\t\t double &gain, double &targetY);\n","prefixes":["libcamera-devel","v3","3/5"]}