[{"id":30312,"web_url":"https://patchwork.libcamera.org/comment/30312/","msgid":"<20240704102317.GB6230@pendragon.ideasonboard.com>","date":"2024-07-04T10:23:17","subject":"Re: [PATCH v3 21/23] 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 Wed, Jul 03, 2024 at 04:17:10PM +0200, Stefan Klug wrote:\n> The current LSC algorithm for the rkisp1 just forwards the LSC tables to\n> the hardware, so absolute factors are needed and not ratios compared to\n> green. Therefore every channel needs to be calculated independently.\n> \n> Signed-off-by: Stefan Klug \n> ---\n> utils/tuning/libtuning/modules/lsc/rkisp1.py | 8 ++++++--\n> 1 file changed, 6 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 512233aeae9d..56df23ec7b42 100644\n> --- a/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> +++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> @@ -38,8 +38,12 @@ 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\nI think this comment can be dropped.\n\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\ngr and gb are not used anymore, so you can drop them from the lines\nabove this hunk.\n\n> +\n> + # The LSC tables in the our rkisp1 algorithm represent gains on the\n> + # corresponding channel, not ratios with respect to green, so calculate\n> + # 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\nNone is the default value for the last parameter so you can drop it.\n\nI think all of this should become\n\n # Perform LSC on each colour channel independently. A future enhancement\n\t# worth investigating would be splitting the luminance and chrominance\n\t# LSC as done by Raspberry Pi.\n cgr, _ = self._lsc_single_channel(image.channels[lt.Color.GR], image)\n cgb, _ = self._lsc_single_channel(image.channels[lt.Color.GB], image)\n cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image)\n cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image)\n\nYou can drop the future enhancement sentence if desired.\n\nReviewed-by: Laurent Pinchart \n\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 1EAD8BD87C\n\tfor ;\n\tThu, 4 Jul 2024 10:23:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E7EA62E24;\n\tThu, 4 Jul 2024 12:23:40 +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 9145C619C8\n\tfor ;\n\tThu, 4 Jul 2024 12:23:38 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A8950502;\n\tThu, 4 Jul 2024 12:23:09 +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=\"LIGDERKw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720088589;\n\tbh=r/GYVnH9oqZnZvdiEfB/PwScj/ZCGry77qKT1OJBUEQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LIGDERKwUKFolPqJyB4riXOvS439iDuqbPeRgY9SX2D2yZdsYyAZl8+70Z1xYBvEM\n\tAuSLG4ZJoQEFy2+so4NSMtLPiaUGl/7o1lL0dX+Le+AKqZxyxpQz+p/x07PY9pBDbK\n\twPj1wnH9gQhKWPhDJ4VzRt7bW/ACpLb7B5G8p5/E=","Date":"Thu, 4 Jul 2024 13:23:17 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 21/23] libtuning: lsc: rkisp1: Do not calculate ratios\n\tto green","Message-ID":"<20240704102317.GB6230@pendragon.ideasonboard.com>","References":"<20240703141726.252368-1-stefan.klug@ideasonboard.com>\n\t<20240703141726.252368-22-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240703141726.252368-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":30338,"web_url":"https://patchwork.libcamera.org/comment/30338/","msgid":"","date":"2024-07-05T06:28:13","subject":"Re: [PATCH v3 21/23] libtuning: lsc: rkisp1: Do not calculate ratios\n\tto green","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Jul 03, 2024 at 04:17:10PM +0200, Stefan Klug wrote:\n> The current LSC algorithm for the rkisp1 just forwards the LSC tables to\n> the hardware, so absolute factors are needed and not ratios compared to\n> green. Therefore every channel needs to be calculated independently.\n> \n> Signed-off-by: Stefan Klug \n\nWith Laurent's comments addressed,\n\nReviewed-by: Paul Elder \n\n> ---\n> utils/tuning/libtuning/modules/lsc/rkisp1.py | 8 ++++++--\n> 1 file changed, 6 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 512233aeae9d..56df23ec7b42 100644\n> --- a/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> +++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py\n> @@ -38,8 +38,12 @@ 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 our rkisp1 algorithm represent gains on the\n> + # corresponding channel, not ratios with respect to green, so calculate\n> + # 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> 2.43.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C5B6CBEFBE\n\tfor ;\n\tFri, 5 Jul 2024 06:28:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4372762E22;\n\tFri, 5 Jul 2024 08:28:22 +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 77F30619BE\n\tfor ;\n\tFri, 5 Jul 2024 08:28:20 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 102217F3;\n\tFri, 5 Jul 2024 08:27: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=\"Je2opbyh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720160871;\n\tbh=ZENfm7Pk5mOWaxWDa9zIlbvsUPFr/GVSxhFSn7q4CjQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Je2opbyhY0ZJKqoGZWpj4YkX0Hq0kkbhzObgjT0Z9ZzQZrCSRQXJHu+9AoVSWIjxl\n\t81ru3lago0atjJ7cKrhtyHoW+8wfrYfQq+2VL0QEswCK1z6hNa0AapiWQ74xDsyF4W\n\t5Op2VJaL8Frz7UqrtvaEQMK/MHihVJfTamFyFhp4=","Date":"Fri, 5 Jul 2024 15:28:13 +0900","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 21/23] libtuning: lsc: rkisp1: Do not calculate ratios\n\tto green","Message-ID":"","References":"<20240703141726.252368-1-stefan.klug@ideasonboard.com>\n\t<20240703141726.252368-22-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240703141726.252368-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\" "}}]