[{"id":29267,"web_url":"https://patchwork.libcamera.org/comment/29267/","msgid":"","date":"2024-04-19T02:54:26","subject":"Re: [PATCH v2 8/8] ipa: rkisp1: Remove bespoke Agc functions","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Dan,\n\nOn Wed, Apr 17, 2024 at 02:15:36PM +0100, Daniel Scally wrote:\n> Now that the rkisp1 Agc algorithm is a derivation of MeanLuminanceAgc\n> we can remove the bespoke functions from the IPA's class.\n> \n> Reviewed-by: Stefan Klug \n> Signed-off-by: Daniel Scally \n\nLooks good to me.\n\nReviewed-by: Paul Elder \n\n> ---\n> Changes in v2:\n> \n> \t- Kept the documentation for estimateLuminance()\n> \n> src/ipa/rkisp1/algorithms/agc.cpp | 231 ++----------------------------\n> src/ipa/rkisp1/algorithms/agc.h | 9 --\n> 2 files changed, 14 insertions(+), 226 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 0d66fcca..27b6f2c1 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -36,30 +36,7 @@ namespace ipa::rkisp1::algorithms {\n> \n> LOG_DEFINE_CATEGORY(RkISP1Agc)\n> \n> -/* Minimum limit for analogue gain value */\n> -static constexpr double kMinAnalogueGain = 1.0;\n> -\n> -/* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> -static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> -\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> - */\n> -static constexpr double kRelativeLuminanceTarget = 0.4;\n> -\n> Agc::Agc()\n> -\t: frameCount_(0), filteredExposure_(0s)\n> {\n> \tsupportsRaw_ = true;\n> }\n> @@ -117,12 +94,6 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\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/*\n> -\t * \\todo Use the upcoming per-frame context API that will provide a\n> -\t * frame index\n> -\t */\n> -\tframeCount_ = 0;\n> -\n> \t/* \\todo Run this again when FrameDurationLimits is passed in */\n> \tconfigureExposureModeHelpers(context.configuration.sensor.minShutterSpeed,\n> \t\t\t\t context.configuration.sensor.maxShutterSpeed,\n> @@ -224,122 +195,24 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n> \tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;\n> }\n> \n> -/**\n> - * \\brief Apply a filter on the exposure value to limit the speed of changes\n> - * \\param[in] exposureValue The target exposure from the AGC algorithm\n> - *\n> - * The speed of the filter is adaptive, and will produce the target quicker\n> - * during startup, or when the target exposure is within 20% of the most recent\n> - * filter output.\n> - *\n> - * \\return The filtered exposure\n> - */\n> -utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n> -{\n> -\tdouble speed = 0.2;\n> -\n> -\t/* Adapt instantly if we are in startup phase. */\n> -\tif (frameCount_ < kNumStartupFrames)\n> -\t\tspeed = 1.0;\n> -\n> -\t/*\n> -\t * If we are close to the desired result, go faster to avoid making\n> -\t * multiple micro-adjustments.\n> -\t * \\todo Make this customisable?\n> -\t */\n> -\tif (filteredExposure_ < 1.2 * exposureValue &&\n> -\t filteredExposure_ > 0.8 * exposureValue)\n> -\t\tspeed = sqrt(speed);\n> -\n> -\tfilteredExposure_ = speed * exposureValue +\n> -\t\t\t filteredExposure_ * (1.0 - speed);\n> -\n> -\tLOG(RkISP1Agc, Debug) << \"After filtering, exposure \" << filteredExposure_;\n> -\n> -\treturn filteredExposure_;\n> -}\n> -\n> -/**\n> - * \\brief Estimate the new exposure and gain values\n> - * \\param[inout] context The shared IPA Context\n> - * \\param[in] frameContext The FrameContext for this frame\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, IPAFrameContext &frameContext,\n> -\t\t\t double yGain, double iqMeanGain)\n> +void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> +\t\t ControlList &metadata)\n> {\n> -\tIPASessionConfiguration &configuration = context.configuration;\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/* Use the highest of the two gain estimates. */\n> -\tdouble evGain = std::max(yGain, iqMeanGain);\n> -\n> -\tutils::Duration minShutterSpeed = configuration.sensor.minShutterSpeed;\n> -\tutils::Duration maxShutterSpeed = std::min(configuration.sensor.maxShutterSpeed,\n> -\t\t\t\t\t\t kMaxShutterSpeed);\n> -\n> -\tdouble minAnalogueGain = std::max(configuration.sensor.minAnalogueGain,\n> -\t\t\t\t\t kMinAnalogueGain);\n> -\tdouble maxAnalogueGain = configuration.sensor.maxAnalogueGain;\n> -\n> -\t/* Consider within 1% of the target as correctly exposed. */\n> -\tif (utils::abs_diff(evGain, 1.0) < 0.01)\n> -\t\treturn;\n> -\n> -\t/* extracted from Rpi::Agc::computeTargetExposure. */\n> -\n> -\t/* Calculate the shutter time in seconds. */\n> -\tutils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> -\n> -\t/*\n> -\t * Update the exposure value for the next computation using the values\n> -\t * of exposure and gain really used by the sensor.\n> -\t */\n> -\tutils::Duration effectiveExposureValue = currentShutter * analogueGain;\n> -\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 \" << 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 * evGain;\n> -\n> -\t/* Clamp the exposure value to the min and max authorized. */\n> -\tutils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain;\n> -\texposureValue = std::min(exposureValue, maxTotalExposure);\n> -\tLOG(RkISP1Agc, Debug) << \"Target total exposure \" << exposureValue\n> -\t\t\t << \", maximum is \" << maxTotalExposure;\n> -\n> -\t/*\n> -\t * Divide the exposure value as new exposure and gain values.\n> -\t * \\todo estimate if we need to desaturate\n> -\t */\n> -\texposureValue = filterExposure(exposureValue);\n> +\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> +\t\t\t\t * frameContext.sensor.exposure;\n> +\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> +\tmetadata.set(controls::ExposureTime, exposureTime.get());\n> \n> -\t/*\n> -\t * Push the shutter time up to the maximum first, and only then\n> -\t * increase the gain.\n> -\t */\n> -\tutils::Duration shutterTime = std::clamp(exposureValue / minAnalogueGain,\n> -\t\t\t\t\t\t\t\t minShutterSpeed, maxShutterSpeed);\n> -\tdouble stepGain = std::clamp(exposureValue / shutterTime,\n> -\t\t\t\t minAnalogueGain, maxAnalogueGain);\n> -\tLOG(RkISP1Agc, Debug) << \"Divided up shutter and gain are \"\n> -\t\t\t << shutterTime << \" and \"\n> -\t\t\t << stepGain;\n> +\t/* \\todo Use VBlank value calculated from each frame exposure. */\n> +\tuint32_t vTotal = context.configuration.sensor.size.height\n> +\t\t\t+ context.configuration.sensor.defVBlank;\n> +\tutils::Duration frameDuration = context.configuration.sensor.lineDuration\n> +\t\t\t\t * vTotal;\n> +\tmetadata.set(controls::FrameDuration, frameDuration.get());\n> }\n> \n> /**\n> * \\brief Estimate the relative luminance of the frame with a given gain\n> - * \\param[in] expMeans The mean luminance values, from the RkISP1 statistics\n> * \\param[in] gain The gain to apply to the frame\n> *\n> * This function estimates the average relative luminance of the frame that\n> @@ -353,8 +226,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> * YUV doesn't take into account the fact that the R, G and B components\n> * contribute differently to the relative luminance.\n> *\n> - * \\todo Have a dedicated YUV algorithm ?\n> - *\n> * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a\n> * theoretical perfect reflector of 100% reference white.\n> *\n> @@ -363,47 +234,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> *\n> * \\return The relative luminance\n> */\n> -double Agc::estimateLuminance(Span expMeans, double gain)\n> -{\n> -\tdouble ySum = 0.0;\n> -\n> -\t/* Sum the averages, saturated to 255. */\n> -\tfor (uint8_t expMean : expMeans)\n> -\t\tySum += std::min(expMean * gain, 255.0);\n> -\n> -\t/* \\todo Weight with the AWB gains */\n> -\n> -\treturn ySum / expMeans.size() / 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 RkISP1\n> - * \\return The mean value of the top 2% of the histogram\n> - */\n> -double Agc::measureBrightness(Span hist) const\n> -{\n> -\tHistogram histogram{ hist };\n> -\t/* Estimate the quantile mean of the top 2% of the histogram. */\n> -\treturn histogram.interQuantileMean(0.98, 1.0);\n> -}\n> -\n> -void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t ControlList &metadata)\n> -{\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t * frameContext.sensor.exposure;\n> -\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> -\tmetadata.set(controls::ExposureTime, exposureTime.get());\n> -\n> -\t/* \\todo Use VBlank value calculated from each frame exposure. */\n> -\tuint32_t vTotal = context.configuration.sensor.size.height\n> -\t\t\t+ context.configuration.sensor.defVBlank;\n> -\tutils::Duration frameDuration = context.configuration.sensor.lineDuration\n> -\t\t\t\t * vTotal;\n> -\tmetadata.set(controls::FrameDuration, frameDuration.get());\n> -}\n> -\n> double Agc::estimateLuminance(double gain)\n> {\n> \tdouble ySum = 0.0;\n> @@ -448,40 +278,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \tconst rkisp1_cif_isp_stat *params = &stats->params;\n> \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n> \n> -\tSpan ae{ params->ae.exp_mean, context.hw->numAeCells };\n> -\tSpan hist{\n> -\t\tparams->hist.hist_bins,\n> -\t\tcontext.hw->numHistogramBins\n> -\t};\n> -\n> -\tdouble iqMean = measureBrightness(hist);\n> -\tdouble iqMeanGain = kEvGainTarget * hist.size() / iqMean;\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> -\t * estimated in an iterative process, as multiplying by a gain will not\n> -\t * increase the relative luminance by the same factor if some image\n> -\t * regions are saturated.\n> -\t */\n> -\tdouble yGain = 1.0;\n> -\tdouble yTarget = kRelativeLuminanceTarget;\n> -\n> -\tfor (unsigned int i = 0; i < 8; i++) {\n> -\t\tdouble yValue = estimateLuminance(ae, yGain);\n> -\t\tdouble extra_gain = std::min(10.0, yTarget / (yValue + .001));\n> -\n> -\t\tyGain *= extra_gain;\n> -\t\tLOG(RkISP1Agc, Debug) << \"Y value: \" << yValue\n> -\t\t\t\t << \", Y target: \" << yTarget\n> -\t\t\t\t << \", gives gain \" << yGain;\n> -\t\tif (extra_gain < 1.01)\n> -\t\t\tbreak;\n> -\t}\n> -\n> -\tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> -\tframeCount_++;\n> -\n> +\tHistogram hist({ params->hist.hist_bins, context.hw->numHistogramBins });\n> \texpMeans_ = { params->ae.exp_mean, context.hw->numAeCells };\n> \n> \t/*\n> @@ -498,7 +295,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \tstd::tie(shutterTime, aGain, dGain) =\n> \t\tcalculateNewEv(context.activeState.agc.constraintMode,\n> \t\t\t context.activeState.agc.exposureMode,\n> -\t\t\t Histogram(hist), effectiveExposureValue);\n> +\t\t\t hist, effectiveExposureValue);\n> \n> \tLOG(RkISP1Agc, Debug)\n> \t\t<< \"Divided up shutter, analogue gain and digital gain are \"\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index a8080228..a2c1f61a 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -44,19 +44,10 @@ public:\n> \t\t ControlList &metadata) override;\n> \n> private:\n> -\tvoid computeExposure(IPAContext &Context, IPAFrameContext &frameContext,\n> -\t\t\t double yGain, double iqMeanGain);\n> -\tutils::Duration filterExposure(utils::Duration exposureValue);\n> -\tdouble estimateLuminance(Span expMeans, double gain);\n> -\tdouble measureBrightness(Span hist) const;\n> \tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> \t\t\t ControlList &metadata);\n> \tdouble estimateLuminance(double gain) override;\n> \n> -\tuint64_t frameCount_;\n> -\n> -\tutils::Duration filteredExposure_;\n> -\n> \tSpan expMeans_;\n> };\n> \n> -- \n> 2.34.1\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 2DCCDC3200\n\tfor ;\n\tFri, 19 Apr 2024 02:54:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D59C3633ED;\n\tFri, 19 Apr 2024 04:54:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 401A361B17\n\tfor ;\n\tFri, 19 Apr 2024 04:54:34 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C7B8A63B;\n\tFri, 19 Apr 2024 04:53:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tTN2+jxJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713495226;\n\tbh=PwRSFJMolqRkfxP/ZQG9WVj5RGZtnFY4sQr3lgR7M1k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tTN2+jxJtqzgM/xdLNFpUs8OuEbac6Uva9YlbWHl872KaFridpiEepklPNvTO9zkP\n\tk3HIxxpj8FTrofb+wdtsQ93+Fy1YW2P3lXda8ZPCDUgZ+wLkNJuPGd/keqpxhruqWf\n\tG1kcDdYp5si9zorQ0OAgMr5A0IhuADm51DHY7184=","Date":"Fri, 19 Apr 2024 11:54:26 +0900","From":"Paul Elder ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org,\n\tStefan Klug ","Subject":"Re: [PATCH v2 8/8] ipa: rkisp1: Remove bespoke Agc functions","Message-ID":"","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-9-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240417131536.484129-9-dan.scally@ideasonboard.com>","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":29293,"web_url":"https://patchwork.libcamera.org/comment/29293/","msgid":"","date":"2024-04-22T10:57:09","subject":"Re: [PATCH v2 8/8] ipa: rkisp1: Remove bespoke Agc functions","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Wed, Apr 17, 2024 at 02:15:36PM +0100, Daniel Scally wrote:\n> Now that the rkisp1 Agc algorithm is a derivation of MeanLuminanceAgc\n> we can remove the bespoke functions from the IPA's class.\n>\n> Reviewed-by: Stefan Klug \n> Signed-off-by: Daniel Scally \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> Changes in v2:\n>\n> \t- Kept the documentation for estimateLuminance()\n>\n> src/ipa/rkisp1/algorithms/agc.cpp | 231 ++----------------------------\n> src/ipa/rkisp1/algorithms/agc.h | 9 --\n> 2 files changed, 14 insertions(+), 226 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 0d66fcca..27b6f2c1 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -36,30 +36,7 @@ namespace ipa::rkisp1::algorithms {\n>\n> LOG_DEFINE_CATEGORY(RkISP1Agc)\n>\n> -/* Minimum limit for analogue gain value */\n> -static constexpr double kMinAnalogueGain = 1.0;\n> -\n> -/* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> -static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> -\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> - */\n> -static constexpr double kRelativeLuminanceTarget = 0.4;\n> -\n> Agc::Agc()\n> -\t: frameCount_(0), filteredExposure_(0s)\n> {\n> \tsupportsRaw_ = true;\n> }\n> @@ -117,12 +94,6 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\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/*\n> -\t * \\todo Use the upcoming per-frame context API that will provide a\n> -\t * frame index\n> -\t */\n> -\tframeCount_ = 0;\n> -\n> \t/* \\todo Run this again when FrameDurationLimits is passed in */\n> \tconfigureExposureModeHelpers(context.configuration.sensor.minShutterSpeed,\n> \t\t\t\t context.configuration.sensor.maxShutterSpeed,\n> @@ -224,122 +195,24 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n> \tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;\n> }\n>\n> -/**\n> - * \\brief Apply a filter on the exposure value to limit the speed of changes\n> - * \\param[in] exposureValue The target exposure from the AGC algorithm\n> - *\n> - * The speed of the filter is adaptive, and will produce the target quicker\n> - * during startup, or when the target exposure is within 20% of the most recent\n> - * filter output.\n> - *\n> - * \\return The filtered exposure\n> - */\n> -utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n> -{\n> -\tdouble speed = 0.2;\n> -\n> -\t/* Adapt instantly if we are in startup phase. */\n> -\tif (frameCount_ < kNumStartupFrames)\n> -\t\tspeed = 1.0;\n> -\n> -\t/*\n> -\t * If we are close to the desired result, go faster to avoid making\n> -\t * multiple micro-adjustments.\n> -\t * \\todo Make this customisable?\n> -\t */\n> -\tif (filteredExposure_ < 1.2 * exposureValue &&\n> -\t filteredExposure_ > 0.8 * exposureValue)\n> -\t\tspeed = sqrt(speed);\n> -\n> -\tfilteredExposure_ = speed * exposureValue +\n> -\t\t\t filteredExposure_ * (1.0 - speed);\n> -\n> -\tLOG(RkISP1Agc, Debug) << \"After filtering, exposure \" << filteredExposure_;\n> -\n> -\treturn filteredExposure_;\n> -}\n> -\n> -/**\n> - * \\brief Estimate the new exposure and gain values\n> - * \\param[inout] context The shared IPA Context\n> - * \\param[in] frameContext The FrameContext for this frame\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, IPAFrameContext &frameContext,\n> -\t\t\t double yGain, double iqMeanGain)\n> +void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> +\t\t ControlList &metadata)\n> {\n> -\tIPASessionConfiguration &configuration = context.configuration;\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/* Use the highest of the two gain estimates. */\n> -\tdouble evGain = std::max(yGain, iqMeanGain);\n> -\n> -\tutils::Duration minShutterSpeed = configuration.sensor.minShutterSpeed;\n> -\tutils::Duration maxShutterSpeed = std::min(configuration.sensor.maxShutterSpeed,\n> -\t\t\t\t\t\t kMaxShutterSpeed);\n> -\n> -\tdouble minAnalogueGain = std::max(configuration.sensor.minAnalogueGain,\n> -\t\t\t\t\t kMinAnalogueGain);\n> -\tdouble maxAnalogueGain = configuration.sensor.maxAnalogueGain;\n> -\n> -\t/* Consider within 1% of the target as correctly exposed. */\n> -\tif (utils::abs_diff(evGain, 1.0) < 0.01)\n> -\t\treturn;\n> -\n> -\t/* extracted from Rpi::Agc::computeTargetExposure. */\n> -\n> -\t/* Calculate the shutter time in seconds. */\n> -\tutils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> -\n> -\t/*\n> -\t * Update the exposure value for the next computation using the values\n> -\t * of exposure and gain really used by the sensor.\n> -\t */\n> -\tutils::Duration effectiveExposureValue = currentShutter * analogueGain;\n> -\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 \" << 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 * evGain;\n> -\n> -\t/* Clamp the exposure value to the min and max authorized. */\n> -\tutils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain;\n> -\texposureValue = std::min(exposureValue, maxTotalExposure);\n> -\tLOG(RkISP1Agc, Debug) << \"Target total exposure \" << exposureValue\n> -\t\t\t << \", maximum is \" << maxTotalExposure;\n> -\n> -\t/*\n> -\t * Divide the exposure value as new exposure and gain values.\n> -\t * \\todo estimate if we need to desaturate\n> -\t */\n> -\texposureValue = filterExposure(exposureValue);\n> +\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> +\t\t\t\t * frameContext.sensor.exposure;\n> +\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> +\tmetadata.set(controls::ExposureTime, exposureTime.get());\n>\n> -\t/*\n> -\t * Push the shutter time up to the maximum first, and only then\n> -\t * increase the gain.\n> -\t */\n> -\tutils::Duration shutterTime = std::clamp(exposureValue / minAnalogueGain,\n> -\t\t\t\t\t\t\t\t minShutterSpeed, maxShutterSpeed);\n> -\tdouble stepGain = std::clamp(exposureValue / shutterTime,\n> -\t\t\t\t minAnalogueGain, maxAnalogueGain);\n> -\tLOG(RkISP1Agc, Debug) << \"Divided up shutter and gain are \"\n> -\t\t\t << shutterTime << \" and \"\n> -\t\t\t << stepGain;\n> +\t/* \\todo Use VBlank value calculated from each frame exposure. */\n> +\tuint32_t vTotal = context.configuration.sensor.size.height\n> +\t\t\t+ context.configuration.sensor.defVBlank;\n> +\tutils::Duration frameDuration = context.configuration.sensor.lineDuration\n> +\t\t\t\t * vTotal;\n> +\tmetadata.set(controls::FrameDuration, frameDuration.get());\n> }\n>\n> /**\n> * \\brief Estimate the relative luminance of the frame with a given gain\n> - * \\param[in] expMeans The mean luminance values, from the RkISP1 statistics\n> * \\param[in] gain The gain to apply to the frame\n> *\n> * This function estimates the average relative luminance of the frame that\n> @@ -353,8 +226,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> * YUV doesn't take into account the fact that the R, G and B components\n> * contribute differently to the relative luminance.\n> *\n> - * \\todo Have a dedicated YUV algorithm ?\n> - *\n> * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a\n> * theoretical perfect reflector of 100% reference white.\n> *\n> @@ -363,47 +234,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> *\n> * \\return The relative luminance\n> */\n> -double Agc::estimateLuminance(Span expMeans, double gain)\n> -{\n> -\tdouble ySum = 0.0;\n> -\n> -\t/* Sum the averages, saturated to 255. */\n> -\tfor (uint8_t expMean : expMeans)\n> -\t\tySum += std::min(expMean * gain, 255.0);\n> -\n> -\t/* \\todo Weight with the AWB gains */\n> -\n> -\treturn ySum / expMeans.size() / 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 RkISP1\n> - * \\return The mean value of the top 2% of the histogram\n> - */\n> -double Agc::measureBrightness(Span hist) const\n> -{\n> -\tHistogram histogram{ hist };\n> -\t/* Estimate the quantile mean of the top 2% of the histogram. */\n> -\treturn histogram.interQuantileMean(0.98, 1.0);\n> -}\n> -\n> -void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t ControlList &metadata)\n> -{\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t * frameContext.sensor.exposure;\n> -\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> -\tmetadata.set(controls::ExposureTime, exposureTime.get());\n> -\n> -\t/* \\todo Use VBlank value calculated from each frame exposure. */\n> -\tuint32_t vTotal = context.configuration.sensor.size.height\n> -\t\t\t+ context.configuration.sensor.defVBlank;\n> -\tutils::Duration frameDuration = context.configuration.sensor.lineDuration\n> -\t\t\t\t * vTotal;\n> -\tmetadata.set(controls::FrameDuration, frameDuration.get());\n> -}\n> -\n> double Agc::estimateLuminance(double gain)\n> {\n> \tdouble ySum = 0.0;\n> @@ -448,40 +278,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \tconst rkisp1_cif_isp_stat *params = &stats->params;\n> \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n>\n> -\tSpan ae{ params->ae.exp_mean, context.hw->numAeCells };\n> -\tSpan hist{\n> -\t\tparams->hist.hist_bins,\n> -\t\tcontext.hw->numHistogramBins\n> -\t};\n> -\n> -\tdouble iqMean = measureBrightness(hist);\n> -\tdouble iqMeanGain = kEvGainTarget * hist.size() / iqMean;\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> -\t * estimated in an iterative process, as multiplying by a gain will not\n> -\t * increase the relative luminance by the same factor if some image\n> -\t * regions are saturated.\n> -\t */\n> -\tdouble yGain = 1.0;\n> -\tdouble yTarget = kRelativeLuminanceTarget;\n> -\n> -\tfor (unsigned int i = 0; i < 8; i++) {\n> -\t\tdouble yValue = estimateLuminance(ae, yGain);\n> -\t\tdouble extra_gain = std::min(10.0, yTarget / (yValue + .001));\n> -\n> -\t\tyGain *= extra_gain;\n> -\t\tLOG(RkISP1Agc, Debug) << \"Y value: \" << yValue\n> -\t\t\t\t << \", Y target: \" << yTarget\n> -\t\t\t\t << \", gives gain \" << yGain;\n> -\t\tif (extra_gain < 1.01)\n> -\t\t\tbreak;\n> -\t}\n> -\n> -\tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> -\tframeCount_++;\n> -\n> +\tHistogram hist({ params->hist.hist_bins, context.hw->numHistogramBins });\n> \texpMeans_ = { params->ae.exp_mean, context.hw->numAeCells };\n>\n> \t/*\n> @@ -498,7 +295,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \tstd::tie(shutterTime, aGain, dGain) =\n> \t\tcalculateNewEv(context.activeState.agc.constraintMode,\n> \t\t\t context.activeState.agc.exposureMode,\n> -\t\t\t Histogram(hist), effectiveExposureValue);\n> +\t\t\t hist, effectiveExposureValue);\n>\n> \tLOG(RkISP1Agc, Debug)\n> \t\t<< \"Divided up shutter, analogue gain and digital gain are \"\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index a8080228..a2c1f61a 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -44,19 +44,10 @@ public:\n> \t\t ControlList &metadata) override;\n>\n> private:\n> -\tvoid computeExposure(IPAContext &Context, IPAFrameContext &frameContext,\n> -\t\t\t double yGain, double iqMeanGain);\n> -\tutils::Duration filterExposure(utils::Duration exposureValue);\n> -\tdouble estimateLuminance(Span expMeans, double gain);\n> -\tdouble measureBrightness(Span hist) const;\n> \tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> \t\t\t ControlList &metadata);\n> \tdouble estimateLuminance(double gain) override;\n>\n> -\tuint64_t frameCount_;\n> -\n> -\tutils::Duration filteredExposure_;\n> -\n> \tSpan expMeans_;\n> };\n>\n> --\n> 2.34.1\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 49478C3200\n\tfor ;\n\tMon, 22 Apr 2024 10:57:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92A4363409;\n\tMon, 22 Apr 2024 12:57:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27A4C63408\n\tfor ;\n\tMon, 22 Apr 2024 12:57:13 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 972AE142E;\n\tMon, 22 Apr 2024 12:56:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fMLLOHY2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713783382;\n\tbh=r1bPZ4+CcmOLGXG7rkfA9ol4FkFDLiEusJtE65Bt5s4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fMLLOHY2VUaPLa47DAvzXINvLUwSsoT5/GcRDYErsMQTJi5cN5TBd8KpqjNvT0NGe\n\t0BuDeoe8tkdLfBOoCXqB8iLNCOPwj+eCGSQ4nNSrQfl5APztt7oDLdp0ksr39wkFIB\n\td/g3AEfT6biKVNc5FdnW/EEH7IwtrFSm1rJJSVxg=","Date":"Mon, 22 Apr 2024 12:57:09 +0200","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org, \n\tStefan Klug ","Subject":"Re: [PATCH v2 8/8] ipa: rkisp1: Remove bespoke Agc functions","Message-ID":"","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-9-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240417131536.484129-9-dan.scally@ideasonboard.com>","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\" "}}]