Patch Detail
Show a patch.
GET /api/1.1/patches/24623/?format=api
{ "id": 24623, "url": "https://patchwork.libcamera.org/api/1.1/patches/24623/?format=api", "web_url": "https://patchwork.libcamera.org/patch/24623/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20251014075252.2876485-8-stefan.klug@ideasonboard.com>", "date": "2025-10-14T07:52:29", "name": "[v1,07/12] ipa: rksip1: lsc: Move function definitions out of class", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "2985ee03fb25fcd564e0ed317da93a0e42bd4b60", "submitter": { "id": 184, "url": "https://patchwork.libcamera.org/api/1.1/people/184/?format=api", "name": "Stefan Klug", "email": "stefan.klug@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/24623/mbox/", "series": [ { "id": 5498, "url": "https://patchwork.libcamera.org/api/1.1/series/5498/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5498", "date": "2025-10-14T07:52:22", "name": "Add resampling support for polynomial LSC data", "version": 1, "mbox": "https://patchwork.libcamera.org/series/5498/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/24623/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/24623/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 83084BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 14 Oct 2025 07:53:22 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26D33605E5;\n\tTue, 14 Oct 2025 09:53:22 +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 A6B476052F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 Oct 2025 09:53:19 +0200 (CEST)", "from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:61fb:8e55:ff1e:be62])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 75C94C73;\n\tTue, 14 Oct 2025 09:51:41 +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=\"uIr13y69\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760428301;\n\tbh=/rju20fUx+t7fLoumYtsmjDtj7B8b7bm7Sk9WU1/3yM=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=uIr13y692kUX04fqMAps+Wr9b9+GG5PuxqoEdfrgj3RI8Yq5ZgM9WP+dsbP35WW0F\n\t4c5osqkNW9rc3kYhQGQOk0pDHkL/XmJh3lIIjT/u24iDeIsdPKZlv3qpQXZ4GxXx39\n\tlG7mT2juUuotNni0R9iIMDEvWXOAr1LEMF8c32nI=", "From": "Stefan Klug <stefan.klug@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Stefan Klug <stefan.klug@ideasonboard.com>", "Subject": "[PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out of\n\tclass", "Date": "Tue, 14 Oct 2025 09:52:29 +0200", "Message-ID": "<20251014075252.2876485-8-stefan.klug@ideasonboard.com>", "X-Mailer": "git-send-email 2.48.1", "In-Reply-To": "<20251014075252.2876485-1-stefan.klug@ideasonboard.com>", "References": "<20251014075252.2876485-1-stefan.klug@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "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": "Move the function definitions out of the related classes. This was noted\nin review after the series was already merged. After trying it out I\nmust admit that it really improves readability and reduces the\nindentation level by one.\n\nSigned-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n\n---\n\nNote: This patch contains no functional changes. `git diff\n--ignore-space-change` helps in verifying that.\n---\n src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------\n 1 file changed, 163 insertions(+), 151 deletions(-)", "diff": "diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\nindex e46943f4bae0..0c1bb470c346 100644\n--- a/src/ipa/rkisp1/algorithms/lsc.cpp\n+++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n@@ -85,184 +85,196 @@ public:\n \t}\n \n \tint parseLscData(const YamlObject &yamlSets,\n-\t\t\t std::map<unsigned int, LensShadingCorrection::Components> &lscData)\n-\t{\n-\t\tconst auto &sets = yamlSets.asList();\n-\t\tfor (const auto &yamlSet : sets) {\n-\t\t\tstd::optional<LscPolynomial> pr, pgr, pgb, pb;\n-\t\t\tuint32_t ct = yamlSet[\"ct\"].get<uint32_t>(0);\n-\n-\t\t\tif (lscData.count(ct)) {\n-\t\t\t\tLOG(RkISP1Lsc, Error)\n-\t\t\t\t\t<< \"Multiple sets found for \"\n-\t\t\t\t\t<< \"color temperature \" << ct;\n-\t\t\t\treturn -EINVAL;\n-\t\t\t}\n-\n-\t\t\tLensShadingCorrection::Components &set = lscData[ct];\n-\t\t\tpr = yamlSet[\"r\"].get<LscPolynomial>();\n-\t\t\tpgr = yamlSet[\"gr\"].get<LscPolynomial>();\n-\t\t\tpgb = yamlSet[\"gb\"].get<LscPolynomial>();\n-\t\t\tpb = yamlSet[\"b\"].get<LscPolynomial>();\n-\n-\t\t\tif (!(pr || pgr || pgb || pb)) {\n-\t\t\t\tLOG(RkISP1Lsc, Error)\n-\t\t\t\t\t<< \"Failed to parse polynomial for \"\n-\t\t\t\t\t<< \"colour temperature \" << ct;\n-\t\t\t\treturn -EINVAL;\n-\t\t\t}\n-\n-\t\t\tpr->setReferenceImageSize(sensorSize_);\n-\t\t\tpgr->setReferenceImageSize(sensorSize_);\n-\t\t\tpgb->setReferenceImageSize(sensorSize_);\n-\t\t\tpb->setReferenceImageSize(sensorSize_);\n-\t\t\tset.r = samplePolynomial(*pr);\n-\t\t\tset.gr = samplePolynomial(*pgr);\n-\t\t\tset.gb = samplePolynomial(*pgb);\n-\t\t\tset.b = samplePolynomial(*pb);\n+\t\t\t std::map<unsigned int, LensShadingCorrection::Components> &lscData);\n+\n+private:\n+\tstd::vector<double> sizesListToPositions(const std::vector<double> &sizes);\n+\tstd::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);\n+\n+\tSize sensorSize_;\n+\tRectangle cropRectangle_;\n+\tconst std::vector<double> &xSizes_;\n+\tconst std::vector<double> &ySizes_;\n+};\n+\n+int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n+\t\t\t\t std::map<unsigned int, LensShadingCorrection::Components> &lscData)\n+{\n+\tconst auto &sets = yamlSets.asList();\n+\tfor (const auto &yamlSet : sets) {\n+\t\tstd::optional<LscPolynomial> pr, pgr, pgb, pb;\n+\t\tuint32_t ct = yamlSet[\"ct\"].get<uint32_t>(0);\n+\n+\t\tif (lscData.count(ct)) {\n+\t\t\tLOG(RkISP1Lsc, Error)\n+\t\t\t\t<< \"Multiple sets found for \"\n+\t\t\t\t<< \"color temperature \" << ct;\n+\t\t\treturn -EINVAL;\n \t\t}\n \n-\t\tif (lscData.empty()) {\n-\t\t\tLOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n+\t\tLensShadingCorrection::Components &set = lscData[ct];\n+\t\tpr = yamlSet[\"r\"].get<LscPolynomial>();\n+\t\tpgr = yamlSet[\"gr\"].get<LscPolynomial>();\n+\t\tpgb = yamlSet[\"gb\"].get<LscPolynomial>();\n+\t\tpb = yamlSet[\"b\"].get<LscPolynomial>();\n+\n+\t\tif (!(pr || pgr || pgb || pb)) {\n+\t\t\tLOG(RkISP1Lsc, Error)\n+\t\t\t\t<< \"Failed to parse polynomial for \"\n+\t\t\t\t<< \"colour temperature \" << ct;\n \t\t\treturn -EINVAL;\n \t\t}\n \n-\t\treturn 0;\n+\t\tpr->setReferenceImageSize(sensorSize_);\n+\t\tpgr->setReferenceImageSize(sensorSize_);\n+\t\tpgb->setReferenceImageSize(sensorSize_);\n+\t\tpb->setReferenceImageSize(sensorSize_);\n+\t\tset.r = samplePolynomial(*pr);\n+\t\tset.gr = samplePolynomial(*pgr);\n+\t\tset.gb = samplePolynomial(*pgb);\n+\t\tset.b = samplePolynomial(*pb);\n \t}\n \n-private:\n-\t/*\n-\t * The rkisp1 LSC grid spacing is defined by the cell sizes on the first\n-\t * half of the grid. This is then mirrored in hardware to the other\n-\t * half. See parseSizes() for further details. For easier handling, this\n-\t * function converts the cell sizes of half the grid to a list of\n-\t * position of the whole grid (on one axis). Example:\n-\t *\n-\t * input: | 0.2 | 0.3 |\n-\t * output: 0.0 0.2 0.5 0.8 1.0\n-\t */\n-\tstd::vector<double> sizesListToPositions(const std::vector<double> &sizes)\n-\t{\n-\t\tconst int half = sizes.size();\n-\t\tstd::vector<double> positions(half * 2 + 1);\n-\t\tdouble x = 0.0;\n-\n-\t\tpositions[half] = 0.5;\n-\t\tfor (int i = 1; i <= half; i++) {\n-\t\t\tx += sizes[half - i];\n-\t\t\tpositions[half - i] = 0.5 - x;\n-\t\t\tpositions[half + i] = 0.5 + x;\n-\t\t}\n+\tif (lscData.empty()) {\n+\t\tLOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n+\t\treturn -EINVAL;\n+\t}\n \n-\t\treturn positions;\n+\treturn 0;\n+}\n+\n+/*\n+ * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of\n+ * the grid. This is then mirrored in hardware to the other half. See\n+ * parseSizes() for further details. For easier handling, this function converts\n+ * the cell sizes of half the grid to a list of position of the whole grid (on\n+ * one axis). Example:\n+ *\n+ * input: | 0.2 | 0.3 |\n+ * output: 0.0 0.2 0.5 0.8 1.0\n+ */\n+std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)\n+{\n+\tconst int half = sizes.size();\n+\tstd::vector<double> positions(half * 2 + 1);\n+\tdouble x = 0.0;\n+\n+\tpositions[half] = 0.5;\n+\tfor (int i = 1; i <= half; i++) {\n+\t\tx += sizes[half - i];\n+\t\tpositions[half - i] = 0.5 - x;\n+\t\tpositions[half + i] = 0.5 + x;\n \t}\n \n-\tstd::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)\n-\t{\n-\t\tconstexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n-\n-\t\tdouble m = poly.getM();\n-\t\tdouble x0 = cropRectangle_.x / m;\n-\t\tdouble y0 = cropRectangle_.y / m;\n-\t\tdouble w = cropRectangle_.width / m;\n-\t\tdouble h = cropRectangle_.height / m;\n-\t\tstd::vector<uint16_t> samples;\n-\n-\t\tASSERT(xSizes_.size() * 2 + 1 == k);\n-\t\tASSERT(ySizes_.size() * 2 + 1 == k);\n-\n-\t\tsamples.reserve(k * k);\n-\n-\t\tstd::vector<double> xPos(sizesListToPositions(xSizes_));\n-\t\tstd::vector<double> yPos(sizesListToPositions(ySizes_));\n-\n-\t\tfor (int y = 0; y < k; y++) {\n-\t\t\tfor (int x = 0; x < k; x++) {\n-\t\t\t\tdouble xp = x0 + xPos[x] * w;\n-\t\t\t\tdouble yp = y0 + yPos[y] * h;\n-\t\t\t\t/*\n-\t\t\t\t * The hardware uses 2.10 fixed point format and\n-\t\t\t\t * limits the legal values to [1..3.999]. Scale\n-\t\t\t\t * and clamp the sampled value accordingly.\n-\t\t\t\t */\n-\t\t\t\tint v = static_cast<int>(\n-\t\t\t\t\tpoly.sampleAtNormalizedPixelPos(xp, yp) *\n-\t\t\t\t\t1024);\n-\t\t\t\tv = std::min(std::max(v, 1024), 4095);\n-\t\t\t\tsamples.push_back(v);\n-\t\t\t}\n+\treturn positions;\n+}\n+\n+std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)\n+{\n+\tconstexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n+\n+\tdouble m = poly.getM();\n+\tdouble x0 = cropRectangle_.x / m;\n+\tdouble y0 = cropRectangle_.y / m;\n+\tdouble w = cropRectangle_.width / m;\n+\tdouble h = cropRectangle_.height / m;\n+\tstd::vector<uint16_t> samples;\n+\n+\tASSERT(xSizes_.size() * 2 + 1 == k);\n+\tASSERT(ySizes_.size() * 2 + 1 == k);\n+\n+\tsamples.reserve(k * k);\n+\n+\tstd::vector<double> xPos(sizesListToPositions(xSizes_));\n+\tstd::vector<double> yPos(sizesListToPositions(ySizes_));\n+\n+\tfor (int y = 0; y < k; y++) {\n+\t\tfor (int x = 0; x < k; x++) {\n+\t\t\tdouble xp = x0 + xPos[x] * w;\n+\t\t\tdouble yp = y0 + yPos[y] * h;\n+\t\t\t/*\n+\t\t\t * The hardware uses 2.10 fixed point format and limits\n+\t\t\t * the legal values to [1..3.999]. Scale and clamp the\n+\t\t\t * sampled value accordingly.\n+\t\t\t */\n+\t\t\tint v = static_cast<int>(\n+\t\t\t\tpoly.sampleAtNormalizedPixelPos(xp, yp) *\n+\t\t\t\t1024);\n+\t\t\tv = std::min(std::max(v, 1024), 4095);\n+\t\t\tsamples.push_back(v);\n \t\t}\n-\t\treturn samples;\n \t}\n-\n-\tSize sensorSize_;\n-\tRectangle cropRectangle_;\n-\tconst std::vector<double> &xSizes_;\n-\tconst std::vector<double> &ySizes_;\n-};\n+\treturn samples;\n+}\n \n class LscTableLoader\n {\n public:\n \tint parseLscData(const YamlObject &yamlSets,\n-\t\t\t std::map<unsigned int, LensShadingCorrection::Components> &lscData)\n-\t{\n-\t\tconst auto &sets = yamlSets.asList();\n-\n-\t\tfor (const auto &yamlSet : sets) {\n-\t\t\tuint32_t ct = yamlSet[\"ct\"].get<uint32_t>(0);\n-\n-\t\t\tif (lscData.count(ct)) {\n-\t\t\t\tLOG(RkISP1Lsc, Error)\n-\t\t\t\t\t<< \"Multiple sets found for color temperature \"\n-\t\t\t\t\t<< ct;\n-\t\t\t\treturn -EINVAL;\n-\t\t\t}\n-\n-\t\t\tLensShadingCorrection::Components &set = lscData[ct];\n-\n-\t\t\tset.r = parseTable(yamlSet, \"r\");\n-\t\t\tset.gr = parseTable(yamlSet, \"gr\");\n-\t\t\tset.gb = parseTable(yamlSet, \"gb\");\n-\t\t\tset.b = parseTable(yamlSet, \"b\");\n-\n-\t\t\tif (set.r.empty() || set.gr.empty() ||\n-\t\t\t set.gb.empty() || set.b.empty()) {\n-\t\t\t\tLOG(RkISP1Lsc, Error)\n-\t\t\t\t\t<< \"Set for color temperature \" << ct\n-\t\t\t\t\t<< \" is missing tables\";\n-\t\t\t\treturn -EINVAL;\n-\t\t\t}\n-\t\t}\n+\t\t\t std::map<unsigned int, LensShadingCorrection::Components> &lscData);\n+\n+private:\n+\tstd::vector<uint16_t> parseTable(const YamlObject &tuningData,\n+\t\t\t\t\t const char *prop);\n+};\n+\n+int LscTableLoader::parseLscData(const YamlObject &yamlSets,\n+\t\t\t\t std::map<unsigned int, LensShadingCorrection::Components> &lscData)\n+{\n+\tconst auto &sets = yamlSets.asList();\n+\n+\tfor (const auto &yamlSet : sets) {\n+\t\tuint32_t ct = yamlSet[\"ct\"].get<uint32_t>(0);\n \n-\t\tif (lscData.empty()) {\n-\t\t\tLOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n+\t\tif (lscData.count(ct)) {\n+\t\t\tLOG(RkISP1Lsc, Error)\n+\t\t\t\t<< \"Multiple sets found for color temperature \"\n+\t\t\t\t<< ct;\n \t\t\treturn -EINVAL;\n \t\t}\n \n-\t\treturn 0;\n-\t}\n+\t\tLensShadingCorrection::Components &set = lscData[ct];\n \n-private:\n-\tstd::vector<uint16_t> parseTable(const YamlObject &tuningData,\n-\t\t\t\t\t const char *prop)\n-\t{\n-\t\tstatic constexpr unsigned int kLscNumSamples =\n-\t\t\tRKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n+\t\tset.r = parseTable(yamlSet, \"r\");\n+\t\tset.gr = parseTable(yamlSet, \"gr\");\n+\t\tset.gb = parseTable(yamlSet, \"gb\");\n+\t\tset.b = parseTable(yamlSet, \"b\");\n \n-\t\tstd::vector<uint16_t> table =\n-\t\t\ttuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});\n-\t\tif (table.size() != kLscNumSamples) {\n+\t\tif (set.r.empty() || set.gr.empty() ||\n+\t\t set.gb.empty() || set.b.empty()) {\n \t\t\tLOG(RkISP1Lsc, Error)\n-\t\t\t\t<< \"Invalid '\" << prop << \"' values: expected \"\n-\t\t\t\t<< kLscNumSamples\n-\t\t\t\t<< \" elements, got \" << table.size();\n-\t\t\treturn {};\n+\t\t\t\t<< \"Set for color temperature \" << ct\n+\t\t\t\t<< \" is missing tables\";\n+\t\t\treturn -EINVAL;\n \t\t}\n+\t}\n \n-\t\treturn table;\n+\tif (lscData.empty()) {\n+\t\tLOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n+\t\treturn -EINVAL;\n \t}\n-};\n+\n+\treturn 0;\n+}\n+\n+std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,\n+\t\t\t\t\t\t const char *prop)\n+{\n+\tstatic constexpr unsigned int kLscNumSamples =\n+\t\tRKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n+\n+\tstd::vector<uint16_t> table =\n+\t\ttuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});\n+\tif (table.size() != kLscNumSamples) {\n+\t\tLOG(RkISP1Lsc, Error)\n+\t\t\t<< \"Invalid '\" << prop << \"' values: expected \"\n+\t\t\t<< kLscNumSamples\n+\t\t\t<< \" elements, got \" << table.size();\n+\t\treturn {};\n+\t}\n+\n+\treturn table;\n+}\n \n static std::vector<double> parseSizes(const YamlObject &tuningData,\n \t\t\t\t const char *prop)\n", "prefixes": [ "v1", "07/12" ] }