[{"id":39681,"web_url":"https://patchwork.libcamera.org/comment/39681/","msgid":"<178395231220.3603632.5026638824247021142@localhost>","date":"2026-07-13T14:18:32","subject":"Re: [PATCH v5 18/36] ipa: libipa: lsc_table: Move LscTable from\n\tRkISP1","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nQuoting Jacopo Mondi (2026-07-08 17:51:00)\n> Move the LscTableImpl class to libipa from RkISP1.\n> \n> Rename the class from LscTableImpl to a simpler LscTable.\n> The class is copied verbatim, only documntation has been added.\n> \n> The implementation still contains RkISP1 specific data which will be\n> removed once the class is generalized.\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Kieran Bingham \n\nLooks good to me.\n\nReviewed-by: Stefan Klug \n\nBest regards,\nStefan\n\n> ---\n> src/ipa/libipa/lsc_table.cpp | 95 +++++++++++++++++++++++++++++++++++++++\n> src/ipa/libipa/lsc_table.h | 50 +++++++++++++++++++++\n> src/ipa/libipa/meson.build | 2 +\n> src/ipa/rkisp1/algorithms/lsc.cpp | 81 +--------------------------------\n> 4 files changed, 149 insertions(+), 79 deletions(-)\n> \n> diff --git a/src/ipa/libipa/lsc_table.cpp b/src/ipa/libipa/lsc_table.cpp\n> new file mode 100644\n> index 000000000000..47dff156963a\n> --- /dev/null\n> +++ b/src/ipa/libipa/lsc_table.cpp\n> @@ -0,0 +1,95 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026, Ideas On Board\n> + *\n> + * Table-based LSC implementation\n> + */\n> +\n> +#include \"lsc_table.h\"\n> +\n> +/* \\todo Remove RkISP1 from libipa. */\n> +#include \"linux/rkisp1-config.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(LscTable)\n> +\n> +namespace ipa {\n> +\n> +/**\n> + * \\class LscTable\n> + * \\brief Table based LSC algorithm implementation\n> + *\n> + * Table based LSC algorithm implementation. The LSCTable class implements LSC\n> + * support using tabular LSC data.\n> + *\n> + * \\sa LscImplementation\n> + */\n> +\n> +/**\n> + * \\brief Parse tabular LSC data\n> + * \\param[in] sets The tuning file content\n> + *\n> + * Parse the LSC data in tabular form from the \\a sets tuning data.\n> + *\n> + * \\return 0 on success or a negative error number otherwise\n> + */\n> +int LscTable::parseLscData(const ValueNode &sets)\n> +{\n> + for (const auto &set : sets.asList()) {\n> + uint32_t ct = set[\"ct\"].get(0);\n> +\n> + if (lscData_.count(ct)) {\n> + LOG(LscTable, Error)\n> + << \"Multiple sets found for color temperature \"\n> + << ct;\n> + return -EINVAL;\n> + }\n> +\n> + lsc::Components components;\n> + components.r = parseTable(set, \"r\");\n> + components.gr = parseTable(set, \"gr\");\n> + components.gb = parseTable(set, \"gb\");\n> + components.b = parseTable(set, \"b\");\n> +\n> + if (components.r.empty() || components.gr.empty() ||\n> + components.gb.empty() || components.b.empty()) {\n> + LOG(LscTable, Error)\n> + << \"Set for color temperature \" << ct\n> + << \" is missing tables\";\n> + return -EINVAL;\n> + }\n> +\n> + lscData_.emplace(ct, std::move(components));\n> + }\n> +\n> + if (lscData_.empty()) {\n> + LOG(LscTable, Error) << \"Failed to load any sets\";\n> + return -EINVAL;\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +std::vector LscTable::parseTable(const ValueNode &tuningData,\n> + const char *prop)\n> +{\n> + static constexpr unsigned int kLscNumSamples =\n> + RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n> +\n> + std::vector table =\n> + tuningData[prop].get>().value_or(utils::defopt);\n> + if (table.size() != kLscNumSamples) {\n> + LOG(LscTable, Error)\n> + << \"Invalid '\" << prop << \"' values: expected \"\n> + << kLscNumSamples\n> + << \" elements, got \" << table.size();\n> + return {};\n> + }\n> +\n> + return table;\n> +}\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/lsc_table.h b/src/ipa/libipa/lsc_table.h\n> new file mode 100644\n> index 000000000000..0983b78c3f6b\n> --- /dev/null\n> +++ b/src/ipa/libipa/lsc_table.h\n> @@ -0,0 +1,50 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026, Ideas On Board\n> + *\n> + * Table-based Lsc implementation\n> + */\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/value_node.h\"\n> +\n> +#include \"lsc_base.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(LscTable)\n> +\n> +namespace ipa {\n> +\n> +class LscTable : public LscImplementation\n> +{\n> +public:\n> + int parseLscData(const ValueNode &sets) override;\n> +\n> + lsc::ComponentsMap\n> + sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,\n> + [[maybe_unused]] Span xSizes,\n> + [[maybe_unused]] Span ySizes) override\n> + {\n> + LOG(LscTable, Warning)\n> + << \"Tabular LSC data doesn't support resampling\";\n> + return lscData_;\n> + }\n> +\n> +private:\n> + std::vector parseTable(const ValueNode &tuningData,\n> + const char *prop);\n> +\n> + lsc::ComponentsMap lscData_;\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index 96611e90ee1e..95d398d99fd6 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -16,6 +16,7 @@ libipa_headers = files([\n> 'interpolator.h',\n> 'lsc_base.h',\n> 'lsc_polynomial.h',\n> + 'lsc_table.h',\n> 'lux.h',\n> 'module.h',\n> 'pwl.h',\n> @@ -39,6 +40,7 @@ libipa_sources = files([\n> 'interpolator.cpp',\n> 'lsc_base.cpp',\n> 'lsc_polynomial.cpp',\n> + 'lsc_table.cpp',\n> 'lux.cpp',\n> 'module.cpp',\n> 'pwl.cpp',\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index b161798763cb..f32c46084dd1 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -17,6 +17,7 @@\n> #include \"libcamera/internal/value_node.h\"\n> \n> #include \"libipa/lsc_polynomial.h\"\n> +#include \"libipa/lsc_table.h\"\n> #include \"linux/rkisp1-config.h\"\n> \n> /**\n> @@ -33,84 +34,6 @@ namespace {\n> \n> constexpr int kColourTemperatureQuantization = 10;\n> \n> -class LscTableImpl : public LscImplementation\n> -{\n> -public:\n> - int parseLscData(const ValueNode &sets) override;\n> -\n> - lsc::ComponentsMap\n> - sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,\n> - [[maybe_unused]] Span xSizes,\n> - [[maybe_unused]] Span ySizes) override\n> - {\n> - LOG(RkISP1Lsc, Warning)\n> - << \"Tabular LSC data doesn't support resampling\";\n> - return lscData_;\n> - }\n> -\n> -private:\n> - std::vector parseTable(const ValueNode &tuningData,\n> - const char *prop);\n> -\n> - lsc::ComponentsMap lscData_;\n> -};\n> -\n> -int LscTableImpl::parseLscData(const ValueNode &sets)\n> -{\n> - for (const auto &set : sets.asList()) {\n> - uint32_t ct = set[\"ct\"].get(0);\n> -\n> - if (lscData_.count(ct)) {\n> - LOG(RkISP1Lsc, Error)\n> - << \"Multiple sets found for color temperature \"\n> - << ct;\n> - return -EINVAL;\n> - }\n> -\n> - lsc::Components components;\n> - components.r = parseTable(set, \"r\");\n> - components.gr = parseTable(set, \"gr\");\n> - components.gb = parseTable(set, \"gb\");\n> - components.b = parseTable(set, \"b\");\n> -\n> - if (components.r.empty() || components.gr.empty() ||\n> - components.gb.empty() || components.b.empty()) {\n> - LOG(RkISP1Lsc, Error)\n> - << \"Set for color temperature \" << ct\n> - << \" is missing tables\";\n> - return -EINVAL;\n> - }\n> -\n> - lscData_.emplace(ct, std::move(components));\n> - }\n> -\n> - if (lscData_.empty()) {\n> - LOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n> - return -EINVAL;\n> - }\n> -\n> - return 0;\n> -}\n> -\n> -std::vector LscTableImpl::parseTable(const ValueNode &tuningData,\n> - const char *prop)\n> -{\n> - static constexpr unsigned int kLscNumSamples =\n> - RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n> -\n> - std::vector table =\n> - tuningData[prop].get>().value_or(utils::defopt);\n> - if (table.size() != kLscNumSamples) {\n> - LOG(RkISP1Lsc, Error)\n> - << \"Invalid '\" << prop << \"' values: expected \"\n> - << kLscNumSamples\n> - << \" elements, got \" << table.size();\n> - return {};\n> - }\n> -\n> - return table;\n> -}\n> -\n> std::vector parseSizes(const ValueNode &tuningData,\n> const char *prop)\n> {\n> @@ -190,7 +113,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> std::string type = tuningData[\"type\"].get(\"table\");\n> if (type == \"table\") {\n> LOG(RkISP1Lsc, Debug) << \"Loading tabular LSC data.\";\n> - algo_ = std::make_unique();\n> + algo_ = std::make_unique();\n> ret = algo_->parseLscData(sets);\n> } else if (type == \"polynomial\") {\n> LOG(RkISP1Lsc, Debug) << \"Loading polynomial LSC data.\";\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 A5997C32CE\n\tfor ;\n\tMon, 13 Jul 2026 14:18:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 998416611A;\n\tMon, 13 Jul 2026 16:18:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5FEA466114\n\tfor ;\n\tMon, 13 Jul 2026 16:18:35 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:6c1f:355d:1c19:aba6])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C65F2103F;\n\tMon, 13 Jul 2026 16:17:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Yzzm6a2r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1783952261;\n\tbh=dNZTOpl3xksySGK5iG61nkoo17FB/at+lkElcNSGlsQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Yzzm6a2rwKBil/ykX4X8/iuEXYYm8O/hPhc5Ojdv969N7n0DhkKewJkPrrnGp0FOY\n\tPR7A53vp1IEpUNRMSj0G+dxiQcORC7gKIq7AStKbQFD4BIEICChgloTYCarmyFco97\n\tSrl+kvlSfeSWD2devrKgMHayatvYevHnY44MdyJM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260708-libipa-algorithms-v5-18-0759d0359f52@ideasonboard.com>","References":"<20260708-libipa-algorithms-v5-0-0759d0359f52@ideasonboard.com>\n\t<20260708-libipa-algorithms-v5-18-0759d0359f52@ideasonboard.com>","Subject":"Re: [PATCH v5 18/36] ipa: libipa: lsc_table: Move LscTable from\n\tRkISP1","From":"Stefan Klug ","Cc":"Jacopo Mondi ,\n\tKieran Bingham ","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 13 Jul 2026 16:18:32 +0200","Message-ID":"<178395231220.3603632.5026638824247021142@localhost>","User-Agent":"alot/0.12.dev43+g2cacc0d03","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\" "}}]