[{"id":31207,"web_url":"https://patchwork.libcamera.org/comment/31207/","msgid":"<172622131083.3474483.12205391110027738276@ping.linuxembedded.co.uk>","date":"2024-09-13T09:55:10","subject":"Re: [PATCH v2 6/9] ipa: rkisp1: Move loader functions into helper\n\tclass","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2024-09-13 08:57:24)\n> In preparation to the polynomial LSC data, move the current loader into\n> it's own helper class.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 125 ++++++++++++++++++------------\n> 1 file changed, 76 insertions(+), 49 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 87a04ec048f8..12867b8f7d0f 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -70,6 +70,70 @@ namespace ipa::rkisp1::algorithms {\n> \n> LOG_DEFINE_CATEGORY(RkISP1Lsc)\n> \n> +class LscClassicLoader\n> +{\n> +public:\n> + int parseLscData(const YamlObject &yamlSets,\n> + std::map &lscData)\n> + {\n> + const auto &sets = yamlSets.asList();\n> +\n> + for (const auto &yamlSet : sets) {\n> + uint32_t ct = yamlSet[\"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> + LensShadingCorrection::Components &set = lscData[ct];\n> +\n> + set.ct = ct;\n> + set.r = parseTable(yamlSet, \"r\");\n> + set.gr = parseTable(yamlSet, \"gr\");\n> + set.gb = parseTable(yamlSet, \"gb\");\n> + set.b = parseTable(yamlSet, \"b\");\n> +\n> + if (set.r.empty() || set.gr.empty() ||\n> + set.gb.empty() || set.b.empty()) {\n> + LOG(RkISP1Lsc, Error)\n> + << \"Set for color temperature \" << ct\n> + << \" is missing tables\";\n> + return -EINVAL;\n> + }\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> +private:\n> + std::vector parseTable(const YamlObject &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].getList().value_or(std::vector{});\n> + if (table.size() != kLscNumSamples) {\n> + LOG(RkISP1Lsc, Error)\n> + << \"Invalid '\" << prop << \"' values: expected \"\n> + << kLscNumSamples\n> + << \" elements, got \" << table.size();\n> + return {};\n\nAha - I think this is where the input validation happens?\n\nThis enforces all the tables are identically sized and we'd never hit\nthe assertion right?\n\nI think that earns the tag for the previous patch ...\n\n\n\n> + }\n> +\n> + return table;\n> + }\n> +};\n> +\n> static std::vector parseSizes(const YamlObject &tuningData,\n> const char *prop)\n> {\n> @@ -100,25 +164,6 @@ static std::vector parseSizes(const YamlObject &tuningData,\n> return sizes;\n> }\n> \n> -static std::vector parseTable(const YamlObject &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].getList().value_or(std::vector{});\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> LensShadingCorrection::LensShadingCorrection()\n> : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n> {\n> @@ -145,39 +190,21 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> return -EINVAL;\n> }\n> \n> - const auto &sets = yamlSets.asList();\n> std::map lscData;\n> - for (const auto &yamlSet : sets) {\n> - uint32_t ct = yamlSet[\"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> - Components &set = lscData[ct];\n> -\n> - set.ct = ct;\n> - set.r = parseTable(yamlSet, \"r\");\n> - set.gr = parseTable(yamlSet, \"gr\");\n> - set.gb = parseTable(yamlSet, \"gb\");\n> - set.b = parseTable(yamlSet, \"b\");\n> -\n> - if (set.r.empty() || set.gr.empty() ||\n> - set.gb.empty() || set.b.empty()) {\n> - LOG(RkISP1Lsc, Error)\n> - << \"Set for color temperature \" << ct\n> - << \" is missing tables\";\n> - return -EINVAL;\n> - }\n> + int res = 0;\n> + std::optional type = tuningData[\"type\"].get();\n> + if (!type.has_value()) {\n> + LOG(RkISP1Lsc, Warning) << \"LSC data is in classic format. \"\n\nI wonder if we'd call this the 'table' format rather than classic ? But\nI don't think it matters.\n\nI could imagine some crazy use case might still want the full control of\nthe table ... so it might not ... get deprecated.\n\nThat makes me ponder if we should just call them 'table' and\n'polynomial'.\n\nBut either way, for me this loader is a good way to abstract and move\nforwards.\n\n\nReviewed-by: Kieran Bingham \n\n> + << \"This will be deprecated soon.\";\n> + auto loader = LscClassicLoader();\n> + res = loader.parseLscData(yamlSets, lscData);\n> + } else {\n> + LOG(RkISP1Lsc, Error) << \"Unsupported LSC type '\" << *type << \"'\";\n> + res = -EINVAL;\n> }\n> \n> - if (lscData.empty()) {\n> - LOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n> - return -EINVAL;\n> - }\n> + if (res)\n> + return res;\n> \n> sets_.setData(std::move(lscData));\n> \n> -- \n> 2.43.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 50FFFC324C\n\tfor ;\n\tFri, 13 Sep 2024 09:55:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B728634FE;\n\tFri, 13 Sep 2024 11:55:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 13EF5634F4\n\tfor ;\n\tFri, 13 Sep 2024 11:55:15 +0200 (CEST)","from pendragon.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 6EA9A9FF;\n\tFri, 13 Sep 2024 11:53:55 +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=\"pRi3WTqH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726221235;\n\tbh=TguTBPzHhme31DyWGzyV3TNw/jbdFIwZnPbCfv5ILZ4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pRi3WTqHFc4YF6riW/qSXGhOO7TqQanZJ0CDjaPloXMk0zIu4JQmTeTd1sg3vTmM0\n\tfFwAoWUfrE9qy/SYExg2Qx1RIHwJEycftLURfS/sV+k5yY1GSdY9XOrJXJ1nsuhZB/\n\t3ZLZlBbuLe7IZXuPvpsizWUrKWq36+U6Fmt0zvYY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240913075750.35115-7-stefan.klug@ideasonboard.com>","References":"<20240913075750.35115-1-stefan.klug@ideasonboard.com>\n\t<20240913075750.35115-7-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v2 6/9] ipa: rkisp1: Move loader functions into helper\n\tclass","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 13 Sep 2024 10:55:10 +0100","Message-ID":"<172622131083.3474483.12205391110027738276@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":31216,"web_url":"https://patchwork.libcamera.org/comment/31216/","msgid":"","date":"2024-09-13T10:41:49","subject":"Re: [PATCH v2 6/9] ipa: rkisp1: Move loader functions into helper\n\tclass","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, Sep 13, 2024 at 10:55:10AM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2024-09-13 08:57:24)\n> > In preparation to the polynomial LSC data, move the current loader into\n> > it's own helper class.\n> > \n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/ipa/rkisp1/algorithms/lsc.cpp | 125 ++++++++++++++++++------------\n> > 1 file changed, 76 insertions(+), 49 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index 87a04ec048f8..12867b8f7d0f 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -70,6 +70,70 @@ namespace ipa::rkisp1::algorithms {\n> > \n> > LOG_DEFINE_CATEGORY(RkISP1Lsc)\n> > \n> > +class LscClassicLoader\n> > +{\n> > +public:\n> > + int parseLscData(const YamlObject &yamlSets,\n> > + std::map &lscData)\n> > + {\n> > + const auto &sets = yamlSets.asList();\n> > +\n> > + for (const auto &yamlSet : sets) {\n> > + uint32_t ct = yamlSet[\"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> > + LensShadingCorrection::Components &set = lscData[ct];\n> > +\n> > + set.ct = ct;\n> > + set.r = parseTable(yamlSet, \"r\");\n> > + set.gr = parseTable(yamlSet, \"gr\");\n> > + set.gb = parseTable(yamlSet, \"gb\");\n> > + set.b = parseTable(yamlSet, \"b\");\n> > +\n> > + if (set.r.empty() || set.gr.empty() ||\n> > + set.gb.empty() || set.b.empty()) {\n> > + LOG(RkISP1Lsc, Error)\n> > + << \"Set for color temperature \" << ct\n> > + << \" is missing tables\";\n> > + return -EINVAL;\n> > + }\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> > +private:\n> > + std::vector parseTable(const YamlObject &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].getList().value_or(std::vector{});\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> Aha - I think this is where the input validation happens?\n> \n> This enforces all the tables are identically sized and we'd never hit\n> the assertion right?\n> \n> I think that earns the tag for the previous patch ...\n> \n> \n> \n> > + }\n> > +\n> > + return table;\n> > + }\n> > +};\n> > +\n> > static std::vector parseSizes(const YamlObject &tuningData,\n> > const char *prop)\n> > {\n> > @@ -100,25 +164,6 @@ static std::vector parseSizes(const YamlObject &tuningData,\n> > return sizes;\n> > }\n> > \n> > -static std::vector parseTable(const YamlObject &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].getList().value_or(std::vector{});\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> > LensShadingCorrection::LensShadingCorrection()\n> > : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n> > {\n> > @@ -145,39 +190,21 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> > return -EINVAL;\n> > }\n> > \n> > - const auto &sets = yamlSets.asList();\n> > std::map lscData;\n> > - for (const auto &yamlSet : sets) {\n> > - uint32_t ct = yamlSet[\"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> > - Components &set = lscData[ct];\n> > -\n> > - set.ct = ct;\n> > - set.r = parseTable(yamlSet, \"r\");\n> > - set.gr = parseTable(yamlSet, \"gr\");\n> > - set.gb = parseTable(yamlSet, \"gb\");\n> > - set.b = parseTable(yamlSet, \"b\");\n> > -\n> > - if (set.r.empty() || set.gr.empty() ||\n> > - set.gb.empty() || set.b.empty()) {\n> > - LOG(RkISP1Lsc, Error)\n> > - << \"Set for color temperature \" << ct\n> > - << \" is missing tables\";\n> > - return -EINVAL;\n> > - }\n> > + int res = 0;\n> > + std::optional type = tuningData[\"type\"].get();\n> > + if (!type.has_value()) {\n> > + LOG(RkISP1Lsc, Warning) << \"LSC data is in classic format. \"\n> \n> I wonder if we'd call this the 'table' format rather than classic ? But\n> I don't think it matters.\n> \n> I could imagine some crazy use case might still want the full control of\n> the table ... so it might not ... get deprecated.\n> \n> That makes me ponder if we should just call them 'table' and\n> 'polynomial'.\n\n+1 if it doesn't get deprecated yeah.\n\n> \n> But either way, for me this loader is a good way to abstract and move\n> forwards.\n\nReviewed-by: Paul Elder \n\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > + << \"This will be deprecated soon.\";\n> > + auto loader = LscClassicLoader();\n> > + res = loader.parseLscData(yamlSets, lscData);\n> > + } else {\n> > + LOG(RkISP1Lsc, Error) << \"Unsupported LSC type '\" << *type << \"'\";\n> > + res = -EINVAL;\n> > }\n> > \n> > - if (lscData.empty()) {\n> > - LOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n> > - return -EINVAL;\n> > - }\n> > + if (res)\n> > + return res;\n> > \n> > sets_.setData(std::move(lscData));\n> > \n> > -- \n> > 2.43.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 6DD80C3257\n\tfor ;\n\tFri, 13 Sep 2024 10:41:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4DCFF634F5;\n\tFri, 13 Sep 2024 12:41:56 +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 7D24F634E3\n\tfor ;\n\tFri, 13 Sep 2024 12:41:54 +0200 (CEST)","from pyrite.rasen.tech (213-229-8-243.static.upcbusiness.at\n\t[213.229.8.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6EA1574C;\n\tFri, 13 Sep 2024 12:40:35 +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=\"LTl/17ua\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726224035;\n\tbh=7GQtSr6vEUbzbyP3F6241jBxxZrG/ninDXEG+CKidwc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LTl/17uaVKcDpGTE+aRljniZsjIuERkriuvPzlgXwjhNlD0qN8FDQgVkDQC023dwu\n\tU2rEZCkwDBAInsWG6dJNoBQ/fGIvI9DjTLkUEH3jRJ0J9rj/28dV/quI+7ilcuRB64\n\tg8XFwowv67Sq08gVezFEMVzGRQX3TvfjbugYcWOE=","Date":"Fri, 13 Sep 2024 12:41:49 +0200","From":"Paul Elder ","To":"Kieran Bingham ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 6/9] ipa: rkisp1: Move loader functions into helper\n\tclass","Message-ID":"","References":"<20240913075750.35115-1-stefan.klug@ideasonboard.com>\n\t<20240913075750.35115-7-stefan.klug@ideasonboard.com>\n\t<172622131083.3474483.12205391110027738276@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<172622131083.3474483.12205391110027738276@ping.linuxembedded.co.uk>","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":31217,"web_url":"https://patchwork.libcamera.org/comment/31217/","msgid":"","date":"2024-09-13T10:44:08","subject":"Re: [PATCH v2 6/9] ipa: rkisp1: Move loader functions into helper\n\tclass","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, Sep 13, 2024 at 09:57:24AM +0200, Stefan Klug wrote:\n> In preparation to the polynomial LSC data, move the current loader into\n\ns/to the/for moving to/ or s/to the/for supporting/\n\n> it's own helper class.\n\ns/it's/its/\n\n\nPaul\n\n> \n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 125 ++++++++++++++++++------------\n> 1 file changed, 76 insertions(+), 49 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 87a04ec048f8..12867b8f7d0f 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -70,6 +70,70 @@ namespace ipa::rkisp1::algorithms {\n> \n> LOG_DEFINE_CATEGORY(RkISP1Lsc)\n> \n> +class LscClassicLoader\n> +{\n> +public:\n> +\tint parseLscData(const YamlObject &yamlSets,\n> +\t\t\t std::map &lscData)\n> +\t{\n> +\t\tconst auto &sets = yamlSets.asList();\n> +\n> +\t\tfor (const auto &yamlSet : sets) {\n> +\t\t\tuint32_t ct = yamlSet[\"ct\"].get(0);\n> +\n> +\t\t\tif (lscData.count(ct)) {\n> +\t\t\t\tLOG(RkISP1Lsc, Error)\n> +\t\t\t\t\t<< \"Multiple sets found for color temperature \"\n> +\t\t\t\t\t<< ct;\n> +\t\t\t\treturn -EINVAL;\n> +\t\t\t}\n> +\n> +\t\t\tLensShadingCorrection::Components &set = lscData[ct];\n> +\n> +\t\t\tset.ct = ct;\n> +\t\t\tset.r = parseTable(yamlSet, \"r\");\n> +\t\t\tset.gr = parseTable(yamlSet, \"gr\");\n> +\t\t\tset.gb = parseTable(yamlSet, \"gb\");\n> +\t\t\tset.b = parseTable(yamlSet, \"b\");\n> +\n> +\t\t\tif (set.r.empty() || set.gr.empty() ||\n> +\t\t\t set.gb.empty() || set.b.empty()) {\n> +\t\t\t\tLOG(RkISP1Lsc, Error)\n> +\t\t\t\t\t<< \"Set for color temperature \" << ct\n> +\t\t\t\t\t<< \" is missing tables\";\n> +\t\t\t\treturn -EINVAL;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tif (lscData.empty()) {\n> +\t\t\tLOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +private:\n> +\tstd::vector parseTable(const YamlObject &tuningData,\n> +\t\t\t\t\t const char *prop)\n> +\t{\n> +\t\tstatic constexpr unsigned int kLscNumSamples =\n> +\t\t\tRKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n> +\n> +\t\tstd::vector table =\n> +\t\t\ttuningData[prop].getList().value_or(std::vector{});\n> +\t\tif (table.size() != kLscNumSamples) {\n> +\t\t\tLOG(RkISP1Lsc, Error)\n> +\t\t\t\t<< \"Invalid '\" << prop << \"' values: expected \"\n> +\t\t\t\t<< kLscNumSamples\n> +\t\t\t\t<< \" elements, got \" << table.size();\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\treturn table;\n> +\t}\n> +};\n> +\n> static std::vector parseSizes(const YamlObject &tuningData,\n> \t\t\t\t const char *prop)\n> {\n> @@ -100,25 +164,6 @@ static std::vector parseSizes(const YamlObject &tuningData,\n> \treturn sizes;\n> }\n> \n> -static std::vector parseTable(const YamlObject &tuningData,\n> -\t\t\t\t\tconst char *prop)\n> -{\n> -\tstatic constexpr unsigned int kLscNumSamples =\n> -\t\tRKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;\n> -\n> -\tstd::vector table =\n> -\t\ttuningData[prop].getList().value_or(std::vector{});\n> -\tif (table.size() != kLscNumSamples) {\n> -\t\tLOG(RkISP1Lsc, Error)\n> -\t\t\t<< \"Invalid '\" << prop << \"' values: expected \"\n> -\t\t\t<< kLscNumSamples\n> -\t\t\t<< \" elements, got \" << table.size();\n> -\t\treturn {};\n> -\t}\n> -\n> -\treturn table;\n> -}\n> -\n> LensShadingCorrection::LensShadingCorrection()\n> \t: lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n> {\n> @@ -145,39 +190,21 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tconst auto &sets = yamlSets.asList();\n> \tstd::map lscData;\n> -\tfor (const auto &yamlSet : sets) {\n> -\t\tuint32_t ct = yamlSet[\"ct\"].get(0);\n> -\n> -\t\tif (lscData.count(ct)) {\n> -\t\t\tLOG(RkISP1Lsc, Error)\n> -\t\t\t\t<< \"Multiple sets found for color temperature \"\n> -\t\t\t\t<< ct;\n> -\t\t\treturn -EINVAL;\n> -\t\t}\n> -\n> -\t\tComponents &set = lscData[ct];\n> -\n> -\t\tset.ct = ct;\n> -\t\tset.r = parseTable(yamlSet, \"r\");\n> -\t\tset.gr = parseTable(yamlSet, \"gr\");\n> -\t\tset.gb = parseTable(yamlSet, \"gb\");\n> -\t\tset.b = parseTable(yamlSet, \"b\");\n> -\n> -\t\tif (set.r.empty() || set.gr.empty() ||\n> -\t\t set.gb.empty() || set.b.empty()) {\n> -\t\t\tLOG(RkISP1Lsc, Error)\n> -\t\t\t\t<< \"Set for color temperature \" << ct\n> -\t\t\t\t<< \" is missing tables\";\n> -\t\t\treturn -EINVAL;\n> -\t\t}\n> +\tint res = 0;\n> +\tstd::optional type = tuningData[\"type\"].get();\n> +\tif (!type.has_value()) {\n> +\t\tLOG(RkISP1Lsc, Warning) << \"LSC data is in classic format. \"\n> +\t\t\t\t\t<< \"This will be deprecated soon.\";\n> +\t\tauto loader = LscClassicLoader();\n> +\t\tres = loader.parseLscData(yamlSets, lscData);\n> +\t} else {\n> +\t\tLOG(RkISP1Lsc, Error) << \"Unsupported LSC type '\" << *type << \"'\";\n> +\t\tres = -EINVAL;\n> \t}\n> \n> -\tif (lscData.empty()) {\n> -\t\tLOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n> -\t\treturn -EINVAL;\n> -\t}\n> +\tif (res)\n> +\t\treturn res;\n> \n> \tsets_.setData(std::move(lscData));\n> \n> -- \n> 2.43.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 EAE19C324C\n\tfor ;\n\tFri, 13 Sep 2024 10:44:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E88F634FE;\n\tFri, 13 Sep 2024 12:44:15 +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 D30AF634F5\n\tfor ;\n\tFri, 13 Sep 2024 12:44:12 +0200 (CEST)","from pyrite.rasen.tech (213-229-8-243.static.upcbusiness.at\n\t[213.229.8.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CCB0974C;\n\tFri, 13 Sep 2024 12:42:53 +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=\"ZYXlGk7H\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726224173;\n\tbh=K2alOEgLOL7OiQZA4I+WyL3KFTx0M91ylLmTMUXeHVo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZYXlGk7HKBBl/0ihV6X1bfohoj/hJD+h2I+2HjFVn8+7AFecd54yOn5XBt5krekKU\n\tJw8LxEYQKOuFtr2dgDZRbFMAAhO4K1HB89FSwyUZZMt+q+rVrnbBJ8MNFXzJHMkztj\n\t+JNKL1ctoODS9uG+L9dc5YxETcpFLecDD2NJwtkg=","Date":"Fri, 13 Sep 2024 12:44:08 +0200","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 6/9] ipa: rkisp1: Move loader functions into helper\n\tclass","Message-ID":"","References":"<20240913075750.35115-1-stefan.klug@ideasonboard.com>\n\t<20240913075750.35115-7-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240913075750.35115-7-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\" "}}]