[{"id":37534,"web_url":"https://patchwork.libcamera.org/comment/37534/","msgid":"<176790378067.4091855.8325011703936528515@ping.linuxembedded.co.uk>","date":"2026-01-08T20:23:00","subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2026-01-08 16:05:52)\n> The quantization functionality in the Interpolator hinders in writing\n> nice code. Don't use it and implement the functionality directly in the\n> algorithm. This patch doesn't introduce any functional changes.\n> \n> Signed-off-by: Stefan Klug \n> \n> ---\n> \n> Changes in v2:\n> - Added this patch\n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 19 +++++++++++++------\n> 1 file changed, 13 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index c42f74557c7364935254d75ee1fb923d176e0dbd..d5e44021906bda22f16a2c0f2a344e4c40d46774 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -308,12 +308,16 @@ std::vector parseSizes(const YamlObject &tuningData,\n> return sizes;\n> }\n> \n> +static unsigned int quantize(unsigned int value, unsigned int step)\n> +{\n> + return std::lround(value / static_cast(step)) * step;\n> +}\n> +\n> } /* namespace */\n> \n> LensShadingCorrection::LensShadingCorrection()\n> : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n> {\n> - sets_.setQuantization(kColourTemperatureChangeThreshhold);\n> }\n> \n> /**\n> @@ -426,17 +430,20 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n> RkISP1Params *params)\n> {\n> uint32_t ct = frameContext.awb.temperatureK;\n> - if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n> - kColourTemperatureChangeThreshhold)\n> + unsigned int quantizedCt = quantize(ct, kColourTemperatureChangeThreshhold);\n> + int ctDiff = static_cast(ct) - static_cast(lastAppliedCt_);\n> +\n> + if (quantizedCt == lastAppliedQuantizedCt_)\n> return;\n\nIs this one redundant? I think it would also return on the next ctDiff\ncheck too? But maybe there's an edge case I'm not thinking about.\n\nAnyway, I don't think it's 'wrong'. If the quantizedCt is the same, then\nwe'll generate the same table so no operations required!\n\n\nReviewed-by: Kieran Bingham \n\n\n\n> - unsigned int quantizedCt;\n> - const Components &set = sets_.getInterpolated(ct, &quantizedCt);\n> - if (lastAppliedQuantizedCt_ == quantizedCt)\n> +\n> + if (std::abs(ctDiff) < kColourTemperatureChangeThreshhold)\n> return;\n> \n> auto config = params->block();\n> config.setEnabled(true);\n> setParameters(*config);\n> +\n> + const Components &set = sets_.getInterpolated(quantizedCt);\n> copyTable(*config, set);\n> \n> lastAppliedCt_ = ct;\n> \n> -- \n> 2.51.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 3D331BDCBF\n\tfor ;\n\tThu, 8 Jan 2026 20:23:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59D0F61FBB;\n\tThu, 8 Jan 2026 21:23:05 +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 A531C615B2\n\tfor ;\n\tThu, 8 Jan 2026 21:23:03 +0100 (CET)","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 D867D103D;\n\tThu, 8 Jan 2026 21:22: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=\"ZKUc0xs+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1767903760;\n\tbh=kV/p9bUeHMj0m9F2kRzjWVRDU9WPe80JBzzWL19FYoA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ZKUc0xs+CkzJZXyTUPLF6q8c/ILIHCPsli9dCuHjeNM43bwMyi30PRbGNx/B6SMOM\n\tkNZPo/9G5GDIHXzGXMjq7nbGLsKFs//7waMZBSj3c0xkLLzAjo4iOpA50gcsOlrjSo\n\t9HGCWCuRS5imSWCJcsLdXFc2QFoZ+c1GMCIDPWgo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260108-sklug-lsc-resampling-v2-dev-v2-9-e682ec4b9893@ideasonboard.com>","References":"<20260108-sklug-lsc-resampling-v2-dev-v2-0-e682ec4b9893@ideasonboard.com>\n\t<20260108-sklug-lsc-resampling-v2-dev-v2-9-e682ec4b9893@ideasonboard.com>","Subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","From":"Kieran Bingham ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 08 Jan 2026 20:23:00 +0000","Message-ID":"<176790378067.4091855.8325011703936528515@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\" "}},{"id":37538,"web_url":"https://patchwork.libcamera.org/comment/37538/","msgid":"<3573c764-fbae-453a-ad16-c48ab0668e62@ideasonboard.com>","date":"2026-01-09T10:03:50","subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 08. 21:23 keltezéssel, Kieran Bingham írta:\n> Quoting Stefan Klug (2026-01-08 16:05:52)\n>> The quantization functionality in the Interpolator hinders in writing\n>> nice code. Don't use it and implement the functionality directly in the\n>> algorithm. This patch doesn't introduce any functional changes.\n>>\n>> Signed-off-by: Stefan Klug \n>>\n>> ---\n>>\n>> Changes in v2:\n>> - Added this patch\n>> ---\n>> src/ipa/rkisp1/algorithms/lsc.cpp | 19 +++++++++++++------\n>> 1 file changed, 13 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n>> index c42f74557c7364935254d75ee1fb923d176e0dbd..d5e44021906bda22f16a2c0f2a344e4c40d46774 100644\n>> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n>> @@ -308,12 +308,16 @@ std::vector parseSizes(const YamlObject &tuningData,\n>> return sizes;\n>> }\n>>\n>> +static unsigned int quantize(unsigned int value, unsigned int step)\n\nNo `static` needed.\n\n\n>> +{\n>> + return std::lround(value / static_cast(step)) * step;\n>> +}\n>> +\n>> } /* namespace */\n>>\n>> LensShadingCorrection::LensShadingCorrection()\n>> : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n>> {\n>> - sets_.setQuantization(kColourTemperatureChangeThreshhold);\n>> }\n>>\n>> /**\n>> @@ -426,17 +430,20 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n>> RkISP1Params *params)\n>> {\n>> uint32_t ct = frameContext.awb.temperatureK;\n>> - if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n>> - kColourTemperatureChangeThreshhold)\n>> + unsigned int quantizedCt = quantize(ct, kColourTemperatureChangeThreshhold);\n>> + int ctDiff = static_cast(ct) - static_cast(lastAppliedCt_);\n>> +\n>> + if (quantizedCt == lastAppliedQuantizedCt_)\n>> return;\n> \n> Is this one redundant? I think it would also return on the next ctDiff\n> check too? But maybe there's an edge case I'm not thinking about.\n\nIf I haven't made a mistake, then my calculations also suggest that it\nis redundant. That is `quantizedCt == lastAppliedQuantizedCt_` implies\nthat `std::abs(ctDiff) < kColourTemperatureChangeThreshhold`, so the\nearlier condition may be dropped; especially since there are no \"expensive\"\ncalculations between the two. (That is, of course, if my calculations are\ncorrect.)\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Anyway, I don't think it's 'wrong'. If the quantizedCt is the same, then\n> we'll generate the same table so no operations required!\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> \n> \n>> - unsigned int quantizedCt;\n>> - const Components &set = sets_.getInterpolated(ct, &quantizedCt);\n>> - if (lastAppliedQuantizedCt_ == quantizedCt)\n>> +\n>> + if (std::abs(ctDiff) < kColourTemperatureChangeThreshhold)\n>> return;\n>>\n>> auto config = params->block();\n>> config.setEnabled(true);\n>> setParameters(*config);\n>> +\n>> + const Components &set = sets_.getInterpolated(quantizedCt);\n>> copyTable(*config, set);\n>>\n>> lastAppliedCt_ = ct;\n>>\n>> --\n>> 2.51.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 A0380BE08B\n\tfor ;\n\tFri, 9 Jan 2026 10:03:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 67A3C61FBB;\n\tFri, 9 Jan 2026 11:03:56 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DC41B606D5\n\tfor ;\n\tFri, 9 Jan 2026 11:03:54 +0100 (CET)","from [192.168.33.17] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 948926DF;\n\tFri, 9 Jan 2026 11:03:31 +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=\"mnuEWzXi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1767953011;\n\tbh=f943y19J4PFmqS6Z95OCvNgr1cklIf6hTelZw7/d4OI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=mnuEWzXiwQrAEdK0uCzBhEgsS3qQDDHZ5IVtPWEoISykekQEZ0zLvXJ4GdokJAPbT\n\tWXwYnrokrqKmQOW26pIl6/CzdkEgP6IwdeaOfN1DG2w6ZmY9mS3iYeyFQWAt5K9EVJ\n\t2Xj4VLBcpzfU7Din/b3PqVpt11vZenKNIj/mi5Ts=","Message-ID":"<3573c764-fbae-453a-ad16-c48ab0668e62@ideasonboard.com>","Date":"Fri, 9 Jan 2026 11:03:50 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","To":"Kieran Bingham ,\n\tStefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260108-sklug-lsc-resampling-v2-dev-v2-0-e682ec4b9893@ideasonboard.com>\n\t<20260108-sklug-lsc-resampling-v2-dev-v2-9-e682ec4b9893@ideasonboard.com>\n\t\n\t<176790378067.4091855.8325011703936528515@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176790378067.4091855.8325011703936528515@ping.linuxembedded.co.uk>","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":37554,"web_url":"https://patchwork.libcamera.org/comment/37554/","msgid":"<176820930471.21455.6937501759740724806@localhost>","date":"2026-01-12T09:15:04","subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Barnabás,\n\nQuoting Barnabás Pőcze (2026-01-09 11:03:50)\n> 2026. 01. 08. 21:23 keltezéssel, Kieran Bingham írta:\n> > Quoting Stefan Klug (2026-01-08 16:05:52)\n> >> The quantization functionality in the Interpolator hinders in writing\n> >> nice code. Don't use it and implement the functionality directly in the\n> >> algorithm. This patch doesn't introduce any functional changes.\n> >>\n> >> Signed-off-by: Stefan Klug \n> >>\n> >> ---\n> >>\n> >> Changes in v2:\n> >> - Added this patch\n> >> ---\n> >> src/ipa/rkisp1/algorithms/lsc.cpp | 19 +++++++++++++------\n> >> 1 file changed, 13 insertions(+), 6 deletions(-)\n> >>\n> >> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> >> index c42f74557c7364935254d75ee1fb923d176e0dbd..d5e44021906bda22f16a2c0f2a344e4c40d46774 100644\n> >> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> >> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> >> @@ -308,12 +308,16 @@ std::vector parseSizes(const YamlObject &tuningData,\n> >> return sizes;\n> >> }\n> >>\n> >> +static unsigned int quantize(unsigned int value, unsigned int step)\n> \n> No `static` needed.\n\nSure.\n\n> \n> \n> >> +{\n> >> + return std::lround(value / static_cast(step)) * step;\n> >> +}\n> >> +\n> >> } /* namespace */\n> >>\n> >> LensShadingCorrection::LensShadingCorrection()\n> >> : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n> >> {\n> >> - sets_.setQuantization(kColourTemperatureChangeThreshhold);\n> >> }\n> >>\n> >> /**\n> >> @@ -426,17 +430,20 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n> >> RkISP1Params *params)\n> >> {\n> >> uint32_t ct = frameContext.awb.temperatureK;\n> >> - if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n> >> - kColourTemperatureChangeThreshhold)\n> >> + unsigned int quantizedCt = quantize(ct, kColourTemperatureChangeThreshhold);\n> >> + int ctDiff = static_cast(ct) - static_cast(lastAppliedCt_);\n> >> +\n> >> + if (quantizedCt == lastAppliedQuantizedCt_)\n> >> return;\n> > \n> > Is this one redundant? I think it would also return on the next ctDiff\n> > check too? But maybe there's an edge case I'm not thinking about.\n> \n> If I haven't made a mistake, then my calculations also suggest that it\n> is redundant. That is `quantizedCt == lastAppliedQuantizedCt_` implies\n> that `std::abs(ctDiff) < kColourTemperatureChangeThreshhold`, so the\n> earlier condition may be dropped; especially since there are no \"expensive\"\n> calculations between the two. (That is, of course, if my calculations are\n> correct.)\n\nNot exactly. Assume we have a quantization of 10, and the input value\noscillates between 44 and 46, then the logic would constantly toggle\nbetween 40 and 50 which could produce visible flickering. So we need a\nthreshold on the input to prevent flickering. With the current threshold\nbeing equal to the quantization we could actually drop the second check\nas the quantization has definitely changed when the threshold was\nreached. There is one downside though. In the above example if you hit\n44, you will not reach a quantization of 50 when the value goes up, as\nthe next point that will be applied is 55 and therefore quantized to 60.\nSo writing this, I think we should reduce the threshold to\nquantization/2 (and possible increase the quantization) but keep the\nlogic (maybe revert the order of ifs to test for the threshold first and\nthen for the quantization which makes it easier to follow logically.\n\nBest regards,\nStefan\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> > \n> > Anyway, I don't think it's 'wrong'. If the quantizedCt is the same, then\n> > we'll generate the same table so no operations required!\n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > \n> > \n> >> - unsigned int quantizedCt;\n> >> - const Components &set = sets_.getInterpolated(ct, &quantizedCt);\n> >> - if (lastAppliedQuantizedCt_ == quantizedCt)\n> >> +\n> >> + if (std::abs(ctDiff) < kColourTemperatureChangeThreshhold)\n> >> return;\n> >>\n> >> auto config = params->block();\n> >> config.setEnabled(true);\n> >> setParameters(*config);\n> >> +\n> >> + const Components &set = sets_.getInterpolated(quantizedCt);\n> >> copyTable(*config, set);\n> >>\n> >> lastAppliedCt_ = ct;\n> >>\n> >> --\n> >> 2.51.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 200FBBDCBF\n\tfor ;\n\tMon, 12 Jan 2026 09:15:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4FE6361FBB;\n\tMon, 12 Jan 2026 10:15:09 +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 CD38661F9F\n\tfor ;\n\tMon, 12 Jan 2026 10:15:07 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:e910:f383:3e14:f5be])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 872B663F;\n\tMon, 12 Jan 2026 10:14:42 +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=\"hncIJPvr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768209282;\n\tbh=YWCAnWb3UMHo2zBVhxNaApEfE7AeuKT8xYgmFAdRfOU=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=hncIJPvrMIb8SvDtIZxsytaX02oRYrIzCla+vzGFZ3TiFFLlfxGhLP7zucSoG+qpr\n\tBFuX//Jx9GfIwYYRi8J6itRFCfsQHbK9Krb9wJOEbhkOBlMVWIT+hM61LE08eKfyxL\n\tH29A0MXRXxQHc0CtrzHaz1v6hpULkRNrM+OtfrTo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<3573c764-fbae-453a-ad16-c48ab0668e62@ideasonboard.com>","References":"<20260108-sklug-lsc-resampling-v2-dev-v2-0-e682ec4b9893@ideasonboard.com>\n\t<20260108-sklug-lsc-resampling-v2-dev-v2-9-e682ec4b9893@ideasonboard.com>\n\t\n\t<176790378067.4091855.8325011703936528515@ping.linuxembedded.co.uk>\n\t<3573c764-fbae-453a-ad16-c48ab0668e62@ideasonboard.com>","Subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","From":"Stefan Klug ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tKieran Bingham , \n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 12 Jan 2026 10:15:04 +0100","Message-ID":"<176820930471.21455.6937501759740724806@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":37568,"web_url":"https://patchwork.libcamera.org/comment/37568/","msgid":"","date":"2026-01-12T12:02:47","subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 12. 10:15 keltezéssel, Stefan Klug írta:\n> Hi Barnabás,\n> \n> Quoting Barnabás Pőcze (2026-01-09 11:03:50)\n>> 2026. 01. 08. 21:23 keltezéssel, Kieran Bingham írta:\n>>> Quoting Stefan Klug (2026-01-08 16:05:52)\n>>>> The quantization functionality in the Interpolator hinders in writing\n>>>> nice code. Don't use it and implement the functionality directly in the\n>>>> algorithm. This patch doesn't introduce any functional changes.\n>>>>\n>>>> Signed-off-by: Stefan Klug \n>>>>\n>>>> ---\n>>>>\n>>>> Changes in v2:\n>>>> - Added this patch\n>>>> ---\n>>>> src/ipa/rkisp1/algorithms/lsc.cpp | 19 +++++++++++++------\n>>>> 1 file changed, 13 insertions(+), 6 deletions(-)\n>>>>\n>>>> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n>>>> index c42f74557c7364935254d75ee1fb923d176e0dbd..d5e44021906bda22f16a2c0f2a344e4c40d46774 100644\n>>>> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n>>>> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n>>>> @@ -308,12 +308,16 @@ std::vector parseSizes(const YamlObject &tuningData,\n>>>> return sizes;\n>>>> }\n>>>>\n>>>> +static unsigned int quantize(unsigned int value, unsigned int step)\n>>\n>> No `static` needed.\n> \n> Sure.\n> \n>>\n>>\n>>>> +{\n>>>> + return std::lround(value / static_cast(step)) * step;\n>>>> +}\n>>>> +\n>>>> } /* namespace */\n>>>>\n>>>> LensShadingCorrection::LensShadingCorrection()\n>>>> : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n>>>> {\n>>>> - sets_.setQuantization(kColourTemperatureChangeThreshhold);\n>>>> }\n>>>>\n>>>> /**\n>>>> @@ -426,17 +430,20 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n>>>> RkISP1Params *params)\n>>>> {\n>>>> uint32_t ct = frameContext.awb.temperatureK;\n>>>> - if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) <\n>>>> - kColourTemperatureChangeThreshhold)\n>>>> + unsigned int quantizedCt = quantize(ct, kColourTemperatureChangeThreshhold);\n>>>> + int ctDiff = static_cast(ct) - static_cast(lastAppliedCt_);\n>>>> +\n>>>> + if (quantizedCt == lastAppliedQuantizedCt_)\n>>>> return;\n>>>\n>>> Is this one redundant? I think it would also return on the next ctDiff\n>>> check too? But maybe there's an edge case I'm not thinking about.\n>>\n>> If I haven't made a mistake, then my calculations also suggest that it\n>> is redundant. That is `quantizedCt == lastAppliedQuantizedCt_` implies\n>> that `std::abs(ctDiff) < kColourTemperatureChangeThreshhold`, so the\n>> earlier condition may be dropped; especially since there are no \"expensive\"\n>> calculations between the two. (That is, of course, if my calculations are\n>> correct.)\n> \n> Not exactly. Assume we have a quantization of 10, and the input value\n> oscillates between 44 and 46, then the logic would constantly toggle\n> between 40 and 50 which could produce visible flickering. So we need a\n> threshold on the input to prevent flickering. With the current threshold\n> being equal to the quantization we could actually drop the second check\n> as the quantization has definitely changed when the threshold was\n> reached. There is one downside though. In the above example if you hit\n\nMaybe I'm misunderstanding, but all I'm saying is that if `quantizedCt == lastAppliedQuantizedCt_`\nthen `std::abs(ctDiff) < kColourTemperatureChangeThreshhold`. Thus the\n`quantizedCt == lastAppliedQuantizedCt_` check can be dropped. This is of\ncourse assuming that the quantization coefficient and the threshold are the same.\n\nIf the `quantizedCt == lastAppliedQuantizedCt_` check is removed, then oscillation\nbetween 44 and 46 will still be \"ignored\" because `std::abs(ctDiff) < kColourTemperatureChangeThreshhold`\nholds true.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> 44, you will not reach a quantization of 50 when the value goes up, as\n> the next point that will be applied is 55 and therefore quantized to 60.\n> So writing this, I think we should reduce the threshold to\n> quantization/2 (and possible increase the quantization) but keep the\n> logic (maybe revert the order of ifs to test for the threshold first and\n> then for the quantization which makes it easier to follow logically.\n> \n> Best regards,\n> Stefan\n> \n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>>\n>>> Anyway, I don't think it's 'wrong'. If the quantizedCt is the same, then\n>>> we'll generate the same table so no operations required!\n>>>\n>>>\n>>> Reviewed-by: Kieran Bingham \n>>>\n>>>\n>>>\n>>>> - unsigned int quantizedCt;\n>>>> - const Components &set = sets_.getInterpolated(ct, &quantizedCt);\n>>>> - if (lastAppliedQuantizedCt_ == quantizedCt)\n>>>> +\n>>>> + if (std::abs(ctDiff) < kColourTemperatureChangeThreshhold)\n>>>> return;\n>>>>\n>>>> auto config = params->block();\n>>>> config.setEnabled(true);\n>>>> setParameters(*config);\n>>>> +\n>>>> + const Components &set = sets_.getInterpolated(quantizedCt);\n>>>> copyTable(*config, set);\n>>>>\n>>>> lastAppliedCt_ = ct;\n>>>>\n>>>> --\n>>>> 2.51.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 6DC7DBDCBF\n\tfor ;\n\tMon, 12 Jan 2026 12:02:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B1E961FA0;\n\tMon, 12 Jan 2026 13:02:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 117E06142F\n\tfor ;\n\tMon, 12 Jan 2026 13:02:51 +0100 (CET)","from [192.168.33.29] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 69B59165B;\n\tMon, 12 Jan 2026 13:02:25 +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=\"NF3OMRBy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768219345;\n\tbh=GPfHhfBcMAQCtFGa6iXgvzuGHwussW1pvr6sXUd9rac=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=NF3OMRByk+IzDc9IVzhLzS2cU8rXh26uXWhx4i1+u3uvAZINj76wiJ4dXb1XRdqng\n\t9xXt1gPy+G+TbwdH3JVFba63tzkj8Bm/+GLnDd8MDZNBQEZV8G5BK3wDs1wm+AdnpV\n\tujritTrvUmB24jyW5avET6HFqGanmpMHpOcqUsY8=","Message-ID":"","Date":"Mon, 12 Jan 2026 13:02:47 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 09/15] ipa: rkisp1: lsc: Handle quantization locally","To":"Stefan Klug ,\n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260108-sklug-lsc-resampling-v2-dev-v2-0-e682ec4b9893@ideasonboard.com>\n\t<20260108-sklug-lsc-resampling-v2-dev-v2-9-e682ec4b9893@ideasonboard.com>\n\t\n\t<176790378067.4091855.8325011703936528515@ping.linuxembedded.co.uk>\n\t<3573c764-fbae-453a-ad16-c48ab0668e62@ideasonboard.com>\n\t<176820930471.21455.6937501759740724806@localhost>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<176820930471.21455.6937501759740724806@localhost>","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\" "}}]