[{"id":22187,"web_url":"https://patchwork.libcamera.org/comment/22187/","msgid":"","date":"2022-02-23T14:23:42","subject":"Re: [libcamera-devel] [PATCH v2 1/4] 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 Wed, Feb 23, 2022 at 11:48:21AM +0100, Jean-Michel Hautbois wrote:\n> Instead of incrementing the frameCount manually in a local counter, use\n> the frame index on the EventStatReay event and store it in a new\n> IPAFrameContext variable named frameId.\n> \n> The frameId may be used by other algorithms later.\n\nI assume the last line is meant to explain why you do this. Let's write\nit as\n\nThis will allow the frameId to be used by other algorithms, without\nhaving to keep multiple private frame counters.\n\nReviewed-by: Laurent Pinchart \n\n> Signed-off-by: Jean-Michel Hautbois \n> Tested-by: Peter Griffin \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 dd97afc0..50980835 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -82,8 +82,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> @@ -255,6 +253,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> @@ -278,7 +278,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 9cb2a9fd..992c9225 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 b94ade0c..c447369f 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 2d79f15f..732ca2bb 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -56,8 +56,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> @@ -239,7 +238,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> @@ -250,8 +248,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> @@ -286,10 +285,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 93607BF415\n\tfor ;\n\tWed, 23 Feb 2022 14:23:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BFEA761144;\n\tWed, 23 Feb 2022 15:23:54 +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 36143601FF\n\tfor ;\n\tWed, 23 Feb 2022 15:23:53 +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 9E2C2DD;\n\tWed, 23 Feb 2022 15:23:52 +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=\"D+kIriiJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1645626232;\n\tbh=TSA0Bq68Q9ScLmv/bWg3r1EU7D9kzhkvm2R8ms7+B3A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=D+kIriiJ+0NDLIQ7/iHGV5sVPGMIA+TwpWoNX+Umdzw2XUNwpKDPoVuIqFFY6nMdz\n\t886+Z5iPDaBqfoVuT4gzRTRghXbkO2nLdrMzuzlAMtKWeyUe9v92tJ9bR1oSNXG8+e\n\t+hVYtOM8l1jBefEKkTZFL8yJrvsz/GgMkftaBWWY=","Date":"Wed, 23 Feb 2022 16:23:42 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20220223104824.25723-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220223104824.25723-2-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220223104824.25723-2-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] 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\" "}}]