[{"id":16348,"web_url":"https://patchwork.libcamera.org/comment/16348/","msgid":"<34f7f074-ad74-429d-5439-b2c18f01c8e3@ideasonboard.com>","date":"2021-04-19T12:49:05","subject":"Re: [libcamera-devel] [PATCH v5 4/4] ipa: ipu3: Add support for\n\tIPU3 AEC/AGC algorithm","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi JM,\n\nOn 16/04/2021 08:49, Jean-Michel Hautbois wrote:\n> Implement basic auto-exposure (AE) and auto-gain (AG) correction functions.\n> The functions computeTargetExposure() and computeGain() are adapted from\n> the Raspberry Pi AGC implementation to suit the IPU3 structures, and\n> filtering is added to reduce visible stepsize when there are large\n> exposure changes.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/ipu3.cpp | 16 ++-\n> src/ipa/ipu3/ipu3_agc.cpp | 206 ++++++++++++++++++++++++++++++++++++++\n> src/ipa/ipu3/ipu3_agc.h | 62 ++++++++++++\n> src/ipa/ipu3/meson.build | 1 +\n> 4 files changed, 282 insertions(+), 3 deletions(-)\n> create mode 100644 src/ipa/ipu3/ipu3_agc.cpp\n> create mode 100644 src/ipa/ipu3/ipu3_agc.h\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index ab052a8a..48dc28c1 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -21,10 +21,11 @@\n> #include \"libcamera/internal/buffer.h\"\n> #include \"libcamera/internal/log.h\"\n> \n> +#include \"ipu3_agc.h\"\n> #include \"ipu3_awb.h\"\n> \n> static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> -static constexpr uint32_t kMaxCellHeightPerSet = 80;\n> +static constexpr uint32_t kMaxCellHeightPerSet = 56;\n\nIs this reduced to solve an issue? or to improve something in particular?\n\nOr does it represent some newly understood hardware limitation?\n\n> namespace libcamera {\n> \n> @@ -70,6 +71,8 @@ private:\n> \n> \t/* Interface to the AWB algorithm */\n> \tstd::unique_ptr awbAlgo_;\n> +\t/* Interface to the AEC/AGC algorithm */\n> +\tstd::unique_ptr agcAlgo_;\n> \n> \t/* Local parameter storage */\n> \tstruct ipu3_uapi_params params_;\n> @@ -168,6 +171,9 @@ void IPAIPU3::configure(const std::map &entityControls\n> \n> \tawbAlgo_ = std::make_unique();\n> \tawbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);\n> +\n> +\tagcAlgo_ = std::make_unique();\n> +\tagcAlgo_->initialise(bdsGrid_);\n> }\n> \n> void IPAIPU3::mapBuffers(const std::vector &buffers)\n> @@ -239,8 +245,8 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n> \n> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> {\n> -\t/* Pass a default gamma of 1.0 (default linear correction) */\n> -\tawbAlgo_->updateWbParameters(params_, 1.0);\n> +\tif (agcAlgo_->updateControls())\n> +\t\tawbAlgo_->updateWbParameters(params_, agcAlgo_->gamma());\n> \n> \t*params = params_;\n> \n> @@ -255,8 +261,12 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n> {\n> \tControlList ctrls(controls::controls);\n> \n> +\tagcAlgo_->process(stats, exposure_, gain_);\n> \tawbAlgo_->calculateWBGains(stats);\n> \n> +\tif (agcAlgo_->updateControls())\n> +\t\tsetControls(frame);\n> +\n> \tipa::ipu3::IPU3Action op;\n> \top.op = ipa::ipu3::ActionMetadataReady;\n> \top.controls = ctrls;\n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n> new file mode 100644\n> index 00000000..04ab55a1\n> --- /dev/null\n> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n> @@ -0,0 +1,206 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * ipu3_agc.cpp - AGC/AEC control algorithm\n> + */\n> +\n> +#include \"ipu3_agc.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +#include \"libipa/histogram.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n\nThis probably applies to the others in this series, but now that the\nIPAIPU3 class is in libcamera::ipa::ipu3, this should be too.\n\n> +\n> +LOG_DEFINE_CATEGORY(IPU3Agc)\n> +\n> +/* Number of frames to wait before calculating stats on minimum exposure */\n> +static constexpr uint32_t kInitialFrameMinAECount = 4;\n> +/* Number of frames to wait between new gain/exposure estimations */\n> +static constexpr uint32_t kFrameSkipCount = 6;\n> +\n> +/* Maximum ISO value for analogue gain */\n> +static constexpr uint32_t kMinISO = 100;\n> +static constexpr uint32_t kMaxISO = 1500;\n> +\n> +/* Maximum analogue gain value\n> + * \\todo grab it from a camera helper */\n> +static constexpr uint32_t kMinGain = kMinISO / 100;\n> +static constexpr uint32_t kMaxGain = kMaxISO / 100;\n> +\n> +/* \\todo use calculated value based on sensor */\n> +static constexpr uint32_t kMinExposure = 1;\n> +static constexpr uint32_t kMaxExposure = 1976;\n> +\n> +/* \\todo those should be get from pipeline handler ! */\n\nPipelinehandler or CameraSensorInfo?\n\n> +/* line duration in microseconds */\n> +static constexpr double kLineDuration = 16.8;\n> +static constexpr double kMaxExposureTime = kMaxExposure * kLineDuration;\n> +\n> +/* Histogram constants */\n> +static constexpr uint32_t knumHistogramBins = 256;\n> +static constexpr double kEvGainTarget = 0.5;\n> +\n> +IPU3Agc::IPU3Agc()\n> +\t: frameCount_(0), lastFrame_(0), converged_(false),\n> +\t updateControls_(false), iqMean_(0.0), gamma_(1.0),\n> +\t prevExposure_(0.0), prevExposureNoDg_(0.0),\n> +\t currentExposure_(0.0), currentExposureNoDg_(0.0)\n> +{\n> +}\n> +\n> +void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid)\n> +{\n> +\taeGrid_ = bdsGrid;\n> +}\n> +\n> +void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n> +{\n> +\tconst struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid;\n> +\tRectangle aeRegion = { statsAeGrid.x_start,\n> +\t\t\t statsAeGrid.y_start,\n> +\t\t\t static_cast(statsAeGrid.x_end - statsAeGrid.x_start) + 1,\n> +\t\t\t static_cast(statsAeGrid.y_end - statsAeGrid.y_start) + 1 };\n> +\tPoint topleft = aeRegion.topLeft();\n> +\tint topleftX = topleft.x >> aeGrid_.block_width_log2;\n> +\tint topleftY = topleft.y >> aeGrid_.block_height_log2;\n> +\n> +\tuint32_t startY = topleftY * aeGrid_.width << aeGrid_.block_width_log2;\n> +\tuint32_t startX = topleftX << aeGrid_.block_width_log2;\n\n\nis startY and startX a Point?\n\nCan we order them alphabetically .. x .. then y?\n\nIs all the shifting simply to make sure they are aligned to the block\nwidth and height?\n\nIf so - do we have any better helpers in geometry.h to help with that ?\nIt looks like we don't - so perhaps we should add\nPoint->alignedDownTo(xAlignment, yAlignment) sometime....\n\n(Not for this series).\n\n\n> +\tuint32_t endX = (startX + (aeRegion.size().width >> aeGrid_.block_width_log2)) << aeGrid_.block_width_log2;\n> +\tuint32_t i, j;\n> +\tuint32_t count = 0;\n> +\n> +\tuint32_t hist[knumHistogramBins] = { 0 };\n> +\tfor (j = topleftY;\n> +\t j < topleftY + (aeRegion.size().height >> aeGrid_.block_height_log2);\n> +\t j++) {\n> +\t\tfor (i = startX + startY; i < endX + startY; i += 8) {\n\nI still don't know what 8 is here?\n\n\n> +\t\t\t/**\n\n/*\n\n> +\t\t\t * The grid width (and maybe height) is not reliable.\n> +\t\t\t * We observed a bit shift which makes the value 160 to be 32 in the stats grid.\n> +\t\t\t * Use the one passed at init time.\n> +\t\t\t */\n> +\t\t\tif (stats->awb_raw_buffer.meta_data[i + 4 + j * aeGrid_.width] == 0) {\n> +\t\t\t\tuint8_t Gr = stats->awb_raw_buffer.meta_data[i + j * aeGrid_.width];\n\ni + 0 + to match the others?\n\n> +\t\t\t\tuint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * aeGrid_.width];\n> +\t\t\t\thist[(Gr + Gb) / 2]++;\n> +\t\t\t\tcount++;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\t/* Limit the gamma effect for now */\n> +\tgamma_ = 1.1;\n> +\n> +\t/* Estimate the quantile mean of the top 2% of the histogram */\n> +\tiqMean_ = Histogram(Span(hist)).interQuantileMean(0.98, 1.0);\n> +}\n> +\n> +void IPU3Agc::filterExposure(bool desaturate)\n> +{\n> +\tdouble speed = 0.2;\n> +\tif (prevExposure_ == 0.0) {\n> +\t\t/* DG stands for digital gain.*/\n> +\t\tprevExposure_ = currentExposure_;\n> +\t\tprevExposureNoDg_ = currentExposureNoDg_;\n> +\t} else {\n> +\t\t/**\n\n/*\n\n> +\t\t * If we are close to the desired result, go faster to avoid making\n> +\t\t * multiple micro-adjustments.\n> +\t\t * \\ todo: Make this customisable?\n> +\t\t */\n> +\t\tif (prevExposure_ < 1.2 * currentExposure_ &&\n> +\t\t prevExposure_ > 0.8 * currentExposure_)\n> +\t\t\tspeed = sqrt(speed);\n> +\t\tprevExposure_ = speed * currentExposure_ +\n> +\t\t\t\tprevExposure_ * (1.0 - speed);\n> +\t\t/**\n\n/*\n\n> +\t\t * When desaturing, take a big jump down in exposure_no_dg,\n\ndesaturating ?\n\n> +\t\t * which we'll hide with digital gain.\n> +\t\t */\n> +\t\tif (desaturate)\n> +\t\t\tprevExposureNoDg_ =\n> +\t\t\t\tcurrentExposureNoDg_;\n> +\t\telse\n> +\t\t\tprevExposureNoDg_ =\n> +\t\t\t\tspeed * currentExposureNoDg_ +\n> +\t\t\t\tprevExposureNoDg_ * (1.0 - speed);\n> +\t}\n> +\t/**\n\n/*\n\n\nWith all those:\n\nReviewed-by: Kieran Bingham \n\n\n> +\t * We can't let the no_dg exposure deviate too far below the\n> +\t * total exposure, as there might not be enough digital gain available\n> +\t * in the ISP to hide it (which will cause nasty oscillation).\n> +\t */\n> +\tdouble fastReduceThreshold = 0.4;\n> +\tif (prevExposureNoDg_ <\n> +\t prevExposure_ * fastReduceThreshold)\n> +\t\tprevExposureNoDg_ = prevExposure_ * fastReduceThreshold;\n> +\tLOG(IPU3Agc, Debug) << \"After filtering, total_exposure \" << prevExposure_;\n> +}\n> +\n> +void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)\n> +{\n> +\tupdateControls_ = false;\n> +\n> +\t/* Algorithm initialization should wait for first valid frames */\n> +\t/* \\todo - have a number of frames given by DelayedControls ?\n> +\t * - implement a function for IIR */\n> +\tif ((frameCount_ < kInitialFrameMinAECount) || (frameCount_ - lastFrame_ < kFrameSkipCount))\n> +\t\treturn;\n> +\n> +\t/* Are we correctly exposed ? */\n> +\tif (std::abs(iqMean_ - kEvGainTarget * knumHistogramBins) <= 1) {\n> +\t\tLOG(IPU3Agc, Debug) << \"!!! Good exposure with iqMean = \" << iqMean_;\n> +\t\tconverged_ = true;\n> +\t} else {\n> +\t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> +\n> +\t\t/* extracted from Rpi::Agc::computeTargetExposure */\n> +\t\tdouble currentShutter = exposure * kLineDuration;\n> +\t\tcurrentExposureNoDg_ = currentShutter * gain;\n> +\t\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n> +\t\t\t\t << \" Shutter speed \" << currentShutter\n> +\t\t\t\t << \" Gain \" << gain;\n> +\t\tcurrentExposure_ = currentExposureNoDg_ * newGain;\n> +\t\tdouble maxTotalExposure = kMaxExposureTime * kMaxGain;\n> +\t\tcurrentExposure_ = std::min(currentExposure_, maxTotalExposure);\n> +\t\tLOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_;\n> +\n> +\t\t/* \\todo: estimate if we need to desaturate */\n> +\t\tfilterExposure(false);\n> +\n> +\t\tdouble newExposure = 0.0;\n> +\t\tif (currentShutter < kMaxExposureTime) {\n> +\t\t\texposure = std::clamp(static_cast(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n> +\t\t\tnewExposure = currentExposure_ / exposure;\n> +\t\t\tgain = std::clamp(static_cast(gain * currentExposure_ / newExposure), kMinGain, kMaxGain);\n> +\t\t\tupdateControls_ = true;\n> +\t\t} else if (currentShutter >= kMaxExposureTime) {\n> +\t\t\tgain = std::clamp(static_cast(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain);\n> +\t\t\tnewExposure = currentExposure_ / gain;\n> +\t\t\texposure = std::clamp(static_cast(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure);\n> +\t\t\tupdateControls_ = true;\n> +\t\t}\n> +\t\tLOG(IPU3Agc, Debug) << \"Adjust exposure \" << exposure * kLineDuration << \" and gain \" << gain;\n> +\t}\n> +\tlastFrame_ = frameCount_;\n> +}\n> +\n> +void IPU3Agc::process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, uint32_t &gain)\n> +{\n> +\tprocessBrightness(stats);\n> +\tlockExposureGain(exposure, gain);\n> +\tframeCount_++;\n> +}\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n> new file mode 100644\n> index 00000000..f59bc420\n> --- /dev/null\n> +++ b/src/ipa/ipu3/ipu3_agc.h\n> @@ -0,0 +1,62 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * ipu3_agc.h - IPU3 AGC/AEC control algorithm\n> + */\n> +#ifndef __LIBCAMERA_IPU3_AGC_H__\n> +#define __LIBCAMERA_IPU3_AGC_H__\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libipa/algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +class IPU3Agc : public Algorithm\n> +{\n> +public:\n> +\tIPU3Agc();\n> +\t~IPU3Agc() = default;\n> +\n> +\tvoid initialise(struct ipu3_uapi_grid_config &bdsGrid);\n> +\tvoid process(const ipu3_uapi_stats_3a *stats, uint32_t &exposure, uint32_t &gain);\n> +\tbool converged() { return converged_; }\n> +\tbool updateControls() { return updateControls_; }\n> +\t/* \\todo Use a metadata exchange between IPAs */\n> +\tdouble gamma() { return gamma_; }\n> +\n> +private:\n> +\tvoid processBrightness(const ipu3_uapi_stats_3a *stats);\n> +\tvoid filterExposure(bool desaturate);\n> +\tvoid lockExposureGain(uint32_t &exposure, uint32_t &gain);\n> +\n> +\tstruct ipu3_uapi_grid_config aeGrid_;\n> +\n> +\tuint64_t frameCount_;\n> +\tuint64_t lastFrame_;\n> +\n> +\tbool converged_;\n> +\tbool updateControls_;\n> +\n> +\tdouble iqMean_;\n> +\tdouble gamma_;\n> +\n> +\tdouble prevExposure_;\n> +\tdouble prevExposureNoDg_;\n> +\tdouble currentExposure_;\n> +\tdouble currentExposureNoDg_;\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPU3_AGC_H__ */\n> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> index 1040698e..0d843846 100644\n> --- a/src/ipa/ipu3/meson.build\n> +++ b/src/ipa/ipu3/meson.build\n> @@ -4,6 +4,7 @@ ipa_name = 'ipa_ipu3'\n> \n> ipu3_ipa_sources = files([\n> 'ipu3.cpp',\n> + 'ipu3_agc.cpp',\n> 'ipu3_awb.cpp',\n> ])\n> \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 5C621BD812\n\tfor ;\n\tMon, 19 Apr 2021 12:49:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B632D6881F;\n\tMon, 19 Apr 2021 14:49:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 08F5168818\n\tfor ;\n\tMon, 19 Apr 2021 14:49:09 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 703E611B8;\n\tMon, 19 Apr 2021 14:49:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IAjuNxXt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618836548;\n\tbh=y08o4H6loQxkbNolW1Etu4X9vXcaDngO8e5joslb7XU=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=IAjuNxXt351M2L1RUqILdYgtBIAZMrKoL8ZqE7Lf73DrXr3FDxeAJg0HSop+7EDjj\n\tDshZqTUo84bcrcPbXqXdmjbWEmYGQ0R4I1b/N4i7/Gc0l/PV45tDsUxr+Qcp+1PvBz\n\tGh9OclH6UWDYWkiu6ONnB6GujWPVf5R3xRjDw8mI=","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210416074909.24218-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210416074909.24218-5-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<34f7f074-ad74-429d-5439-b2c18f01c8e3@ideasonboard.com>","Date":"Mon, 19 Apr 2021 13:49:05 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210416074909.24218-5-jeanmichel.hautbois@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v5 4/4] ipa: ipu3: Add support for\n\tIPU3 AEC/AGC algorithm","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]