Patch Detail
Show a patch.
GET /api/patches/14486/?format=api
{ "id": 14486, "url": "https://patchwork.libcamera.org/api/patches/14486/?format=api", "web_url": "https://patchwork.libcamera.org/patch/14486/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20211108131350.130665-18-jeanmichel.hautbois@ideasonboard.com>", "date": "2021-11-08T13:13:45", "name": "[libcamera-devel,17/22] ipa: ipu3: Use a IPAFrameContext pointer from the per-frame queue", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "8279708fba1b96e6ad5b0ed7e059c547eb095a0a", "submitter": { "id": 75, "url": "https://patchwork.libcamera.org/api/people/75/?format=api", "name": "Jean-Michel Hautbois", "email": "jeanmichel.hautbois@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/14486/mbox/", "series": [ { "id": 2696, "url": "https://patchwork.libcamera.org/api/series/2696/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2696", "date": "2021-11-08T13:13:28", "name": "IPA: IPU3: Introduce per-frame controls", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2696/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/14486/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/14486/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 270ACC324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 8 Nov 2021 13:14:18 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C94A960494;\n\tMon, 8 Nov 2021 14:14:17 +0100 (CET)", "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 B88BF6036F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 8 Nov 2021 14:14:02 +0100 (CET)", "from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:c2bb:76d0:68d7:a9a5])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 77C16110E;\n\tMon, 8 Nov 2021 14:14:02 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OmN7yEkt\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636377242;\n\tbh=Hu5ecbd7XqF8iAIMtPJo8bSofYYRY2Qk2zP2CMlxgF0=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=OmN7yEktdAm4ADi12RaI7cCD0D/2klt73LRmupkqz75IIibRwiCs6wyXPXguv2si6\n\tQgyZPxSwOeFl42HXJEi34nspkndlLgmmPnhURgxPTGgQEi7Jdhbbj1Bgy2ljs4WCt2\n\t4dryd1sTycA1ab7Eec7h9OH3fqmwipY/aBM8C3hU=", "From": "Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 8 Nov 2021 14:13:45 +0100", "Message-Id": "<20211108131350.130665-18-jeanmichel.hautbois@ideasonboard.com>", "X-Mailer": "git-send-email 2.32.0", "In-Reply-To": "<20211108131350.130665-1-jeanmichel.hautbois@ideasonboard.com>", "References": "<20211108131350.130665-1-jeanmichel.hautbois@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 17/22] ipa: ipu3: Use a IPAFrameContext\n\tpointer from the per-frame queue", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "We have one frame context shared between the algorithms thanks to a\nlocal cached context_ variable in IPAIPU3. Now that we can store the\nframe contexts in a queue, implement all the needed functions for that\nand convert the frame context to a pointer.\n\nThe algorithm are now using the values applied on the frame they are\nprocessing, and not the latest one.\n\nSigned-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n---\n src/ipa/ipu3/algorithms/agc.cpp | 12 ++++----\n src/ipa/ipu3/algorithms/agc.h | 2 +-\n src/ipa/ipu3/algorithms/awb.cpp | 14 ++++-----\n src/ipa/ipu3/algorithms/tone_mapping.cpp | 8 ++---\n src/ipa/ipu3/ipa_context.h | 2 +-\n src/ipa/ipu3/ipu3.cpp | 38 +++++++++++++++++++-----\n 6 files changed, 49 insertions(+), 27 deletions(-)", "diff": "diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\nindex 52cf2753..2eee5b6b 100644\n--- a/src/ipa/ipu3/algorithms/agc.cpp\n+++ b/src/ipa/ipu3/algorithms/agc.cpp\n@@ -245,7 +245,7 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain, double curre\n * \\param[in] stats The IPU3 statistics and ISP results\n * \\param[in] currentYGain The gain calculated on the current brightness level\n */\n-double Agc::computeInitialY(IPAFrameContext &frameContext,\n+double Agc::computeInitialY(IPAFrameContext *frameContext,\n \t\t\t const ipu3_uapi_grid_config &grid,\n \t\t\t const ipu3_uapi_stats_3a *stats,\n \t\t\t double currentYGain)\n@@ -271,9 +271,9 @@ double Agc::computeInitialY(IPAFrameContext &frameContext,\n \t * Estimate the sum of the brightness values, weighted with the gains\n \t * applied on the channels in AWB.\n \t */\n-\tdouble Y_sum = redSum * frameContext.awb.gains.red * .299 +\n-\t\t greenSum * frameContext.awb.gains.green * .587 +\n-\t\t blueSum * frameContext.awb.gains.blue * .114;\n+\tdouble Y_sum = redSum * frameContext->awb.gains.red * .299 +\n+\t\t greenSum * frameContext->awb.gains.green * .587 +\n+\t\t blueSum * frameContext->awb.gains.blue * .114;\n \n \t/* And return the average brightness */\n \treturn Y_sum / (grid.height * grid.width);\n@@ -291,8 +291,8 @@ double Agc::computeInitialY(IPAFrameContext &frameContext,\n void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n {\n \t/* Get the latest exposure and gain applied */\n-\tuint32_t &exposure = context.frameContext.agc.exposure;\n-\tdouble &analogueGain = context.frameContext.agc.gain;\n+\tuint32_t &exposure = context.frameContext->agc.exposure;\n+\tdouble &analogueGain = context.frameContext->agc.gain;\n \tmeasureBrightness(stats, context.configuration.grid.bdsGrid);\n \n \tdouble currentYGain = 1.0;\ndiff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\nindex 6f5d71e0..5d6bef9d 100644\n--- a/src/ipa/ipu3/algorithms/agc.h\n+++ b/src/ipa/ipu3/algorithms/agc.h\n@@ -35,7 +35,7 @@ private:\n \t\t\t const ipu3_uapi_grid_config &grid);\n \tvoid filterExposure();\n \tvoid computeExposure(uint32_t &exposure, double &gain, double currentYGain);\n-\tdouble computeInitialY(IPAFrameContext &frameContext,\n+\tdouble computeInitialY(IPAFrameContext *frameContext,\n \t\t\t const ipu3_uapi_grid_config &grid,\n \t\t\t const ipu3_uapi_stats_3a *stats,\n \t\t\t double currentYGain);\ndiff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\nindex a4114659..bd55d377 100644\n--- a/src/ipa/ipu3/algorithms/awb.cpp\n+++ b/src/ipa/ipu3/algorithms/awb.cpp\n@@ -382,9 +382,9 @@ void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n \t * The results are cached, so if no results were calculated, we set the\n \t * cached values from asyncResults_ here.\n \t */\n-\tcontext.frameContext.awb.gains.blue = asyncResults_.blueGain;\n-\tcontext.frameContext.awb.gains.green = asyncResults_.greenGain;\n-\tcontext.frameContext.awb.gains.red = asyncResults_.redGain;\n+\tcontext.frameContext->awb.gains.blue = asyncResults_.blueGain;\n+\tcontext.frameContext->awb.gains.green = asyncResults_.greenGain;\n+\tcontext.frameContext->awb.gains.red = asyncResults_.redGain;\n }\n \n constexpr uint16_t Awb::threshold(float value)\n@@ -432,10 +432,10 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n \tparams->acc_param.bnr.opt_center_sqr.y_sqr_reset = params->acc_param.bnr.opt_center.y_reset\n \t\t\t\t\t\t\t* params->acc_param.bnr.opt_center.y_reset;\n \t/* Convert to u3.13 fixed point values */\n-\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext.awb.gains.green;\n-\tparams->acc_param.bnr.wb_gains.r = 8192 * context.frameContext.awb.gains.red;\n-\tparams->acc_param.bnr.wb_gains.b = 8192 * context.frameContext.awb.gains.blue;\n-\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext.awb.gains.green;\n+\tparams->acc_param.bnr.wb_gains.gr = 8192 * context.frameContext->awb.gains.green;\n+\tparams->acc_param.bnr.wb_gains.r = 8192 * context.frameContext->awb.gains.red;\n+\tparams->acc_param.bnr.wb_gains.b = 8192 * context.frameContext->awb.gains.blue;\n+\tparams->acc_param.bnr.wb_gains.gb = 8192 * context.frameContext->awb.gains.green;\n \n \tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK;\n \ndiff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\nindex 5d74c552..50498f41 100644\n--- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n+++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n@@ -57,7 +57,7 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n {\n \t/* Copy the calculated LUT into the parameters buffer. */\n \tmemcpy(params->acc_param.gamma.gc_lut.lut,\n-\t context.frameContext.toneMapping.gammaCorrection.lut,\n+\t context.frameContext->toneMapping.gammaCorrection.lut,\n \t IPU3_UAPI_GAMMA_CORR_LUT_ENTRIES *\n \t sizeof(params->acc_param.gamma.gc_lut.lut[0]));\n \n@@ -84,11 +84,11 @@ void ToneMapping::process(IPAContext &context,\n \t */\n \tgamma_ = 1.1;\n \n-\tif (context.frameContext.toneMapping.gamma == gamma_)\n+\tif (context.frameContext->toneMapping.gamma == gamma_)\n \t\treturn;\n \n \tstruct ipu3_uapi_gamma_corr_lut &lut =\n-\t\tcontext.frameContext.toneMapping.gammaCorrection;\n+\t\tcontext.frameContext->toneMapping.gammaCorrection;\n \n \tfor (uint32_t i = 0; i < std::size(lut.lut); i++) {\n \t\tdouble j = static_cast<double>(i) / (std::size(lut.lut) - 1);\n@@ -98,7 +98,7 @@ void ToneMapping::process(IPAContext &context,\n \t\tlut.lut[i] = gamma * 8191;\n \t}\n \n-\tcontext.frameContext.toneMapping.gamma = gamma_;\n+\tcontext.frameContext->toneMapping.gamma = gamma_;\n }\n \n } /* namespace ipa::ipu3::algorithms */\ndiff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\nindex ee8f7b55..69780915 100644\n--- a/src/ipa/ipu3/ipa_context.h\n+++ b/src/ipa/ipu3/ipa_context.h\n@@ -57,7 +57,7 @@ struct IPAFrameContext {\n \n struct IPAContext {\n \tIPASessionConfiguration configuration;\n-\tIPAFrameContext frameContext;\n+\tIPAFrameContext *frameContext;\n };\n \n } /* namespace ipa::ipu3 */\ndiff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex 177c5c2f..b60ab7e7 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -522,10 +522,26 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)\n \n void IPAIPU3::frameStarted([[maybe_unused]] unsigned int frame)\n {\n+\t/*\n+\t * Instantiate a new IPAFrameContext to push to the queue. The lifetime\n+\t * of this pointer is indirectly controlled by the\n+\t * ActionSetSensorControls event, as this is where we will delete it.\n+\t */\n+\tstruct IPAFrameContext *frameContext = new IPAFrameContext;\n+\tframeContext->frameId = frame;\n+\tframeContextQueue.push(frameContext);\n }\n \n void IPAIPU3::frameCompleted([[maybe_unused]] unsigned int frame)\n {\n+\t/*\n+\t * Remove the pointer from the queue, it should not be accessed\n+\t * anymore and delete it.\n+\t */\n+\tstruct IPAFrameContext *frameContext = frameContextQueue.front();\n+\tASSERT(frameContext->frameId == frame);\n+\tframeContextQueue.pop();\n+\tdelete frameContext;\n }\n \n /**\n@@ -579,10 +595,6 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n \t\tconst ipu3_uapi_stats_3a *stats =\n \t\t\treinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());\n \n-\t\t/* \\todo move those into processControls */\n-\t\tcontext_.frameContext.agc.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n-\t\tcontext_.frameContext.agc.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());\n-\n \t\tparseStatistics(event.frame, event.frameTimestamp, stats);\n \t\t/*\n \t\t* To save incurring extra IPC calls, we do not send explicit events\n@@ -610,9 +622,12 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n */\n void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n \t\t\t [[maybe_unused]] const ControlList &controls,\n-\t\t\t [[maybe_unused]] const ControlList &sensorCtrls)\n+\t\t\t const ControlList &sensorCtrls)\n {\n-\t/* \\todo Start processing for 'frame' based on 'controls'. */\n+\tstruct IPAFrameContext *frameContext = frameContextQueue.back();\n+\tASSERT(frameContext->frameId == frame);\n+\tframeContext->agc.exposure = sensorCtrls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n+\tframeContext->agc.gain = camHelper_->gain(sensorCtrls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());\n }\n \n /**\n@@ -636,6 +651,9 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n \t */\n \tparams->use = {};\n \n+\t/* Set the reference to the current frame context */\n+\tcontext_.frameContext = frameContextQueue.front();\n+\n \tfor (auto const &algo : algorithms_)\n \t\talgo->prepare(context_, params);\n \n@@ -661,6 +679,9 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n {\n \tControlList ctrls(controls::controls);\n \n+\t/* Set the reference to the current frame context */\n+\tcontext_.frameContext = frameContextQueue.front();\n+\n \tfor (auto const &algo : algorithms_)\n \t\talgo->process(context_, stats);\n \n@@ -690,8 +711,9 @@ void IPAIPU3::setControls(unsigned int frame)\n \tIPU3Action op;\n \top.op = ActionSetSensorControls;\n \n-\texposure_ = context_.frameContext.agc.exposure;\n-\tgain_ = camHelper_->gainCode(context_.frameContext.agc.gain);\n+\t/* Apply the exposure and gain updated values */\n+\texposure_ = context_.frameContext->agc.exposure;\n+\tgain_ = camHelper_->gainCode(context_.frameContext->agc.gain);\n \n \tControlList ctrls(ctrls_);\n \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n", "prefixes": [ "libcamera-devel", "17/22" ] }