{"id":17431,"url":"https://patchwork.libcamera.org/api/patches/17431/?format=json","web_url":"https://patchwork.libcamera.org/patch/17431/","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":"<20220927023642.12341-21-laurent.pinchart@ideasonboard.com>","date":"2022-09-27T02:36:29","name":"[libcamera-devel,v5,20/33] ipa: rkisp1: agc: Store per-frame information in frame context","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"eb4691f6fea11aa52c3703faeb4f76784f288112","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/17431/mbox/","series":[{"id":3506,"url":"https://patchwork.libcamera.org/api/series/3506/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3506","date":"2022-09-27T02:36:09","name":"ipa: Frame context queue, IPU3 & RkISP consolidation, and RkISP1 improvements","version":5,"mbox":"https://patchwork.libcamera.org/series/3506/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/17431/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/17431/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 A09D0C327E\n\tfor ;\n\tTue, 27 Sep 2022 02:37:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 55D366225F;\n\tTue, 27 Sep 2022 04:37:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BCAEE622B3\n\tfor ;\n\tTue, 27 Sep 2022 04:37:28 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3E38F47C\n\tfor ;\n\tTue, 27 Sep 2022 04:37:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1664246251;\n\tbh=0qL8UV+IYV7Thcn8LNEl5Kjr5myNT+7nhu/hzi2ILGw=;\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=TiEUyWafZtmviYFT2ydrXfmpC2cVXXaMWGTGYhkWAaFou6nqAzD93a1ffAOfr0yjr\n\tlFxvFdr1Iijp/3W9cnXwvHUHjrn1FIMKsfHRpcVzm8JeVAnu+kwbh8W0yqmnuYHWOG\n\tfaLQR0NK3huh4OXDEQ15paracuhPjeI1iA4KgnKbiLzzEy2rzDH6lJCOx5eHyZqGrm\n\t+5izJb8MBOYj3W+WJ/Dmk+ZWh/QYZcRK2oOMmEh++YnbQMm8dpuI27xOEOXBIzfeE5\n\t2kFffNMTP493oL3UQxp/0NDs6vxJgvAQWR9WlhzmtZFyi/wi3m7tME7suU9Kr5nCwj\n\th3AZBz++DrFRA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1664246248;\n\tbh=0qL8UV+IYV7Thcn8LNEl5Kjr5myNT+7nhu/hzi2ILGw=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=gb7nXwv1yGtmZQWkgQfwxqWw0H9kHbFmqh28RXFOmB04gZR5i8XXHluM73fDAk0j8\n\tmBF3MaV+zqSYQ7Kq7URbyFs0gu+JiOrLczF/8KLq7Zpit7H3IsdSuUr7db4wG34/Xz\n\t7GZSCo9I6D7//4TCF8bi54hRulQ7H6C9tHCNAPsc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gb7nXwv1\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 27 Sep 2022 05:36:29 +0300","Message-Id":"<20220927023642.12341-21-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.35.1","In-Reply-To":"<20220927023642.12341-1-laurent.pinchart@ideasonboard.com>","References":"<20220927023642.12341-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v5 20/33] ipa: rkisp1: agc: Store\n\tper-frame information in frame context","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Rework the algorithm's usage of the active state to store the value of\ncontrols for the last queued request in the queueRequest() function, and\nstore a copy of the values in the corresponding frame context.\n\nThe frame context is used in the prepare() function to populate the ISP\nparameters with values corresponding to the right frame.\n\nSigned-off-by: Laurent Pinchart \nReviewed-by: Kieran Bingham \n---\nChanges since v4:\n\n- Add todo comments related to sensor controls\n---\n src/ipa/rkisp1/algorithms/agc.cpp | 27 +++++++++++++------\n src/ipa/rkisp1/algorithms/agc.h | 3 ++-\n src/ipa/rkisp1/ipa_context.cpp | 43 ++++++++++++++++++++-----------\n src/ipa/rkisp1/ipa_context.h | 14 ++++++----\n src/ipa/rkisp1/rkisp1.cpp | 14 +++++++---\n 5 files changed, 68 insertions(+), 33 deletions(-)","diff":"diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex 87fc5d1ffec7..4540bde66db4 100644\n--- a/src/ipa/rkisp1/algorithms/agc.cpp\n+++ b/src/ipa/rkisp1/algorithms/agc.cpp\n@@ -144,17 +144,19 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n /**\n * \\brief Estimate the new exposure and gain values\n * \\param[inout] context The shared IPA Context\n+ * \\param[in] frameContext The FrameContext for this frame\n * \\param[in] yGain The gain calculated on the current brightness level\n * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n */\n-void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n+void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n+\t\t\t double yGain, double iqMeanGain)\n {\n \tIPASessionConfiguration &configuration = context.configuration;\n \tIPAActiveState &activeState = context.activeState;\n \n \t/* Get the effective exposure and gain applied on the sensor. */\n-\tuint32_t exposure = activeState.sensor.exposure;\n-\tdouble analogueGain = activeState.sensor.gain;\n+\tuint32_t exposure = frameContext.sensor.exposure;\n+\tdouble analogueGain = frameContext.sensor.gain;\n \n \t/* Use the highest of the two gain estimates. */\n \tdouble evGain = std::max(yGain, iqMeanGain);\n@@ -286,9 +288,16 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const\n * new exposure and gain for the scene.\n */\n void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n-\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n-\t\t const rkisp1_stat_buffer *stats)\n+\t\t IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats)\n {\n+\t/*\n+\t * \\todo Verify that the exposure and gain applied by the sensor for\n+\t * this frame match what has been requested. This isn't a hard\n+\t * requirement for stability of the AGC (the guarantee we need in\n+\t * automatic mode is a perfect match between the frame and the values\n+\t * we receive), but is important in manual mode.\n+\t */\n+\n \tconst rkisp1_cif_isp_stat *params = &stats->params;\n \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n \n@@ -320,7 +329,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \t\t\tbreak;\n \t}\n \n-\tcomputeExposure(context, yGain, iqMeanGain);\n+\tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n \tframeCount_++;\n }\n \n@@ -328,9 +337,11 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n * \\copydoc libcamera::ipa::Algorithm::prepare\n */\n void Agc::prepare(IPAContext &context, const uint32_t frame,\n-\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n-\t\t rkisp1_params_cfg *params)\n+\t\t IPAFrameContext &frameContext, rkisp1_params_cfg *params)\n {\n+\tframeContext.agc.exposure = context.activeState.agc.exposure;\n+\tframeContext.agc.gain = context.activeState.agc.gain;\n+\n \tif (frame > 0)\n \t\treturn;\n \ndiff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\nindex f115ba2ed85c..9ad5c32fd6f6 100644\n--- a/src/ipa/rkisp1/algorithms/agc.h\n+++ b/src/ipa/rkisp1/algorithms/agc.h\n@@ -34,7 +34,8 @@ public:\n \t\t const rkisp1_stat_buffer *stats) override;\n \n private:\n-\tvoid computeExposure(IPAContext &Context, double yGain, double iqMeanGain);\n+\tvoid computeExposure(IPAContext &Context, IPAFrameContext &frameContext,\n+\t\t\t double yGain, double iqMeanGain);\n \tutils::Duration filterExposure(utils::Duration exposureValue);\n \tdouble estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);\n \tdouble measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const;\ndiff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\nindex 78a785f5f982..c7d5b1b6ec5a 100644\n--- a/src/ipa/rkisp1/ipa_context.cpp\n+++ b/src/ipa/rkisp1/ipa_context.cpp\n@@ -104,16 +104,13 @@ namespace libcamera::ipa::rkisp1 {\n * \\var IPAActiveState::agc\n * \\brief State for the Automatic Gain Control algorithm\n *\n- * The exposure and gain determined are expected to be applied to the sensor\n- * at the earliest opportunity.\n+ * The exposure and gain are the latest values computed by the AGC algorithm.\n *\n * \\var IPAActiveState::agc.exposure\n * \\brief Exposure time expressed as a number of lines\n *\n * \\var IPAActiveState::agc.gain\n * \\brief Analogue gain multiplier\n- *\n- * The gain should be adapted to the sensor specific gain code before applying.\n */\n \n /**\n@@ -181,17 +178,6 @@ namespace libcamera::ipa::rkisp1 {\n * \\brief Indicates if ISP parameters need to be updated\n */\n \n-/**\n- * \\var IPAActiveState::sensor\n- * \\brief Effective sensor values\n- *\n- * \\var IPAActiveState::sensor.exposure\n- * \\brief Exposure time expressed as a number of lines\n- *\n- * \\var IPAActiveState::sensor.gain\n- * \\brief Analogue gain multiplier\n- */\n-\n /**\n * \\struct IPAFrameContext\n * \\brief Per-frame context for algorithms\n@@ -199,6 +185,33 @@ namespace libcamera::ipa::rkisp1 {\n * \\todo Populate the frame context for all algorithms\n */\n \n+/**\n+ * \\var IPAFrameContext::agc\n+ * \\brief Automatic Gain Control parameters for this frame\n+ *\n+ * The exposure and gain are provided by the AGC algorithm, and are to be\n+ * applied to the sensor in order to take effect for this frame.\n+ *\n+ * \\var IPAFrameContext::agc.exposure\n+ * \\brief Exposure time expressed as a number of lines\n+ *\n+ * \\var IPAFrameContext::agc.gain\n+ * \\brief Analogue gain multiplier\n+ *\n+ * The gain should be adapted to the sensor specific gain code before applying.\n+ */\n+\n+/**\n+ * \\var IPAFrameContext::sensor\n+ * \\brief Sensor configuration that used been used for this frame\n+ *\n+ * \\var IPAFrameContext::sensor.exposure\n+ * \\brief Exposure time expressed as a number of lines\n+ *\n+ * \\var IPAFrameContext::sensor.gain\n+ * \\brief Analogue gain multiplier\n+ */\n+\n /**\n * \\struct IPAContext\n * \\brief Global IPA context data shared between all algorithms\ndiff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\nindex c7041ea2a214..a4d134e700b5 100644\n--- a/src/ipa/rkisp1/ipa_context.h\n+++ b/src/ipa/rkisp1/ipa_context.h\n@@ -83,14 +83,18 @@ struct IPAActiveState {\n \t\tuint8_t sharpness;\n \t\tbool updateParams;\n \t} filter;\n-\n-\tstruct {\n-\t\tuint32_t exposure;\n-\t\tdouble gain;\n-\t} sensor;\n };\n \n struct IPAFrameContext : public FrameContext {\n+\tstruct {\n+\t\tuint32_t exposure;\n+\t\tdouble gain;\n+\t} agc;\n+\n+\tstruct {\n+\t\tuint32_t exposure;\n+\t\tdouble gain;\n+\t} sensor;\n };\n \n static constexpr uint32_t kMaxFrameContexts = 16;\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 5409c2d5219e..6297916cf86d 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -332,9 +332,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n \t\treinterpret_cast(\n \t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n \n-\tcontext_.activeState.sensor.exposure =\n+\tframeContext.sensor.exposure =\n \t\tsensorControls.get(V4L2_CID_EXPOSURE).get();\n-\tcontext_.activeState.sensor.gain =\n+\tframeContext.sensor.gain =\n \t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n \n \tunsigned int aeState = 0;\n@@ -349,8 +349,14 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n \n void IPARkISP1::setControls(unsigned int frame)\n {\n-\tuint32_t exposure = context_.activeState.agc.exposure;\n-\tuint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);\n+\t/*\n+\t * \\todo The frame number is most likely wrong here, we need to take\n+\t * internal sensor delays and other timing parameters into account.\n+\t */\n+\n+\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n+\tuint32_t exposure = frameContext.agc.exposure;\n+\tuint32_t gain = camHelper_->gainCode(frameContext.agc.gain);\n \n \tControlList ctrls(ctrls_);\n \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure));\n","prefixes":["libcamera-devel","v5","20/33"]}