[{"id":29290,"web_url":"https://patchwork.libcamera.org/comment/29290/","msgid":"","date":"2024-04-22T10:36:39","subject":"Re: [PATCH v2 6/8] ipa: ipu3: Remove bespoke AGC functions from IPU3","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Wed, Apr 17, 2024 at 02:15:34PM +0100, 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> Reviewed-by: Stefan Klug \n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v2:\n>\n> \t- Kept the documentation for estimateLuminance()\n>\n> src/ipa/ipu3/algorithms/agc.cpp | 227 +-------------------------------\n> src/ipa/ipu3/algorithms/agc.h | 13 --\n> 2 files changed, 3 insertions(+), 237 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 3b9761bd..46fc3b33 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -125,8 +125,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> \tcontext.activeState.agc.constraintMode = constraintModes().begin()->first;\n> \tcontext.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;\n>\n> @@ -139,42 +137,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(\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(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> @@ -208,123 +170,9 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\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> - *\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(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> + * \\param[in] gain The gain to apply in estimating luminance\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> @@ -339,42 +187,9 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n> *\n> * More detailed information can be found in:\n> * https://en.wikipedia.org/wiki/Relative_luminance\n> + *\n> + * @return The relative luminance of the frame\n\nWhile this is valid Doxygen, it's usually \"\\return\"\n\nWith this fixed\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\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(\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> \tdouble redSum = 0, greenSum = 0, blueSum = 0;\n> @@ -408,42 +223,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> \tHistogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);\n> \trGain_ = context.activeState.awb.gains.red;\n> \tgGain_ = context.activeState.awb.gains.blue;\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 40f32188..945d1846 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> \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> \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> \tdouble rGain_;\n> \tdouble gGain_;\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 4E791BE08B\n\tfor ;\n\tMon, 22 Apr 2024 10:36:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70299633FF;\n\tMon, 22 Apr 2024 12:36:44 +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 CB609633ED\n\tfor ;\n\tMon, 22 Apr 2024 12:36:42 +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 24FCA142E;\n\tMon, 22 Apr 2024 12:35: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=\"YhUZ6V5F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713782152;\n\tbh=YwnJoDNYbSLZQ5R07ngKXZ2hVBhcchl+i04wTqcIFgQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YhUZ6V5FiArGGotL6UcLrYeuuAl9fv4UoUezKpdzwCEi597buhKsPURA0NPiNO3AE\n\tBsVm7qYozxVFF9SRl18Z7xIXrhA7bOShw0X4rUT0KJ0GxAsr8GU/DkcwuOB02PShue\n\tlTAVhPS2XrsblQ/GlKOm1Ujbo109kLedXmGNnerw=","Date":"Mon, 22 Apr 2024 12:36:39 +0200","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org, \n\tStefan Klug ","Subject":"Re: [PATCH v2 6/8] ipa: ipu3: Remove bespoke AGC functions from IPU3","Message-ID":"","References":"<20240417131536.484129-1-dan.scally@ideasonboard.com>\n\t<20240417131536.484129-7-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240417131536.484129-7-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\" "}}]