Patch Detail
Show a patch.
GET /api/1.1/patches/17431/?format=api
{ "id": 17431, "url": "https://patchwork.libcamera.org/api/1.1/patches/17431/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17431/", "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": "<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/1.1/people/2/?format=api", "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/1.1/series/3506/?format=api", "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": "<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 A09D0C327E\n\tfor <parsemail@patchwork.libcamera.org>;\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 <libcamera-devel@lists.libcamera.org>;\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 <libcamera-devel@lists.libcamera.org>;\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": "<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": "Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "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 <laurent.pinchart@ideasonboard.com>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\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<rkisp1_stat_buffer *>(\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<int32_t>();\n-\tcontext_.activeState.sensor.gain =\n+\tframeContext.sensor.gain =\n \t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());\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<int32_t>(exposure));\n", "prefixes": [ "libcamera-devel", "v5", "20/33" ] }