Patch Detail
Show a patch.
GET /api/patches/12911/?format=api
{ "id": 12911, "url": "https://patchwork.libcamera.org/api/patches/12911/?format=api", "web_url": "https://patchwork.libcamera.org/patch/12911/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20210712131630.73914-3-jeanmichel.hautbois@ideasonboard.com>", "date": "2021-07-12T13:16:30", "name": "[libcamera-devel,2/2] ipa: ipu3: Use metadata and improve the doc", "commit_ref": null, "pull_url": null, "state": "rejected", "archived": false, "hash": "05a75a02293c9af76029a806ba708dcaaadfee83", "submitter": { "id": 75, "url": "https://patchwork.libcamera.org/api/people/75/?format=api", "name": "Jean-Michel Hautbois", "email": "jeanmichel.hautbois@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/12911/mbox/", "series": [ { "id": 2228, "url": "https://patchwork.libcamera.org/api/series/2228/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2228", "date": "2021-07-12T13:16:28", "name": "Introduce Metadata class in ipa", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2228/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/12911/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/12911/checks/", "tags": {}, "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 7245CC3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Jul 2021 13:16:44 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D00DC6852B;\n\tMon, 12 Jul 2021 15:16:40 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4CA068523\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Jul 2021 15:16:36 +0200 (CEST)", "from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:8515:881:eba9:bd61])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A3BBCC;\n\tMon, 12 Jul 2021 15:16:36 +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=\"bxrkm45l\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626095796;\n\tbh=AvrEUNy22xEBAGpLmTRAT/NPybCzo2iEeAXXV4J8b3k=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=bxrkm45l28K2TESX8wkrejQVVcAWt8w7mikewgetM0QEFY7/67rz2IebDUe1ewENq\n\t7Udc06VbXdNP14PvIy2Y8EaRaXVLVcSUy/L+Nz2vkwFRZvglCxBZEo/sAcqokEX9HC\n\tkGqvBWsFRVXMO4fVZ3Y6mD0XZSaJGRcrzeeJoeKs=", "From": "Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 12 Jul 2021 15:16:30 +0200", "Message-Id": "<20210712131630.73914-3-jeanmichel.hautbois@ideasonboard.com>", "X-Mailer": "git-send-email 2.30.2", "In-Reply-To": "<20210712131630.73914-1-jeanmichel.hautbois@ideasonboard.com>", "References": "<20210712131630.73914-1-jeanmichel.hautbois@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 2/2] ipa: ipu3: Use metadata and improve\n\tthe doc", "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>" }, "content": "Using the metadata to exchange the status of the awb algorithm helps to\nsimplify the interface. The structures used by the AWB statistics are in\nipu3_awb.h and documented.\nA bit of the doc has been improved in this same patch.\n\nThere is one metadata variable which will be used and set in the AGC\nalgorithm in a near future, which is the agcGamma. Use it as if it exists, the\nMetadata class won't find it and awb will default to a 1.0 value.\nDoing it now is convenient to have nice process() and updateParamaters() calls.\n\nSigned-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n---\n src/ipa/ipu3/ipu3.cpp | 8 ++-\n src/ipa/ipu3/ipu3_awb.cpp | 116 ++++++++++++++++++++++++++++----------\n src/ipa/ipu3/ipu3_awb.h | 34 ++++++-----\n 3 files changed, 111 insertions(+), 47 deletions(-)", "diff": "diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex 091856f5..1a3d98e9 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -80,6 +80,8 @@ private:\n \tstd::unique_ptr<IPU3Agc> agcAlgo_;\n \t/* Interface to the Camera Helper */\n \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n+\t/* Metadata storage */\n+\tMetadata metadata_;\n \n \t/* Local parameter storage */\n \tstruct ipu3_uapi_params params_;\n@@ -277,7 +279,7 @@ void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n {\n \tif (agcAlgo_->updateControls())\n-\t\tawbAlgo_->updateWbParameters(params_, agcAlgo_->gamma());\n+\t\tawbAlgo_->updateWbParameters(params_, &metadata_);\n \n \t*params = params_;\n \n@@ -297,8 +299,10 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n \tagcAlgo_->process(stats, exposure_, gain);\n \tgain_ = camHelper_->gainCode(gain);\n \n-\tawbAlgo_->calculateWBGains(stats);\n+\t/* Calculate the AWB gains */\n+\tawbAlgo_->process(stats, &metadata_);\n \n+\t/* Update the exposure and gains on sensor side */\n \tif (agcAlgo_->updateControls())\n \t\tsetControls(frame);\n \ndiff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp\nindex 9b409c8f..d441e835 100644\n--- a/src/ipa/ipu3/ipu3_awb.cpp\n+++ b/src/ipa/ipu3/ipu3_awb.cpp\n@@ -18,29 +18,54 @@ namespace ipa::ipu3 {\n \n LOG_DEFINE_CATEGORY(IPU3Awb)\n \n-static constexpr uint32_t kMinZonesCounted = 16;\n-static constexpr uint32_t kMinGreenLevelInZone = 32;\n+/**\n+ * The Grey World algorithm assumes that the scene, in average, is neutral grey.\n+ * Reference: Lam, Edmund & Fung, George. (2008). Automatic White Balancing in\n+ * Digital Photography. 10.1201/9781420054538.ch10.\n+ *\n+ * The IPU3 is generating statistics from the Bayer Demosaic Scaler output\n+ * into a grid defined in the ipu3_uapi_awb_config_s structure.\n+ *\n+ * For example, when the BDS output is 2592x1944 then the grid is calculated to be:\n+ * 81*30 with a cell beeing of size 32*64.\n+ * We then have an average of 2048 R, G and B pixels per cell.\n+ *\n+ * The AWB algorithm could use those variable grid sizes as an input, but it would\n+ * make it a bit more complex. In order to have something consistent with what is\n+ * done on RPi, fix a default grid size to kAwbStatsSizeX x kAwbStatsSizeY.\n+ *\n+ * Before calculating the gains, we will convert the statistics to go from the BDS\n+ * grid configuration to this intern grid size in generateAwbStats.\n+ * When the stats are converted, the saturation flag in the initial grid is used to\n+ * decide if the zone is saturated or not, making the zone relevant or not.\n+ *\n+ * The Grey World algorithm will then estimate the red and blue gains to apply, and\n+ * send those back through metadata.\n+ */\n \n /**\n- * \\struct IspStatsRegion\n+ * \\struct StatsRegion\n * \\brief RGB statistics for a given region\n *\n- * The IspStatsRegion structure is intended to abstract the ISP specific\n- * statistics and use an agnostic algorithm to compute AWB.\n+ * The StatsRegion structure is intended to abstract the ISP specific\n+ * statistics to compute AWB. The Grey World algorithm uses an average\n+ * for a specific counted pixels. When a specific zone in the scene is\n+ * saturated, we want to exclude it from the calculation, and consider\n+ * it as an outlier.\n *\n- * \\var IspStatsRegion::counted\n- * \\brief Number of pixels used to calculate the sums\n+ * \\var StatsRegion::counted\n+ * \\brief Number of unsatured pixels used to calculate the sums\n *\n- * \\var IspStatsRegion::uncounted\n- * \\brief Remaining number of pixels in the region\n+ * \\var StatsRegion::uncounted\n+ * \\brief Remaining number of pixels in the region (ie saturated)\n *\n- * \\var IspStatsRegion::rSum\n+ * \\var StatsRegion::rSum\n * \\brief Sum of the red values in the region\n *\n- * \\var IspStatsRegion::gSum\n+ * \\var StatsRegion::gSum\n * \\brief Sum of the green values in the region\n *\n- * \\var IspStatsRegion::bSum\n+ * \\var StatsRegion::bSum\n * \\brief Sum of the blue values in the region\n */\n \n@@ -48,26 +73,35 @@ static constexpr uint32_t kMinGreenLevelInZone = 32;\n * \\struct AwbStatus\n * \\brief AWB parameters calculated\n *\n- * The AwbStatus structure is intended to store the AWB\n- * parameters calculated by the algorithm\n+ * The AwbStatus structure is intended to store the AWB parameters\n+ * calculated by the algorithm, and shared through the metadata\n+ * object, for other algorithms\n *\n * \\var AwbStatus::temperatureK\n- * \\brief Color temperature calculated\n+ * \\brief Color temperature calculated, in Kelvin\n *\n * \\var AwbStatus::redGain\n- * \\brief Gain calculated for the red channel\n+ * \\brief Gain calculated for the red channel. This is a floating-point\n+ * value used as a multiplier on the ISP side\n *\n * \\var AwbStatus::greenGain\n- * \\brief Gain calculated for the green channel\n+ * \\brief Gain calculated for the green channel. This is a floating-point\n+ * value used as a multiplier on the ISP side\n *\n * \\var AwbStatus::blueGain\n- * \\brief Gain calculated for the blue channel\n+ * \\brief Gain calculated for the blue channel. This is a floating-point\n+ * value used as a multiplier on the ISP side\n */\n \n /**\n * \\struct Ipu3AwbCell\n * \\brief Memory layout for each cell in AWB metadata\n *\n+ * This is the internal layout on IPU3 for one cell of AWB statistics.\n+ * There is ipu3_uapi_awb_config_s->grid.width * 2^block_width_log2 per\n+ * ipu3_uapi_awb_config_s->grid.height * 2^block_height_log2 cells for\n+ * one frame statistics.\n+ *\n * The Ipu3AwbCell structure is used to get individual values\n * such as red average or saturation ratio in a particular cell.\n *\n@@ -84,7 +118,8 @@ static constexpr uint32_t kMinGreenLevelInZone = 32;\n * \\brief Green average for blue lines\n *\n * \\var Ipu3AwbCell::satRatio\n- * \\brief Saturation ratio in the cell\n+ * \\brief Saturation ratio in the cell. It depends on the rgbs_thr_* values, and\n+ * will be set if rgbs_thr_b has IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT bit set.\n *\n * \\var Ipu3AwbCell::padding\n * \\brief array of unused bytes for padding\n@@ -159,6 +194,9 @@ const struct ipu3_uapi_gamma_corr_lut imguCssGammaLut = { {\n \t7807, 7871, 7935, 7999, 8063, 8127, 8191\n } };\n \n+/* Minimum level of green (on a 8 bits base) in a given zone */\n+static constexpr uint32_t kMinGreenLevelInZone = 16;\n+\n IPU3Awb::IPU3Awb()\n \t: Algorithm()\n {\n@@ -166,6 +204,7 @@ IPU3Awb::IPU3Awb()\n \tasyncResults_.greenGain = 1.0;\n \tasyncResults_.redGain = 1.0;\n \tasyncResults_.temperatureK = 4500;\n+\tminZonesCounted_ = 0;\n }\n \n IPU3Awb::~IPU3Awb()\n@@ -202,7 +241,7 @@ void IPU3Awb::initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, st\n \tparams.acc_param.gamma.gc_lut = imguCssGammaLut;\n \tparams.acc_param.gamma.gc_ctrl.enable = 1;\n \n-\tzones_.reserve(kAwbStatsSizeX * kAwbStatsSizeY);\n+\tzones_.reserve(kAwbStatsSize);\n }\n \n /**\n@@ -238,10 +277,10 @@ uint32_t IPU3Awb::estimateCCT(double red, double green, double blue)\n /* Generate an RGB vector with the average values for each region */\n void IPU3Awb::generateZones(std::vector<RGB> &zones)\n {\n-\tfor (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {\n+\tfor (unsigned int i = 0; i < kAwbStatsSize; i++) {\n \t\tRGB zone;\n \t\tdouble counted = awbStats_[i].counted;\n-\t\tif (counted >= kMinZonesCounted) {\n+\t\tif (counted >= minZonesCounted_) {\n \t\t\tzone.G = awbStats_[i].gSum / counted;\n \t\t\tif (zone.G >= kMinGreenLevelInZone) {\n \t\t\t\tzone.R = awbStats_[i].rSum / counted;\n@@ -258,6 +297,7 @@ void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)\n \tuint32_t regionWidth = round(awbGrid_.width / static_cast<double>(kAwbStatsSizeX));\n \tuint32_t regionHeight = round(awbGrid_.height / static_cast<double>(kAwbStatsSizeY));\n \n+\tminZonesCounted_ = ((regionWidth * regionHeight) * 4) / 5;\n \t/*\n \t * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is\n \t * (awbGrid_.width x awbGrid_.height).\n@@ -269,7 +309,7 @@ void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)\n \t\t\tuint32_t cellY = ((cellPosition / awbGrid_.width) / regionHeight) % kAwbStatsSizeY;\n \n \t\t\tuint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX;\n-\t\t\tcellPosition *= 8;\n+\t\t\tcellPosition *= sizeof(Ipu3AwbCell);\n \n \t\t\t/* Cast the initial IPU3 structure to simplify the reading */\n \t\t\tIpu3AwbCell *currentCell = reinterpret_cast<Ipu3AwbCell *>(const_cast<uint8_t *>(&stats->awb_raw_buffer.meta_data[cellPosition]));\n@@ -287,7 +327,7 @@ void IPU3Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)\n \n void IPU3Awb::clearAwbStats()\n {\n-\tfor (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) {\n+\tfor (unsigned int i = 0; i < kAwbStatsSize; i++) {\n \t\tawbStats_[i].bSum = 0;\n \t\tawbStats_[i].rSum = 0;\n \t\tawbStats_[i].gSum = 0;\n@@ -344,24 +384,38 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)\n \tgenerateAwbStats(stats);\n \tgenerateZones(zones_);\n \tLOG(IPU3Awb, Debug) << \"Valid zones: \" << zones_.size();\n-\tif (zones_.size() > 10) {\n+\n+\t/* We need at least 5% of valid zones to estimate the gain correction */\n+\tif (zones_.size() > kAwbStatsSize / 20) {\n \t\tawbGreyWorld();\n \t\tLOG(IPU3Awb, Debug) << \"Gain found for red: \" << asyncResults_.redGain\n \t\t\t\t << \" and for blue: \" << asyncResults_.blueGain;\n \t}\n }\n \n-void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms, double agcGamma)\n+void IPU3Awb::process(const ipu3_uapi_stats_3a *stats, Metadata *imageMetadata)\n+{\n+\tcalculateWBGains(stats);\n+\t/* We need to update the AWB status, to give back the gains */\n+\timageMetadata->set(\"awb.status\", asyncResults_);\n+}\n+\n+void IPU3Awb::updateWbParameters(ipu3_uapi_params ¶ms, Metadata *imageMetadata)\n {\n \t/*\n \t * Green gains should not be touched and considered 1.\n \t * Default is 16, so do not change it at all.\n-\t * 4096 is the value for a gain of 1.0\n+\t * 8192 is the value for a gain of 1.0\n \t */\n-\tparams.acc_param.bnr.wb_gains.gr = 16;\n-\tparams.acc_param.bnr.wb_gains.r = 4096 * asyncResults_.redGain;\n-\tparams.acc_param.bnr.wb_gains.b = 4096 * asyncResults_.blueGain;\n-\tparams.acc_param.bnr.wb_gains.gb = 16;\n+\tparams.acc_param.bnr.wb_gains.gr = 8192;\n+\tparams.acc_param.bnr.wb_gains.r = 8192 * asyncResults_.redGain;\n+\tparams.acc_param.bnr.wb_gains.b = 8192 * asyncResults_.blueGain;\n+\tparams.acc_param.bnr.wb_gains.gb = 8192;\n+\n+\t/* When the AGC algorithm has run, it may have set a new gamma */\n+\tdouble agcGamma = 1.0;\n+\tif (imageMetadata->get(\"agc.gamma\", agcGamma) != 0)\n+\t\tLOG(IPU3Awb, Debug) << \"Awb: no gamma found, defaulted to 1.0\";\n \n \tLOG(IPU3Awb, Debug) << \"Color temperature estimated: \" << asyncResults_.temperatureK\n \t\t\t << \" and gamma calculated: \" << agcGamma;\ndiff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/ipu3_awb.h\nindex 122cf68c..cf12032d 100644\n--- a/src/ipa/ipu3/ipu3_awb.h\n+++ b/src/ipa/ipu3/ipu3_awb.h\n@@ -14,14 +14,18 @@\n #include <libcamera/geometry.h>\n \n #include \"libipa/algorithm.h\"\n+#include \"libipa/metadata.h\"\n \n namespace libcamera {\n \n namespace ipa::ipu3 {\n \n-/* Region size for the statistics generation algorithm */\n+/* Width of the AWB regions used for Grey World calculation */\n static constexpr uint32_t kAwbStatsSizeX = 16;\n+/* Height of the AWB regions used for Grey World calculation */\n static constexpr uint32_t kAwbStatsSizeY = 12;\n+/* Total size of the AWB regions used for Grey World calculation */\n+static constexpr uint32_t kAwbStatsSize = kAwbStatsSizeX * kAwbStatsSizeY;\n \n class IPU3Awb : public Algorithm\n {\n@@ -30,17 +34,8 @@ public:\n \t~IPU3Awb();\n \n \tvoid initialise(ipu3_uapi_params ¶ms, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid);\n-\tvoid calculateWBGains(const ipu3_uapi_stats_3a *stats);\n-\tvoid updateWbParameters(ipu3_uapi_params ¶ms, double agcGamma);\n-\n-\tstruct Ipu3AwbCell {\n-\t\tunsigned char greenRedAvg;\n-\t\tunsigned char redAvg;\n-\t\tunsigned char blueAvg;\n-\t\tunsigned char greenBlueAvg;\n-\t\tunsigned char satRatio;\n-\t\tunsigned char padding[3];\n-\t} __attribute__((packed));\n+\tvoid process(const ipu3_uapi_stats_3a *stats, Metadata *imageMetadata);\n+\tvoid updateWbParameters(ipu3_uapi_params ¶ms, Metadata *imageMetadata);\n \n \t/* \\todo Make these three structs available to all the ISPs ? */\n \tstruct RGB {\n@@ -56,7 +51,7 @@ public:\n \t\t}\n \t};\n \n-\tstruct IspStatsRegion {\n+\tstruct StatsRegion {\n \t\tunsigned int counted;\n \t\tunsigned int uncounted;\n \t\tunsigned long long rSum;\n@@ -71,18 +66,29 @@ public:\n \t\tdouble blueGain;\n \t};\n \n+\tstruct Ipu3AwbCell {\n+\t\tunsigned char greenRedAvg;\n+\t\tunsigned char redAvg;\n+\t\tunsigned char blueAvg;\n+\t\tunsigned char greenBlueAvg;\n+\t\tunsigned char satRatio;\n+\t\tunsigned char padding[3];\n+\t};\n+\n private:\n \tvoid generateZones(std::vector<RGB> &zones);\n \tvoid generateAwbStats(const ipu3_uapi_stats_3a *stats);\n \tvoid clearAwbStats();\n \tvoid awbGreyWorld();\n \tuint32_t estimateCCT(double red, double green, double blue);\n+\tvoid calculateWBGains(const ipu3_uapi_stats_3a *stats);\n \n \tstruct ipu3_uapi_grid_config awbGrid_;\n \n \tstd::vector<RGB> zones_;\n-\tIspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];\n+\tStatsRegion awbStats_[kAwbStatsSize];\n \tAwbStatus asyncResults_;\n+\tuint32_t minZonesCounted_;\n };\n \n } /* namespace ipa::ipu3 */\n", "prefixes": [ "libcamera-devel", "2/2" ] }