[{"id":22463,"web_url":"https://patchwork.libcamera.org/comment/22463/","msgid":"","date":"2022-03-27T20:31:59","subject":"Re: [libcamera-devel] [PATCH v3 4/5] ipa: rkisp1: agc: Add a\n\thistogram-based gain","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, Feb 24, 2022 at 12:33:46PM +0100, Jean-Michel Hautbois wrote:\n> As for the IPU3, we can estimate the histogram of the luminance. The\n> RkISP1 can estimate multiple ones, the R, G and B ones, the Y only one\n> and a combination of RGB. The one we are interested by in AGC is the Y\n> histogram.\n> \n> Use the hardware revision to determine the number of bins of the\n> produced histogram, and use it to populate a vector passed down to the\n> libipa::Histogram class.\n\nYou can drop the second part of the sentence, you don't copy the values\nanymore.\n\n> Signed-off-by: Jean-Michel Hautbois \n> Tested-by: Peter Griffin \n> ---\n> v3:\n> - Don't copy histogram values, pass it to the Histogram class\n> directly\n> - Change the date of the copyright\n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 94 ++++++++++++++++++++++++++-----\n> src/ipa/rkisp1/algorithms/agc.h | 6 +-\n> src/ipa/rkisp1/ipa_context.cpp | 3 +\n> src/ipa/rkisp1/ipa_context.h | 1 +\n> 4 files changed, 88 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index dd97afc0..2e95ee05 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -1,6 +1,6 @@\n> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> /*\n> - * Copyright (C) 2021, Ideas On Board\n> + * Copyright (C) 2021-2022, Ideas On Board\n> *\n> * agc.cpp - AGC/AEC mean-based control algorithm\n> */\n> @@ -16,6 +16,8 @@\n> \n> #include \n> \n> +#include \"libipa/histogram.h\"\n> +\n> /**\n> * \\file agc.h\n> */\n> @@ -43,18 +45,22 @@ static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> /* Number of frames to wait before calculating stats on minimum exposure */\n> static constexpr uint32_t kNumStartupFrames = 10;\n> \n> +/* Target value to reach for the top 2% of the histogram */\n> +static constexpr double kEvGainTarget = 0.5;\n> +\n> /*\n> * Relative luminance target.\n> *\n> * It's a number that's chosen so that, when the camera points at a grey\n> * target, the resulting image brightness is considered right.\n> *\n> - * \\todo Why is the value different between IPU3 and RkISP1 ?\n> + * \\todo The value is not the same for IPU3 and RkIsp1 because we don't have\n> + * sensor degamma yet.\n\nThe sensor output is supposed to be nearly linear already. I'd be very\nsurprised if this alone explained the difference between the two values.\nI'd keep the original comment.\n\nReviewed-by: Laurent Pinchart \n\n> */\n> static constexpr double kRelativeLuminanceTarget = 0.4;\n> \n> Agc::Agc()\n> -\t: frameCount_(0), numCells_(0), filteredExposure_(0s)\n> +\t: frameCount_(0), numCells_(0), numHistBins_(0), filteredExposure_(0s)\n> {\n> }\n> \n> @@ -65,8 +71,7 @@ Agc::Agc()\n> *\n> * \\return 0\n> */\n> -int Agc::configure(IPAContext &context,\n> -\t\t [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> +int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> {\n> \t/* Configure the default exposure and gain. */\n> \tcontext.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> @@ -77,10 +82,22 @@ int Agc::configure(IPAContext &context,\n> \t * - versions < V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V10 entries,\n> \t * - versions >= V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V12 entries.\n> \t */\n> -\tif (context.configuration.hw.revision < RKISP1_V12)\n> +\tif (context.configuration.hw.revision < RKISP1_V12) {\n> \t\tnumCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> -\telse\n> +\t\tnumHistBins_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n> +\t} else {\n> \t\tnumCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> +\t\tnumHistBins_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n> +\t}\n> +\n> +\t/*\n> +\t * Define the measurement window for AGC as a centered rectangle\n> +\t * covering 3/4 of the image width and height.\n> +\t */\n> +\tcontext.configuration.agc.measureWindow.h_offs = configInfo.outputSize.width / 8;\n> +\tcontext.configuration.agc.measureWindow.v_offs = configInfo.outputSize.height / 8;\n> +\tcontext.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;\n> +\tcontext.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n> \n> \t/* \\todo Use actual frame index by populating it in the frameContext. */\n> \tframeCount_ = 0;\n> @@ -126,8 +143,9 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n> * \\brief Estimate the new exposure and gain values\n> * \\param[inout] frameContext The shared IPA frame Context\n> * \\param[in] yGain The gain calculated on the current brightness level\n> + * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n> */\n> -void Agc::computeExposure(IPAContext &context, double yGain)\n> +void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)\n> {\n> \tIPASessionConfiguration &configuration = context.configuration;\n> \tIPAFrameContext &frameContext = context.frameContext;\n> @@ -136,6 +154,9 @@ void Agc::computeExposure(IPAContext &context, double yGain)\n> \tuint32_t exposure = frameContext.sensor.exposure;\n> \tdouble analogueGain = frameContext.sensor.gain;\n> \n> +\t/* Use the highest of the two gain estimates. */\n> +\tdouble evGain = std::max(yGain, iqMeanGain);\n> +\n> \tutils::Duration minShutterSpeed = configuration.agc.minShutterSpeed;\n> \tutils::Duration maxShutterSpeed = std::min(configuration.agc.maxShutterSpeed,\n> \t\t\t\t\t\t kMaxShutterSpeed);\n> @@ -146,7 +167,7 @@ void Agc::computeExposure(IPAContext &context, double yGain)\n> \t\t\t\t\t kMaxAnalogueGain);\n> \n> \t/* Consider within 1% of the target as correctly exposed. */\n> -\tif (utils::abs_diff(yGain, 1.0) < 0.01)\n> +\tif (utils::abs_diff(evGain, 1.0) < 0.01)\n> \t\treturn;\n> \n> \t/* extracted from Rpi::Agc::computeTargetExposure. */\n> @@ -163,13 +184,13 @@ void Agc::computeExposure(IPAContext &context, double yGain)\n> \tLOG(RkISP1Agc, Debug) << \"Actual total exposure \" << currentShutter * analogueGain\n> \t\t\t << \" Shutter speed \" << currentShutter\n> \t\t\t << \" Gain \" << analogueGain\n> -\t\t\t << \" Needed ev gain \" << yGain;\n> +\t\t\t << \" Needed ev gain \" << evGain;\n> \n> \t/*\n> \t * Calculate the current exposure value for the scene as the latest\n> \t * exposure value applied multiplied by the new estimated gain.\n> \t */\n> -\tutils::Duration exposureValue = effectiveExposureValue * yGain;\n> +\tutils::Duration exposureValue = effectiveExposureValue * evGain;\n> \n> \t/* Clamp the exposure value to the min and max authorized. */\n> \tutils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain;\n> @@ -240,6 +261,18 @@ double Agc::estimateLuminance(const rkisp1_cif_isp_ae_stat *ae,\n> \treturn ySum / numCells_ / 255;\n> }\n> \n> +/**\n> + * \\brief Estimate the mean value of the top 2% of the histogram\n> + * \\param[in] hist The histogram statistics computed by the ImgU\n> + * \\return The mean value of the top 2% of the histogram\n> + */\n> +double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const\n> +{\n> +\tHistogram histogram{ Span(hist->hist_bins, numHistBins_) };\n> +\t/* Estimate the quantile mean of the top 2% of the histogram. */\n> +\treturn histogram.interQuantileMean(0.98, 1.0);\n> +}\n> +\n> /**\n> * \\brief Process RkISP1 statistics, and run AGC operations\n> * \\param[in] context The shared IPA context\n> @@ -254,6 +287,10 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)\n> \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n> \n> \tconst rkisp1_cif_isp_ae_stat *ae = ¶ms->ae;\n> +\tconst rkisp1_cif_isp_hist_stat *hist = ¶ms->hist;\n> +\n> +\tdouble iqMean = measureBrightness(hist);\n> +\tdouble iqMeanGain = kEvGainTarget * numHistBins_ / iqMean;\n> \n> \t/*\n> \t * Estimate the gain needed to achieve a relative luminance target. To\n> @@ -277,15 +314,44 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)\n> \t\t\tbreak;\n> \t}\n> \n> -\tcomputeExposure(context, yGain);\n> +\tcomputeExposure(context, yGain, iqMeanGain);\n> \tframeCount_++;\n> }\n> \n> -void Agc::prepare([[maybe_unused]] IPAContext &context,\n> -\t\t rkisp1_params_cfg *params)\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> {\n> +\tif (context.frameContext.frameCount > 0)\n> +\t\treturn;\n> +\n> +\t/* Configure the measurement window. */\n> +\tparams->meas.aec_config.meas_window = context.configuration.agc.measureWindow;\n> +\t/* Use a continuous method for measure. */\n> +\tparams->meas.aec_config.autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0;\n> +\t/* Estimate Y as (R + G + B) x (85/256). */\n> +\tparams->meas.aec_config.mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1;\n> +\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> \tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;\n> \tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\n> +\t/* Configure histogram. */\n> +\tparams->meas.hst_config.meas_window = context.configuration.agc.measureWindow;\n> +\t/* Produce the luminance histogram. */\n> +\tparams->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;\n> +\t/* Set an average weighted histogram. */\n> +\tfor (unsigned int histBin = 0; histBin < numHistBins_; histBin++)\n> +\t\tparams->meas.hst_config.hist_weight[histBin] = 1;\n> +\t/* Step size can't be less than 3. */\n> +\tparams->meas.hst_config.histogram_predivider = 4;\n> +\n> +\t/* Update the configuration for histogram. */\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;\n> +\t/* Enable the histogram measure unit. */\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_HST;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;\n> }\n> \n> } /* namespace ipa::rkisp1::algorithms */\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 942c9d7a..ce1adf27 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -1,6 +1,6 @@\n> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> /*\n> - * Copyright (C) 2021, Ideas On Board\n> + * Copyright (C) 2021-2022, Ideas On Board\n> *\n> * agc.h - RkISP1 AGC/AEC mean-based control algorithm\n> */\n> @@ -32,13 +32,15 @@ public:\n> \tvoid process(IPAContext &context, const rkisp1_stat_buffer *stats) override;\n> \n> private:\n> -\tvoid computeExposure(IPAContext &Context, double yGain);\n> +\tvoid computeExposure(IPAContext &Context, double yGain, double iqMeanGain);\n> \tutils::Duration filterExposure(utils::Duration exposureValue);\n> \tdouble estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);\n> +\tdouble measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const;\n> \n> \tuint64_t frameCount_;\n> \n> \tuint32_t numCells_;\n> +\tuint32_t numHistBins_;\n> \n> \tutils::Duration filteredExposure_;\n> };\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 664f572f..790e159b 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -71,6 +71,9 @@ namespace libcamera::ipa::rkisp1 {\n> * \\var IPASessionConfiguration::agc.maxAnalogueGain\n> * \\brief Maximum analogue gain supported with the configured sensor\n> *\n> + * \\var IPASessionConfiguration::agc.measureWindow\n> + * \\brief AGC measure window\n> + *\n> * \\var IPASessionConfiguration::hw\n> * \\brief RkISP1-specific hardware information\n> *\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 212fa052..35e9b8e5 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -22,6 +22,7 @@ struct IPASessionConfiguration {\n> \t\tutils::Duration maxShutterSpeed;\n> \t\tdouble minAnalogueGain;\n> \t\tdouble maxAnalogueGain;\n> +\t\tstruct rkisp1_cif_isp_window measureWindow;\n> \t} agc;\n> \n> \tstruct {","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 0775EC3256\n\tfor ;\n\tSun, 27 Mar 2022 20:32:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 51A6565631;\n\tSun, 27 Mar 2022 22:32:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5222C600AB\n\tfor ;\n\tSun, 27 Mar 2022 22:32:01 +0200 (CEST)","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 AC3BE2F7;\n\tSun, 27 Mar 2022 22:32:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648413122;\n\tbh=lSBSnzRPwRjd5NmpuypniqZf6VMeGKXZ11yacLy8ZRo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=NfZo8j7yvwCRsrwUZ0BjVm2s144H5y/+GE511/Pu5eyR72Vuw9sjZHfLoMRibL4Bt\n\t3AAMmKIxkmhy1WB5rwBsDkg42hcNDX03n2lRrRySfTvYhFQsoIsrkHNY1EsiSWWbdm\n\t8UWEYBgFqRzqsmZv1GywClTjvVLqhR/Wo4NP4ynC99l8SnacCCoO6JFrnLn2hR2b0X\n\tzodMkAUqnklgJYxBx0cnIUBGyJY4zjPgSXZWwNpbp662CKEf+U38TnzfLyk0vkuUAh\n\t7lSsxGGOVcbcgWXXf/0gDQkH1yDoxWzVcG/LmqdQk5gqtX6rDF3TG2pQJQsIeizU45\n\th4RiOoO0iN9Og==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648413120;\n\tbh=lSBSnzRPwRjd5NmpuypniqZf6VMeGKXZ11yacLy8ZRo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=h9aQ7IoVt/FBz/VX1mkacvEJGLG2csT0AcNI4CzsPl4NGb2H2GyiXmeqnFHf1Iopz\n\tWDzTggwjZkpoaxbtPYwY3c9UFZOBnUn+y21ft5t8Tr80AKn/S0nFdzVClCmGFS58EC\n\tSJKXRWkr+q0B5GIY53qQqGmLhpvEJ1gtVovLM5M0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"h9aQ7IoV\"; dkim-atps=neutral","Date":"Sun, 27 Mar 2022 23:31:59 +0300","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20220224113347.80001-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220224113347.80001-5-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220224113347.80001-5-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/5] ipa: rkisp1: agc: Add a\n\thistogram-based gain","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]