[{"id":29089,"web_url":"https://patchwork.libcamera.org/comment/29089/","msgid":"<20240327151011.5ltaavfn2rry36oi@jasper>","date":"2024-03-27T15:10:11","subject":"Re: [PATCH 07/10] ipa: ipu3: Remove bespoke AGC functions from IPU3","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Daniel,\n\nlooks good to me.\n\nReviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> \n\nCheers,\nStefan\n\nOn Fri, Mar 22, 2024 at 01:14:48PM +0000, Daniel Scally wrote:\n> Now that the IPU3's Agc is derived from MeanLuminanceAgc we can\n> delete all the unecessary bespoke functions.\n> \n> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/algorithms/agc.cpp | 241 --------------------------------\n>  src/ipa/ipu3/algorithms/agc.h   |  13 --\n>  2 files changed, 254 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index a84534ea..08deff0c 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -132,8 +132,6 @@ int Agc::configure(IPAContext &context,\n>  \tactiveState.agc.gain = minAnalogueGain_;\n>  \tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>  \n> -\tframeCount_ = 0;\n> -\n>  \t/*\n>  \t * \\todo We should use the first available mode rather than assume that\n>  \t * the \"Normal\" modes are present in tuning data.\n> @@ -150,42 +148,6 @@ int Agc::configure(IPAContext &context,\n>  \treturn 0;\n>  }\n>  \n> -/**\n> - * \\brief Estimate the mean value of the top 2% of the histogram\n> - * \\param[in] stats The statistics computed by the ImgU\n> - * \\param[in] grid The grid used to store the statistics in the IPU3\n> - * \\return The mean value of the top 2% of the histogram\n> - */\n> -double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,\n> -\t\t\t      const ipu3_uapi_grid_config &grid) const\n> -{\n> -\t/* Initialise the histogram array */\n> -\tuint32_t hist[knumHistogramBins] = { 0 };\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<const ipu3_uapi_awb_set_item *>(\n> -\t\t\t\t\t&stats->awb_raw_buffer.meta_data[cellPosition]\n> -\t\t\t\t);\n> -\n> -\t\t\tuint8_t gr = cell->Gr_avg;\n> -\t\t\tuint8_t gb = cell->Gb_avg;\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[(gr + gb) / 2]++;\n> -\t\t}\n> -\t}\n> -\n> -\t/* Estimate the quantile mean of the top 2% of the histogram. */\n> -\treturn Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);\n> -}\n> -\n>  void Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n>  \t\t\t  const ipu3_uapi_grid_config &grid)\n>  {\n> @@ -219,173 +181,6 @@ void Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n>  \thist_ = Histogram(Span<uint32_t>(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> - *\n> - * The speed of the filter is adaptive, and will produce the target quicker\n> - * during startup, or when the target exposure is within 20% of the most recent\n> - * filter output.\n> - *\n> - * \\return The filtered exposure\n> - */\n> -utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n> -{\n> -\tdouble speed = 0.2;\n> -\n> -\t/* Adapt instantly if we are in startup phase. */\n> -\tif (frameCount_ < kNumStartupFrames)\n> -\t\tspeed = 1.0;\n> -\n> -\t/*\n> -\t * If we are close to the desired result, go faster to avoid making\n> -\t * multiple micro-adjustments.\n> -\t * \\todo Make this customisable?\n> -\t */\n> -\tif (filteredExposure_ < 1.2 * exposureValue &&\n> -\t    filteredExposure_ > 0.8 * exposureValue)\n> -\t\tspeed = sqrt(speed);\n> -\n> -\tfilteredExposure_ = speed * exposureValue +\n> -\t\t\t    filteredExposure_ * (1.0 - speed);\n> -\n> -\tLOG(IPU3Agc, Debug) << \"After filtering, exposure \" << filteredExposure_;\n> -\n> -\treturn filteredExposure_;\n> -}\n> -\n> -/**\n> - * \\brief Estimate the new exposure and gain values\n> - * \\param[inout] frameContext The shared IPA frame Context\n> - * \\param[in] yGain The gain calculated based on the relative luminance target\n> - * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n> - */\n> -void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t\t  double yGain, double iqMeanGain)\n> -{\n> -\tconst IPASessionConfiguration &configuration = context.configuration;\n> -\t/* Get the effective exposure and gain applied on the sensor. */\n> -\tuint32_t exposure = frameContext.sensor.exposure;\n> -\tdouble analogueGain = frameContext.sensor.gain;\n> -\n> -\t/* Use the highest of the two gain estimates. */\n> -\tdouble evGain = std::max(yGain, iqMeanGain);\n> -\n> -\t/* Consider within 1% of the target as correctly exposed */\n> -\tif (utils::abs_diff(evGain, 1.0) < 0.01)\n> -\t\tLOG(IPU3Agc, Debug) << \"We are well exposed (evGain = \"\n> -\t\t\t\t    << evGain << \")\";\n> -\n> -\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\n> -\t/* Calculate the shutter time in seconds */\n> -\tutils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> -\n> -\t/*\n> -\t * Update the exposure value for the next computation using the values\n> -\t * of exposure and gain really used by the sensor.\n> -\t */\n> -\tutils::Duration effectiveExposureValue = currentShutter * analogueGain;\n> -\n> -\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentShutter * analogueGain\n> -\t\t\t    << \" Shutter speed \" << currentShutter\n> -\t\t\t    << \" Gain \" << analogueGain\n> -\t\t\t    << \" Needed ev gain \" << evGain;\n> -\n> -\t/*\n> -\t * Calculate the current exposure value for the scene as the latest\n> -\t * exposure value applied multiplied by the new estimated gain.\n> -\t */\n> -\tutils::Duration exposureValue = effectiveExposureValue * evGain;\n> -\n> -\t/* Clamp the exposure value to the min and max authorized */\n> -\tutils::Duration maxTotalExposure = maxShutterSpeed_ * maxAnalogueGain_;\n> -\texposureValue = std::min(exposureValue, maxTotalExposure);\n> -\tLOG(IPU3Agc, Debug) << \"Target total exposure \" << exposureValue\n> -\t\t\t    << \", maximum is \" << maxTotalExposure;\n> -\n> -\t/*\n> -\t * Filter the exposure.\n> -\t * \\todo estimate if we need to desaturate\n> -\t */\n> -\texposureValue = filterExposure(exposureValue);\n> -\n> -\t/*\n> -\t * Divide the exposure value as new exposure and gain values.\n> -\t *\n> -\t * Push the shutter time up to the maximum first, and only then\n> -\t * increase the gain.\n> -\t */\n> -\tutils::Duration shutterTime =\n> -\t\tstd::clamp<utils::Duration>(exposureValue / minAnalogueGain_,\n> -\t\t\t\t\t    minShutterSpeed_, maxShutterSpeed_);\n> -\tdouble stepGain = std::clamp(exposureValue / shutterTime,\n> -\t\t\t\t     minAnalogueGain_, maxAnalogueGain_);\n> -\tLOG(IPU3Agc, Debug) << \"Divided up shutter and gain are \"\n> -\t\t\t    << shutterTime << \" and \"\n> -\t\t\t    << stepGain;\n> -}\n> -\n> -/**\n> - * \\brief Estimate the relative luminance of the frame with a given gain\n> - * \\param[in] frameContext The shared IPA frame context\n> - * \\param[in] grid The grid used to store the statistics in the IPU3\n> - * \\param[in] stats The IPU3 statistics and ISP results\n> - * \\param[in] gain The gain to apply to the frame\n> - * \\return The relative luminance\n> - *\n> - * This function estimates the average relative luminance of the frame that\n> - * would be output by the sensor if an additional \\a gain was applied.\n> - *\n> - * The estimation is based on the AWB statistics for the current frame. Red,\n> - * green and blue averages for all cells are first multiplied by the gain, and\n> - * then saturated to approximate the sensor behaviour at high brightness\n> - * values. The approximation is quite rough, as it doesn't take into account\n> - * non-linearities when approaching saturation.\n> - *\n> - * The relative luminance (Y) is computed from the linear RGB components using\n> - * the Rec. 601 formula. The values are normalized to the [0.0, 1.0] range,\n> - * where 1.0 corresponds to a theoretical perfect reflector of 100% reference\n> - * white.\n> - *\n> - * More detailed information can be found in:\n> - * https://en.wikipedia.org/wiki/Relative_luminance\n> - */\n> -double Agc::estimateLuminance(IPAActiveState &activeState,\n> -\t\t\t      const ipu3_uapi_grid_config &grid,\n> -\t\t\t      const ipu3_uapi_stats_3a *stats,\n> -\t\t\t      double gain)\n> -{\n> -\tdouble redSum = 0, greenSum = 0, blueSum = 0;\n> -\n> -\t/* Sum the per-channel averages, saturated to 255. */\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<const ipu3_uapi_awb_set_item *>(\n> -\t\t\t\t\t&stats->awb_raw_buffer.meta_data[cellPosition]\n> -\t\t\t\t);\n> -\t\t\tconst uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> -\n> -\t\t\tredSum += std::min(cell->R_avg * gain, 255.0);\n> -\t\t\tgreenSum += std::min(G_avg * gain, 255.0);\n> -\t\t\tblueSum += std::min(cell->B_avg * gain, 255.0);\n> -\t\t}\n> -\t}\n> -\n> -\t/*\n> -\t * Apply the AWB gains to approximate colours correctly, use the Rec.\n> -\t * 601 formula to calculate the relative luminance, and normalize it.\n> -\t */\n> -\tdouble ySum = redSum * activeState.awb.gains.red * 0.299\n> -\t\t    + greenSum * activeState.awb.gains.green * 0.587\n> -\t\t    + blueSum * activeState.awb.gains.blue * 0.114;\n> -\n> -\treturn ySum / (grid.height * grid.width) / 255;\n> -}\n> -\n>  double Agc::estimateLuminance(double gain)\n>  {\n>  \tASSERT(reds_.size() == greens_.size());\n> @@ -422,42 +217,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t\t  const ipu3_uapi_stats_3a *stats,\n>  \t\t  ControlList &metadata)\n>  {\n> -\t/*\n> -\t * Estimate the gain needed to have the proportion of pixels in a given\n> -\t * desired range. iqMean is the mean value of the top 2% of the\n> -\t * cumulative histogram, and we want it to be as close as possible to a\n> -\t * configured target.\n> -\t */\n> -\tdouble iqMean = measureBrightness(stats, context.configuration.grid.bdsGrid);\n> -\tdouble iqMeanGain = kEvGainTarget * knumHistogramBins / iqMean;\n> -\n> -\t/*\n> -\t * Estimate the gain needed to achieve a relative luminance target. To\n> -\t * account for non-linearity caused by saturation, the value needs to be\n> -\t * estimated in an iterative process, as multiplying by a gain will not\n> -\t * increase the relative luminance by the same factor if some image\n> -\t * regions are saturated.\n> -\t */\n> -\tdouble yGain = 1.0;\n> -\tdouble yTarget = kRelativeLuminanceTarget;\n> -\n> -\tfor (unsigned int i = 0; i < 8; i++) {\n> -\t\tdouble yValue = estimateLuminance(context.activeState,\n> -\t\t\t\t\t\t  context.configuration.grid.bdsGrid,\n> -\t\t\t\t\t\t  stats, yGain);\n> -\t\tdouble extraGain = std::min(10.0, yTarget / (yValue + .001));\n> -\n> -\t\tyGain *= extraGain;\n> -\t\tLOG(IPU3Agc, Debug) << \"Y value: \" << yValue\n> -\t\t\t\t    << \", Y target: \" << yTarget\n> -\t\t\t\t    << \", gives gain \" << yGain;\n> -\t\tif (extraGain < 1.01)\n> -\t\t\tbreak;\n> -\t}\n> -\n> -\tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> -\tframeCount_++;\n> -\n>  \tparseStatistics(stats, context.configuration.grid.bdsGrid);\n>  \n>  \t/*\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 8405da9d..78fa3c75 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -38,29 +38,16 @@ public:\n>  \t\t     ControlList &metadata) override;\n>  \n>  private:\n> -\tdouble measureBrightness(const ipu3_uapi_stats_3a *stats,\n> -\t\t\t\t const ipu3_uapi_grid_config &grid) const;\n> -\tutils::Duration filterExposure(utils::Duration currentExposure);\n> -\tvoid computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t\t     double yGain, double iqMeanGain);\n> -\tdouble estimateLuminance(IPAActiveState &activeState,\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>  \tvoid parseStatistics(const ipu3_uapi_stats_3a *stats,\n>  \t\t\t     const ipu3_uapi_grid_config &grid);\n>  \n> -\tuint64_t frameCount_;\n> -\n>  \tutils::Duration minShutterSpeed_;\n>  \tutils::Duration maxShutterSpeed_;\n>  \n>  \tdouble minAnalogueGain_;\n>  \tdouble maxAnalogueGain_;\n>  \n> -\tutils::Duration filteredExposure_;\n> -\n>  \tuint32_t stride_;\n>  \tIPAContext *context_;\n>  \tstd::vector<uint8_t> reds_;\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 87EA6C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Mar 2024 15:10:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C01E6333B;\n\tWed, 27 Mar 2024 16:10:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 44B9961C41\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Mar 2024 16:10:14 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:da09:7e54:ae7f:d731])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A0EF81571;\n\tWed, 27 Mar 2024 16:09:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"v8MQgyVB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711552181;\n\tbh=x13vO627Eq3RNlhmKTpgnd3UNLOUp3JGT71SgxO1Otc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=v8MQgyVBAzkrHj6zLW8KTvUiUjhS6C2vKS5sgffO6iQBD2GTA/HZ0D7XWFi4w8AS6\n\tZI9W7QCNShum82cDyE+wohF/CvI3ljGXXWJnmyEp/lzdaQnY4X9yKiYaTgc63o3kYD\n\tPaDI/ksWftIYEi1V1PfrZWeqYqSXC7lA/iVtDGOk=","Date":"Wed, 27 Mar 2024 16:10:11 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Daniel Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 07/10] ipa: ipu3: Remove bespoke AGC functions from IPU3","Message-ID":"<20240327151011.5ltaavfn2rry36oi@jasper>","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<20240322131451.3092931-8-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240322131451.3092931-8-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29170,"web_url":"https://patchwork.libcamera.org/comment/29170/","msgid":"<20240406013957.GN12507@pendragon.ideasonboard.com>","date":"2024-04-06T01:39:57","subject":"Re: [PATCH 07/10] ipa: ipu3: Remove bespoke AGC functions from IPU3","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dan,\n\nThank you for the patch.\n\nOn Fri, Mar 22, 2024 at 01:14:48PM +0000, Daniel Scally wrote:\n> Now that the IPU3's Agc is derived from MeanLuminanceAgc we can\n> delete all the unecessary bespoke functions.\n> \n> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/algorithms/agc.cpp | 241 --------------------------------\n>  src/ipa/ipu3/algorithms/agc.h   |  13 --\n>  2 files changed, 254 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index a84534ea..08deff0c 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -132,8 +132,6 @@ int Agc::configure(IPAContext &context,\n>  \tactiveState.agc.gain = minAnalogueGain_;\n>  \tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n>  \n> -\tframeCount_ = 0;\n> -\n>  \t/*\n>  \t * \\todo We should use the first available mode rather than assume that\n>  \t * the \"Normal\" modes are present in tuning data.\n> @@ -150,42 +148,6 @@ int Agc::configure(IPAContext &context,\n>  \treturn 0;\n>  }\n>  \n> -/**\n> - * \\brief Estimate the mean value of the top 2% of the histogram\n> - * \\param[in] stats The statistics computed by the ImgU\n> - * \\param[in] grid The grid used to store the statistics in the IPU3\n> - * \\return The mean value of the top 2% of the histogram\n> - */\n> -double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,\n> -\t\t\t      const ipu3_uapi_grid_config &grid) const\n> -{\n> -\t/* Initialise the histogram array */\n> -\tuint32_t hist[knumHistogramBins] = { 0 };\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<const ipu3_uapi_awb_set_item *>(\n> -\t\t\t\t\t&stats->awb_raw_buffer.meta_data[cellPosition]\n> -\t\t\t\t);\n> -\n> -\t\t\tuint8_t gr = cell->Gr_avg;\n> -\t\t\tuint8_t gb = cell->Gb_avg;\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[(gr + gb) / 2]++;\n> -\t\t}\n> -\t}\n> -\n> -\t/* Estimate the quantile mean of the top 2% of the histogram. */\n> -\treturn Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);\n> -}\n> -\n>  void Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n>  \t\t\t  const ipu3_uapi_grid_config &grid)\n>  {\n> @@ -219,173 +181,6 @@ void Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n>  \thist_ = Histogram(Span<uint32_t>(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> - *\n> - * The speed of the filter is adaptive, and will produce the target quicker\n> - * during startup, or when the target exposure is within 20% of the most recent\n> - * filter output.\n> - *\n> - * \\return The filtered exposure\n> - */\n> -utils::Duration Agc::filterExposure(utils::Duration exposureValue)\n> -{\n> -\tdouble speed = 0.2;\n> -\n> -\t/* Adapt instantly if we are in startup phase. */\n> -\tif (frameCount_ < kNumStartupFrames)\n> -\t\tspeed = 1.0;\n> -\n> -\t/*\n> -\t * If we are close to the desired result, go faster to avoid making\n> -\t * multiple micro-adjustments.\n> -\t * \\todo Make this customisable?\n> -\t */\n> -\tif (filteredExposure_ < 1.2 * exposureValue &&\n> -\t    filteredExposure_ > 0.8 * exposureValue)\n> -\t\tspeed = sqrt(speed);\n> -\n> -\tfilteredExposure_ = speed * exposureValue +\n> -\t\t\t    filteredExposure_ * (1.0 - speed);\n> -\n> -\tLOG(IPU3Agc, Debug) << \"After filtering, exposure \" << filteredExposure_;\n> -\n> -\treturn filteredExposure_;\n> -}\n> -\n> -/**\n> - * \\brief Estimate the new exposure and gain values\n> - * \\param[inout] frameContext The shared IPA frame Context\n> - * \\param[in] yGain The gain calculated based on the relative luminance target\n> - * \\param[in] iqMeanGain The gain calculated based on the relative luminance target\n> - */\n> -void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t\t  double yGain, double iqMeanGain)\n> -{\n> -\tconst IPASessionConfiguration &configuration = context.configuration;\n> -\t/* Get the effective exposure and gain applied on the sensor. */\n> -\tuint32_t exposure = frameContext.sensor.exposure;\n> -\tdouble analogueGain = frameContext.sensor.gain;\n> -\n> -\t/* Use the highest of the two gain estimates. */\n> -\tdouble evGain = std::max(yGain, iqMeanGain);\n> -\n> -\t/* Consider within 1% of the target as correctly exposed */\n> -\tif (utils::abs_diff(evGain, 1.0) < 0.01)\n> -\t\tLOG(IPU3Agc, Debug) << \"We are well exposed (evGain = \"\n> -\t\t\t\t    << evGain << \")\";\n> -\n> -\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\n> -\t/* Calculate the shutter time in seconds */\n> -\tutils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> -\n> -\t/*\n> -\t * Update the exposure value for the next computation using the values\n> -\t * of exposure and gain really used by the sensor.\n> -\t */\n> -\tutils::Duration effectiveExposureValue = currentShutter * analogueGain;\n> -\n> -\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentShutter * analogueGain\n> -\t\t\t    << \" Shutter speed \" << currentShutter\n> -\t\t\t    << \" Gain \" << analogueGain\n> -\t\t\t    << \" Needed ev gain \" << evGain;\n> -\n> -\t/*\n> -\t * Calculate the current exposure value for the scene as the latest\n> -\t * exposure value applied multiplied by the new estimated gain.\n> -\t */\n> -\tutils::Duration exposureValue = effectiveExposureValue * evGain;\n> -\n> -\t/* Clamp the exposure value to the min and max authorized */\n> -\tutils::Duration maxTotalExposure = maxShutterSpeed_ * maxAnalogueGain_;\n> -\texposureValue = std::min(exposureValue, maxTotalExposure);\n> -\tLOG(IPU3Agc, Debug) << \"Target total exposure \" << exposureValue\n> -\t\t\t    << \", maximum is \" << maxTotalExposure;\n> -\n> -\t/*\n> -\t * Filter the exposure.\n> -\t * \\todo estimate if we need to desaturate\n> -\t */\n> -\texposureValue = filterExposure(exposureValue);\n> -\n> -\t/*\n> -\t * Divide the exposure value as new exposure and gain values.\n> -\t *\n> -\t * Push the shutter time up to the maximum first, and only then\n> -\t * increase the gain.\n> -\t */\n> -\tutils::Duration shutterTime =\n> -\t\tstd::clamp<utils::Duration>(exposureValue / minAnalogueGain_,\n> -\t\t\t\t\t    minShutterSpeed_, maxShutterSpeed_);\n> -\tdouble stepGain = std::clamp(exposureValue / shutterTime,\n> -\t\t\t\t     minAnalogueGain_, maxAnalogueGain_);\n> -\tLOG(IPU3Agc, Debug) << \"Divided up shutter and gain are \"\n> -\t\t\t    << shutterTime << \" and \"\n> -\t\t\t    << stepGain;\n> -}\n> -\n> -/**\n> - * \\brief Estimate the relative luminance of the frame with a given gain\n> - * \\param[in] frameContext The shared IPA frame context\n> - * \\param[in] grid The grid used to store the statistics in the IPU3\n> - * \\param[in] stats The IPU3 statistics and ISP results\n> - * \\param[in] gain The gain to apply to the frame\n> - * \\return The relative luminance\n> - *\n> - * This function estimates the average relative luminance of the frame that\n> - * would be output by the sensor if an additional \\a gain was applied.\n> - *\n> - * The estimation is based on the AWB statistics for the current frame. Red,\n> - * green and blue averages for all cells are first multiplied by the gain, and\n> - * then saturated to approximate the sensor behaviour at high brightness\n> - * values. The approximation is quite rough, as it doesn't take into account\n> - * non-linearities when approaching saturation.\n> - *\n> - * The relative luminance (Y) is computed from the linear RGB components using\n> - * the Rec. 601 formula. The values are normalized to the [0.0, 1.0] range,\n> - * where 1.0 corresponds to a theoretical perfect reflector of 100% reference\n> - * white.\n> - *\n> - * More detailed information can be found in:\n> - * https://en.wikipedia.org/wiki/Relative_luminance\n\nAll this documentation is list. The first paragraph can be moved to the\ndocumentation in the base class, the rest can go to\nipa::ipu3::algorithms::Agc::estimateLuminance(const double gain). The\nbase class should also explain why luminance estimation is needed (due\nto saturation), and possibly how derived classes can implement that\ndepending on what statistics they have.\n\n> - */\n> -double Agc::estimateLuminance(IPAActiveState &activeState,\n> -\t\t\t      const ipu3_uapi_grid_config &grid,\n> -\t\t\t      const ipu3_uapi_stats_3a *stats,\n> -\t\t\t      double gain)\n> -{\n> -\tdouble redSum = 0, greenSum = 0, blueSum = 0;\n> -\n> -\t/* Sum the per-channel averages, saturated to 255. */\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<const ipu3_uapi_awb_set_item *>(\n> -\t\t\t\t\t&stats->awb_raw_buffer.meta_data[cellPosition]\n> -\t\t\t\t);\n> -\t\t\tconst uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> -\n> -\t\t\tredSum += std::min(cell->R_avg * gain, 255.0);\n> -\t\t\tgreenSum += std::min(G_avg * gain, 255.0);\n> -\t\t\tblueSum += std::min(cell->B_avg * gain, 255.0);\n> -\t\t}\n> -\t}\n> -\n> -\t/*\n> -\t * Apply the AWB gains to approximate colours correctly, use the Rec.\n> -\t * 601 formula to calculate the relative luminance, and normalize it.\n> -\t */\n> -\tdouble ySum = redSum * activeState.awb.gains.red * 0.299\n> -\t\t    + greenSum * activeState.awb.gains.green * 0.587\n> -\t\t    + blueSum * activeState.awb.gains.blue * 0.114;\n> -\n> -\treturn ySum / (grid.height * grid.width) / 255;\n> -}\n> -\n>  double Agc::estimateLuminance(double gain)\n>  {\n>  \tASSERT(reds_.size() == greens_.size());\n> @@ -422,42 +217,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t\t  const ipu3_uapi_stats_3a *stats,\n>  \t\t  ControlList &metadata)\n>  {\n> -\t/*\n> -\t * Estimate the gain needed to have the proportion of pixels in a given\n> -\t * desired range. iqMean is the mean value of the top 2% of the\n> -\t * cumulative histogram, and we want it to be as close as possible to a\n> -\t * configured target.\n> -\t */\n> -\tdouble iqMean = measureBrightness(stats, context.configuration.grid.bdsGrid);\n> -\tdouble iqMeanGain = kEvGainTarget * knumHistogramBins / iqMean;\n> -\n> -\t/*\n> -\t * Estimate the gain needed to achieve a relative luminance target. To\n> -\t * account for non-linearity caused by saturation, the value needs to be\n> -\t * estimated in an iterative process, as multiplying by a gain will not\n> -\t * increase the relative luminance by the same factor if some image\n> -\t * regions are saturated.\n> -\t */\n> -\tdouble yGain = 1.0;\n> -\tdouble yTarget = kRelativeLuminanceTarget;\n> -\n> -\tfor (unsigned int i = 0; i < 8; i++) {\n> -\t\tdouble yValue = estimateLuminance(context.activeState,\n> -\t\t\t\t\t\t  context.configuration.grid.bdsGrid,\n> -\t\t\t\t\t\t  stats, yGain);\n> -\t\tdouble extraGain = std::min(10.0, yTarget / (yValue + .001));\n> -\n> -\t\tyGain *= extraGain;\n> -\t\tLOG(IPU3Agc, Debug) << \"Y value: \" << yValue\n> -\t\t\t\t    << \", Y target: \" << yTarget\n> -\t\t\t\t    << \", gives gain \" << yGain;\n> -\t\tif (extraGain < 1.01)\n> -\t\t\tbreak;\n> -\t}\n> -\n> -\tcomputeExposure(context, frameContext, yGain, iqMeanGain);\n> -\tframeCount_++;\n> -\n>  \tparseStatistics(stats, context.configuration.grid.bdsGrid);\n>  \n>  \t/*\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 8405da9d..78fa3c75 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -38,29 +38,16 @@ public:\n>  \t\t     ControlList &metadata) override;\n>  \n>  private:\n> -\tdouble measureBrightness(const ipu3_uapi_stats_3a *stats,\n> -\t\t\t\t const ipu3_uapi_grid_config &grid) const;\n> -\tutils::Duration filterExposure(utils::Duration currentExposure);\n> -\tvoid computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t\t     double yGain, double iqMeanGain);\n> -\tdouble estimateLuminance(IPAActiveState &activeState,\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>  \tvoid parseStatistics(const ipu3_uapi_stats_3a *stats,\n>  \t\t\t     const ipu3_uapi_grid_config &grid);\n>  \n> -\tuint64_t frameCount_;\n> -\n>  \tutils::Duration minShutterSpeed_;\n>  \tutils::Duration maxShutterSpeed_;\n>  \n>  \tdouble minAnalogueGain_;\n>  \tdouble maxAnalogueGain_;\n>  \n> -\tutils::Duration filteredExposure_;\n> -\n>  \tuint32_t stride_;\n>  \tIPAContext *context_;\n>  \tstd::vector<uint8_t> reds_;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C7A7FC0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat,  6 Apr 2024 01:40:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3A2C63340;\n\tSat,  6 Apr 2024 03:40:09 +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 A5B0561C15\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  6 Apr 2024 03:40:08 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 59DF0231;\n\tSat,  6 Apr 2024 03:39:29 +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=\"ZP+mR4d8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712367569;\n\tbh=qj1bhxiswNRHCyeBxc479YG4srMSAxE5+p5AZeXCgiw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZP+mR4d8exoJueQQXeG+yYfoCXMqCsP4hT5/zHqhM9mlNqjZM5ADmCPq9XeWJz9GM\n\tNAji8tnMuNZoOrUMUJGTd/Pg8++GH8J4u2KmwFq2nMDfLbqs3x0zypsPD9uj1YO3Oe\n\tbcFvoXar56K4NGbXa4vEDV8zvicnpW+Tx9oPIdOE=","Date":"Sat, 6 Apr 2024 04:39:57 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Daniel Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 07/10] ipa: ipu3: Remove bespoke AGC functions from IPU3","Message-ID":"<20240406013957.GN12507@pendragon.ideasonboard.com>","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<20240322131451.3092931-8-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240322131451.3092931-8-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]