[{"id":39689,"web_url":"https://patchwork.libcamera.org/comment/39689/","msgid":"<178401308531.3603632.7165395202401102003@localhost>","date":"2026-07-14T07:11:25","subject":"Re: [PATCH v5 28/36] ipa: libipa: lsc_polynomial: Remove sensorSize_","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:10)\n> The sensorSize_ field is only used during the init() function and is\n> provided to LscPolynomial using the LscDescriptor type.\n> \n> There is no need to store it as a class member and to pass it in\n> at class construction time.\n> \n> Remove it and move the \\todo comment in the LscDescriptor type\n> documentation.\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Kieran Bingham \n> ---\n> src/ipa/libipa/lsc.cpp | 6 +-----\n> src/ipa/libipa/lsc_base.cpp | 3 +++\n> src/ipa/libipa/lsc_polynomial.cpp | 7 -------\n> src/ipa/libipa/lsc_polynomial.h | 6 ------\n> 4 files changed, 4 insertions(+), 18 deletions(-)\n> \n> diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp\n> index 26c3e9968072..1d2f8eb1b41b 100644\n> --- a/src/ipa/libipa/lsc.cpp\n> +++ b/src/ipa/libipa/lsc.cpp\n> @@ -215,11 +215,7 @@ int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &control\n> impl_ = std::make_unique();\n> LOG(Lsc, Debug) << \"Using table-based Lsc\";\n> } else if (type == \"polynomial\") {\n> - /*\n> - * \\todo: Most likely the reference frame should be native_size.\n> - * Let's wait how the internal discussions progress.\n> - */\n> - impl_ = std::make_unique(descriptor.sensorSize);\n> + impl_ = std::make_unique();\n\nI was looking for the code that added the new sensorSize in this patch\nand then understood that the new sensorSize was silently introduced in\n\"[v5,26/36] ipa: libipa: lsc: Make Components a map<>\". Maybe that is\nworth a comment on that patch.\n\nThe overall result is fine.\n\nReviewed-by: Stefan Klug \n\nBest regards,\nStefan\n\n> polynomial_ = true;\n> LOG(Lsc, Debug) << \"Using polynomial Lsc\";\n> } else {\n> diff --git a/src/ipa/libipa/lsc_base.cpp b/src/ipa/libipa/lsc_base.cpp\n> index 919622c6b951..adc09c15752a 100644\n> --- a/src/ipa/libipa/lsc_base.cpp\n> +++ b/src/ipa/libipa/lsc_base.cpp\n> @@ -78,6 +78,9 @@ void Interpolator::\n> * \\var LscDescriptor::sensorSize\n> * \\brief The physical sensor size. This is the largest frame size used to\n> * generate the LSC table. Only used by the polynomial LSC algorithm\n> + *\n> + * \\todo: Most likely the reference frame should be native_size.\n> + * Let's wait how the internal discussions progress.\n> */\n> \n> /**\n> diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp\n> index e17ba85448be..c61c85fbf2b2 100644\n> --- a/src/ipa/libipa/lsc_polynomial.cpp\n> +++ b/src/ipa/libipa/lsc_polynomial.cpp\n> @@ -123,13 +123,6 @@ void Polynomial::setReferenceImageSize(const Size &size)\n> * \\sa LscImplementation\n> */\n> \n> -/**\n> - * \\fn LscPolynomial::LscPolynomial\n> - * \\param[in] sensorSize The physical sensor size\n> - *\n> - * Construct an LscPolynomial\n> - */\n> -\n> /**\n> * \\brief Parse polynomial LSC data\n> * \\param[in] sets The tuning file content\n> diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h\n> index 2a26db499d68..6d19e46a50b7 100644\n> --- a/src/ipa/libipa/lsc_polynomial.h\n> +++ b/src/ipa/libipa/lsc_polynomial.h\n> @@ -57,11 +57,6 @@ private:\n> using PolynomialComponentsMap = std::map;\n> \n> public:\n> - LscPolynomial(const Size &sensorSize)\n> - : sensorSize_(sensorSize)\n> - {\n> - }\n> -\n> int parseLscData(const ValueNode &sets,\n> const LscDescriptor &descriptor) override;\n> \n> @@ -75,7 +70,6 @@ private:\n> Span yPositions,\n> const Rectangle &cropRectangle);\n> PolynomialComponentsMap lscData_;\n> - Size sensorSize_;\n> };\n> \n> } /* namespace ipa */\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 019A3C328C\n\tfor ;\n\tTue, 14 Jul 2026 07:11:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D24286612B;\n\tTue, 14 Jul 2026 09:11:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65352660AE\n\tfor ;\n\tTue, 14 Jul 2026 09:11:28 +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 43B2A103F;\n\tTue, 14 Jul 2026 09:10:34 +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=\"HoUxT1zq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1784013034;\n\tbh=i6cfhwMsmc2N/118G66sFvdlI/4h9QgEH+RLDsG5XvY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=HoUxT1zqXUScKwmPTXxgQnZEBEA0Gnl+KhRI3agOpORYHsTUsyqh0QQZev9TZd6zf\n\tSgnl0PKbDKQ7zASrJUwkbBd0/Gg3VaJHQdXehIzTHnjTPTeGQ8JpXp000dZOH2GOiP\n\t6k7p2IWPPQyPg0JsQCFOCQWaOjSVQ8DRvavJC9Kk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260708-libipa-algorithms-v5-28-0759d0359f52@ideasonboard.com>","References":"<20260708-libipa-algorithms-v5-0-0759d0359f52@ideasonboard.com>\n\t<20260708-libipa-algorithms-v5-28-0759d0359f52@ideasonboard.com>","Subject":"Re: [PATCH v5 28/36] ipa: libipa: lsc_polynomial: Remove sensorSize_","From":"Stefan Klug ","Cc":"Jacopo Mondi ,\n\tKieran Bingham ","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 14 Jul 2026 09:11:25 +0200","Message-ID":"<178401308531.3603632.7165395202401102003@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\" "}},{"id":39692,"web_url":"https://patchwork.libcamera.org/comment/39692/","msgid":"","date":"2026-07-14T08:10:14","subject":"Re: [PATCH v5 28/36] ipa: libipa: lsc_polynomial: Remove sensorSize_","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Tue, Jul 14, 2026 at 09:11:25AM +0200, Stefan Klug wrote:\n> Hi Jacopo,\n>\n> Quoting Jacopo Mondi (2026-07-08 17:51:10)\n> > The sensorSize_ field is only used during the init() function and is\n> > provided to LscPolynomial using the LscDescriptor type.\n> >\n> > There is no need to store it as a class member and to pass it in\n> > at class construction time.\n> >\n> > Remove it and move the \\todo comment in the LscDescriptor type\n> > documentation.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > src/ipa/libipa/lsc.cpp | 6 +-----\n> > src/ipa/libipa/lsc_base.cpp | 3 +++\n> > src/ipa/libipa/lsc_polynomial.cpp | 7 -------\n> > src/ipa/libipa/lsc_polynomial.h | 6 ------\n> > 4 files changed, 4 insertions(+), 18 deletions(-)\n> >\n> > diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp\n> > index 26c3e9968072..1d2f8eb1b41b 100644\n> > --- a/src/ipa/libipa/lsc.cpp\n> > +++ b/src/ipa/libipa/lsc.cpp\n> > @@ -215,11 +215,7 @@ int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &control\n> > impl_ = std::make_unique();\n> > LOG(Lsc, Debug) << \"Using table-based Lsc\";\n> > } else if (type == \"polynomial\") {\n> > - /*\n> > - * \\todo: Most likely the reference frame should be native_size.\n> > - * Let's wait how the internal discussions progress.\n> > - */\n> > - impl_ = std::make_unique(descriptor.sensorSize);\n> > + impl_ = std::make_unique();\n>\n> I was looking for the code that added the new sensorSize in this patch\n> and then understood that the new sensorSize was silently introduced in\n> \"[v5,26/36] ipa: libipa: lsc: Make Components a map<>\". Maybe that is\n\nThe LscPolynomial class always had sensorSize provided to the class\nconstructor. It comes from the LscPolynomialLoader\n\n LscPolynomialLoader(const Size &sensorSize)\n : sensorSize_(sensorSize)\n {\n }\n\n> worth a comment on that patch.\n>\n> The overall result is fine.\n>\n> Reviewed-by: Stefan Klug \n\nThanks\n j\n>\n> Best regards,\n> Stefan\n>\n> > polynomial_ = true;\n> > LOG(Lsc, Debug) << \"Using polynomial Lsc\";\n> > } else {\n> > diff --git a/src/ipa/libipa/lsc_base.cpp b/src/ipa/libipa/lsc_base.cpp\n> > index 919622c6b951..adc09c15752a 100644\n> > --- a/src/ipa/libipa/lsc_base.cpp\n> > +++ b/src/ipa/libipa/lsc_base.cpp\n> > @@ -78,6 +78,9 @@ void Interpolator::\n> > * \\var LscDescriptor::sensorSize\n> > * \\brief The physical sensor size. This is the largest frame size used to\n> > * generate the LSC table. Only used by the polynomial LSC algorithm\n> > + *\n> > + * \\todo: Most likely the reference frame should be native_size.\n> > + * Let's wait how the internal discussions progress.\n> > */\n> >\n> > /**\n> > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp\n> > index e17ba85448be..c61c85fbf2b2 100644\n> > --- a/src/ipa/libipa/lsc_polynomial.cpp\n> > +++ b/src/ipa/libipa/lsc_polynomial.cpp\n> > @@ -123,13 +123,6 @@ void Polynomial::setReferenceImageSize(const Size &size)\n> > * \\sa LscImplementation\n> > */\n> >\n> > -/**\n> > - * \\fn LscPolynomial::LscPolynomial\n> > - * \\param[in] sensorSize The physical sensor size\n> > - *\n> > - * Construct an LscPolynomial\n> > - */\n> > -\n> > /**\n> > * \\brief Parse polynomial LSC data\n> > * \\param[in] sets The tuning file content\n> > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h\n> > index 2a26db499d68..6d19e46a50b7 100644\n> > --- a/src/ipa/libipa/lsc_polynomial.h\n> > +++ b/src/ipa/libipa/lsc_polynomial.h\n> > @@ -57,11 +57,6 @@ private:\n> > using PolynomialComponentsMap = std::map;\n> >\n> > public:\n> > - LscPolynomial(const Size &sensorSize)\n> > - : sensorSize_(sensorSize)\n> > - {\n> > - }\n> > -\n> > int parseLscData(const ValueNode &sets,\n> > const LscDescriptor &descriptor) override;\n> >\n> > @@ -75,7 +70,6 @@ private:\n> > Span yPositions,\n> > const Rectangle &cropRectangle);\n> > PolynomialComponentsMap lscData_;\n> > - Size sensorSize_;\n> > };\n> >\n> > } /* namespace ipa */\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 EE62CBF415\n\tfor ;\n\tTue, 14 Jul 2026 08:10:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 06B9066135;\n\tTue, 14 Jul 2026 10:10:19 +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 9E66A660AE\n\tfor ;\n\tTue, 14 Jul 2026 10:10:17 +0200 (CEST)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 577ADBE;\n\tTue, 14 Jul 2026 10:09:23 +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=\"kcjLxviO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1784016563;\n\tbh=phS9Q2nDzE4tcDoLtCAueKnqamK/2LlFrwCuBNj3r5Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kcjLxviOzQJ/QDqwyAoGcyYV31tG3lG8iO+O40VHVM7bCXcbQ53HJtI3H7FI4x39r\n\t3ZC5enm3cnhVdc0vgw/3wth4kKQntR+r1ksm3nFAK7J5HSDSp+M6StaHcyIvBNspYh\n\ttNMmqrZ/BAGfJoXbzQGewnfYkD7uXpvDxuGRV5zc=","Date":"Tue, 14 Jul 2026 10:10:14 +0200","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","Subject":"Re: [PATCH v5 28/36] ipa: libipa: lsc_polynomial: Remove sensorSize_","Message-ID":"","References":"<20260708-libipa-algorithms-v5-0-0759d0359f52@ideasonboard.com>\n\t<20260708-libipa-algorithms-v5-28-0759d0359f52@ideasonboard.com>\n\t<178401308531.3603632.7165395202401102003@localhost>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<178401308531.3603632.7165395202401102003@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\" "}}]