[{"id":29251,"web_url":"https://patchwork.libcamera.org/comment/29251/","msgid":"<20240418075637.binouhnerhcm4dr3@jasper>","date":"2024-04-18T07:56:37","subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","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\nOn Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:\n> In preparation for switching to a derivation of MeanLuminanceAgc, 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> Signed-off-by: Daniel Scally \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> \n> src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--\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, 136 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 606a237a..3b9761bd 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,16 @@ int Agc::configure(IPAContext &context,\n> \tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n> \n> \tframeCount_ = 0;\n\nIMHO That line can be removed as you call resetFrameCount() below.\n\nWith that fixed\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\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> +\tconfigureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,\n> +\t\t\t\t minAnalogueGain_, maxAnalogueGain_);\n> +\n> +\tresetFrameCount();\n> +\n> \treturn 0;\n> }\n> \n> @@ -142,6 +175,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 +313,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 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> \treturn ySum / (grid.height * grid.width) / 255;\n> }\n> \n> +double Agc::estimateLuminance(double gain)\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 +444,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..40f32188 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) 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> \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 5A2D5BE08B\n\tfor ;\n\tThu, 18 Apr 2024 07:56:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D885633EE;\n\tThu, 18 Apr 2024 09:56:42 +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 9C9E661B43\n\tfor ;\n\tThu, 18 Apr 2024 09:56:40 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:c257:f1f7:8fc8:140f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EF12627C;\n\tThu, 18 Apr 2024 09:55:52 +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=\"Wcn44lKl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713426953;\n\tbh=u/fn0tVpN/3V0hqdoX5H3R/+vOR4jFPf08Ya6fPC218=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Wcn44lKlEmPr277cXBtz3NCZKk8NtmGuCYCaPYJx3rO+8F7v0l5w0UQPaL2xvCU3E\n\teIAU3JMMWDI9nd4DEIcPU+fi1bKk+AfclLdWNRhG0nJRjjitR/t8B9sgRmHCxm+jM2\n\tcApBjS2JB/+m9U8DpSdcvQjYLvsbh6Ls0rYFgeEw=","Date":"Thu, 18 Apr 2024 09:56:37 +0200","From":"Stefan Klug ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","Message-ID":"<20240418075637.binouhnerhcm4dr3@jasper>","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-6-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240417131536.484129-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\" "}},{"id":29289,"web_url":"https://patchwork.libcamera.org/comment/29289/","msgid":"","date":"2024-04-22T10:28:02","subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan,\n in $subject and here\n\ns/MeanLuminanceAgc/AgcMeanLuminance\n\nOn Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:\n> In preparation for switching to a derivation of MeanLuminanceAgc, 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> Signed-off-by: Daniel Scally \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>\n> src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--\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, 136 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 606a237a..3b9761bd 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\nNit:\n\n int ret =\n\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\nWhere is this populated from ?\n\n>\n> \tminShutterSpeed_ = configuration.agc.minShutterSpeed;\n> \tmaxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,\n> @@ -103,6 +126,16 @@ 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> +\tconfigureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,\n> +\t\t\t\t minAnalogueGain_, maxAnalogueGain_);\n> +\n> +\tresetFrameCount();\n> +\n> \treturn 0;\n> }\n>\n> @@ -142,6 +175,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 +313,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 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> \treturn ySum / (grid.height * grid.width) / 255;\n> }\n>\n> +double Agc::estimateLuminance(double gain)\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 +444,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\nAs a general question: what happens if awb is not active ?\n\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\nIs this a Duration ?\n\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..40f32188 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) 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> \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 1075AC3200\n\tfor ;\n\tMon, 22 Apr 2024 10:28:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36345633FA;\n\tMon, 22 Apr 2024 12:28:08 +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 85A9A61B39\n\tfor ;\n\tMon, 22 Apr 2024 12:28:06 +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 8D4EC142E;\n\tMon, 22 Apr 2024 12:27:15 +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=\"HJUtChgE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713781635;\n\tbh=FNDRucKWNvaMzQRabpRUfWDVMZMedPL0wTgLMzk8BZo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HJUtChgEP7hLfgCO4FDDUpsRsrxqqtaaqdqGW49aXLaLlgxOS3ScHYhtkAk+CwBWA\n\tv9wsec47JOi1OYdV7MQqFiHEgV2+zKZIrBs2IZlTOfHQ1aS1L2LoI1RPeN3KsrnGWx\n\tf86Ufeyez1SQ5zwQp+4QRtzIJx3yX6AjPaT4iXVo=","Date":"Mon, 22 Apr 2024 12:28:02 +0200","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","Message-ID":"","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-6-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240417131536.484129-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\" "}},{"id":29310,"web_url":"https://patchwork.libcamera.org/comment/29310/","msgid":"<5ea5fa9f-6e74-4631-ac4d-970ab324b5e8@ideasonboard.com>","date":"2024-04-23T09:20:38","subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 22/04/2024 11:28, Jacopo Mondi wrote:\n> Hi Dan,\n> in $subject and here\n>\n> s/MeanLuminanceAgc/AgcMeanLuminance\n>\n> On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:\n>> In preparation for switching to a derivation of MeanLuminanceAgc, 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>> Signed-off-by: Daniel Scally \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>>\n>> src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--\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, 136 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n>> index 606a237a..3b9761bd 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> Nit:\n>\n> int ret =\n>\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> Where is this populated from ?\n\n\nIn IPAIPU3::calculateBdsGrid()\n\n>\n>> \tminShutterSpeed_ = configuration.agc.minShutterSpeed;\n>> \tmaxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,\n>> @@ -103,6 +126,16 @@ 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>> +\tconfigureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,\n>> +\t\t\t\t minAnalogueGain_, maxAnalogueGain_);\n>> +\n>> +\tresetFrameCount();\n>> +\n>> \treturn 0;\n>> }\n>>\n>> @@ -142,6 +175,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 +313,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 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n>> \treturn ySum / (grid.height * grid.width) / 255;\n>> }\n>>\n>> +double Agc::estimateLuminance(double gain)\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 +444,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> As a general question: what happens if awb is not active ?\n\nIt's hard-coded active in this IPA.\n>\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> Is this a Duration ?\n\n\nYes...isn't it? Perhaps the terminology of \"Exposure Value\" is confusing or wrong? But conceptually \nthat variable is meant to be a duration.\n\n>\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..40f32188 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) 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>> \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 5D24FC3200\n\tfor ;\n\tTue, 23 Apr 2024 09:20:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2940263422;\n\tTue, 23 Apr 2024 11:20:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B5A4E6341C\n\tfor ;\n\tTue, 23 Apr 2024 11:20:41 +0200 (CEST)","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 6C3CF65D;\n\tTue, 23 Apr 2024 11:19:50 +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=\"R1Ccux88\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713863990;\n\tbh=8gwUn+RHEHdzyIezAFSToGSumgu/hEgSjTUVyBKbZnU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=R1Ccux88KyzE+gS1RpjpCkmI6jWqk/yxKaoN6217HPl7mdSYwLHESl1XFnaI3BRrh\n\tB/dlq5LJc+wlgUzDKUJz8Vouk2eJhJB73Fd1JlAJH2DrnQg1wrilxwlnR+jOaIt+Ue\n\t30Xpd+t7AW+tsPaJy3sX6Ib6ZrtBMV+g1WpPeFac=","Message-ID":"<5ea5fa9f-6e74-4631-ac4d-970ab324b5e8@ideasonboard.com>","Date":"Tue, 23 Apr 2024 10:20:38 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-6-dan.scally@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Dan Scally ","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":"","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":"","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":29329,"web_url":"https://patchwork.libcamera.org/comment/29329/","msgid":"<917b2514-a454-4c69-b0fc-ebceb23f64d3@ideasonboard.com>","date":"2024-04-24T09:00:37","subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\nOn 18/04/2024 08:56, Stefan Klug wrote:\n> Hi Daniel,\n>\n> thanks for the patch.\n>\n> On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:\n>> In preparation for switching to a derivation of MeanLuminanceAgc, 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>> Signed-off-by: Daniel Scally \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>>\n>> src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--\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, 136 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n>> index 606a237a..3b9761bd 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,16 @@ int Agc::configure(IPAContext &context,\n>> \tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>> \n>> \tframeCount_ = 0;\n> IMHO That line can be removed as you call resetFrameCount() below.\n\n\nThis is actually libcamera::ipa::ipu3::algorithms::Agc::frameCount_ rather than \nlibcamera::ipa::AgcMeanLuminance::frameCount_, which is what resetFrameCount() touches. It's removed \nalong with the rest of the bespoke implementation in the next patch...does it still need to be \nremoved here?\n\n>\n> With that fixed\n> Reviewed-by: Stefan Klug \n\n\nThanks!\n\n>\n> Cheers,\n> Stefan\n>\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>> +\tconfigureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,\n>> +\t\t\t\t minAnalogueGain_, maxAnalogueGain_);\n>> +\n>> +\tresetFrameCount();\n>> +\n>> \treturn 0;\n>> }\n>> \n>> @@ -142,6 +175,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 +313,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 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n>> \treturn ySum / (grid.height * grid.width) / 255;\n>> }\n>> \n>> +double Agc::estimateLuminance(double gain)\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 +444,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..40f32188 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) 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>> \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 01488C3200\n\tfor ;\n\tWed, 24 Apr 2024 09:00:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1318B633EE;\n\tWed, 24 Apr 2024 11:00:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D69461B14\n\tfor ;\n\tWed, 24 Apr 2024 11:00:40 +0200 (CEST)","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 6C8D966B;\n\tWed, 24 Apr 2024 10:59:48 +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=\"t3DaQCNc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713949188;\n\tbh=Szz5u2p/8Pv8J0MZdAsSiratKISM2QL8t1SaYA6fTM0=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=t3DaQCNckl82ItNCsDHfU5kizEs2HvhWELK/tM5c1JdpG2CnbbptVXJao8Bnfc+7U\n\tIxlWjWBnIme0YcxZ214V1rm//vAWVlF0rllxty7+Hb6Xsa8DsBJSOi+mUUFCm0ruDC\n\tvymVUNIksfuPUIjqltWC8cMjVwOwLBC+gis1ip5Y=","Message-ID":"<917b2514-a454-4c69-b0fc-ebceb23f64d3@ideasonboard.com>","Date":"Wed, 24 Apr 2024 10:00:37 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-6-dan.scally@ideasonboard.com>\n\t<20240418075637.binouhnerhcm4dr3@jasper>","Content-Language":"en-US","From":"Dan Scally ","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":"<20240418075637.binouhnerhcm4dr3@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":"","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":29341,"web_url":"https://patchwork.libcamera.org/comment/29341/","msgid":"<20240425100555.ggubw7sdti23wwac@jasper>","date":"2024-04-25T10:05:55","subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Wed, Apr 24, 2024 at 10:00:37AM +0100, Dan Scally wrote:\n> Hi Stefan\n> \n> On 18/04/2024 08:56, Stefan Klug wrote:\n> > Hi Daniel,\n> > \n> > thanks for the patch.\n> > \n> > On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:\n> > > In preparation for switching to a derivation of MeanLuminanceAgc, 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> > > Signed-off-by: Daniel Scally \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> > > \n> > > src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--\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, 136 insertions(+), 9 deletions(-)\n> > > \n> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > > index 606a237a..3b9761bd 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> > > Agc::Agc()\n> > > -\t: frameCount_(0), minShutterSpeed_(0s),\n> > > -\t maxShutterSpeed_(0s), filteredExposure_(0s)\n> > > +\t: minShutterSpeed_(0s), maxShutterSpeed_(0s)\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> > > \tstride_ = configuration.grid.stride;\n> > > +\tbdsGrid_ = configuration.grid.bdsGrid;\n> > > \tminShutterSpeed_ = configuration.agc.minShutterSpeed;\n> > > \tmaxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,\n> > > @@ -103,6 +126,16 @@ int Agc::configure(IPAContext &context,\n> > > \tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n> > > \tframeCount_ = 0;\n> > IMHO That line can be removed as you call resetFrameCount() below.\n> \n> \n> This is actually libcamera::ipa::ipu3::algorithms::Agc::frameCount_ rather\n> than libcamera::ipa::AgcMeanLuminance::frameCount_, which is what\n> resetFrameCount() touches. It's removed along with the rest of the bespoke\n> implementation in the next patch...does it still need to be removed here?\n\nOh I missed that. Sorry for the noise. Then there is no need to remove\nit here.\n\n> \n> > \n> > With that fixed\n> > Reviewed-by: Stefan Klug \n> \n> \n> Thanks!\n> \n> > \n> > Cheers,\n> > Stefan\n> > \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> > > +\tconfigureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,\n> > > +\t\t\t\t minAnalogueGain_, maxAnalogueGain_);\n> > > +\n> > > +\tresetFrameCount();\n> > > +\n> > > \treturn 0;\n> > > }\n> > > @@ -142,6 +175,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,\n> > > \treturn Histogram(Span(hist)).interQuantileMean(0.98, 1.0);\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 +313,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> > > @@ -314,6 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,\n> > > \treturn ySum / (grid.height * grid.width) / 255;\n> > > }\n> > > +double Agc::estimateLuminance(double gain)\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 +444,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > \tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> > > \tframeCount_++;\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> > > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> > > index 9d6e3ff1..40f32188 100644\n> > > --- a/src/ipa/ipu3/algorithms/agc.h\n> > > +++ b/src/ipa/ipu3/algorithms/agc.h\n> > > @@ -13,6 +13,9 @@\n> > > #include \n> > > +#include \"libipa/agc_mean_luminance.h\"\n> > > +#include \"libipa/histogram.h\"\n> > > +\n> > > #include \"algorithm.h\"\n> > > namespace libcamera {\n> > > @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;\n> > > namespace ipa::ipu3::algorithms {\n> > > -class Agc : public Algorithm\n> > > +class Agc : public Algorithm, public AgcMeanLuminance\n> > > {\n> > > public:\n> > > \tAgc();\n> > > \t~Agc() = default;\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) override;\n> > > +\tHistogram parseStatistics(const ipu3_uapi_stats_3a *stats,\n> > > +\t\t\t\t const ipu3_uapi_grid_config &grid);\n> > > \tuint64_t frameCount_;\n> > > @@ -55,6 +62,11 @@ private:\n> > > \tutils::Duration filteredExposure_;\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> > > } /* 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> > > 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> > > #include \n> > > +#include \n> > > #include \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> > > \t} agc;\n> > > \tstruct {\n> > > @@ -85,6 +88,8 @@ struct IPAContext {\n> > > \tIPAActiveState activeState;\n> > > \tFCQueue frameContexts;\n> > > +\n> > > +\tControlInfoMap::Map ctrlMap;\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> > > IPAIPU3::IPAIPU3()\n> > > -\t: context_({ {}, {}, { kMaxFrameContexts } })\n> > > +\t: context_({ {}, {}, { kMaxFrameContexts }, {} })\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> > > +\tcontrols.merge(context_.ctrlMap);\n> > > \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\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 702E9BE08B\n\tfor ;\n\tThu, 25 Apr 2024 10:06:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7B09D633F4;\n\tThu, 25 Apr 2024 12:05:59 +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 B46BB61A9B\n\tfor ;\n\tThu, 25 Apr 2024 12:05:57 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:6736:b333:3187:60e9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1D0FFB0B;\n\tThu, 25 Apr 2024 12:05:05 +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=\"Y7uTjxIi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1714039505;\n\tbh=AvcneoajbJ+6lTFjJhiimx1/VHgKqo9T8oFrBfwdkeE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Y7uTjxIiM7/uBM2B0lsQjLfijr4KWNlxj7bFIQSFARLAZKrWJZ4CaN6ujzHfAscMD\n\tzUmKD6x8+7AyziaTrmJdYzBW4V3RzSH7bpJDpfCTomqYA+g/jcJxtfUFKAvrQFI/od\n\tMlpgE9AfizqqDubZBkMGCnl8efyye2aYer7PRnAs=","Date":"Thu, 25 Apr 2024 12:05:55 +0200","From":"Stefan Klug ","To":"Dan Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from\n\tMeanLuminanceAgc","Message-ID":"<20240425100555.ggubw7sdti23wwac@jasper>","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-6-dan.scally@ideasonboard.com>\n\t<20240418075637.binouhnerhcm4dr3@jasper>\n\t<917b2514-a454-4c69-b0fc-ebceb23f64d3@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<917b2514-a454-4c69-b0fc-ebceb23f64d3@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\" "}}]