{"id":14486,"url":"https://patchwork.libcamera.org/api/patches/14486/?format=json","web_url":"https://patchwork.libcamera.org/patch/14486/","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":"<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=json","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=json","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"]}