[{"id":30145,"web_url":"https://patchwork.libcamera.org/comment/30145/","msgid":"<20240629002233.GS30900@pendragon.ideasonboard.com>","date":"2024-06-29T00:22:33","subject":"Re: [PATCH v2 21/25] libtuning: lsc: rkisp1: Do not calculate ratios\n\tto green","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nThank you for the patch.\n\nOn Fri, Jun 28, 2024 at 12:47:14PM +0200, Stefan Klug wrote:\n> The hardware in the imx8mp treats the lsc tables as absolute factors and\n\ns/imx8mp/i.MX8MP/\n\nbut maybe you should mention rkisp1 here as that's not specific to the\nversion in the i.MX8MP.\n\ns/lsc/LSC/\n\n> not as ratios compared to green. Therefore every channel must calculated\n> independently.\n\nI don't think this is dictated by the hardware, but by the way the LSC\nalgorithm is implemented in the IPA. Raspberry Pi has a more advanced\nLSC algorithm that splits processing in luminance LSC and chroma LSC,\nand so needs to handle tuning accordingly. The IPA module calculates R,\nG and B LSC tables at runtime, and the hardware operates on the colour\nchannels independently.\n\nThe RkISP1 implementation is (currently) much simpler and handles the\nthree colour channels independently. That's why, as far as I understand,\nwe shouldn't base the computation on ratios.\n\n> Signed-off-by: Stefan Klug \n> ---\n> utils/tuning/libtuning/modules/lsc/rkisp1.py | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n> \n> diff --git a/utils/tuning/libtuning/modules/lsc/rkisp1.py b/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> index 5874f10c936f..a8ba454a7139 100644\n> --- a/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> +++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> @@ -38,8 +38,11 @@ class LSCRkISP1(LSC):\n> \n> # \\todo Should these ratio against the average of both greens or just\n> # each green like we've done here?\n> - cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, gr)\n> - cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, gb)\n> +\n> + # the lsc tables in the rkisp1 are gains on the corresponding channel,\n\ns/the lsc/The LSC/\n\n> + # not ratios with respect to green, so calculate them independently\n> + cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, None)\n> + cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, None)\n> \n> return image.color, cr.flatten(), cb.flatten(), cgr.flatten(), cgb.flatten()\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 7EF95BDB1D\n\tfor ;\n\tSat, 29 Jun 2024 00:22:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD17462C95;\n\tSat, 29 Jun 2024 02:22: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 5252B619E8\n\tfor ;\n\tSat, 29 Jun 2024 02:22:54 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2DCD063B;\n\tSat, 29 Jun 2024 02:22:29 +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=\"UJd7I9cq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719620549;\n\tbh=OVnFKvaCByq0lrzFIE7LlN38BWnTYDBNAvg2dGZ3qOo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UJd7I9cqoEoxC1mkV4jfqVBxCpsa5yvYpoZQ3bWhIRmkTpZzY9ccPU+hH6F07SfVr\n\tVbotjrpesWKMKuIxpLVoLEWPph8nCRo9UNQLkotSPyBjFAgicdwf0mtou2FXyxyzXm\n\twGnb7pFN8oFGGYbasQo1OW6Vo4iqykwWtXeIcfAc=","Date":"Sat, 29 Jun 2024 03:22:33 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 21/25] libtuning: lsc: rkisp1: Do not calculate ratios\n\tto green","Message-ID":"<20240629002233.GS30900@pendragon.ideasonboard.com>","References":"<20240628104828.2928109-1-stefan.klug@ideasonboard.com>\n\t<20240628104828.2928109-22-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240628104828.2928109-22-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30204,"web_url":"https://patchwork.libcamera.org/comment/30204/","msgid":"","date":"2024-07-02T10:25:21","subject":"Re: [PATCH v2 21/25] libtuning: lsc: rkisp1: Do not calculate ratios\n\tto green","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Sat, Jun 29, 2024 at 03:22:33AM +0300, Laurent Pinchart wrote:\n> Hi Stefan,\n> \n> Thank you for the patch.\n> \n> On Fri, Jun 28, 2024 at 12:47:14PM +0200, Stefan Klug wrote:\n> > The hardware in the imx8mp treats the lsc tables as absolute factors and\n> \n> s/imx8mp/i.MX8MP/\n> \n> but maybe you should mention rkisp1 here as that's not specific to the\n> version in the i.MX8MP.\n> \n> s/lsc/LSC/\n> \n> > not as ratios compared to green. Therefore every channel must calculated\n> > independently.\n> \n> I don't think this is dictated by the hardware, but by the way the LSC\n> algorithm is implemented in the IPA. Raspberry Pi has a more advanced\n> LSC algorithm that splits processing in luminance LSC and chroma LSC,\n> and so needs to handle tuning accordingly. The IPA module calculates R,\n> G and B LSC tables at runtime, and the hardware operates on the colour\n> channels independently.\n> \n> The RkISP1 implementation is (currently) much simpler and handles the\n> three colour channels independently. That's why, as far as I understand,\n> we shouldn't base the computation on ratios.\n\nThanks for the clarification. I've updated the comments/commit message\naccordingly.\n\nRegards,\nStefan\n\n> \n> > Signed-off-by: Stefan Klug \n> > ---\n> > utils/tuning/libtuning/modules/lsc/rkisp1.py | 7 +++++--\n> > 1 file changed, 5 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/utils/tuning/libtuning/modules/lsc/rkisp1.py b/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> > index 5874f10c936f..a8ba454a7139 100644\n> > --- a/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> > +++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> > @@ -38,8 +38,11 @@ class LSCRkISP1(LSC):\n> > \n> > # \\todo Should these ratio against the average of both greens or just\n> > # each green like we've done here?\n> > - cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, gr)\n> > - cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, gb)\n> > +\n> > + # the lsc tables in the rkisp1 are gains on the corresponding channel,\n> \n> s/the lsc/The LSC/\n> \n> > + # not ratios with respect to green, so calculate them independently\n> > + cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, None)\n> > + cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, None)\n> > \n> > return image.color, cr.flatten(), cb.flatten(), cgr.flatten(), cgb.flatten()\n> > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 D352FBD87C\n\tfor ;\n\tTue, 2 Jul 2024 10:25:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B253762C96;\n\tTue, 2 Jul 2024 12:25:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02DCA619CC\n\tfor ;\n\tTue, 2 Jul 2024 12:25:23 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:15de:d83a:d962:e44])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9D3885A4;\n\tTue, 2 Jul 2024 12:24:56 +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=\"DhqErNC8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719915896;\n\tbh=+cTRmvXZcWcKHyGIR9GkrvocTUuolp4ygQZqTWoekg4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DhqErNC8khrQW8wA47bmT5p6opqQjosvdY7SoMuDQNNLLkTgIOSw2Ih7mAWEumsQU\n\tjSV7L5xpgh1mXiqaxhxXA7Z6ROVd2bIHQIPUJxqtSf0gi57mMgpyaXmIjAbTKt7r4A\n\tyKtmjpV5hrWXMBIggXOLDUQhT0cUeknDXMW1pl+I=","Date":"Tue, 2 Jul 2024 12:25:21 +0200","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 21/25] libtuning: lsc: rkisp1: Do not calculate ratios\n\tto green","Message-ID":"","References":"<20240628104828.2928109-1-stefan.klug@ideasonboard.com>\n\t<20240628104828.2928109-22-stefan.klug@ideasonboard.com>\n\t<20240629002233.GS30900@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240629002233.GS30900@pendragon.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\" "}}]