[{"id":40060,"web_url":"https://patchwork.libcamera.org/comment/40060/","msgid":"<178594985260.2300677.7848552589454119109@ping.linuxembedded.co.uk>","date":"2026-08-05T17:10:52","subject":"Re: [PATCH v7 27/32] ipa: libipa: lsc: Quantize gains in IPA","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2026-08-05 17:13:09)\n> The libIPA Lsc algorithm currently assumes the register format of\n> the RkISP1 platform (16 bits Q2.10 format), as that's where the\n\n16 or 12 bits?\n\nAh you mean stored in a uint16_t. Does that mean it's a UQ2.10 ?\n\n> implementation has been derived from.\n> \n> Make lsc::Components use floats as the default exchange type between the\n> LscAlgorithm and the IPA module and move the gains quantization to the\n> plaltform IPA module.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/ipa/libipa/lsc_base.h | 2 +-\n> src/ipa/libipa/lsc_polynomial.cpp | 17 +++++------------\n> src/ipa/libipa/lsc_polynomial.h | 8 ++++----\n> src/ipa/libipa/lsc_table.cpp | 22 ++++++++++++++++------\n> src/ipa/libipa/lsc_table.h | 2 +-\n> src/ipa/rkisp1/algorithms/lsc.cpp | 23 +++++++++++++++--------\n> 6 files changed, 42 insertions(+), 32 deletions(-)\n> \n> diff --git a/src/ipa/libipa/lsc_base.h b/src/ipa/libipa/lsc_base.h\n> index 4ad754b3727a..2cff34b32a2b 100644\n> --- a/src/ipa/libipa/lsc_base.h\n> +++ b/src/ipa/libipa/lsc_base.h\n> @@ -26,7 +26,7 @@ namespace ipa {\n> \n> namespace lsc {\n> \n> -using Components = std::map, std::less<>>;\n> +using Components = std::map, std::less<>>;\n> using ComponentsMap = std::map;\n> \n> } /* namespace lsc */\n> diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp\n> index 0eb017de6b50..419de2c12cf0 100644\n> --- a/src/ipa/libipa/lsc_polynomial.cpp\n> +++ b/src/ipa/libipa/lsc_polynomial.cpp\n> @@ -212,7 +212,7 @@ LscPolynomial::sampleForCrop(const Rectangle &cropRectangle,\n> return components;\n> }\n> \n> -std::vector\n> +std::vector\n> LscPolynomial::samplePolynomial(const lsc::Polynomial &poly,\n> Span xPositions,\n> Span yPositions,\n> @@ -223,7 +223,7 @@ LscPolynomial::samplePolynomial(const lsc::Polynomial &poly,\n> double y0 = cropRectangle.y / m;\n> double w = cropRectangle.width / m;\n> double h = cropRectangle.height / m;\n> - std::vector samples;\n> + std::vector samples;\n> \n> samples.reserve(xPositions.size() * yPositions.size());\n> \n> @@ -231,16 +231,9 @@ LscPolynomial::samplePolynomial(const lsc::Polynomial &poly,\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::clamp(v, 1024, 4095);\n> - samples.push_back(v);\n> +\n> + samples.push_back(static_cast\n> + (poly.sampleAtNormalizedPixelPos(xp, yp)));\n> }\n> }\n> return samples;\n> diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h\n> index 548ed94b2461..24b216a1aa25 100644\n> --- a/src/ipa/libipa/lsc_polynomial.h\n> +++ b/src/ipa/libipa/lsc_polynomial.h\n> @@ -65,10 +65,10 @@ public:\n> std::vector xPos, std::vector yPos) override;\n> \n> private:\n> - std::vector samplePolynomial(const lsc::Polynomial &poly,\n> - Span xPositions,\n> - Span yPositions,\n> - const Rectangle &cropRectangle);\n> + std::vector samplePolynomial(const lsc::Polynomial &poly,\n> + Span xPositions,\n> + Span yPositions,\n> + const Rectangle &cropRectangle);\n> PolynomialComponentsMap lscData_;\n> };\n> \n> diff --git a/src/ipa/libipa/lsc_table.cpp b/src/ipa/libipa/lsc_table.cpp\n> index 639f4f6aa8b0..3e8b1a060ce7 100644\n> --- a/src/ipa/libipa/lsc_table.cpp\n> +++ b/src/ipa/libipa/lsc_table.cpp\n> @@ -30,6 +30,11 @@ namespace ipa {\n> *\n> * Parse the LSC data in tabular form from the \\a sets tuning data.\n> *\n> + * \\todo Currently the gain values parsed from tuning file are expressed in the\n> + * platform fixed-point register format. Use gains in floating point\n> + * representation for tabular LSC data in order to facilitate re-use of LSC\n> + * tuning data across different platforms.\n> + *\n> * \\return 0 on success or a negative error number otherwise\n> */\n> int LscTable::parseLscData(const ValueNode &sets,\n> @@ -82,15 +87,20 @@ int LscTable::parseLscComponent(const ValueNode &yamlSet,\n> return 0;\n> }\n> \n> -std::vector LscTable::parseTable(const ValueNode &tuningData,\n> - const char *prop,\n> - unsigned int numHSamples,\n> - unsigned int numVSamples)\n> +std::vector LscTable::parseTable(const ValueNode &tuningData,\n> + const char *prop,\n> + unsigned int numHSamples,\n> + unsigned int numVSamples)\n> {\n> unsigned int lscNumSamples = numHSamples * numVSamples;\n> \n> - std::vector table =\n> - tuningData[prop].get>().value_or(utils::defopt);\n> + /*\n> + * Cast to float even if gains are expressed as fixed-point\n> + * representations. This prepares to express gains in floating point\n> + * formats in tuning files.\n> + */\n> + std::vector table =\n> + tuningData[prop].get>().value_or(utils::defopt);\n\nAs long as this works I'm happy.\n\n\n> if (table.size() != lscNumSamples) {\n> LOG(LscTable, Error)\n> << \"Invalid '\" << prop << \"' values: expected \"\n> diff --git a/src/ipa/libipa/lsc_table.h b/src/ipa/libipa/lsc_table.h\n> index a33208761c15..6d9b0c692b7f 100644\n> --- a/src/ipa/libipa/lsc_table.h\n> +++ b/src/ipa/libipa/lsc_table.h\n> @@ -42,7 +42,7 @@ public:\n> private:\n> int parseLscComponent(const ValueNode &yamlSet,\n> unsigned int ct, const LscDescriptor &descriptor);\n> - std::vector parseTable(const ValueNode &tuningData,\n> + std::vector parseTable(const ValueNode &tuningData,\n> const char *prop,\n> unsigned int numHSamples,\n> unsigned int numVSamples);\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 7a95bfcfb93a..911ac6895a14 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -177,14 +177,21 @@ void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n> void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> const lsc::Components &set)\n> {\n> - const auto &r = set.at(\"r\");\n> - std::copy(r.begin(), r.end(), &config.r_data_tbl[0][0]);\n> - const auto &gr = set.at(\"gr\");\n> - std::copy(gr.begin(), gr.end(), &config.gr_data_tbl[0][0]);\n> - const auto &gb = set.at(\"gb\");\n> - std::copy(gb.begin(), gb.end(), &config.gb_data_tbl[0][0]);\n> - const auto &b = set.at(\"b\");\n> - std::copy(b.begin(), b.end(), &config.b_data_tbl[0][0]);\n> + /*\n> + * The hardware uses 2.10 fixed point format and limits the legal values\n> + * to [1..3.999]. Scale and clamp the sampled values accordingly.\n> + */\n> + const auto quantizeSet = [&](const std::string &key, uint16_t *dst) {\n> + const auto &s = set.at(key);\n> + std::transform(s.begin(), s.end(), dst, [](float f) {\n> + return std::clamp(f * 1024, 1024, 4095);\n> + });\n> + };\n> +\n> + quantizeSet(\"r\", &config.r_data_tbl[0][0]);\n> + quantizeSet(\"gr\", &config.gr_data_tbl[0][0]);\n> + quantizeSet(\"gb\", &config.gb_data_tbl[0][0]);\n> + quantizeSet(\"b\", &config.b_data_tbl[0][0]);\n\nThat looks reasonable to me.\n\nReviewed-by: Kieran Bingham \n\n> }\n> \n> /**\n> \n> -- \n> 2.54.0\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 17320C3301\n\tfor ;\n\tWed, 5 Aug 2026 17:10:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 13CFF68132;\n\tWed, 5 Aug 2026 19:10:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C4DEC68124\n\tfor ;\n\tWed, 5 Aug 2026 19:10:55 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E474A594;\n\tWed, 5 Aug 2026 19:09:44 +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=\"YG1IC640\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785949785;\n\tbh=ezw6t/5D4/mvXWTy/5RAzJkU40NqGpYQrVPEx8spg0o=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YG1IC640z/R9lU6hF/HDs2LiHfo0QZ/rBvilLHO2HbBwwVUqJiFHd1IKfmQCbkYF2\n\tHHlDApMqhcm8/hT4JPL6ELqO9olamXM4Y/xyUajRveBib2ko9iaIZoQzoN/tiXIzSR\n\t1WCNfXcdw63i0pZtwLz0zJU6wsbs53nVKSWAMvo4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260805-libipa-algorithms-v7-27-7425b5b795d4@ideasonboard.com>","References":"<20260805-libipa-algorithms-v7-0-7425b5b795d4@ideasonboard.com>\n\t<20260805-libipa-algorithms-v7-27-7425b5b795d4@ideasonboard.com>","Subject":"Re: [PATCH v7 27/32] ipa: libipa: lsc: Quantize gains in IPA","From":"Kieran Bingham ","Cc":"Jacopo Mondi ","To":"Jacopo Mondi ,\n\tMilan Zamazal ,\n\tStefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 05 Aug 2026 18:10:52 +0100","Message-ID":"<178594985260.2300677.7848552589454119109@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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\" "}}]