[{"id":29393,"web_url":"https://patchwork.libcamera.org/comment/29393/","msgid":"<3rkpk3wd3sdwbtgi2235hajak6ksjdipsbkmwstleure2bksmz@5qkc2kdkkqiq>","date":"2024-05-02T15:51:18","subject":"Re: [PATCH v4 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tAgcMeanLuminance","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Thu, May 02, 2024 at 02:30:43PM GMT, Daniel Scally wrote:\n> In preparation for switching to a derivation of AgcMeanLuminance, add\n> a function to parse and store the statistics for easy retrieval in an\n> overriding estimateLuminance() function.\n>\n> Now that we have a MeanLuminanceAgc class that centralises our AEGC\n> algorithm, derive the IPU3's Agc class from it and plumb in the\n> necessary framework to enable it to be used. For simplicity's sake\n> this commit switches the algorithm to use the derived class, but\n> does not remove the bespoke functions at this time.\n>\n> Reviewed-by: Stefan Klug \n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v4:\n> \t- None\n>\n> Changes in v3:\n> \t- Minor style fixes\n>\n> Changes in v2:\n>\n> \t- Squashed the patch adding parseStatistics()\n> \t- Used the first available constraint and exposure modes rather than\n> \t assuming the \"Normal\" mode was available\n> \t- Used a vector of RGB tuples in estimateLuminance() rather than 3\n> \t vectors\n> \t- Stored a copy of the bdsGrid and AWB gains rather than a pointer to\n> \t the context for use in estimateLuminance()\n> \t- Remembered to report the controls out to IPAIPU3::updateControls()\n> src/ipa/ipu3/algorithms/agc.cpp | 119 ++++++++++++++++++++++++++++++--\n> src/ipa/ipu3/algorithms/agc.h | 14 +++-\n> src/ipa/ipu3/ipa_context.cpp | 3 +\n> src/ipa/ipu3/ipa_context.h | 5 ++\n> src/ipa/ipu3/ipu3.cpp | 3 +-\n> 5 files changed, 135 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 606a237a..b1ba27fe 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -71,11 +71,33 @@ static constexpr uint32_t kNumStartupFrames = 10;\n> static constexpr double kRelativeLuminanceTarget = 0.16;\n>\n> Agc::Agc()\n> -\t: frameCount_(0), minShutterSpeed_(0s),\n> -\t maxShutterSpeed_(0s), filteredExposure_(0s)\n> +\t: minShutterSpeed_(0s), maxShutterSpeed_(0s)\n> {\n> }\n>\n> +/**\n> + * \\brief Initialise the AGC algorithm from tuning files\n> + * \\param[in] context The shared IPA context\n> + * \\param[in] tuningData The YamlObject containing Agc tuning data\n> + *\n> + * This function calls the base class' tuningData parsers to discover which\n> + * control values are supported.\n> + *\n> + * \\return 0 on success or errors from the base class\n> + */\n> +int Agc::init(IPAContext &context, const YamlObject &tuningData)\n> +{\n> +\tint ret;\n> +\n> +\tret = parseTuningData(tuningData);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tcontext.ctrlMap.merge(controls());\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\brief Configure the AGC given a configInfo\n> * \\param[in] context The shared IPA context\n> @@ -90,6 +112,7 @@ int Agc::configure(IPAContext &context,\n> \tIPAActiveState &activeState = context.activeState;\n>\n> \tstride_ = configuration.grid.stride;\n> +\tbdsGrid_ = configuration.grid.bdsGrid;\n>\n> \tminShutterSpeed_ = configuration.agc.minShutterSpeed;\n> \tmaxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,\n> @@ -103,6 +126,15 @@ int Agc::configure(IPAContext &context,\n> \tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>\n> \tframeCount_ = 0;\n> +\n> +\tcontext.activeState.agc.constraintMode = constraintModes().begin()->first;\n> +\tcontext.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;\n> +\n> +\t/* \\todo Run this again when FrameDurationLimits is passed in */\n> +\tsetLimits(minShutterSpeed_, maxShutterSpeed_, minAnalogueGain_,\n> +\t\t maxAnalogueGain_);\n> +\tresetFrameCount();\n> +\n> \treturn 0;\n> }\n>\n> @@ -142,6 +174,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,\n> \treturn Histogram(Span(hist)).interQuantileMean(0.98, 1.0);\n> }\n>\n> +Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n> +\t\t\t const ipu3_uapi_grid_config &grid)\n> +{\n> +\tuint32_t hist[knumHistogramBins] = { 0 };\n> +\n> +\trgbTriples_.clear();\n> +\n> +\tfor (unsigned int cellY = 0; cellY < grid.height; cellY++) {\n> +\t\tfor (unsigned int cellX = 0; cellX < grid.width; cellX++) {\n> +\t\t\tuint32_t cellPosition = cellY * stride_ + cellX;\n> +\n> +\t\t\tconst ipu3_uapi_awb_set_item *cell =\n> +\t\t\t\treinterpret_cast(\n> +\t\t\t\t\t&stats->awb_raw_buffer.meta_data[cellPosition]);\n> +\n> +\t\t\trgbTriples_.push_back({\n> +\t\t\t\tcell->R_avg,\n> +\t\t\t\t(cell->Gr_avg + cell->Gb_avg) / 2,\n> +\t\t\t\tcell->B_avg\n> +\t\t\t});\n> +\n> +\t\t\t/*\n> +\t\t\t * Store the average green value to estimate the\n> +\t\t\t * brightness. Even the overexposed pixels are\n> +\t\t\t * taken into account.\n> +\t\t\t */\n> +\t\t\thist[(cell->Gr_avg + cell->Gb_avg) / 2]++;\n> +\t\t}\n> +\t}\n> +\n> +\treturn Histogram(Span(hist));\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> @@ -247,11 +312,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> \tLOG(IPU3Agc, Debug) << \"Divided up shutter and gain are \"\n> \t\t\t << shutterTime << \" and \"\n> \t\t\t << stepGain;\n> -\n> -\tIPAActiveState &activeState = context.activeState;\n> -\t/* Update the estimated exposure and gain. */\n> -\tactiveState.agc.exposure = shutterTime / configuration.sensor.lineDuration;\n> -\tactiveState.agc.gain = stepGain;\n> }\n>\n> /**\n> @@ -314,6 +374,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> \treturn ySum / (grid.height * grid.width) / 255;\n> }\n>\n> +double Agc::estimateLuminance(double gain) const\n> +{\n> +\tdouble redSum = 0, greenSum = 0, blueSum = 0;\n> +\n> +\tfor (unsigned int i = 0; i < rgbTriples_.size(); i++) {\n> +\t\tredSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);\n> +\t\tgreenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);\n> +\t\tblueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);\n> +\t}\n> +\n> +\tdouble ySum = redSum * rGain_ * 0.299\n> +\t\t + greenSum * gGain_ * 0.587\n> +\t\t + blueSum * bGain_ * 0.114;\n> +\n> +\treturn ySum / (bdsGrid_.height * bdsGrid_.width) / 255;\n> +}\n> +\n> /**\n> * \\brief Process IPU3 statistics, and run AGC operations\n> * \\param[in] context The shared IPA context\n> @@ -366,8 +443,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> \tframeCount_++;\n>\n> +\tHistogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);\n> +\trGain_ = context.activeState.awb.gains.red;\n> +\tgGain_ = context.activeState.awb.gains.blue;\n> +\tbGain_ = context.activeState.awb.gains.green;\n> +\n> +\t/*\n> +\t * The Agc algorithm needs to know the effective exposure value that was\n> +\t * applied to the sensor when the statistics were collected.\n> +\t */\n> \tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> \t\t\t\t * frameContext.sensor.exposure;\n> +\tdouble analogueGain = frameContext.sensor.gain;\n> +\tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n> +\n> +\tutils::Duration shutterTime;\n> +\tdouble aGain, dGain;\n> +\tstd::tie(shutterTime, aGain, dGain) =\n> +\t\tcalculateNewEv(context.activeState.agc.constraintMode,\n> +\t\t\t context.activeState.agc.exposureMode, hist,\n> +\t\t\t effectiveExposureValue);\n> +\n> +\tLOG(IPU3Agc, Debug)\n> +\t\t<< \"Divided up shutter, analogue gain and digital gain are \"\n> +\t\t<< shutterTime << \", \" << aGain << \" and \" << dGain;\n> +\n> +\tIPAActiveState &activeState = context.activeState;\n> +\t/* Update the estimated exposure and gain. */\n> +\tactiveState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;\n> +\tactiveState.agc.gain = aGain;\n> +\n> \tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> \tmetadata.set(controls::ExposureTime, exposureTime.get());\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 9d6e3ff1..1d6855a9 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -13,6 +13,9 @@\n>\n> #include \n>\n> +#include \"libipa/agc_mean_luminance.h\"\n> +#include \"libipa/histogram.h\"\n> +\n> #include \"algorithm.h\"\n>\n> namespace libcamera {\n> @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;\n>\n> namespace ipa::ipu3::algorithms {\n>\n> -class Agc : public Algorithm\n> +class Agc : public Algorithm, public AgcMeanLuminance\n> {\n> public:\n> \tAgc();\n> \t~Agc() = default;\n>\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> \tvoid process(IPAContext &context, const uint32_t frame,\n> \t\t IPAFrameContext &frameContext,\n> @@ -43,6 +47,9 @@ private:\n> \t\t\t\t const ipu3_uapi_grid_config &grid,\n> \t\t\t\t const ipu3_uapi_stats_3a *stats,\n> \t\t\t\t double gain);\n> +\tdouble estimateLuminance(double gain) const override;\n> +\tHistogram parseStatistics(const ipu3_uapi_stats_3a *stats,\n> +\t\t\t\t const ipu3_uapi_grid_config &grid);\n>\n> \tuint64_t frameCount_;\n>\n> @@ -55,6 +62,11 @@ private:\n> \tutils::Duration filteredExposure_;\n>\n> \tuint32_t stride_;\n> +\tdouble rGain_;\n> +\tdouble gGain_;\n> +\tdouble bGain_;\n> +\tipu3_uapi_grid_config bdsGrid_;\n> +\tstd::vector> rgbTriples_;\n> };\n>\n> } /* namespace ipa::ipu3::algorithms */\n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 959f314f..c4fb5642 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -47,6 +47,9 @@ namespace libcamera::ipa::ipu3 {\n> *\n> * \\var IPAContext::activeState\n> * \\brief The current state of IPA algorithms\n> + *\n> + * \\var IPAContext::ctrlMap\n> + * \\brief A ControlInfoMap::Map of controls populated by the algorithms\n> */\n>\n> /**\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index e9a3863b..a92cb6ce 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -12,6 +12,7 @@\n>\n> #include \n>\n> +#include \n> #include \n>\n> #include \n> @@ -55,6 +56,8 @@ struct IPAActiveState {\n> \tstruct {\n> \t\tuint32_t exposure;\n> \t\tdouble gain;\n> +\t\tuint32_t constraintMode;\n> +\t\tuint32_t exposureMode;\n\nMaybe these need to be documented as well ?\n\nThis apart\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n\n> \t} agc;\n>\n> \tstruct {\n> @@ -85,6 +88,8 @@ struct IPAContext {\n> \tIPAActiveState activeState;\n>\n> \tFCQueue frameContexts;\n> +\n> +\tControlInfoMap::Map ctrlMap;\n> };\n>\n> } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 08ee6eb3..4809eb60 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -189,7 +189,7 @@ private:\n> };\n>\n> IPAIPU3::IPAIPU3()\n> -\t: context_({ {}, {}, { kMaxFrameContexts } })\n> +\t: context_({ {}, {}, { kMaxFrameContexts }, {} })\n> {\n> }\n>\n> @@ -287,6 +287,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n> \t\t\t\t\t\t\t frameDurations[1],\n> \t\t\t\t\t\t\t frameDurations[2]);\n>\n> +\tcontrols.merge(context_.ctrlMap);\n> \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\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 9BD5BC3220\n\tfor ;\n\tThu, 2 May 2024 15:51:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 95E9D6341A;\n\tThu, 2 May 2024 17:51:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29742633EB\n\tfor ;\n\tThu, 2 May 2024 17:51:21 +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 63C113A3;\n\tThu, 2 May 2024 17:50:23 +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=\"eHlAouYK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1714665023;\n\tbh=xApmAzNKRTVZkz7pa2SyKyTUSz4D7p5vrFaoWIXQbc0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eHlAouYKnsQaIdUZy+kH025hROxIBBVsq6h954rCfdjsA+HZNmoXjypG/XZlnW/EX\n\tvGEur9n5Sb2hFyv6QcbEMM+0XzN1DwvCgI86dn8A893l1twwH1cRa5vbNJylbEfVkh\n\tNF3wkbSfKkTBfS5Wbe9ndWWgIIa+srY5iCUSK6pA=","Date":"Thu, 2 May 2024 17:51:18 +0200","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org, \n\tStefan Klug ","Subject":"Re: [PATCH v4 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tAgcMeanLuminance","Message-ID":"<3rkpk3wd3sdwbtgi2235hajak6ksjdipsbkmwstleure2bksmz@5qkc2kdkkqiq>","References":"<20240502133046.1976565-1-dan.scally@ideasonboard.com>\n\t<20240502133046.1976565-6-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240502133046.1976565-6-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\" "}}]