[{"id":36446,"web_url":"https://patchwork.libcamera.org/comment/36446/","msgid":"<38ad1e33-cedd-4224-9e07-69f7db293221@ideasonboard.com>","date":"2025-10-24T16:12:04","subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 14. 9:52 keltezéssel, Stefan Klug írta:\n> Move the function definitions out of the related classes. This was noted\n> in review after the series was already merged. After trying it out I\n> must admit that it really improves readability and reduces the\n> indentation level by one.\n> \n> Signed-off-by: Stefan Klug \n> \n> ---\n> \n> Note: This patch contains no functional changes. `git diff\n> --ignore-space-change` helps in verifying that.\n> ---\n\nReviewed-by: Barnabás Pőcze \n\n\n> src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------\n> 1 file changed, 163 insertions(+), 151 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 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 &lscData)\n> -\t{\n> -\t\tconst auto &sets = yamlSets.asList();\n> -\t\tfor (const auto &yamlSet : sets) {\n> -\t\t\tstd::optional pr, pgr, pgb, pb;\n> -\t\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> -\t\t\tpgr = yamlSet[\"gr\"].get();\n> -\t\t\tpgb = yamlSet[\"gb\"].get();\n> -\t\t\tpb = yamlSet[\"b\"].get();\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 &lscData);\n> +\n> +private:\n> +\tstd::vector sizesListToPositions(const std::vector &sizes);\n> +\tstd::vector samplePolynomial(const LscPolynomial &poly);\n> +\n> +\tSize sensorSize_;\n> +\tRectangle cropRectangle_;\n> +\tconst std::vector &xSizes_;\n> +\tconst std::vector &ySizes_;\n> +};\n> +\n> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> +\t\t\t\t std::map &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tstd::optional pr, pgr, pgb, pb;\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> +\t\tpgr = yamlSet[\"gr\"].get();\n> +\t\tpgb = yamlSet[\"gb\"].get();\n> +\t\tpb = yamlSet[\"b\"].get();\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 sizesListToPositions(const std::vector &sizes)\n> -\t{\n> -\t\tconst int half = sizes.size();\n> -\t\tstd::vector 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 LscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> +{\n> +\tconst int half = sizes.size();\n> +\tstd::vector 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 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 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 xPos(sizesListToPositions(xSizes_));\n> -\t\tstd::vector 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(\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 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 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 xPos(sizesListToPositions(xSizes_));\n> +\tstd::vector 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(\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 &xSizes_;\n> -\tconst std::vector &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 &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(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 &lscData);\n> +\n> +private:\n> +\tstd::vector 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 &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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 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 table =\n> -\t\t\ttuningData[prop].getList().value_or(std::vector{});\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 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 table =\n> +\t\ttuningData[prop].getList().value_or(std::vector{});\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 parseSizes(const YamlObject &tuningData,\n> \t\t\t\t const char *prop)","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 7FBFDBE080\n\tfor ;\n\tFri, 24 Oct 2025 16:12:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A4D9609C6;\n\tFri, 24 Oct 2025 18:12:06 +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 935FA608DC\n\tfor ;\n\tFri, 24 Oct 2025 18:12:04 +0200 (CEST)","from [192.168.33.13] (185.221.141.231.nat.pool.zt.hu\n\t[185.221.141.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D77EE1AA6;\n\tFri, 24 Oct 2025 18:10:18 +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=\"d5Z1hzuv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761322219;\n\tbh=0oYfKfp95/miaYR38uVzrx3iD0kNhpPpoDSwl5AjfwA=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=d5Z1hzuvqQbPizBZEsZ1PDfiuMfD8JQwgt80A2GoNzoiIzaRAHcw4RBui7XAfWL93\n\tTd76un4o/2K7O0bE29Nw5OdnJQM1bjoypCSki63n6tdHMpnsP2EEORHcWzg2U6ZUV3\n\tOCG75zVz69ex+7m9x26R78QHOrxpLWCTvzd4yjnA=","Message-ID":"<38ad1e33-cedd-4224-9e07-69f7db293221@ideasonboard.com>","Date":"Fri, 24 Oct 2025 18:12:04 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":36471,"web_url":"https://patchwork.libcamera.org/comment/36471/","msgid":"<1d1de5cc-2b77-4ffe-9535-460a9c0ba64e@ideasonboard.com>","date":"2025-10-26T01:28:50","subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"On 2025-10-14 03:52, Stefan Klug wrote:\n> Move the function definitions out of the related classes. This was noted\n> in review after the series was already merged. After trying it out I\n> must admit that it really improves readability and reduces the\n> indentation level by one.\n>\n> Signed-off-by: Stefan Klug \n>\n> ---\n>\n> Note: 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(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 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 &lscData)\n> -\t{\n> -\t\tconst auto &sets = yamlSets.asList();\n> -\t\tfor (const auto &yamlSet : sets) {\n> -\t\t\tstd::optional pr, pgr, pgb, pb;\n> -\t\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> -\t\t\tpgr = yamlSet[\"gr\"].get();\n> -\t\t\tpgb = yamlSet[\"gb\"].get();\n> -\t\t\tpb = yamlSet[\"b\"].get();\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 &lscData);\n> +\n> +private:\n> +\tstd::vector sizesListToPositions(const std::vector &sizes);\n> +\tstd::vector samplePolynomial(const LscPolynomial &poly);\n> +\n> +\tSize sensorSize_;\n> +\tRectangle cropRectangle_;\n> +\tconst std::vector &xSizes_;\n> +\tconst std::vector &ySizes_;\n> +};\n> +\n> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> +\t\t\t\t std::map &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tstd::optional pr, pgr, pgb, pb;\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> +\t\tpgr = yamlSet[\"gr\"].get();\n> +\t\tpgb = yamlSet[\"gb\"].get();\n> +\t\tpb = yamlSet[\"b\"].get();\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 sizesListToPositions(const std::vector &sizes)\n> -\t{\n> -\t\tconst int half = sizes.size();\n> -\t\tstd::vector 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 LscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> +{\n> +\tconst int half = sizes.size();\n> +\tstd::vector 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 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 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 xPos(sizesListToPositions(xSizes_));\n> -\t\tstd::vector 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(\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 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 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 xPos(sizesListToPositions(xSizes_));\n> +\tstd::vector 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(\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 &xSizes_;\n> -\tconst std::vector &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 &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(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 &lscData);\n> +\n> +private:\n> +\tstd::vector 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 &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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 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 table =\n> -\t\t\ttuningData[prop].getList().value_or(std::vector{});\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 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 table =\n> +\t\ttuningData[prop].getList().value_or(std::vector{});\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 parseSizes(const YamlObject &tuningData,\n> \t\t\t\t const char *prop)\nReviewed by Rui Wang","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 9ED12C3259\n\tfor ;\n\tSun, 26 Oct 2025 01:29:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD4B3606F0;\n\tSun, 26 Oct 2025 02:29:05 +0100 (CET)","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 43DDC606D5\n\tfor ;\n\tSun, 26 Oct 2025 02:29:03 +0100 (CET)","from [192.168.31.114] (unknown [209.216.122.90])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 46CA8664\n\tfor ;\n\tSun, 26 Oct 2025 02:27:16 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ObRAUT7v\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761442036;\n\tbh=W3pf7diOlMlFljuujIKln+eofmAPhaP9eAX68/nNtoc=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ObRAUT7vaRZLpjHQoIqau0iDy/l56T4SJ4vKwNyMWHVkXVEgHxecE4EUTEW5lrFyX\n\tybtSvw1WT5nHJVqyu1wYLWnv8jz4dFoPGwlmRogT78VpcvV26Q8XQkIKcibP7RRmAt\n\tIDw4F+2QXZK+M43FPHmDAfcg0pc17cW5bQN8wof4=","Message-ID":"<1d1de5cc-2b77-4ffe-9535-460a9c0ba64e@ideasonboard.com>","Date":"Sat, 25 Oct 2025 21:28:50 -0400","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","To":"libcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"rui wang ","In-Reply-To":"<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}},{"id":36472,"web_url":"https://patchwork.libcamera.org/comment/36472/","msgid":"<5577e966-5f7d-4d79-a89c-d8165b6cbead@ideasonboard.com>","date":"2025-10-26T02:23:57","subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"On 2025-10-14 03:52, Stefan Klug wrote:\n> Move the function definitions out of the related classes. This was noted\n> in review after the series was already merged. After trying it out I\n> must admit that it really improves readability and reduces the\n> indentation level by one.\n>\n> Signed-off-by: Stefan Klug \n>\n> ---\n>\n> Note: 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(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 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 &lscData)\n> -\t{\n> -\t\tconst auto &sets = yamlSets.asList();\n> -\t\tfor (const auto &yamlSet : sets) {\n> -\t\t\tstd::optional pr, pgr, pgb, pb;\n> -\t\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> -\t\t\tpgr = yamlSet[\"gr\"].get();\n> -\t\t\tpgb = yamlSet[\"gb\"].get();\n> -\t\t\tpb = yamlSet[\"b\"].get();\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 &lscData);\n> +\n> +private:\n> +\tstd::vector sizesListToPositions(const std::vector &sizes);\n> +\tstd::vector samplePolynomial(const LscPolynomial &poly);\n> +\n> +\tSize sensorSize_;\n> +\tRectangle cropRectangle_;\n> +\tconst std::vector &xSizes_;\n> +\tconst std::vector &ySizes_;\n> +};\n> +\n> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> +\t\t\t\t std::map &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tstd::optional pr, pgr, pgb, pb;\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> +\t\tpgr = yamlSet[\"gr\"].get();\n> +\t\tpgb = yamlSet[\"gb\"].get();\n> +\t\tpb = yamlSet[\"b\"].get();\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\n   those 4 need to calculate each channel ,but some of \ninformation are sharing :\n\nlsc table size /center position/ mapping location/ coefs , all those \nwere calculated 4 times.\n\ncan we thinking to provide base structure to save all those and can \nimprove the time complexity\n\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 sizesListToPositions(const std::vector &sizes)\n> -\t{\n> -\t\tconst int half = sizes.size();\n> -\t\tstd::vector 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 LscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> +{\n> +\tconst int half = sizes.size();\n> +\tstd::vector 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 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 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 xPos(sizesListToPositions(xSizes_));\n> -\t\tstd::vector 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(\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 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 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 xPos(sizesListToPositions(xSizes_));\n> +\tstd::vector 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(\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 &xSizes_;\n> -\tconst std::vector &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 &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(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 &lscData);\n> +\n> +private:\n> +\tstd::vector 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 &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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 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 table =\n> -\t\t\ttuningData[prop].getList().value_or(std::vector{});\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 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 table =\n> +\t\ttuningData[prop].getList().value_or(std::vector{});\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 parseSizes(const YamlObject &tuningData,\n> \t\t\t\t const char *prop)","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 5AF86BE080\n\tfor ;\n\tSun, 26 Oct 2025 02:24:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BBE66606DE;\n\tSun, 26 Oct 2025 03:24:11 +0100 (CET)","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 A3DA0606D5\n\tfor ;\n\tSun, 26 Oct 2025 03:24:10 +0100 (CET)","from [192.168.31.114] (unknown [209.216.122.90])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9BB47236\n\tfor ;\n\tSun, 26 Oct 2025 03:22:23 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iag1HAYM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761445343;\n\tbh=qGXpg078pRAO4Meim24OxZ4T5DTcJjo6HGFZq6qB7FE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=iag1HAYM3QnaeuBi1YgsFjYZK9pQkbFrXmwpp1Xmb8kSRjzIdMm5cPj1+4m/NFcNC\n\tVE452tB3Npa0GsCyAr7XLlBirDUG4kwqBJfPx5u/rwrz223ROkRHdgsEMzyBIpvlEf\n\tGpjVDG8p9TvKzTS/1NPM6B+eDmrQwvusTkWOSwUY=","Message-ID":"<5577e966-5f7d-4d79-a89c-d8165b6cbead@ideasonboard.com>","Date":"Sat, 25 Oct 2025 22:23:57 -0400","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","To":"libcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"rui wang ","In-Reply-To":"<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":36503,"web_url":"https://patchwork.libcamera.org/comment/36503/","msgid":"<2ea6e99f-2fda-4278-94e4-e3bee7988007@ideasonboard.com>","date":"2025-10-27T21:29:53","subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","submitter":{"id":241,"url":"https://patchwork.libcamera.org/api/people/241/","name":"Rui Wang","email":"rui.wang@ideasonboard.com"},"content":"On 2025-10-14 03:52, Stefan Klug wrote:\n> Move the function definitions out of the related classes. This was noted\n> in review after the series was already merged. After trying it out I\n> must admit that it really improves readability and reduces the\n> indentation level by one.\n>\n> Signed-off-by: Stefan Klug \n>\n> ---\n>\n> Note: 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(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 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 &lscData)\n> -\t{\n> -\t\tconst auto &sets = yamlSets.asList();\n> -\t\tfor (const auto &yamlSet : sets) {\n> -\t\t\tstd::optional pr, pgr, pgb, pb;\n> -\t\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> -\t\t\tpgr = yamlSet[\"gr\"].get();\n> -\t\t\tpgb = yamlSet[\"gb\"].get();\n> -\t\t\tpb = yamlSet[\"b\"].get();\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 &lscData);\n> +\n> +private:\n> +\tstd::vector sizesListToPositions(const std::vector &sizes);\n> +\tstd::vector samplePolynomial(const LscPolynomial &poly);\n> +\n> +\tSize sensorSize_;\n> +\tRectangle cropRectangle_;\n> +\tconst std::vector &xSizes_;\n> +\tconst std::vector &ySizes_;\n> +};\n> +\n> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> +\t\t\t\t std::map &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tstd::optional pr, pgr, pgb, pb;\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> +\t\tpgr = yamlSet[\"gr\"].get();\n> +\t\tpgb = yamlSet[\"gb\"].get();\n> +\t\tpb = yamlSet[\"b\"].get();\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 sizesListToPositions(const std::vector &sizes)\n> -\t{\n> -\t\tconst int half = sizes.size();\n> -\t\tstd::vector 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 LscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> +{\n> +\tconst int half = sizes.size();\n> +\tstd::vector 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 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 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 xPos(sizesListToPositions(xSizes_));\n> -\t\tstd::vector 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(\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 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 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 xPos(sizesListToPositions(xSizes_));\n> +\tstd::vector 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(\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 &xSizes_;\n> -\tconst std::vector &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 &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(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 &lscData);\n> +\n> +private:\n> +\tstd::vector 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 &lscData)\n> +{\n> +\tconst auto &sets = yamlSets.asList();\n> +\n> +\tfor (const auto &yamlSet : sets) {\n> +\t\tuint32_t ct = yamlSet[\"ct\"].get(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 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 table =\n> -\t\t\ttuningData[prop].getList().value_or(std::vector{});\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 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 table =\n> +\t\ttuningData[prop].getList().value_or(std::vector{});\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 parseSizes(const YamlObject &tuningData,\n> \t\t\t\t const char *prop)\n\n\nReviewed-by: Rui Wang ","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 5D09AC3259\n\tfor ;\n\tMon, 27 Oct 2025 21:30:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 301CF6078C;\n\tMon, 27 Oct 2025 22:30:09 +0100 (CET)","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 589306069A\n\tfor ;\n\tMon, 27 Oct 2025 22:30:07 +0100 (CET)","from [192.168.31.114] (unknown [209.216.122.90])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EF9AEE70\n\tfor ;\n\tMon, 27 Oct 2025 22:28:18 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZgrzQIz9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761600499;\n\tbh=UmFZ2AmTW/9h/XBkNzCLsz6IoivG775aqBdur3//Z9M=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ZgrzQIz9hmd1kE2dEXJvUgEXW6nHZdsevbueecvnhq9YADfK7myjDwSnNDg7sszJe\n\twPTYfvSTE3C/QPpKrA6tICi0A+VsmkT1HRVb02uPYXVv2FGEmDXCVEDAyoPSt7OtLy\n\t7A2DLOGIi/3esBDiqg6WnPNJ6wTOhYTBYtM5HH/I=","Message-ID":"<2ea6e99f-2fda-4278-94e4-e3bee7988007@ideasonboard.com>","Date":"Mon, 27 Oct 2025 17:29:53 -0400","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","To":"libcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"rui wang ","In-Reply-To":"<20251014075252.2876485-8-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}},{"id":36508,"web_url":"https://patchwork.libcamera.org/comment/36508/","msgid":"<20251028093756.GB13023@pendragon.ideasonboard.com>","date":"2025-10-28T09:37:56","subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Rui,\n\nOn Sat, Oct 25, 2025 at 10:23:57PM -0400, Rui Wang wrote:\n> On 2025-10-14 03:52, Stefan Klug wrote:\n> > Move the function definitions out of the related classes. This was noted\n> > in review after the series was already merged. After trying it out I\n> > must admit that it really improves readability and reduces the\n> > indentation level by one.\n> >\n> > Signed-off-by: Stefan Klug \n> >\n> > ---\n> >\n> > Note: 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(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index 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 &lscData)\n> > -\t{\n> > -\t\tconst auto &sets = yamlSets.asList();\n> > -\t\tfor (const auto &yamlSet : sets) {\n> > -\t\t\tstd::optional pr, pgr, pgb, pb;\n> > -\t\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> > -\t\t\tpgr = yamlSet[\"gr\"].get();\n> > -\t\t\tpgb = yamlSet[\"gb\"].get();\n> > -\t\t\tpb = yamlSet[\"b\"].get();\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 &lscData);\n> > +\n> > +private:\n> > +\tstd::vector sizesListToPositions(const std::vector &sizes);\n> > +\tstd::vector samplePolynomial(const LscPolynomial &poly);\n> > +\n> > +\tSize sensorSize_;\n> > +\tRectangle cropRectangle_;\n> > +\tconst std::vector &xSizes_;\n> > +\tconst std::vector &ySizes_;\n> > +};\n> > +\n> > +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > +\t\t\t\t std::map &lscData)\n> > +{\n> > +\tconst auto &sets = yamlSets.asList();\n> > +\tfor (const auto &yamlSet : sets) {\n> > +\t\tstd::optional pr, pgr, pgb, pb;\n> > +\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> > +\t\tpgr = yamlSet[\"gr\"].get();\n> > +\t\tpgb = yamlSet[\"gb\"].get();\n> > +\t\tpb = yamlSet[\"b\"].get();\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> \n>   those 4 need to calculate each channel ,but some of \n> information are sharing :\n> \n> lsc table size /center position/ mapping location/ coefs , all those \n> were calculated 4 times.\n> \n> can we thinking to provide base structure to save all those and can \n> improve the time complexity\n\nThat sounds like a good improvement to explore. It's not an issue\nintroduced by this patch though, which only moves code around. I think\nwe can merge this, and then optimize on top.\n\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 sizesListToPositions(const std::vector &sizes)\n> > -\t{\n> > -\t\tconst int half = sizes.size();\n> > -\t\tstd::vector 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 LscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> > +{\n> > +\tconst int half = sizes.size();\n> > +\tstd::vector 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 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 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 xPos(sizesListToPositions(xSizes_));\n> > -\t\tstd::vector 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(\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 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 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 xPos(sizesListToPositions(xSizes_));\n> > +\tstd::vector 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(\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 &xSizes_;\n> > -\tconst std::vector &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 &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(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 &lscData);\n> > +\n> > +private:\n> > +\tstd::vector 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 &lscData)\n> > +{\n> > +\tconst auto &sets = yamlSets.asList();\n> > +\n> > +\tfor (const auto &yamlSet : sets) {\n> > +\t\tuint32_t ct = yamlSet[\"ct\"].get(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 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 table =\n> > -\t\t\ttuningData[prop].getList().value_or(std::vector{});\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 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 table =\n> > +\t\ttuningData[prop].getList().value_or(std::vector{});\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 parseSizes(const YamlObject &tuningData,\n> > \t\t\t\t const char *prop)","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 C2D00C3259\n\tfor ;\n\tTue, 28 Oct 2025 09:38:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70529607A0;\n\tTue, 28 Oct 2025 10:38:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0AE9603ED\n\tfor ;\n\tTue, 28 Oct 2025 10:38:09 +0100 (CET)","from pendragon.ideasonboard.com (unknown [193.209.96.36])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 639DF176B; \n\tTue, 28 Oct 2025 10:36:21 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"i/vM1pNh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761644181;\n\tbh=taRPqoWEz1fIpvrATaiEhzLW1Uvu/nGSBopbKPlkOR8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i/vM1pNhGvg7YwklXzxFWUwiJys8kTKTiUGoVRwIR6Fyfy8qQTWEAZ2NGkRVyI7L4\n\tIxpin6W0jnGoJMzJZi+4FAXDeFVoXMaBZ4GK4xDmg/U+WXUQhv1zjMDy6lwvXNWVdd\n\tY/0+EJoNnVYZ4ZY+SRwAp5rl5h9c8ODLDtckO/jA=","Date":"Tue, 28 Oct 2025 11:37:56 +0200","From":"Laurent Pinchart ","To":"rui wang ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","Message-ID":"<20251028093756.GB13023@pendragon.ideasonboard.com>","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-8-stefan.klug@ideasonboard.com>\n\t<5577e966-5f7d-4d79-a89c-d8165b6cbead@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<5577e966-5f7d-4d79-a89c-d8165b6cbead@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\" "}},{"id":36509,"web_url":"https://patchwork.libcamera.org/comment/36509/","msgid":"<20251028094452.GC13023@pendragon.ideasonboard.com>","date":"2025-10-28T09:44:52","subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Oct 28, 2025 at 11:37:56AM +0200, Laurent Pinchart wrote:\n> On Sat, Oct 25, 2025 at 10:23:57PM -0400, Rui Wang wrote:\n> > On 2025-10-14 03:52, Stefan Klug wrote:\n> > > Move the function definitions out of the related classes. This was noted\n> > > in review after the series was already merged. After trying it out I\n> > > must admit that it really improves readability and reduces the\n> > > indentation level by one.\n> > >\n> > > Signed-off-by: Stefan Klug \n> > >\n> > > ---\n> > >\n> > > Note: 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(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > index 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 &lscData)\n> > > -\t{\n> > > -\t\tconst auto &sets = yamlSets.asList();\n> > > -\t\tfor (const auto &yamlSet : sets) {\n> > > -\t\t\tstd::optional pr, pgr, pgb, pb;\n> > > -\t\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> > > -\t\t\tpgr = yamlSet[\"gr\"].get();\n> > > -\t\t\tpgb = yamlSet[\"gb\"].get();\n> > > -\t\t\tpb = yamlSet[\"b\"].get();\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 &lscData);\n> > > +\n> > > +private:\n> > > +\tstd::vector sizesListToPositions(const std::vector &sizes);\n> > > +\tstd::vector samplePolynomial(const LscPolynomial &poly);\n> > > +\n> > > +\tSize sensorSize_;\n> > > +\tRectangle cropRectangle_;\n> > > +\tconst std::vector &xSizes_;\n> > > +\tconst std::vector &ySizes_;\n> > > +};\n> > > +\n> > > +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > > +\t\t\t\t std::map &lscData)\n> > > +{\n> > > +\tconst auto &sets = yamlSets.asList();\n> > > +\tfor (const auto &yamlSet : sets) {\n> > > +\t\tstd::optional pr, pgr, pgb, pb;\n> > > +\t\tuint32_t ct = yamlSet[\"ct\"].get(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();\n> > > +\t\tpgr = yamlSet[\"gr\"].get();\n> > > +\t\tpgb = yamlSet[\"gb\"].get();\n> > > +\t\tpb = yamlSet[\"b\"].get();\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> > \n> >   those 4 need to calculate each channel ,but some of \n> > information are sharing :\n> > \n> > lsc table size /center position/ mapping location/ coefs , all those \n> > were calculated 4 times.\n> > \n> > can we thinking to provide base structure to save all those and can \n> > improve the time complexity\n> \n> That sounds like a good improvement to explore. It's not an issue\n> introduced by this patch though, which only moves code around. I think\n> we can merge this, and then optimize on top.\n\nWhich the rest of the series does :-)\n\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 sizesListToPositions(const std::vector &sizes)\n> > > -\t{\n> > > -\t\tconst int half = sizes.size();\n> > > -\t\tstd::vector 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 LscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> > > +{\n> > > +\tconst int half = sizes.size();\n> > > +\tstd::vector 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 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 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 xPos(sizesListToPositions(xSizes_));\n> > > -\t\tstd::vector 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(\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 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 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 xPos(sizesListToPositions(xSizes_));\n> > > +\tstd::vector 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(\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 &xSizes_;\n> > > -\tconst std::vector &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 &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(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 &lscData);\n> > > +\n> > > +private:\n> > > +\tstd::vector 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 &lscData)\n> > > +{\n> > > +\tconst auto &sets = yamlSets.asList();\n> > > +\n> > > +\tfor (const auto &yamlSet : sets) {\n> > > +\t\tuint32_t ct = yamlSet[\"ct\"].get(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 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 table =\n> > > -\t\t\ttuningData[prop].getList().value_or(std::vector{});\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 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 table =\n> > > +\t\ttuningData[prop].getList().value_or(std::vector{});\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 parseSizes(const YamlObject &tuningData,\n> > > \t\t\t\t const char *prop)","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 A2DBEC3259\n\tfor ;\n\tTue, 28 Oct 2025 09:45:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DD8F46079D;\n\tTue, 28 Oct 2025 10:45:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45019603ED\n\tfor ;\n\tTue, 28 Oct 2025 10:45:06 +0100 (CET)","from pendragon.ideasonboard.com (unknown [193.209.96.36])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 82D1F176B; \n\tTue, 28 Oct 2025 10:43:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qElAHaIZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761644597;\n\tbh=2fT7SaQ9yAKSpbBXjoh334gMSYhReLOk8OaMBio3Zb0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qElAHaIZJb1LoqCkQv24Wu7wB1Y1fo3aBRyalD+VqQH8kN93m7ouXP6BtsvHrBtzu\n\tav3wsSTiEAi8ns+W64zZFs8bJz+cCWbOCpPlKjvMV/Fiz3twOSSretONMe1usI3AQc\n\t2DhD7gu7ANnw5S2YUlFD8Q3J/aKoRFkB6CKBv3o8=","Date":"Tue, 28 Oct 2025 11:44:52 +0200","From":"Laurent Pinchart ","To":"rui wang ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 07/12] ipa: rksip1: lsc: Move function definitions out\n\tof class","Message-ID":"<20251028094452.GC13023@pendragon.ideasonboard.com>","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-8-stefan.klug@ideasonboard.com>\n\t<5577e966-5f7d-4d79-a89c-d8165b6cbead@ideasonboard.com>\n\t<20251028093756.GB13023@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20251028093756.GB13023@pendragon.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\" "}}]