[{"id":39773,"web_url":"https://patchwork.libcamera.org/comment/39773/","msgid":"<85v7a8zfzf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-07-21T12:25:56","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Jacopo,\n\nJacopo Mondi writes:\n\n> The series is identical to v5 up to patch 27. The only minor difference\n> is that I back-tracked on making functions class members but in the\n> RkISP1 IPA I kept them as static helpers.\n>\n> Patches 28 and 29 are new.\n>\n> They remove the templating of the LscAlgorithm class hierarchy and\n> instead implement quantization in the LscAlgorithm class.\n>\n> So we now expand polynomials and parse tables as floats, store them\n> in a map and in LscAlgorithm we re-iterate over it to either quantized\n> them in the polynomial case, or simply cast floats to the register bit\n> width if table.\n>\n> Timing the duration of the whole RkISP1 configure operation, it now\n> takes 3 msec compared to 600 usec.\n>\n> \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n>\n> \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n>\n> Also, now each IPA has to define their own Interpolator::interpolate()\n> overload (see rkisp1 IPA as an example of this).\n>\n> As the interface of LscAlgorithm has changed, patches on the list that\n> port the LSC algorithm of other platforms based on v5 will have to be\n> rebased.\n\nI'm having trouble to understand the exact motivation for the change.\nFrom the libipa consumer side, it means that for `simple' pipeline, I\nhad to copy & substitute & paste code from rkisp1 for no very good\nreason. Which looks like a step against the code unification goal,\nwhile it doesn't seem to simplify rkisp1 that much.\n\nThe float representation could be useful for software ISP, where there\nis no inherent reason to use quantized values (although they may still\nhave its use for portable textures, as pointed out by Bryan). But it\nseems using quantized values is still imposed on libipa LSC consumers?\n\n>\n>\n> Signed-off-by: Jacopo Mondi \n> ---\n> Changes in v6:\n> - Address comments received on v5, mostly on documentation\n> - Remove templating of LscPolynomial and perform quantization in\n> LscAlgorithm\n>\n> Changes in v5:\n> - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> to fix and exiting potential issue with Polynomial parsing\n> - Fix Simple, which I broke in v4, thanks Milan\n>\n> Changes in v4:\n> - s/awb/AWB in comments everywhere, same for CCM and LSC\n> - Awb: changelog per patch\n> - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> modules\n> - Reduce comments/renames to reduce patch size and comments\n> - Collect tags\n>\n> Changes in v3:\n> - Address comments on Awb from Stefan:\n> - Introduce AwbImplementation::Result\n> - Move AwbStats implementation to IPAs\n> - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> - Fix Saturation bug in Simple Ccm algorithm\n> - Fix Simple Ccm gain limits using Q<2, 8>\n> - Update Mali CCM and Gamma to latest uAPI patch version\n> - Fix Mali CCM gain masking as reported by Dan\n> - Break down libIPA Lsc implementation as requested by Stefan\n>\n> Changes in v2:\n> - Introduce awb::Context to simplify ActiveState and FrameContext\n> definition\n> - Add mali uAPI header for CCM and Gamma\n> - Do not rename Mali functions for hardware configuration in awb and lsc\n> - Spelling and documentation\n>\n> ---\n> Jacopo Mondi (28):\n> ipa: libipa: awb: Reimplement AwbAlgorithm\n> ipa: mali-c55: awb: Port to use libipa AwbAlgorithm\n> ipa: libipa: Add CcmAlgorithm to libipa\n> ipa: rkisp1: ccm: Port to use CcmAlgorithm\n> include: linux: mali-c55: Update to support CCM and Gamma\n> ipa: mali-c55: Implement Ccm algorithm\n> ipa: rkisp1: lsc: Re-architecture Lsc support\n> ipa: libipa: Introduce lsc_base.h\n> ipa: libipa: lsc_polynomial: Fix polynomial parsing error\n> ipa: libipa: lsc_polynomial: Rename LscPolynomial\n> ipa: libipa: lsc_polynomial: Do not inline functions\n> ipa: libipa: lsc: Move Interpolator to lsc_base\n> ipa: rkisp1: lsc: Remove rogue empty line\n> ipa: libipa: lsc_polynomial: Move LscPolynomial from RkISP1\n> ipa: libipa: lsc_table: Move LscTable from RkISP1\n> ipa: libipa: lsc_base: Add sampleForCrop documentation\n> ipa: libipa: lsc: Introduce LscAlgorithm\n> ipa: libipa: lsc: Move sizesListToPositions() back to rkisp1\n> ipa: rkisp1: lsc: Port to use LscAlgorithm\n> ipa: libipa: lsc: Make Components a map<>\n> ipa: libipa: lsc: Document Components parsing\n> ipa: libipa: lsc_polynomial: Remove sensorSize_\n> ipa: libipa: lsc: Document LscDescriptor::sensorSize usage\n> ipa: libipa: lsc: Quantize lsc gains in IPA\n> ipa: libipa: lsc: Quantize gains in LscAlgorithm\n> ipa: libipa: lsc: Re-sort LscAlgorithmBase documentation\n> ipa: mali-c55: Add sensorInfo to IPAContext\n> ipa: mali-c55: Port to use LscAlgorithm\n>\n> Kieran Bingham (2):\n> ipa: simple: awb: Port to use libipa AwbAlgorithm\n> ipa: simple: Port to use libipa CcmAlgorithm\n>\n> Laurent Pinchart (1):\n> ipa: libipa: lsc_polynomial: Don't inline ValueNode accessor\n>\n> .../internal/software_isp/debayer_params.h | 4 +-\n> include/linux/media/arm/mali-c55-config.h | 86 ++++-\n> src/ipa/libipa/awb.cpp | 418 +++++++++++++++++---\n> src/ipa/libipa/awb.h | 100 ++++-\n> src/ipa/libipa/awb_bayes.cpp | 52 +--\n> src/ipa/libipa/awb_bayes.h | 12 +-\n> src/ipa/libipa/awb_grey.cpp | 16 +-\n> src/ipa/libipa/awb_grey.h | 6 +-\n> src/ipa/libipa/ccm.cpp | 245 ++++++++++++\n> src/ipa/libipa/ccm.h | 77 ++++\n> src/ipa/libipa/lsc.cpp | 372 ++++++++++++++++++\n> src/ipa/libipa/lsc.h | 137 +++++++\n> src/ipa/libipa/lsc_base.cpp | 136 +++++++\n> src/ipa/libipa/lsc_base.h | 56 +++\n> src/ipa/libipa/lsc_polynomial.cpp | 208 +++++++++-\n> src/ipa/libipa/lsc_polynomial.h | 100 ++---\n> src/ipa/libipa/lsc_table.cpp | 107 +++++\n> src/ipa/libipa/lsc_table.h | 55 +++\n> src/ipa/libipa/meson.build | 8 +\n> src/ipa/mali-c55/algorithms/awb.cpp | 223 ++++++-----\n> src/ipa/mali-c55/algorithms/awb.h | 28 +-\n> src/ipa/mali-c55/algorithms/ccm.cpp | 173 +++++++++\n> src/ipa/mali-c55/algorithms/ccm.h | 66 ++++\n> src/ipa/mali-c55/algorithms/lsc.cpp | 156 +++++---\n> src/ipa/mali-c55/algorithms/lsc.h | 32 +-\n> src/ipa/mali-c55/algorithms/meson.build | 1 +\n> src/ipa/mali-c55/ipa_context.cpp | 3 +\n> src/ipa/mali-c55/ipa_context.h | 20 +-\n> src/ipa/mali-c55/mali-c55.cpp | 2 +\n> src/ipa/mali-c55/params.h | 2 +\n> src/ipa/rkisp1/algorithms/awb.cpp | 191 ++-------\n> src/ipa/rkisp1/algorithms/awb.h | 18 +-\n> src/ipa/rkisp1/algorithms/ccm.cpp | 97 +----\n> src/ipa/rkisp1/algorithms/ccm.h | 17 +-\n> src/ipa/rkisp1/algorithms/lsc.cpp | 429 +++------------------\n> src/ipa/rkisp1/algorithms/lsc.h | 74 ++--\n> src/ipa/rkisp1/ipa_context.h | 49 +--\n> src/ipa/simple/algorithms/awb.cpp | 140 +++++--\n> src/ipa/simple/algorithms/awb.h | 28 ++\n> src/ipa/simple/algorithms/blc.cpp | 2 +-\n> src/ipa/simple/algorithms/ccm.cpp | 74 ++--\n> src/ipa/simple/algorithms/ccm.h | 22 +-\n> src/ipa/simple/ipa_context.h | 15 +-\n> src/libcamera/software_isp/debayer_cpu.cpp | 12 +-\n> 44 files changed, 2934 insertions(+), 1135 deletions(-)\n> ---\n> base-commit: 709ad59a8d90af3570be1c8d02dee0ec9e011954\n> change-id: 20260615-libipa-algorithms-59666bca0a2f\n>\n> Best regards,","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 408E2BDE4C\n\tfor ;\n\tTue, 21 Jul 2026 12:26:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 78C6467E62;\n\tTue, 21 Jul 2026 14:26:06 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 33DB76601C\n\tfor ;\n\tTue, 21 Jul 2026 14:26:04 +0200 (CEST)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-252-KSuwf9OFM8uPOvcoHdOnPg-1; Tue, 21 Jul 2026 08:26:00 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-495495ad5ddso28575535e9.3\n\tfor ;\n\tTue, 21 Jul 2026 05:25:59 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb ([213.175.46.86])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-495653d38dbsm61189365e9.15.2026.07.21.05.25.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 21 Jul 2026 05:25:57 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"WSmJLI8c\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1784636762;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=zn02fayp2SlSzK3KZq1vaSJhwLeqof0UwF7tj/Rwggc=;\n\tb=WSmJLI8chduC9JwFboKOXZJjMnPhWodf35pfKqlLFPVmTQ7tnZl62mqsZrF/Lz+e5tMeGy\n\tjFxJVWPKv9rQljS0CGM2/1aIn6jddVvtjmdgdVwTyBzocN/aBN1kB/5kcDTiVphoyLPCgV\n\tNioyfxJ4RV2/3dfUJjCPujE8P7N0MJA=","X-MC-Unique":"KSuwf9OFM8uPOvcoHdOnPg-1","X-Mimecast-MFC-AGG-ID":"KSuwf9OFM8uPOvcoHdOnPg_1784636759","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1784636759; x=1785241559;\n\th=content-transfer-encoding:content-type:mime-version:user-agent\n\t:message-id:date:references:in-reply-to:subject:cc:to:from:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to\n\t:content-type;\n\tbh=DqzZceHAXdTeGU/sP+/VouCsXPlB7DYkh1jFnFOhzBs=;\n\tb=iLAKDMpzQU7gcopw1L+qgNRrzjC17U01LLDZLgWGAvZt+dvorPzK7oV/eUPJBumbH3\n\tqSGmEIwsHIL7cu8A/Ef6R9tnTMdtTRSXFen2/DfGt5i+MACMNHWk7qB3nW9+cb4W7R3c\n\tYVHTdCoXCI0Wh9F70bms9QREvl0ZFkVLracVcI9KJEmoPP6IYl0mdxr62F1iavqCWlq+\n\tE7NOmM0oFQXUNNxQJ7GblUpjtZXBZV7cirdeREandV4ExSKpQX79K3SQ8Pt/ebOAajp+\n\tZUN2S+8qRzzTn+JLAn6Q61Tt7VRI3D/i1DQD84kuYngXPRh2XN/4CVfifC4l4OkLufDt\n\toPIA==","X-Gm-Message-State":"AOJu0YxqEzRZ/T2/pMXgYj9czHj1aAppnqtA3DgT6gPqDXUkaracQlM0\n\t2u6wl8ItC2y0EfxgITIk58xmQbirG2FB/5ZH7WuDSkcXSCzg/fI0wnABOxYZeymykUyGdbWKE05\n\tOqH0JOOHdsK5rfDSyUxToQs7As9hwZEnvJc+0YSP8P+RBFmurAR5VJJ4rWpHTrIyIThMBEJspaY\n\tPg+64Ae+w=","X-Gm-Gg":"AfdE7ckYKCSgn22Abu/CHgp3HBuUYk8s3AM1TlHTzyc0SL/5yRiWbiLcIRgbRvO4X3N\n\tPNXdqjl2eZ4HcIjqcJuILhNgWJFN+aT55Ii+Y9iIf4zKeorOTg3Pak1IlhEZ7qsSvBT7k2uLnYi\n\t8gfeMC8UiM3/NLjj0Py8+XpFPChvlxJNn+Zm5ZLeFYjru+5YmB2F6bkMJgjKX9pWMCoOoXbphl/\n\ty4Jwq/M563v8yEhF5cZxSix+xjHNHE1OL3dKxHr/rQD7JdxgsEDplI/NJX6DX5EAtOeTkCL0b2G\n\tFpQwTc6+Y/UCZ9bFH0tdB+L2c/+tGfaYejDWtyfNb8s1x1gRZeqLMC7Uh9PLpuJEeumk7dxgrPH\n\tpHUOU/hjTwWbgjMaR","X-Received":["by 2002:a05:600c:3114:b0:493:f278:ba2f with SMTP id\n\t5b1f17b1804b1-4954a3ed46bmr215485355e9.9.1784636758919; \n\tTue, 21 Jul 2026 05:25:58 -0700 (PDT)","by 2002:a05:600c:3114:b0:493:f278:ba2f with SMTP id\n\t5b1f17b1804b1-4954a3ed46bmr215484525e9.9.1784636758242; \n\tTue, 21 Jul 2026 05:25:58 -0700 (PDT)"],"From":"Milan Zamazal ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, Stefan Klug\n\t, Kieran Bingham\n\t, Daniel Scally\n\t,\n\tRobert Mader , \n\tLaurent Pinchart ","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","In-Reply-To":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t(Jacopo Mondi's message of \"Mon, 20 Jul 2026 16:59:18 +0200\")","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>","Date":"Tue, 21 Jul 2026 14:25:56 +0200","Message-ID":"<85v7a8zfzf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"4-Ys2Lu12BVYlyR71AKLmPrUInx-xXBkpj-XVeYkmQU_1784636759","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":39775,"web_url":"https://patchwork.libcamera.org/comment/39775/","msgid":"","date":"2026-07-21T13:08:39","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Milan\n thanks for the review\n\nOn Tue, Jul 21, 2026 at 02:25:56PM +0200, Milan Zamazal wrote:\n> Hi Jacopo,\n>\n> Jacopo Mondi writes:\n>\n> > The series is identical to v5 up to patch 27. The only minor difference\n> > is that I back-tracked on making functions class members but in the\n> > RkISP1 IPA I kept them as static helpers.\n> >\n> > Patches 28 and 29 are new.\n> >\n> > They remove the templating of the LscAlgorithm class hierarchy and\n> > instead implement quantization in the LscAlgorithm class.\n> >\n> > So we now expand polynomials and parse tables as floats, store them\n> > in a map and in LscAlgorithm we re-iterate over it to either quantized\n> > them in the polynomial case, or simply cast floats to the register bit\n> > width if table.\n> >\n> > Timing the duration of the whole RkISP1 configure operation, it now\n> > takes 3 msec compared to 600 usec.\n> >\n> > \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n> >\n> > \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n> >\n> > Also, now each IPA has to define their own Interpolator::interpolate()\n> > overload (see rkisp1 IPA as an example of this).\n> >\n> > As the interface of LscAlgorithm has changed, patches on the list that\n> > port the LSC algorithm of other platforms based on v5 will have to be\n> > rebased.\n>\n> I'm having trouble to understand the exact motivation for the change.\n\nI've been suggested to move quantization to LscAlgorithm to remove\ntemplating of LscPolynomial and LscTable.\n\n> From the libipa consumer side, it means that for `simple' pipeline, I\n> had to copy & substitute & paste code from rkisp1 for no very good\n> reason. Which looks like a step against the code unification goal,\n> while it doesn't seem to simplify rkisp1 that much.\n\nAgreed\n\n>\n> The float representation could be useful for software ISP, where there\n> is no inherent reason to use quantized values (although they may still\n> have its use for portable textures, as pointed out by Bryan). But it\n> seems using quantized values is still imposed on libipa LSC consumers?\n\nLet's put LscPolynomial aside for a second and only consider LscTable.\n\nGains in the tuning files are currently expressed in register format\n(iow already quantized). This means Lsc tables are not portable across\nplatforms. I think there is consensus on the desire to express gains\nas floats in tuning files, so that we can share lsc tables among\nplatforms. This is good and will be done on top along with table\nre-scaling to adapt them to different sensor's resolutions.\n\nOn quantization: up until v5 I templated the whole LscImplementation\nclass hierarchy with the platform's Q format, so that when the gain\nvectors are populated (when a polynomial is expanded or a\ntable-resampled in future) they are stored in register format\nready to be consumed by the IPA.\n\nThis required to template a few classes, which was not that bad to me.\nI've been suggested to remove quantization from the LscImplementation\nhierarchy, so that it's simpler and instead perform quantization\neither in the IPA module itself or in LscAlgorithm class. This\nrequires to go over the already populated arrays and quantize the\nentries one by one before passing them to the IPA. I don't think it's ideal.\n\nHowever having floats as an intermediate values might make\ninterpolation more accurate (I don't know and I don't see why,\nI'm just speculating) and the implementation is simpler.\n\nv6 requires however that each IPA defines its own\nInterpolator::interpolate() overload (maybe this could be\ngeneralized?). Is this the code that you needed to add to your IPA ?\n\nOn the gain format: simple is kind of special. It doesn't require\nquantization and you could maybe do with floats. But the same\nreasoning applies to AWB, gamma, CCM and all other algorithms, where\nyou don't have any register format to translate to. If you want floats\nin your IPA to work with, maybe using UQ<0, 32> as register format\nwould do ?\n\n>\n> >\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > Changes in v6:\n> > - Address comments received on v5, mostly on documentation\n> > - Remove templating of LscPolynomial and perform quantization in\n> > LscAlgorithm\n> >\n> > Changes in v5:\n> > - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> > to fix and exiting potential issue with Polynomial parsing\n> > - Fix Simple, which I broke in v4, thanks Milan\n> >\n> > Changes in v4:\n> > - s/awb/AWB in comments everywhere, same for CCM and LSC\n> > - Awb: changelog per patch\n> > - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> > modules\n> > - Reduce comments/renames to reduce patch size and comments\n> > - Collect tags\n> >\n> > Changes in v3:\n> > - Address comments on Awb from Stefan:\n> > - Introduce AwbImplementation::Result\n> > - Move AwbStats implementation to IPAs\n> > - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> > - Fix Saturation bug in Simple Ccm algorithm\n> > - Fix Simple Ccm gain limits using Q<2, 8>\n> > - Update Mali CCM and Gamma to latest uAPI patch version\n> > - Fix Mali CCM gain masking as reported by Dan\n> > - Break down libIPA Lsc implementation as requested by Stefan\n> >\n> > Changes in v2:\n> > - Introduce awb::Context to simplify ActiveState and FrameContext\n> > definition\n> > - Add mali uAPI header for CCM and Gamma\n> > - Do not rename Mali functions for hardware configuration in awb and lsc\n> > - Spelling and documentation\n> >\n> > ---\n> > Jacopo Mondi (28):\n> > ipa: libipa: awb: Reimplement AwbAlgorithm\n> > ipa: mali-c55: awb: Port to use libipa AwbAlgorithm\n> > ipa: libipa: Add CcmAlgorithm to libipa\n> > ipa: rkisp1: ccm: Port to use CcmAlgorithm\n> > include: linux: mali-c55: Update to support CCM and Gamma\n> > ipa: mali-c55: Implement Ccm algorithm\n> > ipa: rkisp1: lsc: Re-architecture Lsc support\n> > ipa: libipa: Introduce lsc_base.h\n> > ipa: libipa: lsc_polynomial: Fix polynomial parsing error\n> > ipa: libipa: lsc_polynomial: Rename LscPolynomial\n> > ipa: libipa: lsc_polynomial: Do not inline functions\n> > ipa: libipa: lsc: Move Interpolator to lsc_base\n> > ipa: rkisp1: lsc: Remove rogue empty line\n> > ipa: libipa: lsc_polynomial: Move LscPolynomial from RkISP1\n> > ipa: libipa: lsc_table: Move LscTable from RkISP1\n> > ipa: libipa: lsc_base: Add sampleForCrop documentation\n> > ipa: libipa: lsc: Introduce LscAlgorithm\n> > ipa: libipa: lsc: Move sizesListToPositions() back to rkisp1\n> > ipa: rkisp1: lsc: Port to use LscAlgorithm\n> > ipa: libipa: lsc: Make Components a map<>\n> > ipa: libipa: lsc: Document Components parsing\n> > ipa: libipa: lsc_polynomial: Remove sensorSize_\n> > ipa: libipa: lsc: Document LscDescriptor::sensorSize usage\n> > ipa: libipa: lsc: Quantize lsc gains in IPA\n> > ipa: libipa: lsc: Quantize gains in LscAlgorithm\n> > ipa: libipa: lsc: Re-sort LscAlgorithmBase documentation\n> > ipa: mali-c55: Add sensorInfo to IPAContext\n> > ipa: mali-c55: Port to use LscAlgorithm\n> >\n> > Kieran Bingham (2):\n> > ipa: simple: awb: Port to use libipa AwbAlgorithm\n> > ipa: simple: Port to use libipa CcmAlgorithm\n> >\n> > Laurent Pinchart (1):\n> > ipa: libipa: lsc_polynomial: Don't inline ValueNode accessor\n> >\n> > .../internal/software_isp/debayer_params.h | 4 +-\n> > include/linux/media/arm/mali-c55-config.h | 86 ++++-\n> > src/ipa/libipa/awb.cpp | 418 +++++++++++++++++---\n> > src/ipa/libipa/awb.h | 100 ++++-\n> > src/ipa/libipa/awb_bayes.cpp | 52 +--\n> > src/ipa/libipa/awb_bayes.h | 12 +-\n> > src/ipa/libipa/awb_grey.cpp | 16 +-\n> > src/ipa/libipa/awb_grey.h | 6 +-\n> > src/ipa/libipa/ccm.cpp | 245 ++++++++++++\n> > src/ipa/libipa/ccm.h | 77 ++++\n> > src/ipa/libipa/lsc.cpp | 372 ++++++++++++++++++\n> > src/ipa/libipa/lsc.h | 137 +++++++\n> > src/ipa/libipa/lsc_base.cpp | 136 +++++++\n> > src/ipa/libipa/lsc_base.h | 56 +++\n> > src/ipa/libipa/lsc_polynomial.cpp | 208 +++++++++-\n> > src/ipa/libipa/lsc_polynomial.h | 100 ++---\n> > src/ipa/libipa/lsc_table.cpp | 107 +++++\n> > src/ipa/libipa/lsc_table.h | 55 +++\n> > src/ipa/libipa/meson.build | 8 +\n> > src/ipa/mali-c55/algorithms/awb.cpp | 223 ++++++-----\n> > src/ipa/mali-c55/algorithms/awb.h | 28 +-\n> > src/ipa/mali-c55/algorithms/ccm.cpp | 173 +++++++++\n> > src/ipa/mali-c55/algorithms/ccm.h | 66 ++++\n> > src/ipa/mali-c55/algorithms/lsc.cpp | 156 +++++---\n> > src/ipa/mali-c55/algorithms/lsc.h | 32 +-\n> > src/ipa/mali-c55/algorithms/meson.build | 1 +\n> > src/ipa/mali-c55/ipa_context.cpp | 3 +\n> > src/ipa/mali-c55/ipa_context.h | 20 +-\n> > src/ipa/mali-c55/mali-c55.cpp | 2 +\n> > src/ipa/mali-c55/params.h | 2 +\n> > src/ipa/rkisp1/algorithms/awb.cpp | 191 ++-------\n> > src/ipa/rkisp1/algorithms/awb.h | 18 +-\n> > src/ipa/rkisp1/algorithms/ccm.cpp | 97 +----\n> > src/ipa/rkisp1/algorithms/ccm.h | 17 +-\n> > src/ipa/rkisp1/algorithms/lsc.cpp | 429 +++------------------\n> > src/ipa/rkisp1/algorithms/lsc.h | 74 ++--\n> > src/ipa/rkisp1/ipa_context.h | 49 +--\n> > src/ipa/simple/algorithms/awb.cpp | 140 +++++--\n> > src/ipa/simple/algorithms/awb.h | 28 ++\n> > src/ipa/simple/algorithms/blc.cpp | 2 +-\n> > src/ipa/simple/algorithms/ccm.cpp | 74 ++--\n> > src/ipa/simple/algorithms/ccm.h | 22 +-\n> > src/ipa/simple/ipa_context.h | 15 +-\n> > src/libcamera/software_isp/debayer_cpu.cpp | 12 +-\n> > 44 files changed, 2934 insertions(+), 1135 deletions(-)\n> > ---\n> > base-commit: 709ad59a8d90af3570be1c8d02dee0ec9e011954\n> > change-id: 20260615-libipa-algorithms-59666bca0a2f\n> >\n> > Best regards,\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 58685BDE4C\n\tfor ;\n\tTue, 21 Jul 2026 13:08:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AACA6601C;\n\tTue, 21 Jul 2026 15:08:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF2F16601C\n\tfor ;\n\tTue, 21 Jul 2026 15:08:42 +0200 (CEST)","from ideasonboard.com (mob-5-90-63-152.net.vodafone.it\n\t[5.90.63.152])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 51105517;\n\tTue, 21 Jul 2026 15:07:43 +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=\"S6Zpzf7H\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1784639263;\n\tbh=2FdujeeFob9ziQ9zgpSTZoywzyvCfiwe69DUrI+hWZc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=S6Zpzf7Hs5a0mufKOfiGK/5SK1FhsjaUlShlxh452PPBO7+cFYMvT0zDXTPubJmT2\n\tOEN/qhmNQ8x+dpeYuLGY256yOABSsaSetdLx2Lz7hLBvUZHkTjCK44KpmdZx4KC8Df\n\tDUwcuBTmtf9sXxVr6mZFk3oARnqIlBGaZHXJlAQc=","Date":"Tue, 21 Jul 2026 15:08:39 +0200","From":"Jacopo Mondi ","To":"Milan Zamazal ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tStefan Klug , \n\tKieran Bingham ,\n\tDaniel Scally , \n\tRobert Mader ,\n\tLaurent Pinchart ","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","Message-ID":"","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<85v7a8zfzf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<85v7a8zfzf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","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":39783,"web_url":"https://patchwork.libcamera.org/comment/39783/","msgid":"<85ik68uho6.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-07-21T22:00:41","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Jacopo Mondi writes:\n\n> Hi Milan\n> thanks for the review\n>\n> On Tue, Jul 21, 2026 at 02:25:56PM +0200, Milan Zamazal wrote:\n>> Hi Jacopo,\n>>\n>> Jacopo Mondi writes:\n>>\n>> > The series is identical to v5 up to patch 27. The only minor difference\n>> > is that I back-tracked on making functions class members but in the\n>> > RkISP1 IPA I kept them as static helpers.\n>> >\n>> > Patches 28 and 29 are new.\n>> >\n>> > They remove the templating of the LscAlgorithm class hierarchy and\n>> > instead implement quantization in the LscAlgorithm class.\n>> >\n>> > So we now expand polynomials and parse tables as floats, store them\n>> > in a map and in LscAlgorithm we re-iterate over it to either quantized\n>> > them in the polynomial case, or simply cast floats to the register bit\n>> > width if table.\n>> >\n>> > Timing the duration of the whole RkISP1 configure operation, it now\n>> > takes 3 msec compared to 600 usec.\n>> >\n>> > \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n>> >\n>> > \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n>> >\n>> > Also, now each IPA has to define their own Interpolator::interpolate()\n>> > overload (see rkisp1 IPA as an example of this).\n>> >\n>> > As the interface of LscAlgorithm has changed, patches on the list that\n>> > port the LSC algorithm of other platforms based on v5 will have to be\n>> > rebased.\n>>\n>> I'm having trouble to understand the exact motivation for the change.\n>\n> I've been suggested to move quantization to LscAlgorithm to remove\n> templating of LscPolynomial and LscTable.\n>\n>> From the libipa consumer side, it means that for `simple' pipeline, I\n>> had to copy & substitute & paste code from rkisp1 for no very good\n>> reason. Which looks like a step against the code unification goal,\n>> while it doesn't seem to simplify rkisp1 that much.\n>\n> Agreed\n>\n>>\n>> The float representation could be useful for software ISP, where there\n>> is no inherent reason to use quantized values (although they may still\n>> have its use for portable textures, as pointed out by Bryan). But it\n>> seems using quantized values is still imposed on libipa LSC consumers?\n>\n> Let's put LscPolynomial aside for a second and only consider LscTable.\n>\n> Gains in the tuning files are currently expressed in register format\n> (iow already quantized). This means Lsc tables are not portable across\n> platforms. I think there is consensus on the desire to express gains\n> as floats in tuning files, so that we can share lsc tables among\n> platforms. This is good and will be done on top along with table\n> re-scaling to adapt them to different sensor's resolutions.\n\nYes.\n\n> On quantization: up until v5 I templated the whole LscImplementation\n> class hierarchy with the platform's Q format, so that when the gain\n> vectors are populated (when a polynomial is expanded or a\n> table-resampled in future) they are stored in register format\n> ready to be consumed by the IPA.\n>\n> This required to template a few classes, which was not that bad to me.\n> I've been suggested to remove quantization from the LscImplementation\n> hierarchy, so that it's simpler and instead perform quantization\n> either in the IPA module itself or in LscAlgorithm class. This\n> requires to go over the already populated arrays and quantize the\n> entries one by one before passing them to the IPA. I don't think it's ideal.\n>\n> However having floats as an intermediate values might make\n> interpolation more accurate (I don't know and I don't see why,\n> I'm just speculating) and the implementation is simpler.\n\nHmm, the whole stuff sounds like enough opportunities for different\nopinions...\n\n> v6 requires however that each IPA defines its own\n> Interpolator::interpolate() overload (maybe this could be\n> generalized?). Is this the code that you needed to add to your IPA ?\n\nYes. If Interpolator::interpolate() specialisation for the map could be\nadded then the IPAs would be happy either way. If not, I'd personally\nprefer IPA simplicity over libipa simplicity.\n\n> On the gain format: simple is kind of special. It doesn't require\n> quantization and you could maybe do with floats. But the same\n> reasoning applies to AWB, gamma, CCM and all other algorithms, where\n> you don't have any register format to translate to. If you want floats\n> in your IPA to work with, maybe using UQ<0, 32> as register format\n> would do ?\n\nIt looks weird but it's not an obstacle. `simple' can live with the\nquantized values; I asked just in case I was missing something.\n\nThanks for clarification.\n\n>>\n>> >\n>> >\n>> > Signed-off-by: Jacopo Mondi \n>> > ---\n>> > Changes in v6:\n>> > - Address comments received on v5, mostly on documentation\n>> > - Remove templating of LscPolynomial and perform quantization in\n>> > LscAlgorithm\n>> >\n>> > Changes in v5:\n>> > - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n>> > to fix and exiting potential issue with Polynomial parsing\n>> > - Fix Simple, which I broke in v4, thanks Milan\n>> >\n>> > Changes in v4:\n>> > - s/awb/AWB in comments everywhere, same for CCM and LSC\n>> > - Awb: changelog per patch\n>> > - Make AwbStats pure virtual and move stats parsing logic to the IPA\n>> > modules\n>> > - Reduce comments/renames to reduce patch size and comments\n>> > - Collect tags\n>> >\n>> > Changes in v3:\n>> > - Address comments on Awb from Stefan:\n>> > - Introduce AwbImplementation::Result\n>> > - Move AwbStats implementation to IPAs\n>> > - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n>> > - Fix Saturation bug in Simple Ccm algorithm\n>> > - Fix Simple Ccm gain limits using Q<2, 8>\n>> > - Update Mali CCM and Gamma to latest uAPI patch version\n>> > - Fix Mali CCM gain masking as reported by Dan\n>> > - Break down libIPA Lsc implementation as requested by Stefan\n>> >\n>> > Changes in v2:\n>> > - Introduce awb::Context to simplify ActiveState and FrameContext\n>> > definition\n>> > - Add mali uAPI header for CCM and Gamma\n>> > - Do not rename Mali functions for hardware configuration in awb and lsc\n>> > - Spelling and documentation\n>> >\n>> > ---\n>> > Jacopo Mondi (28):\n>> > ipa: libipa: awb: Reimplement AwbAlgorithm\n>> > ipa: mali-c55: awb: Port to use libipa AwbAlgorithm\n>> > ipa: libipa: Add CcmAlgorithm to libipa\n>> > ipa: rkisp1: ccm: Port to use CcmAlgorithm\n>> > include: linux: mali-c55: Update to support CCM and Gamma\n>> > ipa: mali-c55: Implement Ccm algorithm\n>> > ipa: rkisp1: lsc: Re-architecture Lsc support\n>> > ipa: libipa: Introduce lsc_base.h\n>> > ipa: libipa: lsc_polynomial: Fix polynomial parsing error\n>> > ipa: libipa: lsc_polynomial: Rename LscPolynomial\n>> > ipa: libipa: lsc_polynomial: Do not inline functions\n>> > ipa: libipa: lsc: Move Interpolator to lsc_base\n>> > ipa: rkisp1: lsc: Remove rogue empty line\n>> > ipa: libipa: lsc_polynomial: Move LscPolynomial from RkISP1\n>> > ipa: libipa: lsc_table: Move LscTable from RkISP1\n>> > ipa: libipa: lsc_base: Add sampleForCrop documentation\n>> > ipa: libipa: lsc: Introduce LscAlgorithm\n>> > ipa: libipa: lsc: Move sizesListToPositions() back to rkisp1\n>> > ipa: rkisp1: lsc: Port to use LscAlgorithm\n>> > ipa: libipa: lsc: Make Components a map<>\n>> > ipa: libipa: lsc: Document Components parsing\n>> > ipa: libipa: lsc_polynomial: Remove sensorSize_\n>> > ipa: libipa: lsc: Document LscDescriptor::sensorSize usage\n>> > ipa: libipa: lsc: Quantize lsc gains in IPA\n>> > ipa: libipa: lsc: Quantize gains in LscAlgorithm\n>> > ipa: libipa: lsc: Re-sort LscAlgorithmBase documentation\n>> > ipa: mali-c55: Add sensorInfo to IPAContext\n>> > ipa: mali-c55: Port to use LscAlgorithm\n>> >\n>> > Kieran Bingham (2):\n>> > ipa: simple: awb: Port to use libipa AwbAlgorithm\n>> > ipa: simple: Port to use libipa CcmAlgorithm\n>> >\n>> > Laurent Pinchart (1):\n>> > ipa: libipa: lsc_polynomial: Don't inline ValueNode accessor\n>> >\n>> > .../internal/software_isp/debayer_params.h | 4 +-\n>> > include/linux/media/arm/mali-c55-config.h | 86 ++++-\n>> > src/ipa/libipa/awb.cpp | 418 +++++++++++++++++---\n>> > src/ipa/libipa/awb.h | 100 ++++-\n>> > src/ipa/libipa/awb_bayes.cpp | 52 +--\n>> > src/ipa/libipa/awb_bayes.h | 12 +-\n>> > src/ipa/libipa/awb_grey.cpp | 16 +-\n>> > src/ipa/libipa/awb_grey.h | 6 +-\n>> > src/ipa/libipa/ccm.cpp | 245 ++++++++++++\n>> > src/ipa/libipa/ccm.h | 77 ++++\n>> > src/ipa/libipa/lsc.cpp | 372 ++++++++++++++++++\n>> > src/ipa/libipa/lsc.h | 137 +++++++\n>> > src/ipa/libipa/lsc_base.cpp | 136 +++++++\n>> > src/ipa/libipa/lsc_base.h | 56 +++\n>> > src/ipa/libipa/lsc_polynomial.cpp | 208 +++++++++-\n>> > src/ipa/libipa/lsc_polynomial.h | 100 ++---\n>> > src/ipa/libipa/lsc_table.cpp | 107 +++++\n>> > src/ipa/libipa/lsc_table.h | 55 +++\n>> > src/ipa/libipa/meson.build | 8 +\n>> > src/ipa/mali-c55/algorithms/awb.cpp | 223 ++++++-----\n>> > src/ipa/mali-c55/algorithms/awb.h | 28 +-\n>> > src/ipa/mali-c55/algorithms/ccm.cpp | 173 +++++++++\n>> > src/ipa/mali-c55/algorithms/ccm.h | 66 ++++\n>> > src/ipa/mali-c55/algorithms/lsc.cpp | 156 +++++---\n>> > src/ipa/mali-c55/algorithms/lsc.h | 32 +-\n>> > src/ipa/mali-c55/algorithms/meson.build | 1 +\n>> > src/ipa/mali-c55/ipa_context.cpp | 3 +\n>> > src/ipa/mali-c55/ipa_context.h | 20 +-\n>> > src/ipa/mali-c55/mali-c55.cpp | 2 +\n>> > src/ipa/mali-c55/params.h | 2 +\n>> > src/ipa/rkisp1/algorithms/awb.cpp | 191 ++-------\n>> > src/ipa/rkisp1/algorithms/awb.h | 18 +-\n>> > src/ipa/rkisp1/algorithms/ccm.cpp | 97 +----\n>> > src/ipa/rkisp1/algorithms/ccm.h | 17 +-\n>> > src/ipa/rkisp1/algorithms/lsc.cpp | 429 +++------------------\n>> > src/ipa/rkisp1/algorithms/lsc.h | 74 ++--\n>> > src/ipa/rkisp1/ipa_context.h | 49 +--\n>> > src/ipa/simple/algorithms/awb.cpp | 140 +++++--\n>> > src/ipa/simple/algorithms/awb.h | 28 ++\n>> > src/ipa/simple/algorithms/blc.cpp | 2 +-\n>> > src/ipa/simple/algorithms/ccm.cpp | 74 ++--\n>> > src/ipa/simple/algorithms/ccm.h | 22 +-\n>> > src/ipa/simple/ipa_context.h | 15 +-\n>> > src/libcamera/software_isp/debayer_cpu.cpp | 12 +-\n>> > 44 files changed, 2934 insertions(+), 1135 deletions(-)\n>> > ---\n>> > base-commit: 709ad59a8d90af3570be1c8d02dee0ec9e011954\n>> > change-id: 20260615-libipa-algorithms-59666bca0a2f\n>> >\n>> > Best regards,\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 B12F0BDE4C\n\tfor ;\n\tTue, 21 Jul 2026 22:00:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74C5A67E86;\n\tWed, 22 Jul 2026 00:00:52 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4782C67E44\n\tfor ;\n\tWed, 22 Jul 2026 00:00:50 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-80-PM4ljmzIND69CbA5ttthdw-1; Tue, 21 Jul 2026 18:00:45 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-4954fd771baso20752705e9.0\n\tfor ;\n\tTue, 21 Jul 2026 15:00:45 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-4.net.vodafone.cz. [77.48.47.4])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4956a500dbfsm19843315e9.6.2026.07.21.15.00.42\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 21 Jul 2026 15:00:42 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"XWAl6Pvq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1784671249;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=93kd1lWoVVO9CMKffCqbTzQy/jXx/hSx/egybLfxEHk=;\n\tb=XWAl6PvqtJ6RjM5gs+Jf4ecA02RM84v5fMl2C7RpxAahou9FrCVYVnZtJ8SKwH50OcMfW2\n\tPp3YJjW1JULcgGLXmENMdxoVnuZlXePq5LcDb14CV0dOx+IYgIpb/r5BCmmh5uqEQQFrfy\n\tjspuQUly3nm/E4xY+q3xDd0T6USaA80=","X-MC-Unique":"PM4ljmzIND69CbA5ttthdw-1","X-Mimecast-MFC-AGG-ID":"PM4ljmzIND69CbA5ttthdw_1784671244","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1784671244; x=1785276044;\n\th=content-transfer-encoding:content-type:mime-version:user-agent\n\t:message-id:date:references:in-reply-to:subject:cc:to:from:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to\n\t:content-type;\n\tbh=vJ/TfPqrmpJyzBN6MmqQ19F+9mNlxdPwhnv1Mhomu24=;\n\tb=Fk8MSErrZJt5xyjn3ICfW0GxPTV/xVfQP7STkZnf7JUP5NtAUy7MueysjlFMdsGcYy\n\thr4Z+gGoS4KWkH7ggsRfEsvbvWNA4RJSvrbxCiFos61kHwvHf7SJF1+u70RU82shezmr\n\t0Sn4Xa6n58cfjsxVeDYrearHXChdjShLubmdWSo3BvzajsWVo3wg6+v1HiH69B9d6cx+\n\tXKqYzdhW8hWUJQIm+JDfcS3huyxZT0Zntta1z4REXzdIub6foxsVp7wPgKPZtRSUHqVB\n\tcA5xDboaJZs6cTEqvdPpy6YuVVMA3+OxngMSiSEFcCXHoADbqUniwXOiz6chmLd7vUVK\n\tjXOg==","X-Gm-Message-State":"AOJu0Yy/wbXyJsv8dHkdO5SOD/LCB3NAdz6XyL0ukujqbjOYaumv/DVi\n\tylINkdbTJVKI5C9SwHahcfw8yh58LcK6GU441E/fgP9QJU7K9lugFoJUioAyEgF7JDDyc+x7kmz\n\tYtHTVrKTLGinZWs3hGxfwcYV5WcNb/ceKcMcZAjy6lgMway3LLJowu+toF2Jk+eyuWLYxl3fraa\n\tM=","X-Gm-Gg":"AR+sD114EmnqXdQG/sBnGR4Uht+MoaDoPZKwBEd+DajB61y9dEVxCXnHDmUQiaDS6AO\n\t25mMk8/B6getrUjjPZSUdxWxyofQEzLPqM2RD9DofsQE3XfYrpJpYqMQhzSjyAU9HZIIooI5qzd\n\tLyJ13qKW5hIqEIjH9JYCM8JUDkmwI9P2skNcwPzaLyxMHXTpSBpH3GXB6csl67ZowcGNBBLI5Zz\n\tjjp2RqQO2rNQuZDO81DmRedfzhHSZfdO8maK5ugN9NX3H95GfyHkrG7aF8lN/TPeaniKbZh2RHO\n\tFnevrbn9gmtayLABaWkVAZWi00bPTpqG9UPKkOCgCFSv6fB+/ud6KVCNfhG4FOk0l1MEWcKShqV\n\tHSCW0HiW4zDMoNQINbK3c/vLEiOlRgKtaUcQj8UKGER89RgNbeUGzWw==","X-Received":["by 2002:a05:600d:8449:b0:495:6022:5a23 with SMTP id\n\t5b1f17b1804b1-4956a51d5c4mr17658325e9.19.1784671243978; \n\tTue, 21 Jul 2026 15:00:43 -0700 (PDT)","by 2002:a05:600d:8449:b0:495:6022:5a23 with SMTP id\n\t5b1f17b1804b1-4956a51d5c4mr17657975e9.19.1784671243563; \n\tTue, 21 Jul 2026 15:00:43 -0700 (PDT)"],"From":"Milan Zamazal ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, Stefan Klug\n\t, Kieran Bingham\n\t, Daniel Scally\n\t,\n\tRobert Mader , \n\tLaurent Pinchart ","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","In-Reply-To":" (Jacopo Mondi's message of \"Tue, 21 Jul\n\t2026 15:08:39 +0200\")","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<85v7a8zfzf.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t","Date":"Wed, 22 Jul 2026 00:00:41 +0200","Message-ID":"<85ik68uho6.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"Hye0hqOjmVCWP3gxAN5di0QX-1w4NEQzkDTg7xKxNfM_1784671244","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":39891,"web_url":"https://patchwork.libcamera.org/comment/39891/","msgid":"<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>","date":"2026-07-28T09:48:51","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n> The series is identical to v5 up to patch 27. The only minor difference\n> is that I back-tracked on making functions class members but in the\n> RkISP1 IPA I kept them as static helpers.\n> \n> Patches 28 and 29 are new.\n> \n> They remove the templating of the LscAlgorithm class hierarchy and\n> instead implement quantization in the LscAlgorithm class.\n\nI have spent some time on this v5 vs. v6 question, and to me it seems there\nare no real significant differences. The interpolation is done on the quantized\ntype in either case, the only real difference is whether the quantization happen\nin the implementation or in the main algorithm. And I think that's largely an\nimplementation detail that doesn't affect users much.\n\nOne potential thing that was already brought up is whether it is even possible\nto do the interpolation on the quantized type? I think as long as we are dealing\nwith unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\nto `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n\"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n\nInitially I thought v6 would defer quantization even more by doing everything with\nfloats and only quantizing the final interpolated data. I feel like that's a nice\napproach that avoids any details of quantization until the last point, but of course\nit has the largest performance penalty as now each time the lsc tables change float\ninterpolation and quantization is carried out, instead of just the integer interpolation\nof v5 and v6.\n\n> \n> So we now expand polynomials and parse tables as floats, store them\n> in a map and in LscAlgorithm we re-iterate over it to either quantized\n> them in the polynomial case, or simply cast floats to the register bit\n> width if table.\n> \n> Timing the duration of the whole RkISP1 configure operation, it now\n> takes 3 msec compared to 600 usec.\n> \n> \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n> \n> \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n\nI'm honestly a bit surprised that there is a 5x time penalty here... is there\nany chance you could attach the tuning file that you used to measure the time?\n\n\n> \n> Also, now each IPA has to define their own Interpolator::interpolate()\n> overload (see rkisp1 IPA as an example of this).\n\nIt seems to me that the it should be possible to provide the specializations\nin a common place like in v5 without making every ipa specialize it itself\n(which will eventually lead to duplicates). The \"Components\" type only\ndepends on the quantized type, but currently there is an unnecessary dependency\non the fixed point representation because it is defined inside `LscAlgorithm`,\nmoving it out would remove this dependency and would allow specialization in a\ncommon place, like in v5.\n\n\n> \n> As the interface of LscAlgorithm has changed, patches on the list that\n> port the LSC algorithm of other platforms based on v5 will have to be rebased.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n\nSo in summary, assuming the plan is to move the table based tuning files to\nfloating point number, I think my preference would be deferring the quantization\nas much as possible (until after interpolation) and dealing with floats. However,\nalthough I have not tested it, I feel the runtime cost of that would be too big\ncompared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n * the interpolation can be moved to a common place like in v5 (as mentioned above);\n * I would like take a look at the 5x runtime gap.\n\n\n> Changes in v6:\n> - Address comments received on v5, mostly on documentation\n> - Remove templating of LscPolynomial and perform quantization in\n> LscAlgorithm\n> \n> Changes in v5:\n> - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> to fix and exiting potential issue with Polynomial parsing\n> - Fix Simple, which I broke in v4, thanks Milan\n> \n> Changes in v4:\n> - s/awb/AWB in comments everywhere, same for CCM and LSC\n> - Awb: changelog per patch\n> - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> modules\n> - Reduce comments/renames to reduce patch size and comments\n> - Collect tags\n> \n> Changes in v3:\n> - Address comments on Awb from Stefan:\n> - Introduce AwbImplementation::Result\n> - Move AwbStats implementation to IPAs\n> - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> - Fix Saturation bug in Simple Ccm algorithm\n> - Fix Simple Ccm gain limits using Q<2, 8>\n> - Update Mali CCM and Gamma to latest uAPI patch version\n> - Fix Mali CCM gain masking as reported by Dan\n> - Break down libIPA Lsc implementation as requested by Stefan\n> \n> Changes in v2:\n> - Introduce awb::Context to simplify ActiveState and FrameContext\n> definition\n> - Add mali uAPI header for CCM and Gamma\n> - Do not rename Mali functions for hardware configuration in awb and lsc\n> - Spelling and documentation\n> \n> ---\n> Jacopo Mondi (28):\n> ipa: libipa: awb: Reimplement AwbAlgorithm\n> ipa: mali-c55: awb: Port to use libipa AwbAlgorithm\n> ipa: libipa: Add CcmAlgorithm to libipa\n> ipa: rkisp1: ccm: Port to use CcmAlgorithm\n> include: linux: mali-c55: Update to support CCM and Gamma\n> ipa: mali-c55: Implement Ccm algorithm\n> ipa: rkisp1: lsc: Re-architecture Lsc support\n> ipa: libipa: Introduce lsc_base.h\n> ipa: libipa: lsc_polynomial: Fix polynomial parsing error\n> ipa: libipa: lsc_polynomial: Rename LscPolynomial\n> ipa: libipa: lsc_polynomial: Do not inline functions\n> ipa: libipa: lsc: Move Interpolator to lsc_base\n> ipa: rkisp1: lsc: Remove rogue empty line\n> ipa: libipa: lsc_polynomial: Move LscPolynomial from RkISP1\n> ipa: libipa: lsc_table: Move LscTable from RkISP1\n> ipa: libipa: lsc_base: Add sampleForCrop documentation\n> ipa: libipa: lsc: Introduce LscAlgorithm\n> ipa: libipa: lsc: Move sizesListToPositions() back to rkisp1\n> ipa: rkisp1: lsc: Port to use LscAlgorithm\n> ipa: libipa: lsc: Make Components a map<>\n> ipa: libipa: lsc: Document Components parsing\n> ipa: libipa: lsc_polynomial: Remove sensorSize_\n> ipa: libipa: lsc: Document LscDescriptor::sensorSize usage\n> ipa: libipa: lsc: Quantize lsc gains in IPA\n> ipa: libipa: lsc: Quantize gains in LscAlgorithm\n> ipa: libipa: lsc: Re-sort LscAlgorithmBase documentation\n> ipa: mali-c55: Add sensorInfo to IPAContext\n> ipa: mali-c55: Port to use LscAlgorithm\n> \n> Kieran Bingham (2):\n> ipa: simple: awb: Port to use libipa AwbAlgorithm\n> ipa: simple: Port to use libipa CcmAlgorithm\n> \n> Laurent Pinchart (1):\n> ipa: libipa: lsc_polynomial: Don't inline ValueNode accessor\n> \n> .../internal/software_isp/debayer_params.h | 4 +-\n> include/linux/media/arm/mali-c55-config.h | 86 ++++-\n> src/ipa/libipa/awb.cpp | 418 +++++++++++++++++---\n> src/ipa/libipa/awb.h | 100 ++++-\n> src/ipa/libipa/awb_bayes.cpp | 52 +--\n> src/ipa/libipa/awb_bayes.h | 12 +-\n> src/ipa/libipa/awb_grey.cpp | 16 +-\n> src/ipa/libipa/awb_grey.h | 6 +-\n> src/ipa/libipa/ccm.cpp | 245 ++++++++++++\n> src/ipa/libipa/ccm.h | 77 ++++\n> src/ipa/libipa/lsc.cpp | 372 ++++++++++++++++++\n> src/ipa/libipa/lsc.h | 137 +++++++\n> src/ipa/libipa/lsc_base.cpp | 136 +++++++\n> src/ipa/libipa/lsc_base.h | 56 +++\n> src/ipa/libipa/lsc_polynomial.cpp | 208 +++++++++-\n> src/ipa/libipa/lsc_polynomial.h | 100 ++---\n> src/ipa/libipa/lsc_table.cpp | 107 +++++\n> src/ipa/libipa/lsc_table.h | 55 +++\n> src/ipa/libipa/meson.build | 8 +\n> src/ipa/mali-c55/algorithms/awb.cpp | 223 ++++++-----\n> src/ipa/mali-c55/algorithms/awb.h | 28 +-\n> src/ipa/mali-c55/algorithms/ccm.cpp | 173 +++++++++\n> src/ipa/mali-c55/algorithms/ccm.h | 66 ++++\n> src/ipa/mali-c55/algorithms/lsc.cpp | 156 +++++---\n> src/ipa/mali-c55/algorithms/lsc.h | 32 +-\n> src/ipa/mali-c55/algorithms/meson.build | 1 +\n> src/ipa/mali-c55/ipa_context.cpp | 3 +\n> src/ipa/mali-c55/ipa_context.h | 20 +-\n> src/ipa/mali-c55/mali-c55.cpp | 2 +\n> src/ipa/mali-c55/params.h | 2 +\n> src/ipa/rkisp1/algorithms/awb.cpp | 191 ++-------\n> src/ipa/rkisp1/algorithms/awb.h | 18 +-\n> src/ipa/rkisp1/algorithms/ccm.cpp | 97 +----\n> src/ipa/rkisp1/algorithms/ccm.h | 17 +-\n> src/ipa/rkisp1/algorithms/lsc.cpp | 429 +++------------------\n> src/ipa/rkisp1/algorithms/lsc.h | 74 ++--\n> src/ipa/rkisp1/ipa_context.h | 49 +--\n> src/ipa/simple/algorithms/awb.cpp | 140 +++++--\n> src/ipa/simple/algorithms/awb.h | 28 ++\n> src/ipa/simple/algorithms/blc.cpp | 2 +-\n> src/ipa/simple/algorithms/ccm.cpp | 74 ++--\n> src/ipa/simple/algorithms/ccm.h | 22 +-\n> src/ipa/simple/ipa_context.h | 15 +-\n> src/libcamera/software_isp/debayer_cpu.cpp | 12 +-\n> 44 files changed, 2934 insertions(+), 1135 deletions(-)\n> ---\n> base-commit: 709ad59a8d90af3570be1c8d02dee0ec9e011954\n> change-id: 20260615-libipa-algorithms-59666bca0a2f\n> \n> Best regards,","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 305BDBDE4C\n\tfor ;\n\tTue, 28 Jul 2026 09:48:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 44D4F67F96;\n\tTue, 28 Jul 2026 11:48: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 615CB67EC4\n\tfor ;\n\tTue, 28 Jul 2026 11:48:54 +0200 (CEST)","from [192.168.33.47] (185.182.215.156.nat.pool.zt.hu\n\t[185.182.215.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 78FFC2F8;\n\tTue, 28 Jul 2026 11:47:49 +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=\"RXmZl5tW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785232069;\n\tbh=JyQQ3KUMrV/D5BX8wuH6or5Dtmto5flFjW4R2iD6aTs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=RXmZl5tWSJLCbZfuylEfVocvwvNb5zBmergq7RdXpgygnfkNkdHlQDFhp3ZIFRn+E\n\tD7PU1eaiMw2c2YQ4LHrbmsKGnOq3iBK+sKQayq66iXgh9k2QXaYcsLWSKAMx/f39bf\n\tSQuCN+P94mKV2rAeMhE6QLUotSu78NzhQznQR/Q8=","Message-ID":"<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>","Date":"Tue, 28 Jul 2026 11:48:51 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org,\n\tStefan Klug ,\n\tMilan Zamazal ","Cc":"Kieran Bingham ,\n\tDaniel Scally ,\n\tRobert Mader ,\n\tLaurent Pinchart ","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>","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":39893,"web_url":"https://patchwork.libcamera.org/comment/39893/","msgid":"","date":"2026-07-28T12:12:58","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Jul 28, 2026 at 11:48:51AM +0200, Barnabás Pőcze wrote:\n> Hi\n>\n> 2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n> > The series is identical to v5 up to patch 27. The only minor difference\n> > is that I back-tracked on making functions class members but in the\n> > RkISP1 IPA I kept them as static helpers.\n> >\n> > Patches 28 and 29 are new.\n> >\n> > They remove the templating of the LscAlgorithm class hierarchy and\n> > instead implement quantization in the LscAlgorithm class.\n>\n> I have spent some time on this v5 vs. v6 question, and to me it seems there\n\nThanks a lot\n\n> are no real significant differences. The interpolation is done on the quantized\n> type in either case, the only real difference is whether the quantization happen\n> in the implementation or in the main algorithm. And I think that's largely an\n\nNote there is a third option suggested by Stefan to move quantization\nin the single IPA after the interpolation in the float domain\n\n> implementation detail that doesn't affect users much.\n>\n> One potential thing that was already brought up is whether it is even possible\n> to do the interpolation on the quantized type? I think as long as we are dealing\n> with unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\n> to `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n> \"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n>\n> Initially I thought v6 would defer quantization even more by doing everything with\n> floats and only quantizing the final interpolated data. I feel like that's a nice\n\nThat would require moving quantization completely to the IPAs, right ?\n\n> approach that avoids any details of quantization until the last point, but of course\n> it has the largest performance penalty as now each time the lsc tables change float\n> interpolation and quantization is carried out, instead of just the integer interpolation\n> of v5 and v6.\n>\n> >\n> > So we now expand polynomials and parse tables as floats, store them\n> > in a map and in LscAlgorithm we re-iterate over it to either quantized\n> > them in the polynomial case, or simply cast floats to the register bit\n> > width if table.\n> >\n> > Timing the duration of the whole RkISP1 configure operation, it now\n> > takes 3 msec compared to 600 usec.\n> >\n> > \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n> >\n> > \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n>\n> I'm honestly a bit surprised that there is a 5x time penalty here... is there\n> any chance you could attach the tuning file that you used to measure the time?\n\nSure, I think it's just the imx219 tuning file we have in the mainline\ntree\n\nFrom the branch I sent to the ml:\nhttps://gitlab.freedesktop.org/camera/libcamera/-/blob/patchwork/6067/src/ipa/rkisp1/data/imx219.yaml\n\n>\n>\n> >\n> > Also, now each IPA has to define their own Interpolator::interpolate()\n> > overload (see rkisp1 IPA as an example of this).\n>\n> It seems to me that the it should be possible to provide the specializations\n> in a common place like in v5 without making every ipa specialize it itself\n> (which will eventually lead to duplicates). The \"Components\" type only\n> depends on the quantized type, but currently there is an unnecessary dependency\n> on the fixed point representation because it is defined inside `LscAlgorithm`,\n> moving it out would remove this dependency and would allow specialization in a\n> common place, like in v5.\n>\n>\n> >\n> > As the interface of LscAlgorithm has changed, patches on the list that\n> > port the LSC algorithm of other platforms based on v5 will have to be rebased.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n>\n> So in summary, assuming the plan is to move the table based tuning files to\n> floating point number, I think my preference would be deferring the quantization\n> as much as possible (until after interpolation) and dealing with floats. However,\n> although I have not tested it, I feel the runtime cost of that would be too big\n> compared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n> * the interpolation can be moved to a common place like in v5 (as mentioned above);\n\nI presume there is way to do that by moving Components<> out of\nLscAlgorithm. As you noted, I have defined it inside the class because\nof the template dependency.\n\n> * I would like take a look at the 5x runtime gap.\n\nThe latest version is available in the above branch while v5 is\navailable in\nhttps://gitlab.freedesktop.org/camera/libcamera/-/tree/patchwork/6048/\n\nThe timing measurement was performed with:\n\n-------------------------------------------------------------------------------\n--- a/src/ipa/rkisp1/algorithms/lsc.cpp\n+++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n@@ -140,6 +140,8 @@ int LensShadingCorrection::configure(IPAContext &context,\n \tconst Size &size = context.configuration.sensor.size;\n \tSize totalSize{};\n\n+std::chrono::high_resolution_clock::time_point begin = std::chrono::high_resolution_clock::now();\n+\n \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {\n \t\txSizes_[i] = xSize_[i] * size.width;\n \t\tySizes_[i] = ySize_[i] * size.height;\n@@ -162,8 +164,13 @@ int LensShadingCorrection::configure(IPAContext &context,\n \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n \t}\n\n-\treturn lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n+\tlscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n \t\t\t\t xPos_, yPos_);\n+\tstd::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();\n+\n+\tLOG(RkISP1Lsc, Error) << \"Configure took = \" << std::chrono::duration_cast(end - begin).count() << \"[µs]\";\n+\n+\treturn 0;\n }\n\n void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n-------------------------------------------------------------------------------\n\n\n\n>\n>\n> > Changes in v6:\n> > - Address comments received on v5, mostly on documentation\n> > - Remove templating of LscPolynomial and perform quantization in\n> > LscAlgorithm\n> >\n> > Changes in v5:\n> > - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> > to fix and exiting potential issue with Polynomial parsing\n> > - Fix Simple, which I broke in v4, thanks Milan\n> >\n> > Changes in v4:\n> > - s/awb/AWB in comments everywhere, same for CCM and LSC\n> > - Awb: changelog per patch\n> > - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> > modules\n> > - Reduce comments/renames to reduce patch size and comments\n> > - Collect tags\n> >\n> > Changes in v3:\n> > - Address comments on Awb from Stefan:\n> > - Introduce AwbImplementation::Result\n> > - Move AwbStats implementation to IPAs\n> > - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> > - Fix Saturation bug in Simple Ccm algorithm\n> > - Fix Simple Ccm gain limits using Q<2, 8>\n> > - Update Mali CCM and Gamma to latest uAPI patch version\n> > - Fix Mali CCM gain masking as reported by Dan\n> > - Break down libIPA Lsc implementation as requested by Stefan\n> >\n> > Changes in v2:\n> > - Introduce awb::Context to simplify ActiveState and FrameContext\n> > definition\n> > - Add mali uAPI header for CCM and Gamma\n> > - Do not rename Mali functions for hardware configuration in awb and lsc\n> > - Spelling and documentation\n> >\n> > ---\n> > Jacopo Mondi (28):\n> > ipa: libipa: awb: Reimplement AwbAlgorithm\n> > ipa: mali-c55: awb: Port to use libipa AwbAlgorithm\n> > ipa: libipa: Add CcmAlgorithm to libipa\n> > ipa: rkisp1: ccm: Port to use CcmAlgorithm\n> > include: linux: mali-c55: Update to support CCM and Gamma\n> > ipa: mali-c55: Implement Ccm algorithm\n> > ipa: rkisp1: lsc: Re-architecture Lsc support\n> > ipa: libipa: Introduce lsc_base.h\n> > ipa: libipa: lsc_polynomial: Fix polynomial parsing error\n> > ipa: libipa: lsc_polynomial: Rename LscPolynomial\n> > ipa: libipa: lsc_polynomial: Do not inline functions\n> > ipa: libipa: lsc: Move Interpolator to lsc_base\n> > ipa: rkisp1: lsc: Remove rogue empty line\n> > ipa: libipa: lsc_polynomial: Move LscPolynomial from RkISP1\n> > ipa: libipa: lsc_table: Move LscTable from RkISP1\n> > ipa: libipa: lsc_base: Add sampleForCrop documentation\n> > ipa: libipa: lsc: Introduce LscAlgorithm\n> > ipa: libipa: lsc: Move sizesListToPositions() back to rkisp1\n> > ipa: rkisp1: lsc: Port to use LscAlgorithm\n> > ipa: libipa: lsc: Make Components a map<>\n> > ipa: libipa: lsc: Document Components parsing\n> > ipa: libipa: lsc_polynomial: Remove sensorSize_\n> > ipa: libipa: lsc: Document LscDescriptor::sensorSize usage\n> > ipa: libipa: lsc: Quantize lsc gains in IPA\n> > ipa: libipa: lsc: Quantize gains in LscAlgorithm\n> > ipa: libipa: lsc: Re-sort LscAlgorithmBase documentation\n> > ipa: mali-c55: Add sensorInfo to IPAContext\n> > ipa: mali-c55: Port to use LscAlgorithm\n> >\n> > Kieran Bingham (2):\n> > ipa: simple: awb: Port to use libipa AwbAlgorithm\n> > ipa: simple: Port to use libipa CcmAlgorithm\n> >\n> > Laurent Pinchart (1):\n> > ipa: libipa: lsc_polynomial: Don't inline ValueNode accessor\n> >\n> > .../internal/software_isp/debayer_params.h | 4 +-\n> > include/linux/media/arm/mali-c55-config.h | 86 ++++-\n> > src/ipa/libipa/awb.cpp | 418 +++++++++++++++++---\n> > src/ipa/libipa/awb.h | 100 ++++-\n> > src/ipa/libipa/awb_bayes.cpp | 52 +--\n> > src/ipa/libipa/awb_bayes.h | 12 +-\n> > src/ipa/libipa/awb_grey.cpp | 16 +-\n> > src/ipa/libipa/awb_grey.h | 6 +-\n> > src/ipa/libipa/ccm.cpp | 245 ++++++++++++\n> > src/ipa/libipa/ccm.h | 77 ++++\n> > src/ipa/libipa/lsc.cpp | 372 ++++++++++++++++++\n> > src/ipa/libipa/lsc.h | 137 +++++++\n> > src/ipa/libipa/lsc_base.cpp | 136 +++++++\n> > src/ipa/libipa/lsc_base.h | 56 +++\n> > src/ipa/libipa/lsc_polynomial.cpp | 208 +++++++++-\n> > src/ipa/libipa/lsc_polynomial.h | 100 ++---\n> > src/ipa/libipa/lsc_table.cpp | 107 +++++\n> > src/ipa/libipa/lsc_table.h | 55 +++\n> > src/ipa/libipa/meson.build | 8 +\n> > src/ipa/mali-c55/algorithms/awb.cpp | 223 ++++++-----\n> > src/ipa/mali-c55/algorithms/awb.h | 28 +-\n> > src/ipa/mali-c55/algorithms/ccm.cpp | 173 +++++++++\n> > src/ipa/mali-c55/algorithms/ccm.h | 66 ++++\n> > src/ipa/mali-c55/algorithms/lsc.cpp | 156 +++++---\n> > src/ipa/mali-c55/algorithms/lsc.h | 32 +-\n> > src/ipa/mali-c55/algorithms/meson.build | 1 +\n> > src/ipa/mali-c55/ipa_context.cpp | 3 +\n> > src/ipa/mali-c55/ipa_context.h | 20 +-\n> > src/ipa/mali-c55/mali-c55.cpp | 2 +\n> > src/ipa/mali-c55/params.h | 2 +\n> > src/ipa/rkisp1/algorithms/awb.cpp | 191 ++-------\n> > src/ipa/rkisp1/algorithms/awb.h | 18 +-\n> > src/ipa/rkisp1/algorithms/ccm.cpp | 97 +----\n> > src/ipa/rkisp1/algorithms/ccm.h | 17 +-\n> > src/ipa/rkisp1/algorithms/lsc.cpp | 429 +++------------------\n> > src/ipa/rkisp1/algorithms/lsc.h | 74 ++--\n> > src/ipa/rkisp1/ipa_context.h | 49 +--\n> > src/ipa/simple/algorithms/awb.cpp | 140 +++++--\n> > src/ipa/simple/algorithms/awb.h | 28 ++\n> > src/ipa/simple/algorithms/blc.cpp | 2 +-\n> > src/ipa/simple/algorithms/ccm.cpp | 74 ++--\n> > src/ipa/simple/algorithms/ccm.h | 22 +-\n> > src/ipa/simple/ipa_context.h | 15 +-\n> > src/libcamera/software_isp/debayer_cpu.cpp | 12 +-\n> > 44 files changed, 2934 insertions(+), 1135 deletions(-)\n> > ---\n> > base-commit: 709ad59a8d90af3570be1c8d02dee0ec9e011954\n> > change-id: 20260615-libipa-algorithms-59666bca0a2f\n> >\n> > Best regards,\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 4A86EBDE4C\n\tfor ;\n\tTue, 28 Jul 2026 12:13:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A81DE67F8F;\n\tTue, 28 Jul 2026 14:13:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9807E67E44\n\tfor ;\n\tTue, 28 Jul 2026 14:13:02 +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 D73A722E;\n\tTue, 28 Jul 2026 14:11:57 +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=\"aM0tIHM8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785240718;\n\tbh=9cMdl/8LHbJbfWsozFAR5q6F2Xm18ndQh/Fl28rbevg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aM0tIHM8XCLI3XDJHPd2zcpi1DzTH0XcDbYSFlIjw3K4Sz4sIVfn1qt1sXguTKR+R\n\t7LaRo7y5iymI+ASOn8XD2axuc2S0b0VNISoIfF+SGjCwQC3VHV8TwXBLeSZQPRNBch\n\t4y4ptvexeMFuiXlokL2hX+Kz+DBcTmWBEfbA37Zs=","Date":"Tue, 28 Jul 2026 14:12:58 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tStefan Klug , \n\tMilan Zamazal ,\n\tKieran Bingham , \n\tDaniel Scally ,\n\tRobert Mader , \n\tLaurent Pinchart ","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","Message-ID":"","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@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\" "}},{"id":39894,"web_url":"https://patchwork.libcamera.org/comment/39894/","msgid":"<80740149-87b6-4d87-8e1f-b64a6515f7e7@ideasonboard.com>","date":"2026-07-28T13:10:48","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 07. 28. 14:12 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Tue, Jul 28, 2026 at 11:48:51AM +0200, Barnabás Pőcze wrote:\n>> Hi\n>>\n>> 2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n>>> The series is identical to v5 up to patch 27. The only minor difference\n>>> is that I back-tracked on making functions class members but in the\n>>> RkISP1 IPA I kept them as static helpers.\n>>>\n>>> Patches 28 and 29 are new.\n>>>\n>>> They remove the templating of the LscAlgorithm class hierarchy and\n>>> instead implement quantization in the LscAlgorithm class.\n>>\n>> I have spent some time on this v5 vs. v6 question, and to me it seems there\n> \n> Thanks a lot\n> \n>> are no real significant differences. The interpolation is done on the quantized\n>> type in either case, the only real difference is whether the quantization happen\n>> in the implementation or in the main algorithm. And I think that's largely an\n> \n> Note there is a third option suggested by Stefan to move quantization\n> in the single IPA after the interpolation in the float domain\n\nThat sounds like what I'm suggesting: load and interpolate with `float`,\nquantize the interpolated data \"in the IPA module\" right before applying.\n\n\n> \n>> implementation detail that doesn't affect users much.\n>>\n>> One potential thing that was already brought up is whether it is even possible\n>> to do the interpolation on the quantized type? I think as long as we are dealing\n>> with unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\n>> to `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n>> \"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n>>\n>> Initially I thought v6 would defer quantization even more by doing everything with\n>> floats and only quantizing the final interpolated data. I feel like that's a nice\n> \n> That would require moving quantization completely to the IPAs, right ?\n\nI was thinking of something like this:\n\nstruct Components {\n std::map, std::less<>> sets;\n\n template\n void quantize(std::string_key key, typename T::QuantizedType *dst)\n {\n const auto &s = sets.at(key);\n std::transform(s.begin(), s.end(), dst, [](float x) { return T(x).quantized(); });\n }\n};\n\n...\n\nvoid LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n\t\t\t\t const lsc::Components &set)\n{\n set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n ...\n}\n\n> \n>> approach that avoids any details of quantization until the last point, but of course\n>> it has the largest performance penalty as now each time the lsc tables change float\n>> interpolation and quantization is carried out, instead of just the integer interpolation\n>> of v5 and v6.\n>>\n>>>\n>>> So we now expand polynomials and parse tables as floats, store them\n>>> in a map and in LscAlgorithm we re-iterate over it to either quantized\n>>> them in the polynomial case, or simply cast floats to the register bit\n>>> width if table.\n>>>\n>>> Timing the duration of the whole RkISP1 configure operation, it now\n>>> takes 3 msec compared to 600 usec.\n>>>\n>>> \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n>>>\n>>> \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n>>\n>> I'm honestly a bit surprised that there is a 5x time penalty here... is there\n>> any chance you could attach the tuning file that you used to measure the time?\n> \n> Sure, I think it's just the imx219 tuning file we have in the mainline\n> tree\n> \n> From the branch I sent to the ml:\n> https://gitlab.freedesktop.org/camera/libcamera/-/blob/patchwork/6067/src/ipa/rkisp1/data/imx219.yaml\n> \n>>\n>>\n>>>\n>>> Also, now each IPA has to define their own Interpolator::interpolate()\n>>> overload (see rkisp1 IPA as an example of this).\n>>\n>> It seems to me that the it should be possible to provide the specializations\n>> in a common place like in v5 without making every ipa specialize it itself\n>> (which will eventually lead to duplicates). The \"Components\" type only\n>> depends on the quantized type, but currently there is an unnecessary dependency\n>> on the fixed point representation because it is defined inside `LscAlgorithm`,\n>> moving it out would remove this dependency and would allow specialization in a\n>> common place, like in v5.\n>>\n>>\n>>>\n>>> As the interface of LscAlgorithm has changed, patches on the list that\n>>> port the LSC algorithm of other platforms based on v5 will have to be rebased.\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>\n>> So in summary, assuming the plan is to move the table based tuning files to\n>> floating point number, I think my preference would be deferring the quantization\n>> as much as possible (until after interpolation) and dealing with floats. However,\n>> although I have not tested it, I feel the runtime cost of that would be too big\n>> compared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n>> * the interpolation can be moved to a common place like in v5 (as mentioned above);\n> \n> I presume there is way to do that by moving Components<> out of\n> LscAlgorithm. As you noted, I have defined it inside the class because\n> of the template dependency.\n\nMy point is that I don't think there is a real dependency, I'm attaching a proof\nof concept patch that should apply to v6.\n\n\n> \n>> * I would like take a look at the 5x runtime gap.\n> \n> The latest version is available in the above branch while v5 is\n> available in\n> https://gitlab.freedesktop.org/camera/libcamera/-/tree/patchwork/6048/\n> \n> The timing measurement was performed with:\n> \n> -------------------------------------------------------------------------------\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -140,6 +140,8 @@ int LensShadingCorrection::configure(IPAContext &context,\n> \tconst Size &size = context.configuration.sensor.size;\n> \tSize totalSize{};\n> \n> +std::chrono::high_resolution_clock::time_point begin = std::chrono::high_resolution_clock::now();\n> +\n> \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {\n> \t\txSizes_[i] = xSize_[i] * size.width;\n> \t\tySizes_[i] = ySize_[i] * size.height;\n> @@ -162,8 +164,13 @@ int LensShadingCorrection::configure(IPAContext &context,\n> \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n> \t}\n> \n> -\treturn lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> +\tlscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> \t\t\t\t xPos_, yPos_);\n> +\tstd::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();\n> +\n> +\tLOG(RkISP1Lsc, Error) << \"Configure took = \" << std::chrono::duration_cast(end - begin).count() << \"[µs]\";\n> +\n> +\treturn 0;\n> }\n\nInteresting, I'm only measuring the `lscAlgo_.configure()` call because the parts before\nseem to be the same, and I'm getting around ~120us for v5 and ~140us for v6\nwith imx219.yml.\n\n\n> \n> void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n> -------------------------------------------------------------------------------\n> \n> \n> \n>>\n>>\n>>> Changes in v6:\n>>> - Address comments received on v5, mostly on documentation\n>>> - Remove templating of LscPolynomial and perform quantization in\n>>> LscAlgorithm\n>>>\n>>> Changes in v5:\n>>> - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n>>> to fix and exiting potential issue with Polynomial parsing\n>>> - Fix Simple, which I broke in v4, thanks Milan\n>>>\n>>> Changes in v4:\n>>> - s/awb/AWB in comments everywhere, same for CCM and LSC\n>>> - Awb: changelog per patch\n>>> - Make AwbStats pure virtual and move stats parsing logic to the IPA\n>>> modules\n>>> - Reduce comments/renames to reduce patch size and comments\n>>> - Collect tags\n>>>\n>>> Changes in v3:\n>>> - Address comments on Awb from Stefan:\n>>> - Introduce AwbImplementation::Result\n>>> - Move AwbStats implementation to IPAs\n>>> - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n>>> - Fix Saturation bug in Simple Ccm algorithm\n>>> - Fix Simple Ccm gain limits using Q<2, 8>\n>>> - Update Mali CCM and Gamma to latest uAPI patch version\n>>> - Fix Mali CCM gain masking as reported by Dan\n>>> - Break down libIPA Lsc implementation as requested by Stefan\n>>>\n>>> Changes in v2:\n>>> - Introduce awb::Context to simplify ActiveState and FrameContext\n>>> definition\n>>> - Add mali uAPI header for CCM and Gamma\n>>> - Do not rename Mali functions for hardware configuration in awb and lsc\n>>> - Spelling and documentation\n>>>\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 3443ABDE17\n\tfor ;\n\tTue, 28 Jul 2026 13:10:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0BFBE67F9B;\n\tTue, 28 Jul 2026 15:10:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8584767E44\n\tfor ;\n\tTue, 28 Jul 2026 15:10:51 +0200 (CEST)","from [192.168.33.47] (185.182.215.156.nat.pool.zt.hu\n\t[185.182.215.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7AB4B2F8;\n\tTue, 28 Jul 2026 15:09:46 +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=\"C1zRxO7B\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785244186;\n\tbh=Kb9tGZtgEObzCvx3n4DdJ1dVjp5UAfHsmyX0j0biNFo=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=C1zRxO7BbH04PamVMec9ZAIX0XPrJmNulMjXUJ39YYQsL4FD4MTJTl3+SkpIn/j/i\n\takbU9nS5fu2uYD3RHhEtKWzqxoSGT/oRgNoSI4mNTQk8rjTXb0cAdyDRN21YF7TjDu\n\tbXQYNd/M3Q3s1+HekZNKPX7RdViHC+3pOko4aAl8=","Content-Type":"multipart/mixed;\n\tboundary=\"------------U2BuqzaXesZXWnO5HucldDXR\"","Message-ID":"<80740149-87b6-4d87-8e1f-b64a6515f7e7@ideasonboard.com>","Date":"Tue, 28 Jul 2026 15:10:48 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org,\n\tStefan Klug ,\n\tMilan Zamazal ,\n\tKieran Bingham ,\n\tDaniel Scally ,\n\tRobert Mader ,\n\tLaurent Pinchart ","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":39895,"web_url":"https://patchwork.libcamera.org/comment/39895/","msgid":"","date":"2026-07-28T13:42:22","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Tue, Jul 28, 2026 at 03:10:48PM +0200, Barnabás Pőcze wrote:\n> 2026. 07. 28. 14:12 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Tue, Jul 28, 2026 at 11:48:51AM +0200, Barnabás Pőcze wrote:\n> > > Hi\n> > >\n> > > 2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n> > > > The series is identical to v5 up to patch 27. The only minor difference\n> > > > is that I back-tracked on making functions class members but in the\n> > > > RkISP1 IPA I kept them as static helpers.\n> > > >\n> > > > Patches 28 and 29 are new.\n> > > >\n> > > > They remove the templating of the LscAlgorithm class hierarchy and\n> > > > instead implement quantization in the LscAlgorithm class.\n> > >\n> > > I have spent some time on this v5 vs. v6 question, and to me it seems there\n> >\n> > Thanks a lot\n> >\n> > > are no real significant differences. The interpolation is done on the quantized\n> > > type in either case, the only real difference is whether the quantization happen\n> > > in the implementation or in the main algorithm. And I think that's largely an\n> >\n> > Note there is a third option suggested by Stefan to move quantization\n> > in the single IPA after the interpolation in the float domain\n>\n> That sounds like what I'm suggesting: load and interpolate with `float`,\n> quantize the interpolated data \"in the IPA module\" right before applying.\n>\n\nEven if we use std::transform<> doesn't it mean we have to walk all\nthe vectors and quantize them, while if we quantize once storing the\nvalues in the vector before interpolation we avoid that ?\n\nThat's the bulk of the difference between the two versions, isn't it ?\n\n>\n> >\n> > > implementation detail that doesn't affect users much.\n> > >\n> > > One potential thing that was already brought up is whether it is even possible\n> > > to do the interpolation on the quantized type? I think as long as we are dealing\n> > > with unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\n> > > to `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n> > > \"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n> > >\n> > > Initially I thought v6 would defer quantization even more by doing everything with\n> > > floats and only quantizing the final interpolated data. I feel like that's a nice\n> >\n> > That would require moving quantization completely to the IPAs, right ?\n>\n> I was thinking of something like this:\n>\n> struct Components {\n> std::map, std::less<>> sets;\n>\n> template\n> void quantize(std::string_key key, typename T::QuantizedType *dst)\n> {\n> const auto &s = sets.at(key);\n> std::transform(s.begin(), s.end(), dst, [](float x) { return T(x).quantized(); });\n> }\n> };\n>\n> ...\n>\n> void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> \t\t\t\t const lsc::Components &set)\n> {\n> set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n> ...\n> }\n>\n\nThat's nice.\n\nI had starting by defining Components with 'using' instead of defining\nthem as a type. I can expand their definition to also do the\nquantization.\n\n> >\n> > > approach that avoids any details of quantization until the last point, but of course\n> > > it has the largest performance penalty as now each time the lsc tables change float\n> > > interpolation and quantization is carried out, instead of just the integer interpolation\n> > > of v5 and v6.\n\nYes, and let me point out that v5 did the quantization when populating\nthe vector of gains (when expanding polynomial or (in the future)\nre-sampling the lsc table), while v6 does that by walking the vectors\nafter they have been populated by LscPolynomial/LscTable.\n\n\n> > >\n> > > >\n> > > > So we now expand polynomials and parse tables as floats, store them\n> > > > in a map and in LscAlgorithm we re-iterate over it to either quantized\n> > > > them in the polynomial case, or simply cast floats to the register bit\n> > > > width if table.\n> > > >\n> > > > Timing the duration of the whole RkISP1 configure operation, it now\n> > > > takes 3 msec compared to 600 usec.\n> > > >\n> > > > \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n> > > >\n> > > > \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n> > >\n> > > I'm honestly a bit surprised that there is a 5x time penalty here... is there\n> > > any chance you could attach the tuning file that you used to measure the time?\n> >\n> > Sure, I think it's just the imx219 tuning file we have in the mainline\n> > tree\n> >\n> > From the branch I sent to the ml:\n> > https://gitlab.freedesktop.org/camera/libcamera/-/blob/patchwork/6067/src/ipa/rkisp1/data/imx219.yaml\n> >\n> > >\n> > >\n> > > >\n> > > > Also, now each IPA has to define their own Interpolator::interpolate()\n> > > > overload (see rkisp1 IPA as an example of this).\n> > >\n> > > It seems to me that the it should be possible to provide the specializations\n> > > in a common place like in v5 without making every ipa specialize it itself\n> > > (which will eventually lead to duplicates). The \"Components\" type only\n> > > depends on the quantized type, but currently there is an unnecessary dependency\n> > > on the fixed point representation because it is defined inside `LscAlgorithm`,\n> > > moving it out would remove this dependency and would allow specialization in a\n> > > common place, like in v5.\n> > >\n> > >\n> > > >\n> > > > As the interface of LscAlgorithm has changed, patches on the list that\n> > > > port the LSC algorithm of other platforms based on v5 will have to be rebased.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > >\n> > > So in summary, assuming the plan is to move the table based tuning files to\n> > > floating point number, I think my preference would be deferring the quantization\n> > > as much as possible (until after interpolation) and dealing with floats. However,\n> > > although I have not tested it, I feel the runtime cost of that would be too big\n> > > compared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n> > > * the interpolation can be moved to a common place like in v5 (as mentioned above);\n> >\n> > I presume there is way to do that by moving Components<> out of\n> > LscAlgorithm. As you noted, I have defined it inside the class because\n> > of the template dependency.\n>\n> My point is that I don't think there is a real dependency, I'm attaching a proof\n> of concept patch that should apply to v6.\n\nThere isn't if you have the IPA provide to the Components.quantize()\nfunction the Q format\n\n set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n\nAs LscAlgorithm is already templated with the Q format it would be\nnice to re-use this instead of having to specify.\n\nAlternatively, let's remove the templating completely from\nLscAlgorithm and only template quantize() with the Q format.\n\nThis still requires you to\n\n1) LscImplementation: expand polynomial/resample grid in float\n2) IPA module: interpolate on floats\n3) IPA module: quantize the interpolated values\n\nwhile v5:\n\n1) LscImplementation: expand polynomial/resample grid and quantize\n2) IPA module: interpolate on quantized\n\nin v6:\n\n1) LscImplementation: expand polynomial/resample grid in floats\n2) LscAlgorithm: quantize values\n3) IPA module: interpolate on quantized\n\n\n>\n>\n> >\n> > > * I would like take a look at the 5x runtime gap.\n> >\n> > The latest version is available in the above branch while v5 is\n> > available in\n> > https://gitlab.freedesktop.org/camera/libcamera/-/tree/patchwork/6048/\n> >\n> > The timing measurement was performed with:\n> >\n> > -------------------------------------------------------------------------------\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -140,6 +140,8 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > \tconst Size &size = context.configuration.sensor.size;\n> > \tSize totalSize{};\n> >\n> > +std::chrono::high_resolution_clock::time_point begin = std::chrono::high_resolution_clock::now();\n> > +\n> > \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {\n> > \t\txSizes_[i] = xSize_[i] * size.width;\n> > \t\tySizes_[i] = ySize_[i] * size.height;\n> > @@ -162,8 +164,13 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n> > \t}\n> >\n> > -\treturn lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > +\tlscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > \t\t\t\t xPos_, yPos_);\n> > +\tstd::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();\n> > +\n> > +\tLOG(RkISP1Lsc, Error) << \"Configure took = \" << std::chrono::duration_cast(end - begin).count() << \"[µs]\";\n> > +\n> > +\treturn 0;\n> > }\n>\n> Interesting, I'm only measuring the `lscAlgo_.configure()` call because the parts before\n> seem to be the same, and I'm getting around ~120us for v5 and ~140us for v6\n> with imx219.yml.\n\nWith the above patch applied on top of v6:\n\n ERROR RkISP1Lsc lsc.cpp:171 Configure took = 3052[µs]\n\nWhat am I doing differently ?\n\n>\n>\n> >\n> > void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n> > -------------------------------------------------------------------------------\n> >\n> >\n> >\n> > >\n> > >\n> > > > Changes in v6:\n> > > > - Address comments received on v5, mostly on documentation\n> > > > - Remove templating of LscPolynomial and perform quantization in\n> > > > LscAlgorithm\n> > > >\n> > > > Changes in v5:\n> > > > - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> > > > to fix and exiting potential issue with Polynomial parsing\n> > > > - Fix Simple, which I broke in v4, thanks Milan\n> > > >\n> > > > Changes in v4:\n> > > > - s/awb/AWB in comments everywhere, same for CCM and LSC\n> > > > - Awb: changelog per patch\n> > > > - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> > > > modules\n> > > > - Reduce comments/renames to reduce patch size and comments\n> > > > - Collect tags\n> > > >\n> > > > Changes in v3:\n> > > > - Address comments on Awb from Stefan:\n> > > > - Introduce AwbImplementation::Result\n> > > > - Move AwbStats implementation to IPAs\n> > > > - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> > > > - Fix Saturation bug in Simple Ccm algorithm\n> > > > - Fix Simple Ccm gain limits using Q<2, 8>\n> > > > - Update Mali CCM and Gamma to latest uAPI patch version\n> > > > - Fix Mali CCM gain masking as reported by Dan\n> > > > - Break down libIPA Lsc implementation as requested by Stefan\n> > > >\n> > > > Changes in v2:\n> > > > - Introduce awb::Context to simplify ActiveState and FrameContext\n> > > > definition\n> > > > - Add mali uAPI header for CCM and Gamma\n> > > > - Do not rename Mali functions for hardware configuration in awb and lsc\n> > > > - Spelling and documentation\n> > > >\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 9FFCDBDE17\n\tfor ;\n\tTue, 28 Jul 2026 13:42:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DC4667F9D;\n\tTue, 28 Jul 2026 15:42:26 +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 C440D67E44\n\tfor ;\n\tTue, 28 Jul 2026 15:42:25 +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 EA4592F8;\n\tTue, 28 Jul 2026 15:41:20 +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=\"i9xyb/Rs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785246081;\n\tbh=RH8We9gjMCNLqwzB4lMbj3MXiC8SphQzFoNcLHxpbHo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i9xyb/Rsn9U2E5GBBhI7ruqWqU6IIxR6d8KGJfJDxTMl7CSa2m6qQfHNcgRUlE+8P\n\tijh8hIpr1O47rzZQrGG7RJjNUXlXOopFl5jXv4miTSv6o5N8M/ytHQcubYEBH890EG\n\trRBzoRunCMRpiz8sST4yBGPfPCDbFbxEfVR1NZY0=","Date":"Tue, 28 Jul 2026 15:42:22 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tStefan Klug , \n\tMilan Zamazal ,\n\tKieran Bingham , \n\tDaniel Scally ,\n\tRobert Mader , \n\tLaurent Pinchart ","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","Message-ID":"","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>\n\t\n\t<80740149-87b6-4d87-8e1f-b64a6515f7e7@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<80740149-87b6-4d87-8e1f-b64a6515f7e7@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\" "}},{"id":39896,"web_url":"https://patchwork.libcamera.org/comment/39896/","msgid":"<71a272c8-a52c-49bb-8eb5-76a46213aa1b@ideasonboard.com>","date":"2026-07-28T13:57:31","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 07. 28. 15:42 keltezéssel, Jacopo Mondi írta:\n> On Tue, Jul 28, 2026 at 03:10:48PM +0200, Barnabás Pőcze wrote:\n>> 2026. 07. 28. 14:12 keltezéssel, Jacopo Mondi írta:\n>>> Hi Barnabás\n>>>\n>>> On Tue, Jul 28, 2026 at 11:48:51AM +0200, Barnabás Pőcze wrote:\n>>>> Hi\n>>>>\n>>>> 2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n>>>>> The series is identical to v5 up to patch 27. The only minor difference\n>>>>> is that I back-tracked on making functions class members but in the\n>>>>> RkISP1 IPA I kept them as static helpers.\n>>>>>\n>>>>> Patches 28 and 29 are new.\n>>>>>\n>>>>> They remove the templating of the LscAlgorithm class hierarchy and\n>>>>> instead implement quantization in the LscAlgorithm class.\n>>>>\n>>>> I have spent some time on this v5 vs. v6 question, and to me it seems there\n>>>\n>>> Thanks a lot\n>>>\n>>>> are no real significant differences. The interpolation is done on the quantized\n>>>> type in either case, the only real difference is whether the quantization happen\n>>>> in the implementation or in the main algorithm. And I think that's largely an\n>>>\n>>> Note there is a third option suggested by Stefan to move quantization\n>>> in the single IPA after the interpolation in the float domain\n>>\n>> That sounds like what I'm suggesting: load and interpolate with `float`,\n>> quantize the interpolated data \"in the IPA module\" right before applying.\n>>\n> \n> Even if we use std::transform<> doesn't it mean we have to walk all\n> the vectors and quantize them, while if we quantize once storing the\n> values in the vector before interpolation we avoid that ?\n> \n> That's the bulk of the difference between the two versions, isn't it ?\n\nYes, and that's why I have mostly disregarded this approach in my first\nreply because while I kind of prefer the design, I think the runtime\ncost is probably not worth it -- assuming interpolation on the quantized\nvalues is possible, which it seems to be.\n\n\n> \n>>\n>>>\n>>>> implementation detail that doesn't affect users much.\n>>>>\n>>>> One potential thing that was already brought up is whether it is even possible\n>>>> to do the interpolation on the quantized type? I think as long as we are dealing\n>>>> with unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\n>>>> to `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n>>>> \"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n>>>>\n>>>> Initially I thought v6 would defer quantization even more by doing everything with\n>>>> floats and only quantizing the final interpolated data. I feel like that's a nice\n>>>\n>>> That would require moving quantization completely to the IPAs, right ?\n>>\n>> I was thinking of something like this:\n>>\n>> struct Components {\n>> std::map, std::less<>> sets;\n>>\n>> template\n>> void quantize(std::string_key key, typename T::QuantizedType *dst)\n>> {\n>> const auto &s = sets.at(key);\n>> std::transform(s.begin(), s.end(), dst, [](float x) { return T(x).quantized(); });\n>> }\n>> };\n>>\n>> ...\n>>\n>> void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n>> \t\t\t\t const lsc::Components &set)\n>> {\n>> set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n>> ...\n>> }\n>>\n> \n> That's nice.\n> \n> I had starting by defining Components with 'using' instead of defining\n> them as a type. I can expand their definition to also do the\n> quantization.\n> \n>>>\n>>>> approach that avoids any details of quantization until the last point, but of course\n>>>> it has the largest performance penalty as now each time the lsc tables change float\n>>>> interpolation and quantization is carried out, instead of just the integer interpolation\n>>>> of v5 and v6.\n> \n> Yes, and let me point out that v5 did the quantization when populating\n> the vector of gains (when expanding polynomial or (in the future)\n> re-sampling the lsc table), while v6 does that by walking the vectors\n> after they have been populated by LscPolynomial/LscTable.\n> \n> \n>>>>\n>>>>>\n>>>>> So we now expand polynomials and parse tables as floats, store them\n>>>>> in a map and in LscAlgorithm we re-iterate over it to either quantized\n>>>>> them in the polynomial case, or simply cast floats to the register bit\n>>>>> width if table.\n>>>>>\n>>>>> Timing the duration of the whole RkISP1 configure operation, it now\n>>>>> takes 3 msec compared to 600 usec.\n>>>>>\n>>>>> \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n>>>>>\n>>>>> \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n>>>>\n>>>> I'm honestly a bit surprised that there is a 5x time penalty here... is there\n>>>> any chance you could attach the tuning file that you used to measure the time?\n>>>\n>>> Sure, I think it's just the imx219 tuning file we have in the mainline\n>>> tree\n>>>\n>>> From the branch I sent to the ml:\n>>> https://gitlab.freedesktop.org/camera/libcamera/-/blob/patchwork/6067/src/ipa/rkisp1/data/imx219.yaml\n>>>\n>>>>\n>>>>\n>>>>>\n>>>>> Also, now each IPA has to define their own Interpolator::interpolate()\n>>>>> overload (see rkisp1 IPA as an example of this).\n>>>>\n>>>> It seems to me that the it should be possible to provide the specializations\n>>>> in a common place like in v5 without making every ipa specialize it itself\n>>>> (which will eventually lead to duplicates). The \"Components\" type only\n>>>> depends on the quantized type, but currently there is an unnecessary dependency\n>>>> on the fixed point representation because it is defined inside `LscAlgorithm`,\n>>>> moving it out would remove this dependency and would allow specialization in a\n>>>> common place, like in v5.\n>>>>\n>>>>\n>>>>>\n>>>>> As the interface of LscAlgorithm has changed, patches on the list that\n>>>>> port the LSC algorithm of other platforms based on v5 will have to be rebased.\n>>>>>\n>>>>> Signed-off-by: Jacopo Mondi \n>>>>> ---\n>>>>\n>>>> So in summary, assuming the plan is to move the table based tuning files to\n>>>> floating point number, I think my preference would be deferring the quantization\n>>>> as much as possible (until after interpolation) and dealing with floats. However,\n>>>> although I have not tested it, I feel the runtime cost of that would be too big\n>>>> compared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n>>>> * the interpolation can be moved to a common place like in v5 (as mentioned above);\n>>>\n>>> I presume there is way to do that by moving Components<> out of\n>>> LscAlgorithm. As you noted, I have defined it inside the class because\n>>> of the template dependency.\n>>\n>> My point is that I don't think there is a real dependency, I'm attaching a proof\n>> of concept patch that should apply to v6.\n> \n> There isn't if you have the IPA provide to the Components.quantize()\n> function the Q format\n> \n> set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n> \n> As LscAlgorithm is already templated with the Q format it would be\n> nice to re-use this instead of having to specify.\n\nThe way I see it, this `quantize` function in `Components` is only needed in the\n\"third\" (not currently implemented approach) that uses floats everywhere. In that\ncase `LscAlgorithm` wouldn't be a template, and it would work with floats all the way.\n\n\n> \n> Alternatively, let's remove the templating completely from\n> LscAlgorithm and only template quantize() with the Q format.\n> \n> This still requires you to\n> \n> 1) LscImplementation: expand polynomial/resample grid in float\n> 2) IPA module: interpolate on floats\n> 3) IPA module: quantize the interpolated values\n> \n> while v5:\n> \n> 1) LscImplementation: expand polynomial/resample grid and quantize\n> 2) IPA module: interpolate on quantized\n> \n> in v6:\n> \n> 1) LscImplementation: expand polynomial/resample grid in floats\n> 2) LscAlgorithm: quantize values\n> 3) IPA module: interpolate on quantized\n> \n> \n>>\n>>\n>>>\n>>>> * I would like take a look at the 5x runtime gap.\n>>>\n>>> The latest version is available in the above branch while v5 is\n>>> available in\n>>> https://gitlab.freedesktop.org/camera/libcamera/-/tree/patchwork/6048/\n>>>\n>>> The timing measurement was performed with:\n>>>\n>>> -------------------------------------------------------------------------------\n>>> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n>>> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n>>> @@ -140,6 +140,8 @@ int LensShadingCorrection::configure(IPAContext &context,\n>>> \tconst Size &size = context.configuration.sensor.size;\n>>> \tSize totalSize{};\n>>>\n>>> +std::chrono::high_resolution_clock::time_point begin = std::chrono::high_resolution_clock::now();\n>>> +\n>>> \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {\n>>> \t\txSizes_[i] = xSize_[i] * size.width;\n>>> \t\tySizes_[i] = ySize_[i] * size.height;\n>>> @@ -162,8 +164,13 @@ int LensShadingCorrection::configure(IPAContext &context,\n>>> \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n>>> \t}\n>>>\n>>> -\treturn lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n>>> +\tlscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n>>> \t\t\t\t xPos_, yPos_);\n>>> +\tstd::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();\n>>> +\n>>> +\tLOG(RkISP1Lsc, Error) << \"Configure took = \" << std::chrono::duration_cast(end - begin).count() << \"[µs]\";\n>>> +\n>>> +\treturn 0;\n>>> }\n>>\n>> Interesting, I'm only measuring the `lscAlgo_.configure()` call because the parts before\n>> seem to be the same, and I'm getting around ~120us for v5 and ~140us for v6\n>> with imx219.yml.\n> \n> With the above patch applied on top of v6:\n> \n> ERROR RkISP1Lsc lsc.cpp:171 Configure took = 3052[µs]\n> \n> What am I doing differently ?\n\nI'll test again.\n\n\n> \n>>\n>>\n>>>\n>>> void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n>>> -------------------------------------------------------------------------------\n>>>\n>>>\n>>>\n>>>>\n>>>>\n>>>>> Changes in v6:\n>>>>> - Address comments received on v5, mostly on documentation\n>>>>> - Remove templating of LscPolynomial and perform quantization in\n>>>>> LscAlgorithm\n>>>>>\n>>>>> Changes in v5:\n>>>>> - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n>>>>> to fix and exiting potential issue with Polynomial parsing\n>>>>> - Fix Simple, which I broke in v4, thanks Milan\n>>>>>\n>>>>> Changes in v4:\n>>>>> - s/awb/AWB in comments everywhere, same for CCM and LSC\n>>>>> - Awb: changelog per patch\n>>>>> - Make AwbStats pure virtual and move stats parsing logic to the IPA\n>>>>> modules\n>>>>> - Reduce comments/renames to reduce patch size and comments\n>>>>> - Collect tags\n>>>>>\n>>>>> Changes in v3:\n>>>>> - Address comments on Awb from Stefan:\n>>>>> - Introduce AwbImplementation::Result\n>>>>> - Move AwbStats implementation to IPAs\n>>>>> - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n>>>>> - Fix Saturation bug in Simple Ccm algorithm\n>>>>> - Fix Simple Ccm gain limits using Q<2, 8>\n>>>>> - Update Mali CCM and Gamma to latest uAPI patch version\n>>>>> - Fix Mali CCM gain masking as reported by Dan\n>>>>> - Break down libIPA Lsc implementation as requested by Stefan\n>>>>>\n>>>>> Changes in v2:\n>>>>> - Introduce awb::Context to simplify ActiveState and FrameContext\n>>>>> definition\n>>>>> - Add mali uAPI header for CCM and Gamma\n>>>>> - Do not rename Mali functions for hardware configuration in awb and lsc\n>>>>> - Spelling and documentation\n>>>>>\n>>>>> ---\n>>> [...]\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 DFC91BDE4C\n\tfor ;\n\tTue, 28 Jul 2026 13:57:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2AC467F96;\n\tTue, 28 Jul 2026 15:57:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 26A3B67E44\n\tfor ;\n\tTue, 28 Jul 2026 15:57:36 +0200 (CEST)","from [192.168.33.47] (185.182.215.156.nat.pool.zt.hu\n\t[185.182.215.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 276AB2F8;\n\tTue, 28 Jul 2026 15:56:31 +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=\"CGzqRJYf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785246991;\n\tbh=GfeUFg1KadmH2jpXEL2SDgChNqDPjVS2IFsZs1p4vpw=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=CGzqRJYf/zXrjZL/vIhF8JWs5YtbGnYW18dBU/LlPD99PZjsT7MQRlf5tkJjLb/Gv\n\tdEddh22RLzPfuWIsU4nhXv99IxCdGBCCiYi0YBFsp6YuIm2mM+X64U/erWIdTcaYfL\n\tnh8zjHsJ/LqljKQPbEy7P+Vno0f/1QglfbDFD7KI=","Message-ID":"<71a272c8-a52c-49bb-8eb5-76a46213aa1b@ideasonboard.com>","Date":"Tue, 28 Jul 2026 15:57:31 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org,\n\tStefan Klug ,\n\tMilan Zamazal ,\n\tKieran Bingham ,\n\tDaniel Scally ,\n\tRobert Mader ,\n\tLaurent Pinchart ","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>\n\t\n\t<80740149-87b6-4d87-8e1f-b64a6515f7e7@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":39897,"web_url":"https://patchwork.libcamera.org/comment/39897/","msgid":"","date":"2026-07-28T14:04:52","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Jul 28, 2026 at 03:57:31PM +0200, Barnabás Pőcze wrote:\n> 2026. 07. 28. 15:42 keltezéssel, Jacopo Mondi írta:\n> > On Tue, Jul 28, 2026 at 03:10:48PM +0200, Barnabás Pőcze wrote:\n> > > 2026. 07. 28. 14:12 keltezéssel, Jacopo Mondi írta:\n> > > > Hi Barnabás\n> > > >\n> > > > On Tue, Jul 28, 2026 at 11:48:51AM +0200, Barnabás Pőcze wrote:\n> > > > > Hi\n> > > > >\n> > > > > 2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n> > > > > > The series is identical to v5 up to patch 27. The only minor difference\n> > > > > > is that I back-tracked on making functions class members but in the\n> > > > > > RkISP1 IPA I kept them as static helpers.\n> > > > > >\n> > > > > > Patches 28 and 29 are new.\n> > > > > >\n> > > > > > They remove the templating of the LscAlgorithm class hierarchy and\n> > > > > > instead implement quantization in the LscAlgorithm class.\n> > > > >\n> > > > > I have spent some time on this v5 vs. v6 question, and to me it seems there\n> > > >\n> > > > Thanks a lot\n> > > >\n> > > > > are no real significant differences. The interpolation is done on the quantized\n> > > > > type in either case, the only real difference is whether the quantization happen\n> > > > > in the implementation or in the main algorithm. And I think that's largely an\n> > > >\n> > > > Note there is a third option suggested by Stefan to move quantization\n> > > > in the single IPA after the interpolation in the float domain\n> > >\n> > > That sounds like what I'm suggesting: load and interpolate with `float`,\n> > > quantize the interpolated data \"in the IPA module\" right before applying.\n> > >\n> >\n> > Even if we use std::transform<> doesn't it mean we have to walk all\n> > the vectors and quantize them, while if we quantize once storing the\n> > values in the vector before interpolation we avoid that ?\n> >\n> > That's the bulk of the difference between the two versions, isn't it ?\n>\n> Yes, and that's why I have mostly disregarded this approach in my first\n> reply because while I kind of prefer the design, I think the runtime\n> cost is probably not worth it -- assuming interpolation on the quantized\n> values is possible, which it seems to be.\n>\n>\n> >\n> > >\n> > > >\n> > > > > implementation detail that doesn't affect users much.\n> > > > >\n> > > > > One potential thing that was already brought up is whether it is even possible\n> > > > > to do the interpolation on the quantized type? I think as long as we are dealing\n> > > > > with unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\n> > > > > to `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n> > > > > \"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n> > > > >\n> > > > > Initially I thought v6 would defer quantization even more by doing everything with\n> > > > > floats and only quantizing the final interpolated data. I feel like that's a nice\n> > > >\n> > > > That would require moving quantization completely to the IPAs, right ?\n> > >\n> > > I was thinking of something like this:\n> > >\n> > > struct Components {\n> > > std::map, std::less<>> sets;\n> > >\n> > > template\n> > > void quantize(std::string_key key, typename T::QuantizedType *dst)\n> > > {\n> > > const auto &s = sets.at(key);\n> > > std::transform(s.begin(), s.end(), dst, [](float x) { return T(x).quantized(); });\n> > > }\n> > > };\n> > >\n> > > ...\n> > >\n> > > void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> > > \t\t\t\t const lsc::Components &set)\n> > > {\n> > > set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n> > > ...\n> > > }\n> > >\n> >\n> > That's nice.\n> >\n> > I had starting by defining Components with 'using' instead of defining\n> > them as a type. I can expand their definition to also do the\n> > quantization.\n> >\n> > > >\n> > > > > approach that avoids any details of quantization until the last point, but of course\n> > > > > it has the largest performance penalty as now each time the lsc tables change float\n> > > > > interpolation and quantization is carried out, instead of just the integer interpolation\n> > > > > of v5 and v6.\n> >\n> > Yes, and let me point out that v5 did the quantization when populating\n> > the vector of gains (when expanding polynomial or (in the future)\n> > re-sampling the lsc table), while v6 does that by walking the vectors\n> > after they have been populated by LscPolynomial/LscTable.\n> >\n> >\n> > > > >\n> > > > > >\n> > > > > > So we now expand polynomials and parse tables as floats, store them\n> > > > > > in a map and in LscAlgorithm we re-iterate over it to either quantized\n> > > > > > them in the polynomial case, or simply cast floats to the register bit\n> > > > > > width if table.\n> > > > > >\n> > > > > > Timing the duration of the whole RkISP1 configure operation, it now\n> > > > > > takes 3 msec compared to 600 usec.\n> > > > > >\n> > > > > > \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n> > > > > >\n> > > > > > \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n> > > > >\n> > > > > I'm honestly a bit surprised that there is a 5x time penalty here... is there\n> > > > > any chance you could attach the tuning file that you used to measure the time?\n> > > >\n> > > > Sure, I think it's just the imx219 tuning file we have in the mainline\n> > > > tree\n> > > >\n> > > > From the branch I sent to the ml:\n> > > > https://gitlab.freedesktop.org/camera/libcamera/-/blob/patchwork/6067/src/ipa/rkisp1/data/imx219.yaml\n> > > >\n> > > > >\n> > > > >\n> > > > > >\n> > > > > > Also, now each IPA has to define their own Interpolator::interpolate()\n> > > > > > overload (see rkisp1 IPA as an example of this).\n> > > > >\n> > > > > It seems to me that the it should be possible to provide the specializations\n> > > > > in a common place like in v5 without making every ipa specialize it itself\n> > > > > (which will eventually lead to duplicates). The \"Components\" type only\n> > > > > depends on the quantized type, but currently there is an unnecessary dependency\n> > > > > on the fixed point representation because it is defined inside `LscAlgorithm`,\n> > > > > moving it out would remove this dependency and would allow specialization in a\n> > > > > common place, like in v5.\n> > > > >\n> > > > >\n> > > > > >\n> > > > > > As the interface of LscAlgorithm has changed, patches on the list that\n> > > > > > port the LSC algorithm of other platforms based on v5 will have to be rebased.\n> > > > > >\n> > > > > > Signed-off-by: Jacopo Mondi \n> > > > > > ---\n> > > > >\n> > > > > So in summary, assuming the plan is to move the table based tuning files to\n> > > > > floating point number, I think my preference would be deferring the quantization\n> > > > > as much as possible (until after interpolation) and dealing with floats. However,\n> > > > > although I have not tested it, I feel the runtime cost of that would be too big\n> > > > > compared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n> > > > > * the interpolation can be moved to a common place like in v5 (as mentioned above);\n> > > >\n> > > > I presume there is way to do that by moving Components<> out of\n> > > > LscAlgorithm. As you noted, I have defined it inside the class because\n> > > > of the template dependency.\n> > >\n> > > My point is that I don't think there is a real dependency, I'm attaching a proof\n> > > of concept patch that should apply to v6.\n> >\n> > There isn't if you have the IPA provide to the Components.quantize()\n> > function the Q format\n> >\n> > set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n> >\n> > As LscAlgorithm is already templated with the Q format it would be\n> > nice to re-use this instead of having to specify.\n>\n> The way I see it, this `quantize` function in `Components` is only needed in the\n> \"third\" (not currently implemented approach) that uses floats everywhere. In that\n> case `LscAlgorithm` wouldn't be a template, and it would work with floats all the way.\n>\n>\n> >\n> > Alternatively, let's remove the templating completely from\n> > LscAlgorithm and only template quantize() with the Q format.\n> >\n> > This still requires you to\n> >\n> > 1) LscImplementation: expand polynomial/resample grid in float\n> > 2) IPA module: interpolate on floats\n> > 3) IPA module: quantize the interpolated values\n> >\n\nSo basically you mean this one..\n\nI can give it a go, so we can compare all three designs as v5, v6 and v7 :)\n\n> > while v5:\n> >\n> > 1) LscImplementation: expand polynomial/resample grid and quantize\n> > 2) IPA module: interpolate on quantized\n> >\n> > in v6:\n> >\n> > 1) LscImplementation: expand polynomial/resample grid in floats\n> > 2) LscAlgorithm: quantize values\n> > 3) IPA module: interpolate on quantized\n> >\n> >\n> > >\n> > >\n> > > >\n> > > > > * I would like take a look at the 5x runtime gap.\n> > > >\n> > > > The latest version is available in the above branch while v5 is\n> > > > available in\n> > > > https://gitlab.freedesktop.org/camera/libcamera/-/tree/patchwork/6048/\n> > > >\n> > > > The timing measurement was performed with:\n> > > >\n> > > > -------------------------------------------------------------------------------\n> > > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > > @@ -140,6 +140,8 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > > > \tconst Size &size = context.configuration.sensor.size;\n> > > > \tSize totalSize{};\n> > > >\n> > > > +std::chrono::high_resolution_clock::time_point begin = std::chrono::high_resolution_clock::now();\n> > > > +\n> > > > \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {\n> > > > \t\txSizes_[i] = xSize_[i] * size.width;\n> > > > \t\tySizes_[i] = ySize_[i] * size.height;\n> > > > @@ -162,8 +164,13 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > > > \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n> > > > \t}\n> > > >\n> > > > -\treturn lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > > > +\tlscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > > > \t\t\t\t xPos_, yPos_);\n> > > > +\tstd::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();\n> > > > +\n> > > > +\tLOG(RkISP1Lsc, Error) << \"Configure took = \" << std::chrono::duration_cast(end - begin).count() << \"[µs]\";\n> > > > +\n> > > > +\treturn 0;\n> > > > }\n> > >\n> > > Interesting, I'm only measuring the `lscAlgo_.configure()` call because the parts before\n> > > seem to be the same, and I'm getting around ~120us for v5 and ~140us for v6\n> > > with imx219.yml.\n> >\n> > With the above patch applied on top of v6:\n> >\n> > ERROR RkISP1Lsc lsc.cpp:171 Configure took = 3052[µs]\n> >\n> > What am I doing differently ?\n>\n> I'll test again.\n>\n>\n> >\n> > >\n> > >\n> > > >\n> > > > void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n> > > > -------------------------------------------------------------------------------\n> > > >\n> > > >\n> > > >\n> > > > >\n> > > > >\n> > > > > > Changes in v6:\n> > > > > > - Address comments received on v5, mostly on documentation\n> > > > > > - Remove templating of LscPolynomial and perform quantization in\n> > > > > > LscAlgorithm\n> > > > > >\n> > > > > > Changes in v5:\n> > > > > > - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> > > > > > to fix and exiting potential issue with Polynomial parsing\n> > > > > > - Fix Simple, which I broke in v4, thanks Milan\n> > > > > >\n> > > > > > Changes in v4:\n> > > > > > - s/awb/AWB in comments everywhere, same for CCM and LSC\n> > > > > > - Awb: changelog per patch\n> > > > > > - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> > > > > > modules\n> > > > > > - Reduce comments/renames to reduce patch size and comments\n> > > > > > - Collect tags\n> > > > > >\n> > > > > > Changes in v3:\n> > > > > > - Address comments on Awb from Stefan:\n> > > > > > - Introduce AwbImplementation::Result\n> > > > > > - Move AwbStats implementation to IPAs\n> > > > > > - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> > > > > > - Fix Saturation bug in Simple Ccm algorithm\n> > > > > > - Fix Simple Ccm gain limits using Q<2, 8>\n> > > > > > - Update Mali CCM and Gamma to latest uAPI patch version\n> > > > > > - Fix Mali CCM gain masking as reported by Dan\n> > > > > > - Break down libIPA Lsc implementation as requested by Stefan\n> > > > > >\n> > > > > > Changes in v2:\n> > > > > > - Introduce awb::Context to simplify ActiveState and FrameContext\n> > > > > > definition\n> > > > > > - Add mali uAPI header for CCM and Gamma\n> > > > > > - Do not rename Mali functions for hardware configuration in awb and lsc\n> > > > > > - Spelling and documentation\n> > > > > >\n> > > > > > ---\n> > > > [...]\n> >\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 55F74BDE17\n\tfor ;\n\tTue, 28 Jul 2026 14:04:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5015A67F96;\n\tTue, 28 Jul 2026 16:04: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 68C8A67E44\n\tfor ;\n\tTue, 28 Jul 2026 16:04:55 +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 90BF22F8;\n\tTue, 28 Jul 2026 16:03:50 +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=\"A0doPV3Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785247430;\n\tbh=mj0gOelpZoY3zQj2I2BR02tWfBE5G/vCQPcZZRCW53U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=A0doPV3Z51mLT7gRKLd3YaIXJXLblNsm8mpPWLgfBGQE8GnUeGIjm1IPtyBFlVOxR\n\tWBdJ8SwnC3N4lwwACC9WT7ruEUCDtWPouNffuCQjmPX7n52whWw7j4KGI/O0ppgfkb\n\ty4+IIAeVp2zN7tq9oJ7s+wNIVjYNqTh1FAB6hbRg=","Date":"Tue, 28 Jul 2026 16:04:52 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tStefan Klug , \n\tMilan Zamazal ,\n\tKieran Bingham , \n\tDaniel Scally ,\n\tRobert Mader , \n\tLaurent Pinchart ","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","Message-ID":"","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>\n\t\n\t<80740149-87b6-4d87-8e1f-b64a6515f7e7@ideasonboard.com>\n\t\n\t<71a272c8-a52c-49bb-8eb5-76a46213aa1b@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<71a272c8-a52c-49bb-8eb5-76a46213aa1b@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\" "}},{"id":39898,"web_url":"https://patchwork.libcamera.org/comment/39898/","msgid":"","date":"2026-07-28T14:20:26","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"One small correction\n\nOn Tue, Jul 28, 2026 at 04:04:52PM +0200, Jacopo Mondi wrote:\n> Hi Barnabás\n>\n> On Tue, Jul 28, 2026 at 03:57:31PM +0200, Barnabás Pőcze wrote:\n> > 2026. 07. 28. 15:42 keltezéssel, Jacopo Mondi írta:\n> > > On Tue, Jul 28, 2026 at 03:10:48PM +0200, Barnabás Pőcze wrote:\n> > > > 2026. 07. 28. 14:12 keltezéssel, Jacopo Mondi írta:\n> > > > > Hi Barnabás\n> > > > >\n> > > > > On Tue, Jul 28, 2026 at 11:48:51AM +0200, Barnabás Pőcze wrote:\n> > > > > > Hi\n> > > > > >\n> > > > > > 2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n> > > > > > > The series is identical to v5 up to patch 27. The only minor difference\n> > > > > > > is that I back-tracked on making functions class members but in the\n> > > > > > > RkISP1 IPA I kept them as static helpers.\n> > > > > > >\n> > > > > > > Patches 28 and 29 are new.\n> > > > > > >\n> > > > > > > They remove the templating of the LscAlgorithm class hierarchy and\n> > > > > > > instead implement quantization in the LscAlgorithm class.\n> > > > > >\n> > > > > > I have spent some time on this v5 vs. v6 question, and to me it seems there\n> > > > >\n> > > > > Thanks a lot\n> > > > >\n> > > > > > are no real significant differences. The interpolation is done on the quantized\n> > > > > > type in either case, the only real difference is whether the quantization happen\n> > > > > > in the implementation or in the main algorithm. And I think that's largely an\n> > > > >\n> > > > > Note there is a third option suggested by Stefan to move quantization\n> > > > > in the single IPA after the interpolation in the float domain\n> > > >\n> > > > That sounds like what I'm suggesting: load and interpolate with `float`,\n> > > > quantize the interpolated data \"in the IPA module\" right before applying.\n> > > >\n> > >\n> > > Even if we use std::transform<> doesn't it mean we have to walk all\n> > > the vectors and quantize them, while if we quantize once storing the\n> > > values in the vector before interpolation we avoid that ?\n> > >\n> > > That's the bulk of the difference between the two versions, isn't it ?\n> >\n> > Yes, and that's why I have mostly disregarded this approach in my first\n> > reply because while I kind of prefer the design, I think the runtime\n> > cost is probably not worth it -- assuming interpolation on the quantized\n> > values is possible, which it seems to be.\n> >\n> >\n> > >\n> > > >\n> > > > >\n> > > > > > implementation detail that doesn't affect users much.\n> > > > > >\n> > > > > > One potential thing that was already brought up is whether it is even possible\n> > > > > > to do the interpolation on the quantized type? I think as long as we are dealing\n> > > > > > with unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\n> > > > > > to `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n> > > > > > \"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n> > > > > >\n> > > > > > Initially I thought v6 would defer quantization even more by doing everything with\n> > > > > > floats and only quantizing the final interpolated data. I feel like that's a nice\n> > > > >\n> > > > > That would require moving quantization completely to the IPAs, right ?\n> > > >\n> > > > I was thinking of something like this:\n> > > >\n> > > > struct Components {\n> > > > std::map, std::less<>> sets;\n> > > >\n> > > > template\n> > > > void quantize(std::string_key key, typename T::QuantizedType *dst)\n> > > > {\n> > > > const auto &s = sets.at(key);\n> > > > std::transform(s.begin(), s.end(), dst, [](float x) { return T(x).quantized(); });\n> > > > }\n> > > > };\n> > > >\n> > > > ...\n> > > >\n> > > > void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> > > > \t\t\t\t const lsc::Components &set)\n> > > > {\n> > > > set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n> > > > ...\n> > > > }\n> > > >\n> > >\n> > > That's nice.\n> > >\n> > > I had starting by defining Components with 'using' instead of defining\n> > > them as a type. I can expand their definition to also do the\n> > > quantization.\n> > >\n> > > > >\n> > > > > > approach that avoids any details of quantization until the last point, but of course\n> > > > > > it has the largest performance penalty as now each time the lsc tables change float\n> > > > > > interpolation and quantization is carried out, instead of just the integer interpolation\n> > > > > > of v5 and v6.\n> > >\n> > > Yes, and let me point out that v5 did the quantization when populating\n> > > the vector of gains (when expanding polynomial or (in the future)\n> > > re-sampling the lsc table), while v6 does that by walking the vectors\n> > > after they have been populated by LscPolynomial/LscTable.\n> > >\n> > >\n> > > > > >\n> > > > > > >\n> > > > > > > So we now expand polynomials and parse tables as floats, store them\n> > > > > > > in a map and in LscAlgorithm we re-iterate over it to either quantized\n> > > > > > > them in the polynomial case, or simply cast floats to the register bit\n> > > > > > > width if table.\n> > > > > > >\n> > > > > > > Timing the duration of the whole RkISP1 configure operation, it now\n> > > > > > > takes 3 msec compared to 600 usec.\n> > > > > > >\n> > > > > > > \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n> > > > > > >\n> > > > > > > \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n> > > > > >\n> > > > > > I'm honestly a bit surprised that there is a 5x time penalty here... is there\n> > > > > > any chance you could attach the tuning file that you used to measure the time?\n> > > > >\n> > > > > Sure, I think it's just the imx219 tuning file we have in the mainline\n> > > > > tree\n> > > > >\n> > > > > From the branch I sent to the ml:\n> > > > > https://gitlab.freedesktop.org/camera/libcamera/-/blob/patchwork/6067/src/ipa/rkisp1/data/imx219.yaml\n> > > > >\n> > > > > >\n> > > > > >\n> > > > > > >\n> > > > > > > Also, now each IPA has to define their own Interpolator::interpolate()\n> > > > > > > overload (see rkisp1 IPA as an example of this).\n> > > > > >\n> > > > > > It seems to me that the it should be possible to provide the specializations\n> > > > > > in a common place like in v5 without making every ipa specialize it itself\n> > > > > > (which will eventually lead to duplicates). The \"Components\" type only\n> > > > > > depends on the quantized type, but currently there is an unnecessary dependency\n> > > > > > on the fixed point representation because it is defined inside `LscAlgorithm`,\n> > > > > > moving it out would remove this dependency and would allow specialization in a\n> > > > > > common place, like in v5.\n> > > > > >\n> > > > > >\n> > > > > > >\n> > > > > > > As the interface of LscAlgorithm has changed, patches on the list that\n> > > > > > > port the LSC algorithm of other platforms based on v5 will have to be rebased.\n> > > > > > >\n> > > > > > > Signed-off-by: Jacopo Mondi \n> > > > > > > ---\n> > > > > >\n> > > > > > So in summary, assuming the plan is to move the table based tuning files to\n> > > > > > floating point number, I think my preference would be deferring the quantization\n> > > > > > as much as possible (until after interpolation) and dealing with floats. However,\n> > > > > > although I have not tested it, I feel the runtime cost of that would be too big\n> > > > > > compared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n> > > > > > * the interpolation can be moved to a common place like in v5 (as mentioned above);\n> > > > >\n> > > > > I presume there is way to do that by moving Components<> out of\n> > > > > LscAlgorithm. As you noted, I have defined it inside the class because\n> > > > > of the template dependency.\n> > > >\n> > > > My point is that I don't think there is a real dependency, I'm attaching a proof\n> > > > of concept patch that should apply to v6.\n> > >\n> > > There isn't if you have the IPA provide to the Components.quantize()\n> > > function the Q format\n> > >\n> > > set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n> > >\n> > > As LscAlgorithm is already templated with the Q format it would be\n> > > nice to re-use this instead of having to specify.\n> >\n> > The way I see it, this `quantize` function in `Components` is only needed in the\n> > \"third\" (not currently implemented approach) that uses floats everywhere. In that\n> > case `LscAlgorithm` wouldn't be a template, and it would work with floats all the way.\n> >\n> >\n> > >\n> > > Alternatively, let's remove the templating completely from\n> > > LscAlgorithm and only template quantize() with the Q format.\n> > >\n> > > This still requires you to\n> > >\n> > > 1) LscImplementation: expand polynomial/resample grid in float\n> > > 2) IPA module: interpolate on floats\n> > > 3) IPA module: quantize the interpolated values\n> > >\n>\n> So basically you mean this one..\n>\n> I can give it a go, so we can compare all three designs as v5, v6 and v7 :)\n>\n\nAs long as we have tables in quantized format, we can't have\nLscAlgorithm work with floats.\n\nSo we need to quantize the result of the polynomial expansion before\nsending them to the IPA, which means we're left with the two below\npossibilities:\n\n> > > while v5:\n> > >\n> > > 1) LscImplementation: expand polynomial/resample grid and quantize\n> > > 2) IPA module: interpolate on quantized\n> > >\n> > > in v6:\n> > >\n> > > 1) LscImplementation: expand polynomial/resample grid in floats\n> > > 2) LscAlgorithm: quantize values\n> > > 3) IPA module: interpolate on quantized\n> > >\n\nThese ones ^\n\nNote that \"resample grid\" above is not yet implemented and we simply\nreturn the (quantized) values as parsed from the tuning file\n\nDoes it make sense ?\n\nThanks\n j\n\n> > >\n> > > >\n> > > >\n> > > > >\n> > > > > > * I would like take a look at the 5x runtime gap.\n> > > > >\n> > > > > The latest version is available in the above branch while v5 is\n> > > > > available in\n> > > > > https://gitlab.freedesktop.org/camera/libcamera/-/tree/patchwork/6048/\n> > > > >\n> > > > > The timing measurement was performed with:\n> > > > >\n> > > > > -------------------------------------------------------------------------------\n> > > > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > > > @@ -140,6 +140,8 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > > > > \tconst Size &size = context.configuration.sensor.size;\n> > > > > \tSize totalSize{};\n> > > > >\n> > > > > +std::chrono::high_resolution_clock::time_point begin = std::chrono::high_resolution_clock::now();\n> > > > > +\n> > > > > \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {\n> > > > > \t\txSizes_[i] = xSize_[i] * size.width;\n> > > > > \t\tySizes_[i] = ySize_[i] * size.height;\n> > > > > @@ -162,8 +164,13 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > > > > \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n> > > > > \t}\n> > > > >\n> > > > > -\treturn lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > > > > +\tlscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > > > > \t\t\t\t xPos_, yPos_);\n> > > > > +\tstd::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();\n> > > > > +\n> > > > > +\tLOG(RkISP1Lsc, Error) << \"Configure took = \" << std::chrono::duration_cast(end - begin).count() << \"[µs]\";\n> > > > > +\n> > > > > +\treturn 0;\n> > > > > }\n> > > >\n> > > > Interesting, I'm only measuring the `lscAlgo_.configure()` call because the parts before\n> > > > seem to be the same, and I'm getting around ~120us for v5 and ~140us for v6\n> > > > with imx219.yml.\n> > >\n> > > With the above patch applied on top of v6:\n> > >\n> > > ERROR RkISP1Lsc lsc.cpp:171 Configure took = 3052[µs]\n> > >\n> > > What am I doing differently ?\n> >\n> > I'll test again.\n> >\n> >\n> > >\n> > > >\n> > > >\n> > > > >\n> > > > > void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n> > > > > -------------------------------------------------------------------------------\n> > > > >\n> > > > >\n> > > > >\n> > > > > >\n> > > > > >\n> > > > > > > Changes in v6:\n> > > > > > > - Address comments received on v5, mostly on documentation\n> > > > > > > - Remove templating of LscPolynomial and perform quantization in\n> > > > > > > LscAlgorithm\n> > > > > > >\n> > > > > > > Changes in v5:\n> > > > > > > - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> > > > > > > to fix and exiting potential issue with Polynomial parsing\n> > > > > > > - Fix Simple, which I broke in v4, thanks Milan\n> > > > > > >\n> > > > > > > Changes in v4:\n> > > > > > > - s/awb/AWB in comments everywhere, same for CCM and LSC\n> > > > > > > - Awb: changelog per patch\n> > > > > > > - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> > > > > > > modules\n> > > > > > > - Reduce comments/renames to reduce patch size and comments\n> > > > > > > - Collect tags\n> > > > > > >\n> > > > > > > Changes in v3:\n> > > > > > > - Address comments on Awb from Stefan:\n> > > > > > > - Introduce AwbImplementation::Result\n> > > > > > > - Move AwbStats implementation to IPAs\n> > > > > > > - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> > > > > > > - Fix Saturation bug in Simple Ccm algorithm\n> > > > > > > - Fix Simple Ccm gain limits using Q<2, 8>\n> > > > > > > - Update Mali CCM and Gamma to latest uAPI patch version\n> > > > > > > - Fix Mali CCM gain masking as reported by Dan\n> > > > > > > - Break down libIPA Lsc implementation as requested by Stefan\n> > > > > > >\n> > > > > > > Changes in v2:\n> > > > > > > - Introduce awb::Context to simplify ActiveState and FrameContext\n> > > > > > > definition\n> > > > > > > - Add mali uAPI header for CCM and Gamma\n> > > > > > > - Do not rename Mali functions for hardware configuration in awb and lsc\n> > > > > > > - Spelling and documentation\n> > > > > > >\n> > > > > > > ---\n> > > > > [...]\n> > >\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 4E6DCBDE4C\n\tfor ;\n\tTue, 28 Jul 2026 14:20:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 061F967FA0;\n\tTue, 28 Jul 2026 16:20:31 +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 A66B967E44\n\tfor ;\n\tTue, 28 Jul 2026 16:20:29 +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 B31782F8;\n\tTue, 28 Jul 2026 16:19:24 +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=\"hoQUdaZe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785248364;\n\tbh=jXvN8o24P6B//kF8e3ZhsJb9nO6FI5M8JFkKK7HPrRI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hoQUdaZe/1AdtteGPuWF1SOGGRUDScFQ4q/H39aPIyLx3ccytTlPLjHGBFiLJ656q\n\tiaHuzRkbQaw0/L8VGlEZq87gorFZMWuVq2aLWrCkKATRVI5Ktr6puqS8sbBWULhRP5\n\tDe5qPhZGPaT1J2DmzD2elQZov/o8B3Sm4iyfBXfc=","Date":"Tue, 28 Jul 2026 16:20:26 +0200","From":"Jacopo Mondi ","To":"Jacopo Mondi ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org, Stefan Klug\n\t, Milan Zamazal ,\n\tKieran Bingham , Daniel Scally\n\t, \n\tRobert Mader , Laurent Pinchart\n\t","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","Message-ID":"","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>\n\t\n\t<80740149-87b6-4d87-8e1f-b64a6515f7e7@ideasonboard.com>\n\t\n\t<71a272c8-a52c-49bb-8eb5-76a46213aa1b@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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":39899,"web_url":"https://patchwork.libcamera.org/comment/39899/","msgid":"","date":"2026-07-28T14:48:17","subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"HI Barnabas\n\nOn Tue, Jul 28, 2026 at 03:10:48PM +0200, Barnabás Pőcze wrote:\n> 2026. 07. 28. 14:12 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Tue, Jul 28, 2026 at 11:48:51AM +0200, Barnabás Pőcze wrote:\n> > > Hi\n> > >\n> > > 2026. 07. 20. 16:59 keltezéssel, Jacopo Mondi írta:\n> > > > The series is identical to v5 up to patch 27. The only minor difference\n> > > > is that I back-tracked on making functions class members but in the\n> > > > RkISP1 IPA I kept them as static helpers.\n> > > >\n> > > > Patches 28 and 29 are new.\n> > > >\n> > > > They remove the templating of the LscAlgorithm class hierarchy and\n> > > > instead implement quantization in the LscAlgorithm class.\n> > >\n> > > I have spent some time on this v5 vs. v6 question, and to me it seems there\n> >\n> > Thanks a lot\n> >\n> > > are no real significant differences. The interpolation is done on the quantized\n> > > type in either case, the only real difference is whether the quantization happen\n> > > in the implementation or in the main algorithm. And I think that's largely an\n> >\n> > Note there is a third option suggested by Stefan to move quantization\n> > in the single IPA after the interpolation in the float domain\n>\n> That sounds like what I'm suggesting: load and interpolate with `float`,\n> quantize the interpolated data \"in the IPA module\" right before applying.\n>\n\nAs of now, tables are already quantized, so was can't assume\nLscImplementation::ComponentsMap as your patch does. That's\nwhere I got stuck when trying to remove the Components definition from\nLscAlgorithm.\n\nI feel like I've tried hard enough to explore all possible options and\nstill I didn't get any real argument against the v5 version if not\ngeneric complaints against templates.\n\nOr have I missed what your opinion is on that subject..\n\nShould we simply resume v5 and re-consider when we can work with\nfloats in LscImplementation ? (iow when we'll switch format in all\ntuning files).\n\nWhat do you think ?\n\n>\n> >\n> > > implementation detail that doesn't affect users much.\n> > >\n> > > One potential thing that was already brought up is whether it is even possible\n> > > to do the interpolation on the quantized type? I think as long as we are dealing\n> > > with unsigned types, it should be fine (so maybe it would be best to add a `static_assert`\n> > > to `LscAlgorithm` enforcing that), but if a platform appears that needs some kid of\n> > > \"special\" quantization from float, then `LscAlgorithm` will have to be adjusted.\n> > >\n> > > Initially I thought v6 would defer quantization even more by doing everything with\n> > > floats and only quantizing the final interpolated data. I feel like that's a nice\n> >\n> > That would require moving quantization completely to the IPAs, right ?\n>\n> I was thinking of something like this:\n>\n> struct Components {\n> std::map, std::less<>> sets;\n>\n> template\n> void quantize(std::string_key key, typename T::QuantizedType *dst)\n> {\n> const auto &s = sets.at(key);\n> std::transform(s.begin(), s.end(), dst, [](float x) { return T(x).quantized(); });\n> }\n> };\n>\n> ...\n>\n> void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> \t\t\t\t const lsc::Components &set)\n> {\n> set.quantize>(\"r\", &config.r_data_tbl[0][0]);\n> ...\n> }\n>\n> >\n> > > approach that avoids any details of quantization until the last point, but of course\n> > > it has the largest performance penalty as now each time the lsc tables change float\n> > > interpolation and quantization is carried out, instead of just the integer interpolation\n> > > of v5 and v6.\n> > >\n> > > >\n> > > > So we now expand polynomials and parse tables as floats, store them\n> > > > in a map and in LscAlgorithm we re-iterate over it to either quantized\n> > > > them in the polynomial case, or simply cast floats to the register bit\n> > > > width if table.\n> > > >\n> > > > Timing the duration of the whole RkISP1 configure operation, it now\n> > > > takes 3 msec compared to 600 usec.\n> > > >\n> > > > \tRkISP1Lsc lsc.cpp:171 Configure took = 3081[µs]\n> > > >\n> > > > \tRkISP1Lsc lsc.cpp:172 Configure took = 583[µs]\n> > >\n> > > I'm honestly a bit surprised that there is a 5x time penalty here... is there\n> > > any chance you could attach the tuning file that you used to measure the time?\n> >\n> > Sure, I think it's just the imx219 tuning file we have in the mainline\n> > tree\n> >\n> > From the branch I sent to the ml:\n> > https://gitlab.freedesktop.org/camera/libcamera/-/blob/patchwork/6067/src/ipa/rkisp1/data/imx219.yaml\n> >\n> > >\n> > >\n> > > >\n> > > > Also, now each IPA has to define their own Interpolator::interpolate()\n> > > > overload (see rkisp1 IPA as an example of this).\n> > >\n> > > It seems to me that the it should be possible to provide the specializations\n> > > in a common place like in v5 without making every ipa specialize it itself\n> > > (which will eventually lead to duplicates). The \"Components\" type only\n> > > depends on the quantized type, but currently there is an unnecessary dependency\n> > > on the fixed point representation because it is defined inside `LscAlgorithm`,\n> > > moving it out would remove this dependency and would allow specialization in a\n> > > common place, like in v5.\n> > >\n> > >\n> > > >\n> > > > As the interface of LscAlgorithm has changed, patches on the list that\n> > > > port the LSC algorithm of other platforms based on v5 will have to be rebased.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > >\n> > > So in summary, assuming the plan is to move the table based tuning files to\n> > > floating point number, I think my preference would be deferring the quantization\n> > > as much as possible (until after interpolation) and dealing with floats. However,\n> > > although I have not tested it, I feel the runtime cost of that would be too big\n> > > compared to the other approaches. So my (very) slight preference is for v6 with two caveats:\n> > > * the interpolation can be moved to a common place like in v5 (as mentioned above);\n> >\n> > I presume there is way to do that by moving Components<> out of\n> > LscAlgorithm. As you noted, I have defined it inside the class because\n> > of the template dependency.\n>\n> My point is that I don't think there is a real dependency, I'm attaching a proof\n> of concept patch that should apply to v6.\n>\n>\n> >\n> > > * I would like take a look at the 5x runtime gap.\n> >\n> > The latest version is available in the above branch while v5 is\n> > available in\n> > https://gitlab.freedesktop.org/camera/libcamera/-/tree/patchwork/6048/\n> >\n> > The timing measurement was performed with:\n> >\n> > -------------------------------------------------------------------------------\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -140,6 +140,8 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > \tconst Size &size = context.configuration.sensor.size;\n> > \tSize totalSize{};\n> >\n> > +std::chrono::high_resolution_clock::time_point begin = std::chrono::high_resolution_clock::now();\n> > +\n> > \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE; ++i) {\n> > \t\txSizes_[i] = xSize_[i] * size.width;\n> > \t\tySizes_[i] = ySize_[i] * size.height;\n> > @@ -162,8 +164,13 @@ int LensShadingCorrection::configure(IPAContext &context,\n> > \t\tyGrad_[i] = std::round(32768 / ySizes_[i]);\n> > \t}\n> >\n> > -\treturn lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > +\tlscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop,\n> > \t\t\t\t xPos_, yPos_);\n> > +\tstd::chrono::high_resolution_clock::time_point end = std::chrono::high_resolution_clock::now();\n> > +\n> > +\tLOG(RkISP1Lsc, Error) << \"Configure took = \" << std::chrono::duration_cast(end - begin).count() << \"[µs]\";\n> > +\n> > +\treturn 0;\n> > }\n>\n> Interesting, I'm only measuring the `lscAlgo_.configure()` call because the parts before\n> seem to be the same, and I'm getting around ~120us for v5 and ~140us for v6\n> with imx219.yml.\n>\n>\n> >\n> > void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config)\n> > -------------------------------------------------------------------------------\n> >\n> >\n> >\n> > >\n> > >\n> > > > Changes in v6:\n> > > > - Address comments received on v5, mostly on documentation\n> > > > - Remove templating of LscPolynomial and perform quantization in\n> > > > LscAlgorithm\n> > > >\n> > > > Changes in v5:\n> > > > - Add \"ipa: libipa: lsc_polynomial: Fix polynomial parsing error\"\n> > > > to fix and exiting potential issue with Polynomial parsing\n> > > > - Fix Simple, which I broke in v4, thanks Milan\n> > > >\n> > > > Changes in v4:\n> > > > - s/awb/AWB in comments everywhere, same for CCM and LSC\n> > > > - Awb: changelog per patch\n> > > > - Make AwbStats pure virtual and move stats parsing logic to the IPA\n> > > > modules\n> > > > - Reduce comments/renames to reduce patch size and comments\n> > > > - Collect tags\n> > > >\n> > > > Changes in v3:\n> > > > - Address comments on Awb from Stefan:\n> > > > - Introduce AwbImplementation::Result\n> > > > - Move AwbStats implementation to IPAs\n> > > > - Drop AwbStats::rg()/bg() and re-implement AwbGray accordingly\n> > > > - Fix Saturation bug in Simple Ccm algorithm\n> > > > - Fix Simple Ccm gain limits using Q<2, 8>\n> > > > - Update Mali CCM and Gamma to latest uAPI patch version\n> > > > - Fix Mali CCM gain masking as reported by Dan\n> > > > - Break down libIPA Lsc implementation as requested by Stefan\n> > > >\n> > > > Changes in v2:\n> > > > - Introduce awb::Context to simplify ActiveState and FrameContext\n> > > > definition\n> > > > - Add mali uAPI header for CCM and Gamma\n> > > > - Do not rename Mali functions for hardware configuration in awb and lsc\n> > > > - Spelling and documentation\n> > > >\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 A2180BDE4C\n\tfor ;\n\tTue, 28 Jul 2026 14:48:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B02C67F9B;\n\tTue, 28 Jul 2026 16:48:21 +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 7948367E44\n\tfor ;\n\tTue, 28 Jul 2026 16:48:20 +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 A31642F8;\n\tTue, 28 Jul 2026 16:47:15 +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=\"K1pApqCm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785250035;\n\tbh=W9CiIB4hYowMQeVz9yWNFLVhXyXzBUUOdm+WO5n8Iuw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K1pApqCm4Ddlf+o4NQvSsOecaojQtw6YSE2DUFvOcsVrq2iPBeq8w+tVcN6wd4HDl\n\tCCUcyTftATcT+EZ4xrKxde6Tzj5lOXPuYd/UkgtRpln2R9G1Sbhxf8Csy1fFc4pBBH\n\t3Np+xu8xlt5NSxBI6kS07F47qFaJbIWoyjMDRzsA=","Date":"Tue, 28 Jul 2026 16:48:17 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org,\n\tStefan Klug , \n\tMilan Zamazal ,\n\tKieran Bingham , \n\tDaniel Scally ,\n\tRobert Mader , \n\tLaurent Pinchart ","Subject":"Re: [PATCH v6 00/31] ipa: libipa: Introduce libipa algorithms","Message-ID":"","References":"<20260720-libipa-algorithms-v6-0-ececb73f97cb@ideasonboard.com>\n\t<6139a6ea-dbfe-4ffa-a928-cbbdc6366404@ideasonboard.com>\n\t\n\t<80740149-87b6-4d87-8e1f-b64a6515f7e7@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<80740149-87b6-4d87-8e1f-b64a6515f7e7@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\" "}}]