[{"id":20896,"web_url":"https://patchwork.libcamera.org/comment/20896/","msgid":"<13296a58-30c9-e81b-af20-43fdb2d2be96@ideasonboard.com>","date":"2021-11-12T08:10:55","subject":"Re: [libcamera-devel] [PATCH v4 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi JM,\n\nThank you for the pathc.\n\nOn 11/11/21 7:39 PM, Jean-Michel Hautbois wrote:\n> The pipeline handler populates the new sensorControls ControlList, to\n> have the effective exposure and gain values for the current frame. This\n> is done when a statistics buffer is received.\n>\n> Make those values the frameContext::sensor values for the frame when the\n> EventStatReady event is received.\n>\n> AGC also needs to use frameContext.sensor as its input values and\n> frameContext.agc as its output values. Modify computeExposure by passing\n> it the frameContext instead of individual exposure and gain values.\n>\n> Signed-off-by: Jean-Michel Hautbois \n> Reviewed-by: Kieran Bingham \n\n\nLooks good to me individually as per the rationale written in the commit \nmessage\n\nReviewed-by: Umang Jain \n\n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 19 ++++++++++---------\n> src/ipa/ipu3/algorithms/agc.h | 2 +-\n> src/ipa/ipu3/ipa_context.cpp | 11 +++++++++++\n> src/ipa/ipu3/ipa_context.h | 5 +++++\n> src/ipa/ipu3/ipu3.cpp | 3 +++\n> 5 files changed, 30 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index b5d736c1..5723f6f3 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -169,10 +169,9 @@ void Agc::filterExposure()\n> \n> /**\n> * \\brief Estimate the new exposure and gain values\n> - * \\param[inout] exposure The exposure value reference as a number of lines\n> - * \\param[inout] gain The gain reference to be updated\n> + * \\param[inout] frameContext The shared IPA frame Context\n> */\n> -void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> +void Agc::computeExposure(IPAFrameContext &frameContext)\n> {\n> \t/* Algorithm initialization should wait for first valid frames */\n> \t/* \\todo - have a number of frames given by DelayedControls ?\n> @@ -189,6 +188,10 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> \t\treturn;\n> \t}\n> \n> +\t/* Get the effective exposure and gain applied on the sensor. */\n> +\tuint32_t exposure = frameContext.sensor.exposure;\n> +\tdouble analogueGain = frameContext.sensor.gain;\n> +\n> \t/* Estimate the gain needed to have the proportion wanted */\n> \tdouble evGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> \n> @@ -233,8 +236,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> \t\t\t << shutterTime << \" and \"\n> \t\t\t << stepGain;\n> \n> -\texposure = shutterTime / lineDuration_;\n> -\tanalogueGain = stepGain;\n> +\t/* Update the estimated exposure and gain. */\n> +\tframeContext.agc.exposure = shutterTime / lineDuration_;\n> +\tframeContext.agc.gain = stepGain;\n> \n> \t/*\n> \t * Update the exposure value for the next process call.\n> @@ -257,11 +261,8 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> */\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> \tmeasureBrightness(stats, context.configuration.grid.bdsGrid);\n> -\tcomputeExposure(exposure, analogueGain);\n> +\tcomputeExposure(context.frameContext);\n> \tframeCount_++;\n> }\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 69e0b831..f0db25ee 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -34,7 +34,7 @@ private:\n> \tvoid measureBrightness(const ipu3_uapi_stats_3a *stats,\n> \t\t\t const ipu3_uapi_grid_config &grid);\n> \tvoid filterExposure();\n> -\tvoid computeExposure(uint32_t &exposure, double &gain);\n> +\tvoid computeExposure(IPAFrameContext &frameContext);\n> \n> \tuint64_t frameCount_;\n> \tuint64_t lastFrame_;\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 2355a9c7..a7ff957d 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -119,6 +119,17 @@ namespace libcamera::ipa::ipu3 {\n> * \\brief White balance gain for B channel\n> */\n> \n> +/**\n> + * \\var IPAFrameContext::sensor\n> + * \\brief Effective sensor values\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> * \\var IPAFrameContext::toneMapping\n> * \\brief Context for ToneMapping and Gamma control\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 1e46c61a..a5a19800 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -47,6 +47,11 @@ struct IPAFrameContext {\n> \t\t} gains;\n> \t} awb;\n> \n> +\tstruct {\n> +\t\tuint32_t exposure;\n> +\t\tdouble gain;\n> +\t} sensor;\n> +\n> \tstruct {\n> \t\tdouble gamma;\n> \t\tstruct ipu3_uapi_gamma_corr_lut gammaCorrection;\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index bcc3863b..38e86e58 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -549,6 +549,9 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n> \t\tconst ipu3_uapi_stats_3a *stats =\n> \t\t\treinterpret_cast(mem.data());\n> \n> +\t\tcontext_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get();\n> +\t\tcontext_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> +\n> \t\tparseStatistics(event.frame, event.frameTimestamp, stats);\n> \t\tbreak;\n> \t}","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 55F17BF415\n\tfor ;\n\tFri, 12 Nov 2021 08:11:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF1426034D;\n\tFri, 12 Nov 2021 09:11:01 +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 CB38560234\n\tfor ;\n\tFri, 12 Nov 2021 09:11:00 +0100 (CET)","from [192.168.1.106] (unknown [103.251.226.254])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CE44B276;\n\tFri, 12 Nov 2021 09:10:59 +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=\"iYdbC5h6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636704660;\n\tbh=WIbWMRKt6EVD87+FaM9GE8H49IHX/jIwehjTvV2CIbk=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=iYdbC5h62YbiJwkUzAwBk+QXHq7QFKob7ijZv9X7evG0E6t5HpXaIDpL51013WmV4\n\tYgf5ihhOuB82fOTEN8AYF29w/+nlayH/5zZ6UCnfm+PRk0w6DvzLpJ4YOaRPW0flyb\n\tCkZzLeRLUyVx8CSKC9mrUFaOCV0O8Yz4OINJhUlU=","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211111140928.136111-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211111140928.136111-4-jeanmichel.hautbois@ideasonboard.com>","From":"Umang Jain ","Message-ID":"<13296a58-30c9-e81b-af20-43fdb2d2be96@ideasonboard.com>","Date":"Fri, 12 Nov 2021 13:40:55 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20211111140928.136111-4-jeanmichel.hautbois@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v4 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20931,"web_url":"https://patchwork.libcamera.org/comment/20931/","msgid":"<20211112225516.GA8453@jade.amanokami.net>","date":"2021-11-12T22:55:16","subject":"Re: [libcamera-devel] [PATCH v4 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Thu, Nov 11, 2021 at 03:09:17PM +0100, Jean-Michel Hautbois wrote:\n> The pipeline handler populates the new sensorControls ControlList, to\n> have the effective exposure and gain values for the current frame. This\n> is done when a statistics buffer is received.\n> \n> Make those values the frameContext::sensor values for the frame when the\n> EventStatReady event is received.\n> \n> AGC also needs to use frameContext.sensor as its input values and\n> frameContext.agc as its output values. Modify computeExposure by passing\n> it the frameContext instead of individual exposure and gain values.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 19 ++++++++++---------\n> src/ipa/ipu3/algorithms/agc.h | 2 +-\n> src/ipa/ipu3/ipa_context.cpp | 11 +++++++++++\n> src/ipa/ipu3/ipa_context.h | 5 +++++\n> src/ipa/ipu3/ipu3.cpp | 3 +++\n> 5 files changed, 30 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index b5d736c1..5723f6f3 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -169,10 +169,9 @@ void Agc::filterExposure()\n> \n> /**\n> * \\brief Estimate the new exposure and gain values\n> - * \\param[inout] exposure The exposure value reference as a number of lines\n> - * \\param[inout] gain The gain reference to be updated\n> + * \\param[inout] frameContext The shared IPA frame Context\n> */\n> -void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> +void Agc::computeExposure(IPAFrameContext &frameContext)\n> {\n> \t/* Algorithm initialization should wait for first valid frames */\n> \t/* \\todo - have a number of frames given by DelayedControls ?\n> @@ -189,6 +188,10 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> \t\treturn;\n> \t}\n> \n> +\t/* Get the effective exposure and gain applied on the sensor. */\n> +\tuint32_t exposure = frameContext.sensor.exposure;\n> +\tdouble analogueGain = frameContext.sensor.gain;\n> +\n> \t/* Estimate the gain needed to have the proportion wanted */\n> \tdouble evGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> \n> @@ -233,8 +236,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> \t\t\t << shutterTime << \" and \"\n> \t\t\t << stepGain;\n> \n> -\texposure = shutterTime / lineDuration_;\n> -\tanalogueGain = stepGain;\n> +\t/* Update the estimated exposure and gain. */\n> +\tframeContext.agc.exposure = shutterTime / lineDuration_;\n> +\tframeContext.agc.gain = stepGain;\n> \n> \t/*\n> \t * Update the exposure value for the next process call.\n> @@ -257,11 +261,8 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)\n> */\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> \tmeasureBrightness(stats, context.configuration.grid.bdsGrid);\n> -\tcomputeExposure(exposure, analogueGain);\n> +\tcomputeExposure(context.frameContext);\n> \tframeCount_++;\n> }\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 69e0b831..f0db25ee 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -34,7 +34,7 @@ private:\n> \tvoid measureBrightness(const ipu3_uapi_stats_3a *stats,\n> \t\t\t const ipu3_uapi_grid_config &grid);\n> \tvoid filterExposure();\n> -\tvoid computeExposure(uint32_t &exposure, double &gain);\n> +\tvoid computeExposure(IPAFrameContext &frameContext);\n> \n> \tuint64_t frameCount_;\n> \tuint64_t lastFrame_;\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 2355a9c7..a7ff957d 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -119,6 +119,17 @@ namespace libcamera::ipa::ipu3 {\n> * \\brief White balance gain for B channel\n> */\n> \n> +/**\n> + * \\var IPAFrameContext::sensor\n> + * \\brief Effective sensor values\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> * \\var IPAFrameContext::toneMapping\n> * \\brief Context for ToneMapping and Gamma control\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 1e46c61a..a5a19800 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -47,6 +47,11 @@ struct IPAFrameContext {\n> \t\t} gains;\n> \t} awb;\n> \n> +\tstruct {\n> +\t\tuint32_t exposure;\n> +\t\tdouble gain;\n> +\t} sensor;\n> +\n> \tstruct {\n> \t\tdouble gamma;\n> \t\tstruct ipu3_uapi_gamma_corr_lut gammaCorrection;\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index bcc3863b..38e86e58 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -549,6 +549,9 @@ void IPAIPU3::processEvent(const IPU3Event &event)\n> \t\tconst ipu3_uapi_stats_3a *stats =\n> \t\t\treinterpret_cast(mem.data());\n> \n> +\t\tcontext_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get();\n> +\t\tcontext_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> +\n> \t\tparseStatistics(event.frame, event.frameTimestamp, stats);\n> \t\tbreak;\n> \t}\n> -- \n> 2.32.0\n>","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 50A59BDB1C\n\tfor ;\n\tFri, 12 Nov 2021 22:55:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69DD46036B;\n\tFri, 12 Nov 2021 23:55:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A6E036032C\n\tfor ;\n\tFri, 12 Nov 2021 23:55:27 +0100 (CET)","from jade.amanokami.net (KD027085206038.au-net.ne.jp\n\t[27.85.206.38])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 814DB74C;\n\tFri, 12 Nov 2021 23:55:25 +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=\"RqR2EWOg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636757727;\n\tbh=Nxa+vPTGEpq5UGctsblml4JSU5RcUWYcIve/2ytiNTc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RqR2EWOgs4Ionx8NatNJDeDcobLSjIXSwGWC2s53B+EpAhQyrcA19GI+m9YXUiof7\n\tZew9ouqiTTW2RkNzbMtxgxpGXByq6xvZ1YBg2bGcnn5MtE9BhXBLAKRgiEvq7E2Cz0\n\tPhFXGJJ4XDDG1+/EuVv1nV8lyJypqZkI8nM3YADw=","Date":"Sat, 13 Nov 2021 07:55:16 +0900","From":"Paul Elder ","To":"Jean-Michel Hautbois ","Message-ID":"<20211112225516.GA8453@jade.amanokami.net>","References":"<20211111140928.136111-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211111140928.136111-4-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20211111140928.136111-4-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 03/14] ipa: ipu3: Use sensor\n\tcontrols to update frameContext","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\" "}}]