[{"id":29097,"web_url":"https://patchwork.libcamera.org/comment/29097/","msgid":"<20240327162210.ixpvpprj6j3klfyl@jasper>","date":"2024-03-27T16:22:10","subject":"Re: [PATCH 10/10] ipa: rkisp1: Remove bespoke Agc functions","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Daniel,\n\nthanks for the patch.\n\nI didn't test it, but that will get cought by CI. Does every commit\nbuild? I just noticed, that the filterExposure missed an overwrite in\nthe previous patch.\n\nOtherwise looks good to me.\n\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> \n\nCheers,\nStefan\n\nOn Fri, Mar 22, 2024 at 01:14:51PM +0000, 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> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> ---\n>  src/ipa/rkisp1/algorithms/agc.cpp | 222 ------------------------------\n>  src/ipa/rkisp1/algorithms/agc.h   |   9 --\n>  2 files changed, 231 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 3389c471..5e6a8ba0 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -42,24 +42,7 @@ static constexpr double kMinAnalogueGain = 1.0;\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> @@ -127,12 +110,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>  \tfor (auto &[id, helper] : exposureModeHelpers()) {\n>  \t\t/* \\todo Run this again when FrameDurationLimits is passed in */\n>  \t\thelper->configure(context.configuration.sensor.minShutterSpeed,\n> @@ -234,170 +211,6 @@ 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> -{\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> -\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<utils::Duration>(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> -}\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> - * would be output by the sensor if an additional \\a gain was applied.\n> - *\n> - * The estimation is based on the AE statistics for the current frame. Y\n> - * averages for all cells are first multiplied by the gain, and then saturated\n> - * to approximate the sensor behaviour at high brightness values. The\n> - * approximation is quite rough, as it doesn't take into account non-linearities\n> - * when approaching saturation. In this case, saturating after the conversion to\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> - * More detailed information can be found in:\n> - * https://en.wikipedia.org/wiki/Relative_luminance\n> - *\n> - * \\return The relative luminance\n> - */\n> -double Agc::estimateLuminance(Span<const uint8_t> 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<const uint32_t> 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> @@ -465,43 +278,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t * we receive), but is important in manual mode.\n>  \t */\n>  \n> -\tconst rkisp1_cif_isp_stat *params = &stats->params;\n>  \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n>  \n> -\tSpan<const uint8_t> ae{ params->ae.exp_mean, context.hw->numAeCells };\n> -\tSpan<const uint32_t> 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>  \tparseStatistics(stats, context);\n>  \n>  \t/*\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 1271741e..311d4e94 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -44,21 +44,12 @@ 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<const uint8_t> expMeans, double gain);\n> -\tdouble measureBrightness(Span<const uint32_t> hist) const;\n>  \tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n>  \t\t\t  ControlList &metadata);\n>  \tvoid parseStatistics(const rkisp1_stat_buffer *stats,\n>  \t\t\t     IPAContext &context);\n>  \tdouble estimateLuminance(double gain) override;\n>  \n> -\tuint64_t frameCount_;\n> -\n> -\tutils::Duration filteredExposure_;\n> -\n>  \tHistogram hist_;\n>  \tSpan<const uint8_t> expMeans_;\n>  };\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 9A39FBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Mar 2024 16:22:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9440963352;\n\tWed, 27 Mar 2024 17:22:15 +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 AA17263331\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Mar 2024 17:22:13 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:da09:7e54:ae7f:d731])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3106013AC;\n\tWed, 27 Mar 2024 17:21:41 +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=\"V+/YETrF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711556501;\n\tbh=86/TbxcRVqFkT8wCuGFaDwubp0pOI1Y2lISaRryOuWw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=V+/YETrF2li0T1dIJUjih/DD7VDmq6ruAWMqcskUG8ei6TA8YSKM8bcZaqeKBNEHa\n\tMGjZB0DHuQwHugwX1ytGp2sRQCV5ophIY7qrBfWO0bZ+I/BtQ1NdADh4o0SegFOqAs\n\tSz049hfhDOyEJIA8hjayyN0GHIvk+a2I2faXg3rA=","Date":"Wed, 27 Mar 2024 17:22:10 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Daniel Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 10/10] ipa: rkisp1: Remove bespoke Agc functions","Message-ID":"<20240327162210.ixpvpprj6j3klfyl@jasper>","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<20240322131451.3092931-11-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240322131451.3092931-11-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29099,"web_url":"https://patchwork.libcamera.org/comment/29099/","msgid":"<92c8a344-abd8-41fd-b26f-10d9bc2ff3d4@ideasonboard.com>","date":"2024-03-27T16:38:17","subject":"Re: [PATCH 10/10] ipa: rkisp1: Remove bespoke Agc functions","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\nOn 27/03/2024 16:22, Stefan Klug wrote:\n> Hi Daniel,\n>\n> thanks for the patch.\n>\n> I didn't test it, but that will get cought by CI. Does every commit\n> build? I just noticed, that the filterExposure missed an overwrite in\n> the previous patch.\n\n\nEvery commit builds yes. You mean the bespoke filterExposure() is still present? That's true but it \nshouldn't matter for the base class to use the function, should it?\n\n\n>\n> Otherwise looks good to me.\n>\n> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>\n\n\nThanks!\n\n\nDan\n\n>\n> Cheers,\n> Stefan\n>\n> On Fri, Mar 22, 2024 at 01:14:51PM +0000, 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>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n>> ---\n>>   src/ipa/rkisp1/algorithms/agc.cpp | 222 ------------------------------\n>>   src/ipa/rkisp1/algorithms/agc.h   |   9 --\n>>   2 files changed, 231 deletions(-)\n>>\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n>> index 3389c471..5e6a8ba0 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n>> @@ -42,24 +42,7 @@ static constexpr double kMinAnalogueGain = 1.0;\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>> @@ -127,12 +110,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>>   \tfor (auto &[id, helper] : exposureModeHelpers()) {\n>>   \t\t/* \\todo Run this again when FrameDurationLimits is passed in */\n>>   \t\thelper->configure(context.configuration.sensor.minShutterSpeed,\n>> @@ -234,170 +211,6 @@ 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>> -{\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>> -\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<utils::Duration>(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>> -}\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>> - * would be output by the sensor if an additional \\a gain was applied.\n>> - *\n>> - * The estimation is based on the AE statistics for the current frame. Y\n>> - * averages for all cells are first multiplied by the gain, and then saturated\n>> - * to approximate the sensor behaviour at high brightness values. The\n>> - * approximation is quite rough, as it doesn't take into account non-linearities\n>> - * when approaching saturation. In this case, saturating after the conversion to\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>> - * More detailed information can be found in:\n>> - * https://en.wikipedia.org/wiki/Relative_luminance\n>> - *\n>> - * \\return The relative luminance\n>> - */\n>> -double Agc::estimateLuminance(Span<const uint8_t> 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<const uint32_t> 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>> @@ -465,43 +278,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>>   \t * we receive), but is important in manual mode.\n>>   \t */\n>>   \n>> -\tconst rkisp1_cif_isp_stat *params = &stats->params;\n>>   \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n>>   \n>> -\tSpan<const uint8_t> ae{ params->ae.exp_mean, context.hw->numAeCells };\n>> -\tSpan<const uint32_t> 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>>   \tparseStatistics(stats, context);\n>>   \n>>   \t/*\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n>> index 1271741e..311d4e94 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.h\n>> +++ b/src/ipa/rkisp1/algorithms/agc.h\n>> @@ -44,21 +44,12 @@ 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<const uint8_t> expMeans, double gain);\n>> -\tdouble measureBrightness(Span<const uint32_t> hist) const;\n>>   \tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n>>   \t\t\t  ControlList &metadata);\n>>   \tvoid parseStatistics(const rkisp1_stat_buffer *stats,\n>>   \t\t\t     IPAContext &context);\n>>   \tdouble estimateLuminance(double gain) override;\n>>   \n>> -\tuint64_t frameCount_;\n>> -\n>> -\tutils::Duration filteredExposure_;\n>> -\n>>   \tHistogram hist_;\n>>   \tSpan<const uint8_t> expMeans_;\n>>   };\n>> -- \n>> 2.34.1\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EA367C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Mar 2024 16:38:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9851D63331;\n\tWed, 27 Mar 2024 17:38:23 +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 7F30361C41\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Mar 2024 17:38:21 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 132821890;\n\tWed, 27 Mar 2024 17:37:49 +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=\"d3QBNPUu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711557469;\n\tbh=C81ByYSMQ76Aqr/96GfiMAirv/tv9ObrQbeV2vqwjp8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=d3QBNPUuuESp9koeDm1Mt+Yj7fsPAfRiT1RcOIfjZslCS+88M3Rh0mXFsj+oUUyrj\n\tojxco+O+jYZvfPBDRK60zpb2OBc4TOfs91U2aYnaMtD9Tr2rbR/PwnpEjQpPEEklJD\n\tKrAUesGD8nLd1vpCYF4n8/OoIhax3NQXxHCy1LLY=","Message-ID":"<92c8a344-abd8-41fd-b26f-10d9bc2ff3d4@ideasonboard.com>","Date":"Wed, 27 Mar 2024 16:38:17 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 10/10] ipa: rkisp1: Remove bespoke Agc functions","Content-Language":"en-US","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<20240322131451.3092931-11-dan.scally@ideasonboard.com>\n\t<20240327162210.ixpvpprj6j3klfyl@jasper>","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240327162210.ixpvpprj6j3klfyl@jasper>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29186,"web_url":"https://patchwork.libcamera.org/comment/29186/","msgid":"<ZhUW8VdVaPrNgvxf@pyrite.rasen.tech>","date":"2024-04-09T10:22:41","subject":"Re: [PATCH 10/10] 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":"On Fri, Mar 22, 2024 at 01:14:51PM +0000, 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> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n\nMy comments are in a patch! (\"fixup: ipa: rkisp1: Remove bespoke Agc\nfunctions\" [1])\n\n[1] https://patchwork.libcamera.org/patch/19853/\n\n\nThanks,\n\nPaul\n\n> ---\n>  src/ipa/rkisp1/algorithms/agc.cpp | 222 ------------------------------\n>  src/ipa/rkisp1/algorithms/agc.h   |   9 --\n>  2 files changed, 231 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 3389c471..5e6a8ba0 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -42,24 +42,7 @@ static constexpr double kMinAnalogueGain = 1.0;\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> @@ -127,12 +110,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>  \tfor (auto &[id, helper] : exposureModeHelpers()) {\n>  \t\t/* \\todo Run this again when FrameDurationLimits is passed in */\n>  \t\thelper->configure(context.configuration.sensor.minShutterSpeed,\n> @@ -234,170 +211,6 @@ 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> -{\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> -\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<utils::Duration>(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> -}\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> - * would be output by the sensor if an additional \\a gain was applied.\n> - *\n> - * The estimation is based on the AE statistics for the current frame. Y\n> - * averages for all cells are first multiplied by the gain, and then saturated\n> - * to approximate the sensor behaviour at high brightness values. The\n> - * approximation is quite rough, as it doesn't take into account non-linearities\n> - * when approaching saturation. In this case, saturating after the conversion to\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> - * More detailed information can be found in:\n> - * https://en.wikipedia.org/wiki/Relative_luminance\n> - *\n> - * \\return The relative luminance\n> - */\n> -double Agc::estimateLuminance(Span<const uint8_t> 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<const uint32_t> 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> @@ -465,43 +278,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t * we receive), but is important in manual mode.\n>  \t */\n>  \n> -\tconst rkisp1_cif_isp_stat *params = &stats->params;\n>  \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n>  \n> -\tSpan<const uint8_t> ae{ params->ae.exp_mean, context.hw->numAeCells };\n> -\tSpan<const uint32_t> 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>  \tparseStatistics(stats, context);\n>  \n>  \t/*\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 1271741e..311d4e94 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -44,21 +44,12 @@ 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<const uint8_t> expMeans, double gain);\n> -\tdouble measureBrightness(Span<const uint32_t> hist) const;\n>  \tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n>  \t\t\t  ControlList &metadata);\n>  \tvoid parseStatistics(const rkisp1_stat_buffer *stats,\n>  \t\t\t     IPAContext &context);\n>  \tdouble estimateLuminance(double gain) override;\n>  \n> -\tuint64_t frameCount_;\n> -\n> -\tutils::Duration filteredExposure_;\n> -\n>  \tHistogram hist_;\n>  \tSpan<const uint8_t> expMeans_;\n>  };\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C9C72BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  9 Apr 2024 10:22:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE49363352;\n\tTue,  9 Apr 2024 12:22:49 +0200 (CEST)","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 E50A76333B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Apr 2024 12:22:48 +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 64A008B9;\n\tTue,  9 Apr 2024 12:22:06 +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=\"IvjMkC6J\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712658127;\n\tbh=28talPHw3PGhiiGJcHOKZeJ2V+BlON7CcuNswki5nXI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IvjMkC6JJP9HGg13ZFezr3lfHB/g7GFYNT6Gytl4Dwk+i425wh+KB6nbnKwyW+y06\n\tjz5gYGtUJqG51cRYiaXDRT+N3HuXkrBgO3Thi1pXc9eGYDXmEfhWAuH7WhthS5PvHi\n\tGxXdldq7cKuC9P+IHyEJmpRi8hr9t9Ja7z+r45eg=","Date":"Tue, 9 Apr 2024 19:22:41 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Daniel Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 10/10] ipa: rkisp1: Remove bespoke Agc functions","Message-ID":"<ZhUW8VdVaPrNgvxf@pyrite.rasen.tech>","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<20240322131451.3092931-11-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240322131451.3092931-11-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]