Patch Detail
Show a patch.
GET /api/1.1/patches/19944/?format=api
{ "id": 19944, "url": "https://patchwork.libcamera.org/api/1.1/patches/19944/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19944/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20240424124917.1250837-9-dan.scally@ideasonboard.com>", "date": "2024-04-24T12:49:17", "name": "[v3,8/8] ipa: rkisp1: Remove bespoke Agc functions", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "af265442a0b6a51e63403e41de9623e0a22cbdc2", "submitter": { "id": 156, "url": "https://patchwork.libcamera.org/api/1.1/people/156/?format=api", "name": "Dan Scally", "email": "dan.scally@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/19944/mbox/", "series": [ { "id": 4270, "url": "https://patchwork.libcamera.org/api/1.1/series/4270/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4270", "date": "2024-04-24T12:49:09", "name": "Centralise Agc into libipa", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4270/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/19944/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/19944/checks/", "tags": {}, "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 4B696C32CA\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Apr 2024 12:49:53 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8F78B63413;\n\tWed, 24 Apr 2024 14:49:48 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E954A633EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Apr 2024 14:49:36 +0200 (CEST)", "from mail.ideasonboard.com\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 DF2EB7E9;\n\tWed, 24 Apr 2024 14:48: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=\"tPcoVlUq\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713962925;\n\tbh=icsVYqpzWKMdxWoJx49k/I/aMqjnFH3qlC4wOOXU8Bc=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=tPcoVlUqs/Y2oWZK0nnsKmEkhyNyWR3zTL+tdgkCChBm7b7Jj8UTfUZwaKOLx+SU6\n\tEP1sIlN6NBVEaY9z8cgQ90018uzRlKiYmk8+l/DrEj93XaWp3bCbU04i0Y1X1LgqlO\n\tuueOGvz4IH+IUo9NW165bw12/BcIB596FXBjMGyg=", "From": "Daniel Scally <dan.scally@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Daniel Scally <dan.scally@ideasonboard.com>,\n\tStefan Klug <stefan.klug@ideasonboard.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>", "Subject": "[PATCH v3 8/8] ipa: rkisp1: Remove bespoke Agc functions", "Date": "Wed, 24 Apr 2024 13:49:17 +0100", "Message-Id": "<20240424124917.1250837-9-dan.scally@ideasonboard.com>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20240424124917.1250837-1-dan.scally@ideasonboard.com>", "References": "<20240424124917.1250837-1-dan.scally@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "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>" }, "content": "Now that the rkisp1 Agc algorithm is a derivation of MeanLuminanceAgc\nwe can remove the bespoke functions from the IPA's class.\n\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\nSigned-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n---\nChanges in v3:\n\n\t- None\n\nChanges 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(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex f8d1b9a3..3e98496b 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@@ -116,12 +93,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 \tsetLimits(context.configuration.sensor.minShutterSpeed,\n \t\t context.configuration.sensor.maxShutterSpeed,\n@@ -223,122 +194,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<std::micro>());\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+\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<std::micro>());\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@@ -352,8 +225,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@@ -362,47 +233,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\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-\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<std::micro>());\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<std::micro>());\n-}\n-\n double Agc::estimateLuminance(double gain) const\n {\n \tdouble ySum = 0.0;\n@@ -447,40 +277,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<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+\tHistogram hist({ params->hist.hist_bins, context.hw->numHistogramBins });\n \texpMeans_ = { params->ae.exp_mean, context.hw->numAeCells };\n \n \t/*\n@@ -497,7 +294,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 \"\ndiff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\nindex 34504459..f2f5b59d 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<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 \tdouble estimateLuminance(double gain) const override;\n \n-\tuint64_t frameCount_;\n-\n-\tutils::Duration filteredExposure_;\n-\n \tSpan<const uint8_t> expMeans_;\n };\n \n", "prefixes": [ "v3", "8/8" ] }