[{"id":21561,"web_url":"https://patchwork.libcamera.org/comment/21561/","msgid":"","date":"2021-12-03T02:26:18","subject":"Re: [libcamera-devel] [PATCH v1 2/6] ipa: rkisp1: Use frame index","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Thu, Dec 02, 2021 at 07:04:06PM +0100, Jean-Michel Hautbois wrote:\n> Instead of incrementing the frameCount manually, use the frame index on\n> the EventStatReay event and store it in a new IPAFrameContext variable\n> named frameId.\n\nWhy ?\n\n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 5 ++---\n> src/ipa/rkisp1/ipa_context.cpp | 5 +++++\n> src/ipa/rkisp1/ipa_context.h | 2 ++\n> src/ipa/rkisp1/rkisp1.cpp | 11 +++++------\n> 4 files changed, 14 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index d6abdc310..f95ecfde5 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -81,8 +81,6 @@ int Agc::configure(IPAContext &context,\n> \telse\n> \t\tnumCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> \n> -\t/* \\todo Use actual frame index by populating it in the frameContext. */\n> -\tframeCount_ = 0;\n> \treturn 0;\n> }\n> \n> @@ -254,6 +252,8 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)\n> \n> \tconst rkisp1_cif_isp_ae_stat *ae = ¶ms->ae;\n> \n> +\tframeCount_ = context.frameContext.frameId;\n> +\n> \t/*\n> \t * Estimate the gain needed to achieve a relative luminance target. To\n> \t * account for non-linearity caused by saturation, the value needs to be\n> @@ -277,7 +277,6 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)\n> \t}\n> \n> \tcomputeExposure(context, yGain);\n> -\tframeCount_++;\n> }\n> \n> void Agc::prepare([[maybe_unused]] IPAContext &context,\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 9cb2a9fda..992c92256 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -113,4 +113,9 @@ namespace libcamera::ipa::rkisp1 {\n> * \\brief Analogue gain multiplier\n> */\n> \n> +/**\n> + * \\var IPAFrameContext::frameId\n> + * \\brief Frame number for this frame context\n> + */\n> +\n> } /* namespace libcamera::ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index b94ade0c5..c447369f3 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -43,6 +43,8 @@ struct IPAFrameContext {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> \t} sensor;\n> +\n> +\tunsigned int frameId;\n> };\n> \n> struct IPAContext {\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 07f1f1c87..cd425a2e1 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -57,8 +57,7 @@ public:\n> private:\n> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> \t\t\t const ControlList &controls);\n> -\tvoid updateStatistics(unsigned int frame,\n> -\t\t\t const rkisp1_stat_buffer *stats);\n> +\tvoid updateStatistics(const rkisp1_stat_buffer *stats);\n> \n> \tvoid setControls(unsigned int frame);\n> \tvoid metadataReady(unsigned int frame, unsigned int aeState);\n> @@ -241,7 +240,6 @@ void IPARkISP1::processEvent(const RkISP1Event &event)\n> {\n> \tswitch (event.op) {\n> \tcase EventSignalStatBuffer: {\n> -\t\tunsigned int frame = event.frame;\n> \t\tunsigned int bufferId = event.bufferId;\n> \n> \t\tconst rkisp1_stat_buffer *stats =\n> @@ -252,8 +250,9 @@ void IPARkISP1::processEvent(const RkISP1Event &event)\n> \t\t\tevent.sensorControls.get(V4L2_CID_EXPOSURE).get();\n> \t\tcontext_.frameContext.sensor.gain =\n> \t\t\tcamHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> +\t\tcontext_.frameContext.frameId = event.frame;\n> \n> -\t\tupdateStatistics(frame, stats);\n> +\t\tupdateStatistics(stats);\n> \t\tbreak;\n> \t}\n> \tcase EventQueueRequest: {\n> @@ -288,10 +287,10 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> \tqueueFrameAction.emit(frame, op);\n> }\n> \n> -void IPARkISP1::updateStatistics(unsigned int frame,\n> -\t\t\t\t const rkisp1_stat_buffer *stats)\n> +void IPARkISP1::updateStatistics(const rkisp1_stat_buffer *stats)\n> {\n> \tunsigned int aeState = 0;\n> +\tunsigned int frame = context_.frameContext.frameId;\n> \n> \tfor (auto const &algo : algorithms_)\n> \t\talgo->process(context_, stats);","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 F1A93BDB13\n\tfor ;\n\tFri, 3 Dec 2021 02:26:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79D74607DE;\n\tFri, 3 Dec 2021 03:26:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E74DB60118\n\tfor ;\n\tFri, 3 Dec 2021 03:26:44 +0100 (CET)","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 5B45EA59;\n\tFri, 3 Dec 2021 03:26:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"O7eK1QW4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638498404;\n\tbh=rQjPazSPN6L6UybXJV3AlUBma3BEnuNofg284oBVVgw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O7eK1QW4hhltmUXsp43ysqzgH+Cn+FEc0Mu5YXO80PpY3V2IsbJK/T2JLtrwPQA06\n\tvL/Wi1+fsopRzEJURmG/VS/n+g2CWOpW0BEoXWpBkvwVP7y+yPG9Wi1duGDCiT9mRa\n\tPw/BSgcqz0+owNR+zUep/Vnm4a9X0gNyCdxYLzYk=","Date":"Fri, 3 Dec 2021 04:26:18 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211202180410.518232-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211202180410.518232-3-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211202180410.518232-3-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 2/6] ipa: rkisp1: Use frame index","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]