[{"id":36240,"web_url":"https://patchwork.libcamera.org/comment/36240/","msgid":"","date":"2025-10-14T08:59:00","subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 10. 14. 9:52 keltezéssel, Stefan Klug írta:\n> The lens shading correction is always applied based on the sensor crop\n> bounds. This leads to incorrect lens shading correction for analog crops\n> that do not cover the whole sensor.\n> \n> To fix that, we need to adapt the lens shading table for the selected\n> analog crop at configure time. Introduce an abstract ShadingDescriptor\n> class that holds the lens shading information that can then be sampled\n> at configure time for a specific crop rectangle.\n> \n> Resampling for a specific crop is only implemented for polynomial lsc\n> data. For tabular data, a warning is logged and the unmodified table is\n> returned. This matches the current functionality for tabular data and is\n> a huge improvement for polynomial data.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 230 +++++++++++++++++++-----------\n> src/ipa/rkisp1/algorithms/lsc.h | 14 ++\n> 2 files changed, 157 insertions(+), 87 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 29cd7efd83ef..68cf35064182 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -70,38 +70,124 @@ namespace ipa::rkisp1::algorithms {\n> \n> LOG_DEFINE_CATEGORY(RkISP1Lsc)\n> \n> -class LscPolynomialLoader\n> +class LscPolynomialShadingDescriptor : public LensShadingCorrection::ShadingDescriptor\n\nIf you're already here, could you put all \"private\" Lsc* types into an anon namespace?\nI feel we should strive to have everything translation-unit local in there.\n\n\n> {\n> public:\n> -\tLscPolynomialLoader(const Size &sensorSize,\n> -\t\t\t const Rectangle &cropRectangle,\n> -\t\t\t const std::vector &xSizes,\n> -\t\t\t const std::vector &ySizes)\n> -\t\t: sensorSize_(sensorSize),\n> -\t\t cropRectangle_(cropRectangle),\n> -\t\t xSizes_(xSizes),\n> -\t\t ySizes_(ySizes)\n> +\tLscPolynomialShadingDescriptor(const LscPolynomial &pr, const LscPolynomial &pgr,\n> +\t\t\t\t const LscPolynomial &pgb, const LscPolynomial &pb)\n> +\t\t: pr_(pr), pgr_(pgr), pgb_(pgb), pb_(pb)\n> \t{\n> \t}\n> \n> -\tint parseLscData(const YamlObject &yamlSets,\n> -\t\t\t std::map &lscData);\n> +\tvoid sampleForCrop(const Rectangle &cropRectangle,\n> +\t\t\t const std::vector &xSizes,\n> +\t\t\t const std::vector &ySizes,\n> +\t\t\t LensShadingCorrection::Components &components) override;\n> \n> private:\n> -\tstd::vector sizesListToPositions(const std::vector &sizes);\n> \tstd::vector samplePolynomial(const LscPolynomial &poly,\n> \t\t\t\t\t const std::vector &xPositions,\n> \t\t\t\t\t const std::vector &yPositions,\n> \t\t\t\t\t const Rectangle &cropRectangle);\n> \n> +\tstd::vector sizesListToPositions(const std::vector &sizes);\n> +\n> +\tLscPolynomial pr_;\n> +\tLscPolynomial pgr_;\n> +\tLscPolynomial pgb_;\n> +\tLscPolynomial pb_;\n> +};\n> [...]\n> @@ -393,6 +441,14 @@ int LensShadingCorrection::configure(IPAContext &context,\n> \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n> \t}\n> \n> +\tLOG(RkISP1Lsc, Debug) << \"Sample LSC data for \" << configInfo.analogCrop;\n> +\tstd::map shadingData;\n> +\tfor (auto const &[t, descriptor] : shadingDescriptors_)\n\nauto const or const auto is the preferred?\n\n\n> +\t\tdescriptor->sampleForCrop(configInfo.analogCrop, xSize_, ySize_,\n> +\t\t\t\t\t shadingData[t]);\n> +\n> +\tsets_.setData(std::move(shadingData));\n> +\n> \tcontext.configuration.lsc.enabled = true;\n> \treturn 0;\n> }\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> index 7b68dda1a0d4..43fee337931f 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.h\n> +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> \n> #include \"libipa/interpolator.h\"\n> \n> @@ -36,10 +37,23 @@ public:\n> \t\tstd::vector b;\n> \t};\n> \n> +\tclass ShadingDescriptor\n> +\t{\n> +\tpublic:\n> +\t\tvirtual ~ShadingDescriptor() = default;\n> +\t\tvirtual void sampleForCrop(const Rectangle &cropRectangle,\n> +\t\t\t\t\t const std::vector &xSizes,\n> +\t\t\t\t\t const std::vector &ySizes,\n> +\t\t\t\t\t Components &components) = 0;\n> +\t};\n> +\n> +\tusing ShadingDescriptorMap = std::map>;\n\n\nSince the set of possibilities appears quite closed to me. I was wondering if maybe `std::variant`\nis something worth trying.\n\nThat is,\n\n std::map> shadingDescriptors_;\n\nor even\n\n std::variant<\n std::map,\n std::map\n > shadingDescriptors_;\n\nand then in the first case\n\n for (auto const &[t, descriptor] : shadingDescriptors_) {\n std::visit([&](auto &x) {\n x.sampleForCrop(...);\n }, descriptor);\n }\n\nand in the second case\n\n std::visit([&](auto &shadingDescriptors) {\n for (const auto &[t, descriptor] : shadingDescriptors) {\n descriptor.sampleForCrop(...);\n }\n }, shadingDescriptors_);\n\n\nThoughts?\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +\n> private:\n> \tvoid setParameters(rkisp1_cif_isp_lsc_config &config);\n> \tvoid copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);\n> \n> +\tstd::map> shadingDescriptors_;\n> \tipa::Interpolator sets_;\n> \tstd::vector xSize_;\n> \tstd::vector ySize_;","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 21719BE080\n\tfor ;\n\tTue, 14 Oct 2025 08:59:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 65A85605D7;\n\tTue, 14 Oct 2025 10:59:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EE4D360580\n\tfor ;\n\tTue, 14 Oct 2025 10:59:03 +0200 (CEST)","from [192.168.33.20] (185.182.214.105.nat.pool.zt.hu\n\t[185.182.214.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 804F4C73;\n\tTue, 14 Oct 2025 10:57:25 +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=\"VLoVSUda\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760432245;\n\tbh=qzFVksUtQwJiNK87v6nFKugRbMU6IwDcIEfNOYW5vUc=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=VLoVSUda7Bo1h+fAZlmyiOBSy+bAnWb9P5a0cnrIU3VVaCo2fS7X0c0I8iPgVGdV7\n\tHtq+D7E9XWKAcgUWC+dChr2VGCFdeE/uw/1jOqvMmq3CVVXFsZi7BBSNW/9hi/kQWK\n\thPZymflUMoevZyy7Zy7zlAiQxmkun8M8Kavl+X4M=","Message-ID":"","Date":"Tue, 14 Oct 2025 10:59:00 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-11-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-11-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":36448,"web_url":"https://patchwork.libcamera.org/comment/36448/","msgid":"<1731c430-f3f0-41ed-9c61-4349d0037434@ideasonboard.com>","date":"2025-10-24T16:20:30","subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","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> The lens shading correction is always applied based on the sensor crop\n> bounds. This leads to incorrect lens shading correction for analog crops\n> that do not cover the whole sensor.\n>\n> To fix that, we need to adapt the lens shading table for the selected\n> analog crop at configure time. Introduce an abstract ShadingDescriptor\n> class that holds the lens shading information that can then be sampled\n> at configure time for a specific crop rectangle.\n>\n> Resampling for a specific crop is only implemented for polynomial lsc\n> data. For tabular data, a warning is logged and the unmodified table is\n> returned. This matches the current functionality for tabular data and is\n> a huge improvement for polynomial data.\n>\n> Signed-off-by: Stefan Klug\n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 230 +++++++++++++++++++-----------\n> src/ipa/rkisp1/algorithms/lsc.h | 14 ++\n> 2 files changed, 157 insertions(+), 87 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 29cd7efd83ef..68cf35064182 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -70,38 +70,124 @@ namespaceipa::rkisp1::algorithms {\n> \n> LOG_DEFINE_CATEGORY(RkISP1Lsc)\n> \n> -class LscPolynomialLoader\n> +class LscPolynomialShadingDescriptor : publicLensShadingCorrection::ShadingDescriptor\n> {\n> public:\n> -\tLscPolynomialLoader(const Size &sensorSize,\n> -\t\t\t const Rectangle &cropRectangle,\n> -\t\t\t conststd::vector &xSizes,\n> -\t\t\t conststd::vector &ySizes)\n> -\t\t: sensorSize_(sensorSize),\n> -\t\t cropRectangle_(cropRectangle),\n> -\t\t xSizes_(xSizes),\n> -\t\t ySizes_(ySizes)\n> +\tLscPolynomialShadingDescriptor(const LscPolynomial &pr, const LscPolynomial &pgr,\n> +\t\t\t\t const LscPolynomial &pgb, const LscPolynomial &pb)\n> +\t\t: pr_(pr), pgr_(pgr), pgb_(pgb), pb_(pb)\n> \t{\n> \t}\n> \n> -\tint parseLscData(const YamlObject &yamlSets,\n> -\t\t\tstd::map &lscData);\n> +\tvoid sampleForCrop(const Rectangle &cropRectangle,\n> +\t\t\t conststd::vector &xSizes,\n> +\t\t\t conststd::vector &ySizes,\n> +\t\t\tLensShadingCorrection::Components &components) override;\n> \n> private:\n> -\tstd::vector sizesListToPositions(conststd::vector &sizes);\n> \tstd::vector samplePolynomial(const LscPolynomial &poly,\n> \t\t\t\t\t conststd::vector &xPositions,\n> \t\t\t\t\t conststd::vector &yPositions,\n> \t\t\t\t\t const Rectangle &cropRectangle);\n> \n> +\tstd::vector sizesListToPositions(conststd::vector &sizes);\n> +\n> +\tLscPolynomial pr_;\n> +\tLscPolynomial pgr_;\n> +\tLscPolynomial pgb_;\n> +\tLscPolynomial pb_;\n> +};\n     if the class declaration can be move into header file , the \nrelation of descriptor will looks more clear.\n> +\n> +voidLscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,\n> +\t\t\t\t\t\t conststd::vector &xSizes,\n> +\t\t\t\t\t\t conststd::vector &ySizes,\n> +\t\t\t\t\t\tLensShadingCorrection::Components &components)\n> +{\n> +\tstd::vector xPos = sizesListToPositions(xSizes);\n> +\tstd::vector yPos = sizesListToPositions(ySizes);\n> +\tcomponents.r = samplePolynomial(pr_, xPos, yPos, cropRectangle);\n> +\tcomponents.gr = samplePolynomial(pgr_, xPos, yPos, cropRectangle);\n> +\tcomponents.gb = samplePolynomial(pgb_, xPos, yPos, cropRectangle);\n> +\tcomponents.b = samplePolynomial(pb_, xPos, yPos, cropRectangle);\n> +}\n\n      Move components as return value looks more efficient, the invoker \ndoes not need to create a local object to keep the content.\n\n     and don't need to consider about rvalue move and memory leak or \nmemory release\n\nLensShadingCorrection::Components LscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,\n+\t\t\t\t\t\t conststd::vector &xSizes,\n+\t\t\t\t\t\t conststd::vector &ySizes)\n\n> +\n> +std::vector\n> +LscPolynomialShadingDescriptor::samplePolynomial(const LscPolynomial &poly,\n> +\t\t\t\t\t\t conststd::vector &xPositions,\n> +\t\t\t\t\t\t conststd::vector &yPositions,\n> +\t\t\t\t\t\t const Rectangle &cropRectangle)\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> +\tsamples.reserve(xPositions.size() * yPositions.size());\n> +\n> +\tfor (double y : yPositions) {\n> +\t\tfor (double x : xPositions) {\n> +\t\t\tdouble xp = x0 + x * w;\n> +\t\t\tdouble yp = y0 + 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\n                                              int_32 to replace int , \nand std::clamp() can show more clear meaning.\n\nint v = \nstd::clamp(static_cast(poly.sampleAtNormalizedPixelPos(xp, yp) * \n1024), 1024, 4095)\n\n> +\t\t\tsamples.push_back(v);\n> +\t\t}\n> +\t}\n> +\treturn samples;\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\n> +LscPolynomialShadingDescriptor::sizesListToPositions(conststd::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> +\treturn positions;\n> +}\n> +\n> +class LscPolynomialLoader\n> +{\n> +public:\n> +\tLscPolynomialLoader(const Size &sensorSize) : sensorSize_(sensorSize)\n> +\t{\n> +\t}\n> +\n> +\tint parseLscData(const YamlObject &yamlSets,\n> +\t\t\tLensShadingCorrection::ShadingDescriptorMap &lscData);\n> +\n> +private:\n> \tSize sensorSize_;\n> -\tRectangle cropRectangle_;\n> -\tconststd::vector &xSizes_;\n> -\tconststd::vector &ySizes_;\n> };\n> \n> intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> -\t\t\t\tstd::map &lscData)\n> +\t\t\t\tLensShadingCorrection::ShadingDescriptorMap &lscData)\n> {\n> \tconst auto &sets = yamlSets.asList();\n> \tfor (const auto &yamlSet : sets) {\n> @@ -115,7 +201,6 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> \t\t\treturn -EINVAL;\n> \t\t}\n> \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> @@ -133,12 +218,9 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> \t\tpgb->setReferenceImageSize(sensorSize_);\n> \t\tpb->setReferenceImageSize(sensorSize_);\n> \n> -\t\tstd::vector xPos(sizesListToPositions(xSizes_));\n> -\t\tstd::vector yPos(sizesListToPositions(ySizes_));\n> -\t\tset.r = samplePolynomial(*pr, xPos, yPos, cropRectangle_);\n> -\t\tset.gr = samplePolynomial(*pgr, xPos, yPos, cropRectangle_);\n> -\t\tset.gb = samplePolynomial(*pgb, xPos, yPos, cropRectangle_);\n> -\t\tset.b = samplePolynomial(*pb, xPos, yPos, cropRectangle_);\n> +\t\tlscData.emplace(\n> +\t\t\tct,std::make_unique(\n> +\t\t\t\t *pr, *pgr, *pgb, *pb));\n> \t}\n> \n> \tif (lscData.empty()) {\n> @@ -149,70 +231,33 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\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::vectorLscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> +class LscTableShadingDescriptor : publicLensShadingCorrection::ShadingDescriptor\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> +public:\n> +\tLscTableShadingDescriptor(LensShadingCorrection::Components components)\n> +\t\t: lscData_(std::move(components))\n> +\t{\n> \t}\n> \n> -\treturn positions;\n> -}\n> -\n> -std::vectorLscPolynomialLoader::samplePolynomial(const LscPolynomial &poly,\n> -\t\t\t\t\t\t\t conststd::vector &xPositions,\n> -\t\t\t\t\t\t\t conststd::vector &yPositions,\n> -\t\t\t\t\t\t\t const Rectangle &cropRectangle)\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> -\tsamples.reserve(xPositions.size() * yPositions.size());\n> -\n> -\tfor (double y : yPositions) {\n> -\t\tfor (double x : xPositions) {\n> -\t\t\tdouble xp = x0 + x * w;\n> -\t\t\tdouble yp = y0 + 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> +\tvoid sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,\n> +\t\t\t [[maybe_unused]] conststd::vector &xSizes,\n> +\t\t\t [[maybe_unused]] conststd::vector &ySizes,\n> +\t\t\tLensShadingCorrection::Components &components)\n> +\t{\n> +\t\tLOG(RkISP1Lsc, Warning)\n> +\t\t\t<< \"Tabular LSC data doesn't support resampling.\";\n> +\t\tcomponents = lscData_;\n> \t}\n> -\treturn samples;\n> -}\n> +\n> +private:\n> +\tLensShadingCorrection::Components lscData_;\n> +};\n> \n> class LscTableLoader\n> {\n> public:\n> \tint parseLscData(const YamlObject &yamlSets,\n> -\t\t\tstd::map &lscData);\n> +\t\t\tLensShadingCorrection::ShadingDescriptorMap &lscData);\n> \n> private:\n> \tstd::vector parseTable(const YamlObject &tuningData,\n> @@ -220,7 +265,7 @@ private:\n> };\n> \n> intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> -\t\t\t\tstd::map &lscData)\n> +\t\t\t\tLensShadingCorrection::ShadingDescriptorMap &lscData)\n> {\n> \tconst auto &sets = yamlSets.asList();\n> \n> @@ -234,8 +279,7 @@ intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> \t\t\treturn -EINVAL;\n> \t\t}\n> \n> -\t\tLensShadingCorrection::Components &set = lscData[ct];\n> -\n> +\t\tLensShadingCorrection::Components set;\n> \t\tset.r = parseTable(yamlSet, \"r\");\n> \t\tset.gr = parseTable(yamlSet, \"gr\");\n> \t\tset.gb = parseTable(yamlSet, \"gb\");\n> @@ -248,6 +292,9 @@ intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> \t\t\t\t<< \" is missing tables\";\n> \t\t\treturn -EINVAL;\n> \t\t}\n> +\n> +\t\tlscData.emplace(\n> +\t\t\tct,std::make_unique(std::move(set)));\n> \t}\n> \n> \tif (lscData.empty()) {\n> @@ -334,7 +381,7 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \t}\n> \n> \tint ret = 0;\n> -\tstd::map lscData;\n> +\tstd::map> lscData;\n                already declare shadingDescriptors_ in header file\n> \n> \tstd::string type = tuningData[\"type\"].get(\"table\");\n> \tif (type == \"table\") {\n> @@ -343,10 +390,11 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \t\tret = loader.parseLscData(yamlSets, lscData);\n> \t} else if (type == \"polynomial\") {\n> \t\tLOG(RkISP1Lsc, Debug) << \"Loading polynomial LSC data.\";\n> -\t\tauto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,\n> -\t\t\t\t\t\t context.sensorInfo.analogCrop,\n> -\t\t\t\t\t\t xSize_,\n> -\t\t\t\t\t\t ySize_);\n> +\t\t/*\n> +\t\t * \\todo: Most likely the reference frame should be native_size.\n> +\t\t * Let's wait how the internal discussions progress.\n> +\t\t */\n> +\t\tauto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize);\n> \t\tret = loader.parseLscData(yamlSets, lscData);\n> \t} else {\n> \t\tLOG(RkISP1Lsc, Error) << \"Unsupported LSC data type '\"\n> @@ -357,7 +405,7 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tsets_.setData(std::move(lscData));\n> +\tshadingDescriptors_.swap(lscData);\n> \n> \treturn 0;\n> }\n> @@ -393,6 +441,14 @@ intLensShadingCorrection::configure(IPAContext &context,\n> \t\tyGrad_[i] =std::round(32768 / ySizes_[i]);\n> \t}\n> \n> +\tLOG(RkISP1Lsc, Debug) << \"Sample LSC data for \" << configInfo.analogCrop;\n> +\tstd::map shadingData;\n> +\tfor (auto const &[t, descriptor] : shadingDescriptors_)\n> +\t\tdescriptor->sampleForCrop(configInfo.analogCrop, xSize_, ySize_,\n> +\t\t\t\t\t shadingData[t]);\n> +\n> +\tsets_.setData(std::move(shadingData));\n> +\n> \tcontext.configuration.lsc.enabled = true;\n> \treturn 0;\n> }\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> index 7b68dda1a0d4..43fee337931f 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.h\n> +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> \n> #include \"libipa/interpolator.h\"\n> \n> @@ -36,10 +37,23 @@ public:\n> \t\tstd::vector b;\n> \t};\n> \n> +\tclass ShadingDescriptor\n> +\t{\n> +\tpublic:\n> +\t\tvirtual ~ShadingDescriptor() = default;\n> +\t\tvirtual void sampleForCrop(const Rectangle &cropRectangle,\n> +\t\t\t\t\t conststd::vector &xSizes,\n> +\t\t\t\t\t conststd::vector &ySizes,\n> +\t\t\t\t\t Components &components) = 0;\n> +\t};\n\n\tsampleForCrop is defined as interface, it looks just remove =0 as abstract method looks more convinient , and by default\n\t\n\n> \tvirtual Components sampleForCrop(const Rectangle &cropRectangle,\n> +\t\t\t\t\t conststd::vector &xSizes,\n> +\t\t\t\t\t conststd::vector &ySizes) {\n                     return _lsc***;\n> }\n> +\n> +\tusing ShadingDescriptorMap =std::map>;\nunsigned int, can be replaced by uint32_t with include \n> +\n> private:\n> \tvoid setParameters(rkisp1_cif_isp_lsc_config &config);\n> \tvoid copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);\n> \n> +\tstd::map> shadingDescriptors_;\n\n              declare using ShadingDescriptorMap above , either remove \nShadingDescriptorMap or\n\nShadingDescriptorMap shadingDescriptors_;\n\n> \tipa::Interpolator sets_;\n> \tstd::vector xSize_;\n> \tstd::vector ySize_;","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 3DD2DBE080\n\tfor ;\n\tFri, 24 Oct 2025 16:20:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 783EE6099E;\n\tFri, 24 Oct 2025 18:20:44 +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 11832608DC\n\tfor ;\n\tFri, 24 Oct 2025 18:20:43 +0200 (CEST)","from [192.168.31.114] (unknown [209.216.122.90])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C4A61183\n\tfor ;\n\tFri, 24 Oct 2025 18:18:56 +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=\"kFQmvVlO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761322737;\n\tbh=quAWTTDL3C9CyXFvo6PSsxMbcw7WCAhAQGY3HNq4rHk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=kFQmvVlOZWHfEV0QP9rGa1yjh3K5V/U10BTBitn3s3S8pZ1zKlUdHRehuPkOJoCfA\n\tkeqhYp9TUPaGnZDX81qmPpoOVUcvsHZ3eMyhm+iusNhJZySpFQr6A7l8jSapD8BLgZ\n\th1BwKenyOGnKwtfOr509JtwB5dvJtN5xZo3jzmYI=","Content-Type":"multipart/alternative;\n\tboundary=\"------------Z2oZApzA951ZVXXJB6JMAaCY\"","Message-ID":"<1731c430-f3f0-41ed-9c61-4349d0037434@ideasonboard.com>","Date":"Fri, 24 Oct 2025 12:20:30 -0400","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","To":"libcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-11-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"rui wang ","In-Reply-To":"<20251014075252.2876485-11-stefan.klug@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":36450,"web_url":"https://patchwork.libcamera.org/comment/36450/","msgid":"<7c7d498e-3dc1-405a-8966-81bfc90b758e@ideasonboard.com>","date":"2025-10-24T16:44:16","subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","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> The lens shading correction is always applied based on the sensor crop\n> bounds. This leads to incorrect lens shading correction for analog crops\n> that do not cover the whole sensor.\n> \n> To fix that, we need to adapt the lens shading table for the selected\n> analog crop at configure time. Introduce an abstract ShadingDescriptor\n> class that holds the lens shading information that can then be sampled\n> at configure time for a specific crop rectangle.\n> \n> Resampling for a specific crop is only implemented for polynomial lsc\n> data. For tabular data, a warning is logged and the unmodified table is\n> returned. This matches the current functionality for tabular data and is\n> a huge improvement for polynomial data.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 230 +++++++++++++++++++-----------\n> src/ipa/rkisp1/algorithms/lsc.h | 14 ++\n> 2 files changed, 157 insertions(+), 87 deletions(-)\n> \n> [...]\n> +std::vector\n> +LscPolynomialShadingDescriptor::samplePolynomial(const LscPolynomial &poly,\n> +\t\t\t\t\t\t const std::vector &xPositions,\n> +\t\t\t\t\t\t const std::vector &yPositions,\n> +\t\t\t\t\t\t const Rectangle &cropRectangle)\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> +\tsamples.reserve(xPositions.size() * yPositions.size());\n> +\n> +\tfor (double y : yPositions) {\n> +\t\tfor (double x : xPositions) {\n> +\t\t\tdouble xp = x0 + x * w;\n> +\t\t\tdouble yp = y0 + 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\nIsn't this just `std::clamp()`?\n\n\n> +\t\t\tsamples.push_back(v);\n> +\t\t}\n> +\t}\n> +\treturn samples;\n> +}\n> [...]\n> int LscTableLoader::parseLscData(const YamlObject &yamlSets,\n> -\t\t\t\t std::map &lscData)\n> +\t\t\t\t LensShadingCorrection::ShadingDescriptorMap &lscData)\n> {\n> \tconst auto &sets = yamlSets.asList();\n> \n> @@ -234,8 +279,7 @@ int LscTableLoader::parseLscData(const YamlObject &yamlSets,\n> \t\t\treturn -EINVAL;\n> \t\t}\n> \n> -\t\tLensShadingCorrection::Components &set = lscData[ct];\n> -\n> +\t\tLensShadingCorrection::Components set;\n> \t\tset.r = parseTable(yamlSet, \"r\");\n> \t\tset.gr = parseTable(yamlSet, \"gr\");\n> \t\tset.gb = parseTable(yamlSet, \"gb\");\n> @@ -248,6 +292,9 @@ int LscTableLoader::parseLscData(const YamlObject &yamlSets,\n> \t\t\t\t<< \" is missing tables\";\n> \t\t\treturn -EINVAL;\n> \t\t}\n> +\n> +\t\tlscData.emplace(\n> +\t\t\tct, std::make_unique(std::move(set)));\n> \t}\n> \n> \tif (lscData.empty()) {\n> @@ -334,7 +381,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \t}\n> \n> \tint ret = 0;\n> -\tstd::map lscData;\n> +\tstd::map> lscData;\n> \n> \tstd::string type = tuningData[\"type\"].get(\"table\");\n> \tif (type == \"table\") {\n> @@ -343,10 +390,11 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \t\tret = loader.parseLscData(yamlSets, lscData);\n> \t} else if (type == \"polynomial\") {\n> \t\tLOG(RkISP1Lsc, Debug) << \"Loading polynomial LSC data.\";\n> -\t\tauto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,\n> -\t\t\t\t\t\t context.sensorInfo.analogCrop,\n> -\t\t\t\t\t\t xSize_,\n> -\t\t\t\t\t\t ySize_);\n> +\t\t/*\n> +\t\t * \\todo: Most likely the reference frame should be native_size.\n> +\t\t * Let's wait how the internal discussions progress.\n> +\t\t */\n> +\t\tauto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize);\n> \t\tret = loader.parseLscData(yamlSets, lscData);\n> \t} else {\n> \t\tLOG(RkISP1Lsc, Error) << \"Unsupported LSC data type '\"\n> @@ -357,7 +405,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tsets_.setData(std::move(lscData));\n> +\tshadingDescriptors_.swap(lscData);\n\n = std::move(lscData)\n\nor am I missing something? A preference?\n\n\n> \n> \treturn 0;\n> }\n> @@ -393,6 +441,14 @@ int LensShadingCorrection::configure(IPAContext &context,\n> \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n> \t}\n> \n> +\tLOG(RkISP1Lsc, Debug) << \"Sample LSC data for \" << configInfo.analogCrop;\n> +\tstd::map shadingData;\n> +\tfor (auto const &[t, descriptor] : shadingDescriptors_)\n> +\t\tdescriptor->sampleForCrop(configInfo.analogCrop, xSize_, ySize_,\n> +\t\t\t\t\t shadingData[t]);\n> +\n> +\tsets_.setData(std::move(shadingData));\n> +\n> \tcontext.configuration.lsc.enabled = true;\n> \treturn 0;\n> }\n> [...]","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7407FBE080\n\tfor ;\n\tFri, 24 Oct 2025 16:44:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 27519609D7;\n\tFri, 24 Oct 2025 18:44:21 +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 F3A4860990\n\tfor ;\n\tFri, 24 Oct 2025 18:44:19 +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 0FF12EFF;\n\tFri, 24 Oct 2025 18:42:33 +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=\"qHeK06nh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761324154;\n\tbh=oBJ5JGGinIwHi/84jTqd+C7q+h1lLW0omfFX9G3Cf5U=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=qHeK06nhbU6C88lLwy08qw8dejIH6axJmWnBdQYHKtpGcol9fNMq9RLUYrJOiEi0e\n\tf4Xg/B9XPD7UAHaWhW/rfYTKctw5W2ESay64vvQ0274e0jXllcFg4n1xJrt+HLPRcP\n\tbbvH2M9Ngb7N6FiB1Pe+TjFAuVnXYRDxNira1AJA=","Message-ID":"<7c7d498e-3dc1-405a-8966-81bfc90b758e@ideasonboard.com>","Date":"Fri, 24 Oct 2025 18:44:16 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-11-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-11-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":37518,"web_url":"https://patchwork.libcamera.org/comment/37518/","msgid":"<176780939928.21455.15999728103922746903@localhost>","date":"2026-01-07T18:09:59","subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Rui,\n\nThank you for the review. Finally I got time to work on it again.\n\nQuoting rui wang (2025-10-24 18:20:30)\n> \n> On 2025-10-14 03:52, Stefan Klug wrote:\n> > The lens shading correction is always applied based on the sensor crop\n> > bounds. This leads to incorrect lens shading correction for analog crops\n> > that do not cover the whole sensor.\n> >\n> > To fix that, we need to adapt the lens shading table for the selected\n> > analog crop at configure time. Introduce an abstract ShadingDescriptor\n> > class that holds the lens shading information that can then be sampled\n> > at configure time for a specific crop rectangle.\n> >\n> > Resampling for a specific crop is only implemented for polynomial lsc\n> > data. For tabular data, a warning is logged and the unmodified table is\n> > returned. This matches the current functionality for tabular data and is\n> > a huge improvement for polynomial data.\n> >\n> > Signed-off-by: Stefan Klug\n> > ---\n> > src/ipa/rkisp1/algorithms/lsc.cpp | 230 +++++++++++++++++++-----------\n> > src/ipa/rkisp1/algorithms/lsc.h | 14 ++\n> > 2 files changed, 157 insertions(+), 87 deletions(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index 29cd7efd83ef..68cf35064182 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -70,38 +70,124 @@ namespaceipa::rkisp1::algorithms {\n> > \n> > LOG_DEFINE_CATEGORY(RkISP1Lsc)\n> > \n> > -class LscPolynomialLoader\n> > +class LscPolynomialShadingDescriptor : publicLensShadingCorrection::ShadingDescriptor\n> > {\n> > public:\n> > - LscPolynomialLoader(const Size &sensorSize,\n> > - const Rectangle &cropRectangle,\n> > - conststd::vector &xSizes,\n> > - conststd::vector &ySizes)\n> > - : sensorSize_(sensorSize),\n> > - cropRectangle_(cropRectangle),\n> > - xSizes_(xSizes),\n> > - ySizes_(ySizes)\n> > + LscPolynomialShadingDescriptor(const LscPolynomial &pr, const LscPolynomial &pgr,\n> > + const LscPolynomial &pgb, const LscPolynomial &pb)\n> > + : pr_(pr), pgr_(pgr), pgb_(pgb), pb_(pb)\n> > {\n> > }\n> > \n> > - int parseLscData(const YamlObject &yamlSets,\n> > - std::map &lscData);\n> > + void sampleForCrop(const Rectangle &cropRectangle,\n> > + conststd::vector &xSizes,\n> > + conststd::vector &ySizes,\n> > + LensShadingCorrection::Components &components) override;\n> > \n> > private:\n> > - std::vector sizesListToPositions(conststd::vector &sizes);\n> > std::vector samplePolynomial(const LscPolynomial &poly,\n> > conststd::vector &xPositions,\n> > conststd::vector &yPositions,\n> > const Rectangle &cropRectangle);\n> > \n> > + std::vector sizesListToPositions(conststd::vector &sizes);\n> > +\n> > + LscPolynomial pr_;\n> > + LscPolynomial pgr_;\n> > + LscPolynomial pgb_;\n> > + LscPolynomial pb_;\n> > +};\n>     if the class declaration can be move into header file , the \n> relation of descriptor will looks more clear.\n\nI don't really understand what you mean here. The declaration of\nLscPolynomialShadingDescriptor doesn't belong into the header imho.\n\n> > +\n> > +voidLscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,\n> > + conststd::vector &xSizes,\n> > + conststd::vector &ySizes,\n> > + LensShadingCorrection::Components &components)\n> > +{\n> > + std::vector xPos = sizesListToPositions(xSizes);\n> > + std::vector yPos = sizesListToPositions(ySizes);\n> > + components.r = samplePolynomial(pr_, xPos, yPos, cropRectangle);\n> > + components.gr = samplePolynomial(pgr_, xPos, yPos, cropRectangle);\n> > + components.gb = samplePolynomial(pgb_, xPos, yPos, cropRectangle);\n> > + components.b = samplePolynomial(pb_, xPos, yPos, cropRectangle);\n> > +}\n> \n>      Move components as return value looks more efficient, the invoker \n> does not need to create a local object to keep the content.\n> \n>     and don't need to consider about rvalue move and memory leak or \n> memory release\n\nYou're right, I made components a return value for v2.\n\n> \n> LensShadingCorrection::Components LscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,\n> + conststd::vector &xSizes,\n> + conststd::vector &ySizes)\n> \n> > +\n> > +std::vector\n> > +LscPolynomialShadingDescriptor::samplePolynomial(const LscPolynomial &poly,\n> > + conststd::vector &xPositions,\n> > + conststd::vector &yPositions,\n> > + const Rectangle &cropRectangle)\n> > +{\n> > + double m = poly.getM();\n> > + double x0 = cropRectangle.x / m;\n> > + double y0 = cropRectangle.y / m;\n> > + double w = cropRectangle.width / m;\n> > + double h = cropRectangle.height / m;\n> > + std::vector samples;\n> > +\n> > + samples.reserve(xPositions.size() * yPositions.size());\n> > +\n> > + for (double y : yPositions) {\n> > + for (double x : xPositions) {\n> > + double xp = x0 + x * w;\n> > + double yp = y0 + y * h;\n> > + /*\n> > + * The hardware uses 2.10 fixed point format and limits\n> > + * the legal values to [1..3.999]. Scale and clamp the\n> > + * sampled value accordingly.\n> > + */\n> > + int v = static_cast(\n> > + poly.sampleAtNormalizedPixelPos(xp, yp) *\n> > + 1024);\n> > + v =std::min(std::max(v, 1024), 4095);\n> \n>                                              int_32 to replace int , \n> and std::clamp() can show more clear meaning.\n\nWhy do you recommend to use int32_t (I guess that is what you meant)?\nI'll replace the min/max by clamp. That is definitely better.\n\n> \n> int v = \n> std::clamp(static_cast(poly.sampleAtNormalizedPixelPos(xp, yp) * \n> 1024), 1024, 4095)\n\nI don't think this is easier to read. Is there a technical reason to\ncollate it into one expression?\n\n> \n> > + samples.push_back(v);\n> > + }\n> > + }\n> > + return samples;\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\n> > +LscPolynomialShadingDescriptor::sizesListToPositions(conststd::vector &sizes)\n> > +{\n> > + const int half = sizes.size();\n> > + std::vector positions(half * 2 + 1);\n> > + double x = 0.0;\n> > +\n> > + positions[half] = 0.5;\n> > + for (int i = 1; i <= half; i++) {\n> > + x += sizes[half - i];\n> > + positions[half - i] = 0.5 - x;\n> > + positions[half + i] = 0.5 + x;\n> > + }\n> > +\n> > + return positions;\n> > +}\n> > +\n> > +class LscPolynomialLoader\n> > +{\n> > +public:\n> > + LscPolynomialLoader(const Size &sensorSize) : sensorSize_(sensorSize)\n> > + {\n> > + }\n> > +\n> > + int parseLscData(const YamlObject &yamlSets,\n> > + LensShadingCorrection::ShadingDescriptorMap &lscData);\n> > +\n> > +private:\n> > Size sensorSize_;\n> > - Rectangle cropRectangle_;\n> > - conststd::vector &xSizes_;\n> > - conststd::vector &ySizes_;\n> > };\n> > \n> > intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > - std::map &lscData)\n> > + LensShadingCorrection::ShadingDescriptorMap &lscData)\n> > {\n> > const auto &sets = yamlSets.asList();\n> > for (const auto &yamlSet : sets) {\n> > @@ -115,7 +201,6 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > return -EINVAL;\n> > }\n> > \n> > - LensShadingCorrection::Components &set = lscData[ct];\n> > pr = yamlSet[\"r\"].get();\n> > pgr = yamlSet[\"gr\"].get();\n> > pgb = yamlSet[\"gb\"].get();\n> > @@ -133,12 +218,9 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > pgb->setReferenceImageSize(sensorSize_);\n> > pb->setReferenceImageSize(sensorSize_);\n> > \n> > - std::vector xPos(sizesListToPositions(xSizes_));\n> > - std::vector yPos(sizesListToPositions(ySizes_));\n> > - set.r = samplePolynomial(*pr, xPos, yPos, cropRectangle_);\n> > - set.gr = samplePolynomial(*pgr, xPos, yPos, cropRectangle_);\n> > - set.gb = samplePolynomial(*pgb, xPos, yPos, cropRectangle_);\n> > - set.b = samplePolynomial(*pb, xPos, yPos, cropRectangle_);\n> > + lscData.emplace(\n> > + ct,std::make_unique(\n> > + *pr, *pgr, *pgb, *pb));\n> > }\n> > \n> > if (lscData.empty()) {\n> > @@ -149,70 +231,33 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > return 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::vectorLscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> > +class LscTableShadingDescriptor : publicLensShadingCorrection::ShadingDescriptor\n> > {\n> > - const int half = sizes.size();\n> > - std::vector positions(half * 2 + 1);\n> > - double x = 0.0;\n> > -\n> > - positions[half] = 0.5;\n> > - for (int i = 1; i <= half; i++) {\n> > - x += sizes[half - i];\n> > - positions[half - i] = 0.5 - x;\n> > - positions[half + i] = 0.5 + x;\n> > +public:\n> > + LscTableShadingDescriptor(LensShadingCorrection::Components components)\n> > + : lscData_(std::move(components))\n> > + {\n> > }\n> > \n> > - return positions;\n> > -}\n> > -\n> > -std::vectorLscPolynomialLoader::samplePolynomial(const LscPolynomial &poly,\n> > - conststd::vector &xPositions,\n> > - conststd::vector &yPositions,\n> > - const Rectangle &cropRectangle)\n> > -{\n> > - double m = poly.getM();\n> > - double x0 = cropRectangle.x / m;\n> > - double y0 = cropRectangle.y / m;\n> > - double w = cropRectangle.width / m;\n> > - double h = cropRectangle.height / m;\n> > - std::vector samples;\n> > -\n> > - samples.reserve(xPositions.size() * yPositions.size());\n> > -\n> > - for (double y : yPositions) {\n> > - for (double x : xPositions) {\n> > - double xp = x0 + x * w;\n> > - double yp = y0 + y * h;\n> > - /*\n> > - * The hardware uses 2.10 fixed point format and limits\n> > - * the legal values to [1..3.999]. Scale and clamp the\n> > - * sampled value accordingly.\n> > - */\n> > - int v = static_cast(\n> > - poly.sampleAtNormalizedPixelPos(xp, yp) *\n> > - 1024);\n> > - v =std::min(std::max(v, 1024), 4095);\n> > - samples.push_back(v);\n> > - }\n> > + void sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,\n> > + [[maybe_unused]] conststd::vector &xSizes,\n> > + [[maybe_unused]] conststd::vector &ySizes,\n> > + LensShadingCorrection::Components &components)\n> > + {\n> > + LOG(RkISP1Lsc, Warning)\n> > + << \"Tabular LSC data doesn't support resampling.\";\n> > + components = lscData_;\n> > }\n> > - return samples;\n> > -}\n> > +\n> > +private:\n> > + LensShadingCorrection::Components lscData_;\n> > +};\n> > \n> > class LscTableLoader\n> > {\n> > public:\n> > int parseLscData(const YamlObject &yamlSets,\n> > - std::map &lscData);\n> > + LensShadingCorrection::ShadingDescriptorMap &lscData);\n> > \n> > private:\n> > std::vector parseTable(const YamlObject &tuningData,\n> > @@ -220,7 +265,7 @@ private:\n> > };\n> > \n> > intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> > - std::map &lscData)\n> > + LensShadingCorrection::ShadingDescriptorMap &lscData)\n> > {\n> > const auto &sets = yamlSets.asList();\n> > \n> > @@ -234,8 +279,7 @@ intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> > return -EINVAL;\n> > }\n> > \n> > - LensShadingCorrection::Components &set = lscData[ct];\n> > -\n> > + LensShadingCorrection::Components set;\n> > set.r = parseTable(yamlSet, \"r\");\n> > set.gr = parseTable(yamlSet, \"gr\");\n> > set.gb = parseTable(yamlSet, \"gb\");\n> > @@ -248,6 +292,9 @@ intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> > << \" is missing tables\";\n> > return -EINVAL;\n> > }\n> > +\n> > + lscData.emplace(\n> > + ct,std::make_unique(std::move(set)));\n> > }\n> > \n> > if (lscData.empty()) {\n> > @@ -334,7 +381,7 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > }\n> > \n> > int ret = 0;\n> > - std::map lscData;\n> > + std::map> lscData;\n>                already declare shadingDescriptors_ in header file\n> > \n> > std::string type = tuningData[\"type\"].get(\"table\");\n> > if (type == \"table\") {\n> > @@ -343,10 +390,11 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > ret = loader.parseLscData(yamlSets, lscData);\n> > } else if (type == \"polynomial\") {\n> > LOG(RkISP1Lsc, Debug) << \"Loading polynomial LSC data.\";\n> > - auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,\n> > - context.sensorInfo.analogCrop,\n> > - xSize_,\n> > - ySize_);\n> > + /*\n> > + * \\todo: Most likely the reference frame should be native_size.\n> > + * Let's wait how the internal discussions progress.\n> > + */\n> > + auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize);\n> > ret = loader.parseLscData(yamlSets, lscData);\n> > } else {\n> > LOG(RkISP1Lsc, Error) << \"Unsupported LSC data type '\"\n> > @@ -357,7 +405,7 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > if (ret)\n> > return ret;\n> > \n> > - sets_.setData(std::move(lscData));\n> > + shadingDescriptors_.swap(lscData);\n> > \n> > return 0;\n> > }\n> > @@ -393,6 +441,14 @@ intLensShadingCorrection::configure(IPAContext &context,\n> > yGrad_[i] =std::round(32768 / ySizes_[i]);\n> > }\n> > \n> > + LOG(RkISP1Lsc, Debug) << \"Sample LSC data for \" << configInfo.analogCrop;\n> > + std::map shadingData;\n> > + for (auto const &[t, descriptor] : shadingDescriptors_)\n> > + descriptor->sampleForCrop(configInfo.analogCrop, xSize_, ySize_,\n> > + shadingData[t]);\n> > +\n> > + sets_.setData(std::move(shadingData));\n> > +\n> > context.configuration.lsc.enabled = true;\n> > return 0;\n> > }\n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> > index 7b68dda1a0d4..43fee337931f 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.h\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> > \n> > #include \n> > +#include \n> > \n> > #include \"libipa/interpolator.h\"\n> > \n> > @@ -36,10 +37,23 @@ public:\n> > std::vector b;\n> > };\n> > \n> > + class ShadingDescriptor\n> > + {\n> > + public:\n> > + virtual ~ShadingDescriptor() = default;\n> > + virtual void sampleForCrop(const Rectangle &cropRectangle,\n> > + conststd::vector &xSizes,\n> > + conststd::vector &ySizes,\n> > + Components &components) = 0;\n> > + };\n> \n> sampleForCrop is defined as interface, it looks just remove =0 as abstract method looks more convinient , and by default\n\nIf I get you right, you propose to drop the = 0. But why would that be\nbeneficial?\n\n> \n> \n> > virtual Components sampleForCrop(const Rectangle &cropRectangle,\n> > + conststd::vector &xSizes,\n> > + conststd::vector &ySizes) {\n>                     return _lsc***;\n> > }\n> > +\n> > + using ShadingDescriptorMap =std::map>;\n> unsigned int, can be replaced by uint32_t with include \n> > +\n> > private:\n> > void setParameters(rkisp1_cif_isp_lsc_config &config);\n> > void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);\n> > \n> > + std::map> shadingDescriptors_;\n> \n>              declare using ShadingDescriptorMap above , either remove \n> ShadingDescriptorMap or\n> \n> ShadingDescriptorMap shadingDescriptors_;\n\nOh yes, that is more compact. I fixed it in v2.\n\nBest regards,\nStefan\n\n> \n> > ipa::Interpolator sets_;\n> > std::vector xSize_;\n> > std::vector ySize_;","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 17E44BE08B\n\tfor ;\n\tWed, 7 Jan 2026 18:10:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C1EA61FBB;\n\tWed, 7 Jan 2026 19:10:04 +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 E46A961F84\n\tfor ;\n\tWed, 7 Jan 2026 19:10:02 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:d809:713a:29af:b139])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id EE65D741;\n\tWed, 7 Jan 2026 19:09:40 +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=\"l7FarfIA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1767809381;\n\tbh=nyroKNDfUqSTPL8TSp8Sh5BQCqOTzCFSwYSqTm5TDFM=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=l7FarfIAB4PXdIg3Cf79sFoVOb2ugVXHkxZIrQ81o2MxmGb2VEwboIyC9DqlQgYno\n\tzIYC4kPR7DrpUhDMqw49ZWyeuGgrRlNQVzKcNIHRmQPL95x+RDV2lV3GqhRSGMEURZ\n\t0481B3T+DkthHzyVLaJ/FQ//eRFgLhO0OnfGGorY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<1731c430-f3f0-41ed-9c61-4349d0037434@ideasonboard.com>","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-11-stefan.klug@ideasonboard.com>\n\t<1731c430-f3f0-41ed-9c61-4349d0037434@ideasonboard.com>","Subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","From":"Stefan Klug ","To":"libcamera-devel@lists.libcamera.org, rui wang ","Date":"Wed, 07 Jan 2026 19:09:59 +0100","Message-ID":"<176780939928.21455.15999728103922746903@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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":37519,"web_url":"https://patchwork.libcamera.org/comment/37519/","msgid":"<20260107185256.GD20168@pendragon.ideasonboard.com>","date":"2026-01-07T18:52:56","subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jan 07, 2026 at 07:09:59PM +0100, Stefan Klug wrote:\n> Quoting rui wang (2025-10-24 18:20:30)\n> > On 2025-10-14 03:52, Stefan Klug wrote:\n> > > The lens shading correction is always applied based on the sensor crop\n> > > bounds. This leads to incorrect lens shading correction for analog crops\n> > > that do not cover the whole sensor.\n> > >\n> > > To fix that, we need to adapt the lens shading table for the selected\n> > > analog crop at configure time. Introduce an abstract ShadingDescriptor\n> > > class that holds the lens shading information that can then be sampled\n> > > at configure time for a specific crop rectangle.\n> > >\n> > > Resampling for a specific crop is only implemented for polynomial lsc\n> > > data. For tabular data, a warning is logged and the unmodified table is\n> > > returned. This matches the current functionality for tabular data and is\n> > > a huge improvement for polynomial data.\n> > >\n> > > Signed-off-by: Stefan Klug\n> > > ---\n> > > src/ipa/rkisp1/algorithms/lsc.cpp | 230 +++++++++++++++++++-----------\n> > > src/ipa/rkisp1/algorithms/lsc.h | 14 ++\n> > > 2 files changed, 157 insertions(+), 87 deletions(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > index 29cd7efd83ef..68cf35064182 100644\n> > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > @@ -70,38 +70,124 @@ namespaceipa::rkisp1::algorithms {\n> > > \n> > > LOG_DEFINE_CATEGORY(RkISP1Lsc)\n> > > \n> > > -class LscPolynomialLoader\n> > > +class LscPolynomialShadingDescriptor : publicLensShadingCorrection::ShadingDescriptor\n> > > {\n> > > public:\n> > > - LscPolynomialLoader(const Size &sensorSize,\n> > > - const Rectangle &cropRectangle,\n> > > - conststd::vector &xSizes,\n> > > - conststd::vector &ySizes)\n> > > - : sensorSize_(sensorSize),\n> > > - cropRectangle_(cropRectangle),\n> > > - xSizes_(xSizes),\n> > > - ySizes_(ySizes)\n> > > + LscPolynomialShadingDescriptor(const LscPolynomial &pr, const LscPolynomial &pgr,\n> > > + const LscPolynomial &pgb, const LscPolynomial &pb)\n> > > + : pr_(pr), pgr_(pgr), pgb_(pgb), pb_(pb)\n> > > {\n> > > }\n> > > \n> > > - int parseLscData(const YamlObject &yamlSets,\n> > > - std::map &lscData);\n> > > + void sampleForCrop(const Rectangle &cropRectangle,\n> > > + conststd::vector &xSizes,\n> > > + conststd::vector &ySizes,\n> > > + LensShadingCorrection::Components &components) override;\n> > > \n> > > private:\n> > > - std::vector sizesListToPositions(conststd::vector &sizes);\n> > > std::vector samplePolynomial(const LscPolynomial &poly,\n> > > conststd::vector &xPositions,\n> > > conststd::vector &yPositions,\n> > > const Rectangle &cropRectangle);\n> > > \n> > > + std::vector sizesListToPositions(conststd::vector &sizes);\n> > > +\n> > > + LscPolynomial pr_;\n> > > + LscPolynomial pgr_;\n> > > + LscPolynomial pgb_;\n> > > + LscPolynomial pb_;\n> > > +};\n> >     if the class declaration can be move into header file , the \n> > relation of descriptor will looks more clear.\n> \n> I don't really understand what you mean here. The declaration of\n> LscPolynomialShadingDescriptor doesn't belong into the header imho.\n> \n> > > +\n> > > +voidLscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,\n> > > + conststd::vector &xSizes,\n> > > + conststd::vector &ySizes,\n> > > + LensShadingCorrection::Components &components)\n> > > +{\n> > > + std::vector xPos = sizesListToPositions(xSizes);\n> > > + std::vector yPos = sizesListToPositions(ySizes);\n> > > + components.r = samplePolynomial(pr_, xPos, yPos, cropRectangle);\n> > > + components.gr = samplePolynomial(pgr_, xPos, yPos, cropRectangle);\n> > > + components.gb = samplePolynomial(pgb_, xPos, yPos, cropRectangle);\n> > > + components.b = samplePolynomial(pb_, xPos, yPos, cropRectangle);\n> > > +}\n> > \n> >      Move components as return value looks more efficient, the invoker \n> > does not need to create a local object to keep the content.\n> > \n> >     and don't need to consider about rvalue move and memory leak or \n> > memory release\n> \n> You're right, I made components a return value for v2.\n> \n> > LensShadingCorrection::Components LscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,\n> > + conststd::vector &xSizes,\n> > + conststd::vector &ySizes)\n\nYou can add a line break after the return type, it will be more\nreadable.\n\n> > > +\n> > > +std::vector\n> > > +LscPolynomialShadingDescriptor::samplePolynomial(const LscPolynomial &poly,\n> > > + conststd::vector &xPositions,\n> > > + conststd::vector &yPositions,\n> > > + const Rectangle &cropRectangle)\n> > > +{\n> > > + double m = poly.getM();\n> > > + double x0 = cropRectangle.x / m;\n> > > + double y0 = cropRectangle.y / m;\n> > > + double w = cropRectangle.width / m;\n> > > + double h = cropRectangle.height / m;\n> > > + std::vector samples;\n> > > +\n> > > + samples.reserve(xPositions.size() * yPositions.size());\n> > > +\n> > > + for (double y : yPositions) {\n> > > + for (double x : xPositions) {\n> > > + double xp = x0 + x * w;\n> > > + double yp = y0 + y * h;\n> > > + /*\n> > > + * The hardware uses 2.10 fixed point format and limits\n> > > + * the legal values to [1..3.999]. Scale and clamp the\n> > > + * sampled value accordingly.\n> > > + */\n> > > + int v = static_cast(\n> > > + poly.sampleAtNormalizedPixelPos(xp, yp) *\n> > > + 1024);\n> > > + v =std::min(std::max(v, 1024), 4095);\n> > \n> >                                              int_32 to replace int , \n> > and std::clamp() can show more clear meaning.\n> \n> Why do you recommend to use int32_t (I guess that is what you meant)?\n> I'll replace the min/max by clamp. That is definitely better.\n> \n> > int v = \n> > std::clamp(static_cast(poly.sampleAtNormalizedPixelPos(xp, yp) * \n> > 1024), 1024, 4095)\n> \n> I don't think this is easier to read. Is there a technical reason to\n> collate it into one expression?\n> \n> > > + samples.push_back(v);\n> > > + }\n> > > + }\n> > > + return samples;\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\n> > > +LscPolynomialShadingDescriptor::sizesListToPositions(conststd::vector &sizes)\n> > > +{\n> > > + const int half = sizes.size();\n> > > + std::vector positions(half * 2 + 1);\n> > > + double x = 0.0;\n> > > +\n> > > + positions[half] = 0.5;\n> > > + for (int i = 1; i <= half; i++) {\n> > > + x += sizes[half - i];\n> > > + positions[half - i] = 0.5 - x;\n> > > + positions[half + i] = 0.5 + x;\n> > > + }\n> > > +\n> > > + return positions;\n> > > +}\n> > > +\n> > > +class LscPolynomialLoader\n> > > +{\n> > > +public:\n> > > + LscPolynomialLoader(const Size &sensorSize) : sensorSize_(sensorSize)\n> > > + {\n> > > + }\n> > > +\n> > > + int parseLscData(const YamlObject &yamlSets,\n> > > + LensShadingCorrection::ShadingDescriptorMap &lscData);\n> > > +\n> > > +private:\n> > > Size sensorSize_;\n> > > - Rectangle cropRectangle_;\n> > > - conststd::vector &xSizes_;\n> > > - conststd::vector &ySizes_;\n> > > };\n> > > \n> > > intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > > - std::map &lscData)\n> > > + LensShadingCorrection::ShadingDescriptorMap &lscData)\n> > > {\n> > > const auto &sets = yamlSets.asList();\n> > > for (const auto &yamlSet : sets) {\n> > > @@ -115,7 +201,6 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > > return -EINVAL;\n> > > }\n> > > \n> > > - LensShadingCorrection::Components &set = lscData[ct];\n> > > pr = yamlSet[\"r\"].get();\n> > > pgr = yamlSet[\"gr\"].get();\n> > > pgb = yamlSet[\"gb\"].get();\n> > > @@ -133,12 +218,9 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > > pgb->setReferenceImageSize(sensorSize_);\n> > > pb->setReferenceImageSize(sensorSize_);\n> > > \n> > > - std::vector xPos(sizesListToPositions(xSizes_));\n> > > - std::vector yPos(sizesListToPositions(ySizes_));\n> > > - set.r = samplePolynomial(*pr, xPos, yPos, cropRectangle_);\n> > > - set.gr = samplePolynomial(*pgr, xPos, yPos, cropRectangle_);\n> > > - set.gb = samplePolynomial(*pgb, xPos, yPos, cropRectangle_);\n> > > - set.b = samplePolynomial(*pb, xPos, yPos, cropRectangle_);\n> > > + lscData.emplace(\n> > > + ct,std::make_unique(\n> > > + *pr, *pgr, *pgb, *pb));\n> > > }\n> > > \n> > > if (lscData.empty()) {\n> > > @@ -149,70 +231,33 @@ intLscPolynomialLoader::parseLscData(const YamlObject &yamlSets,\n> > > return 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::vectorLscPolynomialLoader::sizesListToPositions(const std::vector &sizes)\n> > > +class LscTableShadingDescriptor : publicLensShadingCorrection::ShadingDescriptor\n> > > {\n> > > - const int half = sizes.size();\n> > > - std::vector positions(half * 2 + 1);\n> > > - double x = 0.0;\n> > > -\n> > > - positions[half] = 0.5;\n> > > - for (int i = 1; i <= half; i++) {\n> > > - x += sizes[half - i];\n> > > - positions[half - i] = 0.5 - x;\n> > > - positions[half + i] = 0.5 + x;\n> > > +public:\n> > > + LscTableShadingDescriptor(LensShadingCorrection::Components components)\n> > > + : lscData_(std::move(components))\n> > > + {\n> > > }\n> > > \n> > > - return positions;\n> > > -}\n> > > -\n> > > -std::vectorLscPolynomialLoader::samplePolynomial(const LscPolynomial &poly,\n> > > - conststd::vector &xPositions,\n> > > - conststd::vector &yPositions,\n> > > - const Rectangle &cropRectangle)\n> > > -{\n> > > - double m = poly.getM();\n> > > - double x0 = cropRectangle.x / m;\n> > > - double y0 = cropRectangle.y / m;\n> > > - double w = cropRectangle.width / m;\n> > > - double h = cropRectangle.height / m;\n> > > - std::vector samples;\n> > > -\n> > > - samples.reserve(xPositions.size() * yPositions.size());\n> > > -\n> > > - for (double y : yPositions) {\n> > > - for (double x : xPositions) {\n> > > - double xp = x0 + x * w;\n> > > - double yp = y0 + y * h;\n> > > - /*\n> > > - * The hardware uses 2.10 fixed point format and limits\n> > > - * the legal values to [1..3.999]. Scale and clamp the\n> > > - * sampled value accordingly.\n> > > - */\n> > > - int v = static_cast(\n> > > - poly.sampleAtNormalizedPixelPos(xp, yp) *\n> > > - 1024);\n> > > - v =std::min(std::max(v, 1024), 4095);\n> > > - samples.push_back(v);\n> > > - }\n> > > + void sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,\n> > > + [[maybe_unused]] conststd::vector &xSizes,\n> > > + [[maybe_unused]] conststd::vector &ySizes,\n> > > + LensShadingCorrection::Components &components)\n> > > + {\n> > > + LOG(RkISP1Lsc, Warning)\n> > > + << \"Tabular LSC data doesn't support resampling.\";\n> > > + components = lscData_;\n> > > }\n> > > - return samples;\n> > > -}\n> > > +\n> > > +private:\n> > > + LensShadingCorrection::Components lscData_;\n> > > +};\n> > > \n> > > class LscTableLoader\n> > > {\n> > > public:\n> > > int parseLscData(const YamlObject &yamlSets,\n> > > - std::map &lscData);\n> > > + LensShadingCorrection::ShadingDescriptorMap &lscData);\n> > > \n> > > private:\n> > > std::vector parseTable(const YamlObject &tuningData,\n> > > @@ -220,7 +265,7 @@ private:\n> > > };\n> > > \n> > > intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> > > - std::map &lscData)\n> > > + LensShadingCorrection::ShadingDescriptorMap &lscData)\n> > > {\n> > > const auto &sets = yamlSets.asList();\n> > > \n> > > @@ -234,8 +279,7 @@ intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> > > return -EINVAL;\n> > > }\n> > > \n> > > - LensShadingCorrection::Components &set = lscData[ct];\n> > > -\n> > > + LensShadingCorrection::Components set;\n> > > set.r = parseTable(yamlSet, \"r\");\n> > > set.gr = parseTable(yamlSet, \"gr\");\n> > > set.gb = parseTable(yamlSet, \"gb\");\n> > > @@ -248,6 +292,9 @@ intLscTableLoader::parseLscData(const YamlObject &yamlSets,\n> > > << \" is missing tables\";\n> > > return -EINVAL;\n> > > }\n> > > +\n> > > + lscData.emplace(\n> > > + ct,std::make_unique(std::move(set)));\n> > > }\n> > > \n> > > if (lscData.empty()) {\n> > > @@ -334,7 +381,7 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > > }\n> > > \n> > > int ret = 0;\n> > > - std::map lscData;\n> > > + std::map> lscData;\n> >                already declare shadingDescriptors_ in header file\n> > > \n> > > std::string type = tuningData[\"type\"].get(\"table\");\n> > > if (type == \"table\") {\n> > > @@ -343,10 +390,11 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > > ret = loader.parseLscData(yamlSets, lscData);\n> > > } else if (type == \"polynomial\") {\n> > > LOG(RkISP1Lsc, Debug) << \"Loading polynomial LSC data.\";\n> > > - auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,\n> > > - context.sensorInfo.analogCrop,\n> > > - xSize_,\n> > > - ySize_);\n> > > + /*\n> > > + * \\todo: Most likely the reference frame should be native_size.\n> > > + * Let's wait how the internal discussions progress.\n> > > + */\n> > > + auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize);\n> > > ret = loader.parseLscData(yamlSets, lscData);\n> > > } else {\n> > > LOG(RkISP1Lsc, Error) << \"Unsupported LSC data type '\"\n> > > @@ -357,7 +405,7 @@ intLensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > > if (ret)\n> > > return ret;\n> > > \n> > > - sets_.setData(std::move(lscData));\n> > > + shadingDescriptors_.swap(lscData);\n> > > \n> > > return 0;\n> > > }\n> > > @@ -393,6 +441,14 @@ intLensShadingCorrection::configure(IPAContext &context,\n> > > yGrad_[i] =std::round(32768 / ySizes_[i]);\n> > > }\n> > > \n> > > + LOG(RkISP1Lsc, Debug) << \"Sample LSC data for \" << configInfo.analogCrop;\n> > > + std::map shadingData;\n> > > + for (auto const &[t, descriptor] : shadingDescriptors_)\n> > > + descriptor->sampleForCrop(configInfo.analogCrop, xSize_, ySize_,\n> > > + shadingData[t]);\n> > > +\n> > > + sets_.setData(std::move(shadingData));\n> > > +\n> > > context.configuration.lsc.enabled = true;\n> > > return 0;\n> > > }\n> > > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> > > index 7b68dda1a0d4..43fee337931f 100644\n> > > --- a/src/ipa/rkisp1/algorithms/lsc.h\n> > > +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> > > @@ -8,6 +8,7 @@\n> > > #pragma once\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > #include \"libipa/interpolator.h\"\n> > > \n> > > @@ -36,10 +37,23 @@ public:\n> > > std::vector b;\n> > > };\n> > > \n> > > + class ShadingDescriptor\n> > > + {\n> > > + public:\n> > > + virtual ~ShadingDescriptor() = default;\n> > > + virtual void sampleForCrop(const Rectangle &cropRectangle,\n> > > + conststd::vector &xSizes,\n> > > + conststd::vector &ySizes,\n> > > + Components &components) = 0;\n> > > + };\n> > \n> > sampleForCrop is defined as interface, it looks just remove =0 as abstract method looks more convinient , and by default\n> \n> If I get you right, you propose to drop the = 0. But why would that be\n> beneficial?\n> \n> > > virtual Components sampleForCrop(const Rectangle &cropRectangle,\n> > > + conststd::vector &xSizes,\n> > > + conststd::vector &ySizes) {\n> >                     return _lsc***;\n> > > }\n> > > +\n> > > + using ShadingDescriptorMap =std::map>;\n> > unsigned int, can be replaced by uint32_t with include \n> > > +\n> > > private:\n> > > void setParameters(rkisp1_cif_isp_lsc_config &config);\n> > > void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);\n> > > \n> > > + std::map> shadingDescriptors_;\n> > \n> >              declare using ShadingDescriptorMap above , either remove \n> > ShadingDescriptorMap or\n> > \n> > ShadingDescriptorMap shadingDescriptors_;\n> \n> Oh yes, that is more compact. I fixed it in v2.\n> \n> > > ipa::Interpolator sets_;\n> > > std::vector xSize_;\n> > > std::vector ySize_;","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 190B3BE08B\n\tfor ;\n\tWed, 7 Jan 2026 18:53:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4F79761F84;\n\tWed, 7 Jan 2026 19:53:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 922F661F84\n\tfor ;\n\tWed, 7 Jan 2026 19:53:15 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 6B027741;\n\tWed, 7 Jan 2026 19:52:53 +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=\"mfauMIr7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1767811973;\n\tbh=8SPXXT+CkgJCGpMuqUvbz3p+/E9DfcUyL93qDg/XoPg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mfauMIr7ze6fMG+lvbbAhGfaUK7+7mYu7GtmDULwkEu5Q48d8CZjVvDiQpJCAvA1n\n\tw4u8mzYeyYcpvmawb0TFYsCaLik1Q8hNuewn9+QpYlXWAB+BbGHqMk8qiQhSmlyI9T\n\tn7fXcrw+uENPQTJFp0MUgWeDCPAzqVS2zy/cO+Eg=","Date":"Wed, 7 Jan 2026 20:52:56 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org, rui wang ","Subject":"Re: [PATCH v1 10/12] ipa: rkisp1: lsc: Resample polynomial lens\n\tshading tables at configure time","Message-ID":"<20260107185256.GD20168@pendragon.ideasonboard.com>","References":"<20251014075252.2876485-1-stefan.klug@ideasonboard.com>\n\t<20251014075252.2876485-11-stefan.klug@ideasonboard.com>\n\t<1731c430-f3f0-41ed-9c61-4349d0037434@ideasonboard.com>\n\t<176780939928.21455.15999728103922746903@localhost>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<176780939928.21455.15999728103922746903@localhost>","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\" "}}]