Patch Detail
Show a patch.
GET /api/patches/13853/?format=api
{ "id": 13853, "url": "https://patchwork.libcamera.org/api/patches/13853/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13853/", "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": "<20210914163709.85751-5-jeanmichel.hautbois@ideasonboard.com>", "date": "2021-09-14T16:37:08", "name": "[libcamera-devel,v8,4/5] ipa: ipu3: Make the naming consistent", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "d976d9ea0c85a756bbff4b7da1524251e0b2246e", "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/13853/mbox/", "series": [ { "id": 2531, "url": "https://patchwork.libcamera.org/api/series/2531/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2531", "date": "2021-09-14T16:37:04", "name": "Move and improve AWB structures", "version": 8, "mbox": "https://patchwork.libcamera.org/series/2531/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13853/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13853/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 EA984BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 14 Sep 2021 16:37:18 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE7FB69195;\n\tTue, 14 Sep 2021 18:37:18 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4D0EA69187\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 Sep 2021 18:37:14 +0200 (CEST)", "from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:d0a7:2575:a724:b30a])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 01FFE499;\n\tTue, 14 Sep 2021 18:37:13 +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=\"GVCgPsdk\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631637434;\n\tbh=4BxLt+cgpqkV6Mc6MIdLNyI53xFrhDsdk9onnuBBels=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=GVCgPsdkm7MXUcOeCedgXQBsgB2soJ3Vlg2k+7HLC32ZfmbLxJW1bQSUDngQVxdIl\n\tfaWPL8ALmPHLDEzOnEaWASMFpY+GaQbxYloLX4WqRKCZXqAKBekdBd+NGMhuGqG6PU\n\tUGmBx0HDUMi4Zsgfy9fWYnW4IYpnbuOck+RiFbnI=", "From": "Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 14 Sep 2021 18:37:08 +0200", "Message-Id": "<20210914163709.85751-5-jeanmichel.hautbois@ideasonboard.com>", "X-Mailer": "git-send-email 2.30.2", "In-Reply-To": "<20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com>", "References": "<20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v8 4/5] ipa: ipu3: Make the naming\n\tconsistent", "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": "The variables mix the terms cell, region and zone. It can confuse the\nreader, and make the algorithm more difficult to follow. Rename the\nlocal variables consistent with their definitions:\n- Cells are defined in Pixels\n- Zones are defined in Cells\nThere is no \"region\" as such, so replace it with the correct term.\n\nWhile reading again the code for this renaming, the\nAgc::processBrightness() function was really too difficult to follow,\nand mixed pixels, cells, region of interest. The history is that AGC was\nmeant to be done in one particular region of the input frame (the\ncenter, or anything else) but there is no need for that, and there is no\neasy way to define this region of interest right now. It will be better\nto introduce metering instead.\n\nWhile we are renaming and making things more consistent, let's improve\nAgc at the same time:\n- Remove the aeRegion reference, as start_x and y_start are always 0\n- Use only the grid passed to the function, and not the one in the stats\n- rename i and j accordingly, to follow the Awb way\n- remove the unused variables (count, kCellSize, etc.)\n\nSigned-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n---\n src/ipa/ipu3/algorithms/agc.cpp | 44 +++++++++------------------------\n src/ipa/ipu3/algorithms/awb.cpp | 26 +++++++++----------\n 2 files changed, 25 insertions(+), 45 deletions(-)", "diff": "diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\nindex 5ff50f4a..52e8015e 100644\n--- a/src/ipa/ipu3/algorithms/agc.cpp\n+++ b/src/ipa/ipu3/algorithms/agc.cpp\n@@ -6,6 +6,7 @@\n */\n \n #include \"agc.h\"\n+#include \"awb.h\"\n \n #include <algorithm>\n #include <chrono>\n@@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976;\n static constexpr uint32_t knumHistogramBins = 256;\n static constexpr double kEvGainTarget = 0.5;\n \n-/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */\n-static constexpr uint8_t kCellSize = 8;\n-\n Agc::Agc()\n \t: frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s),\n \t maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s),\n@@ -70,37 +68,19 @@ int Agc::configure([[maybe_unused]] IPAContext &context,\n void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,\n \t\t\t const ipu3_uapi_grid_config &grid)\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<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1,\n-\t\t\t static_cast<unsigned int>(statsAeGrid.y_end - statsAeGrid.y_start) + 1 };\n-\tPoint topleft = aeRegion.topLeft();\n-\tint topleftX = topleft.x >> grid.block_width_log2;\n-\tint topleftY = topleft.y >> grid.block_height_log2;\n-\n-\t/* Align to the grid cell width and height */\n-\tuint32_t startX = topleftX << grid.block_width_log2;\n-\tuint32_t startY = topleftY * grid.width << grid.block_width_log2;\n-\tuint32_t endX = (startX + (aeRegion.size().width >> grid.block_width_log2)) << grid.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 >> grid.block_height_log2);\n-\t j++) {\n-\t\tfor (i = startX + startY; i < endX + startY; i += kCellSize) {\n-\t\t\t/*\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 * grid.width] == 0) {\n-\t\t\t\tuint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width];\n-\t\t\t\tuint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width];\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 * grid.width + cellX)\n+\t\t\t\t\t * 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+\t\t\tif (currentCell->satRatio == 0) {\n+\t\t\t\tuint8_t Gr = currentCell->greenRedAvg;\n+\t\t\t\tuint8_t Gb = currentCell->greenBlueAvg;\n \t\t\t\thist[(Gr + Gb) / 2]++;\n-\t\t\t\tcount++;\n \t\t\t}\n \t\t}\n \t}\ndiff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\nindex d78ae4f4..b96a6ebf 100644\n--- a/src/ipa/ipu3/algorithms/awb.cpp\n+++ b/src/ipa/ipu3/algorithms/awb.cpp\n@@ -220,31 +220,31 @@ void Awb::generateZones(std::vector<RGB> &zones)\n void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats,\n \t\t\t const ipu3_uapi_grid_config &grid)\n {\n-\tuint32_t regionWidth = round(grid.width / static_cast<double>(kAwbStatsSizeX));\n-\tuint32_t regionHeight = round(grid.height / static_cast<double>(kAwbStatsSizeY));\n+\tuint32_t cellsPerZoneX = round(grid.width / static_cast<double>(kAwbStatsSizeX));\n+\tuint32_t cellsPerZoneY = round(grid.height / static_cast<double>(kAwbStatsSizeY));\n \n \t/*\n \t * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is\n \t * (grid.width x grid.height).\n \t */\n-\tfor (unsigned int j = 0; j < kAwbStatsSizeY * regionHeight; j++) {\n-\t\tfor (unsigned int i = 0; i < kAwbStatsSizeX * regionWidth; i++) {\n-\t\t\tuint32_t cellPosition = j * grid.width + i;\n-\t\t\tuint32_t cellX = (cellPosition / regionWidth) % kAwbStatsSizeX;\n-\t\t\tuint32_t cellY = ((cellPosition / grid.width) / regionHeight) % kAwbStatsSizeY;\n+\tfor (unsigned int cellY = 0; cellY < kAwbStatsSizeY * cellsPerZoneY; cellY++) {\n+\t\tfor (unsigned int cellX = 0; cellX < kAwbStatsSizeX * cellsPerZoneX; cellX++) {\n+\t\t\tuint32_t cellPosition = (cellY * grid.width + cellX)\n+\t\t\t\t\t * sizeof(Ipu3AwbCell);\n+\t\t\tuint32_t zoneX = cellX / cellsPerZoneX;\n+\t\t\tuint32_t zoneY = cellY / cellsPerZoneY;\n \n-\t\t\tuint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX;\n-\t\t\tcellPosition *= 8;\n+\t\t\tuint32_t awbZonePosition = zoneY * kAwbStatsSizeX + zoneX;\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 \t\t\tif (currentCell->satRatio == 0) {\n \t\t\t\t/* The cell is not saturated, use the current cell */\n-\t\t\t\tawbStats_[awbRegionPosition].counted++;\n+\t\t\t\tawbStats_[awbZonePosition].counted++;\n \t\t\t\tuint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg;\n-\t\t\t\tawbStats_[awbRegionPosition].sum.green += greenValue / 2;\n-\t\t\t\tawbStats_[awbRegionPosition].sum.red += currentCell->redAvg;\n-\t\t\t\tawbStats_[awbRegionPosition].sum.blue += currentCell->blueAvg;\n+\t\t\t\tawbStats_[awbZonePosition].sum.green += greenValue / 2;\n+\t\t\t\tawbStats_[awbZonePosition].sum.red += currentCell->redAvg;\n+\t\t\t\tawbStats_[awbZonePosition].sum.blue += currentCell->blueAvg;\n \t\t\t}\n \t\t}\n \t}\n", "prefixes": [ "libcamera-devel", "v8", "4/5" ] }