[{"id":30369,"web_url":"https://patchwork.libcamera.org/comment/30369/","msgid":"<299694f1-39e3-4594-b6af-c7764a1eb077@ideasonboard.com>","date":"2024-07-05T19:19:04","subject":"Re: [PATCH v4 20/23] tuning: rkisp1: Add some static modules","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\nOn 05/07/2024 15:41, Stefan Klug wrote:\n> Add awb, blc, cproc, filter, and gamma to the tuning file.  These don't\n> need any configuration.\n>\n> At the moment there are no inter-module dependencies in the tuning\n> process. We can therefore safely sort them alphabetically. As soon as\n> the first dependency gets introduced (most likely lsc -> ccm) we will\n> see how to solve that.\n>\n> The output order controls the order of processing in the IPA. It is now\n> also in alphabetical order which happens to be no change for the modules\n> that existed previously. For the others, there is no need for a specific\n> order.\n>\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>   utils/tuning/rkisp1.py | 21 +++++++++++++++++----\n>   1 file changed, 17 insertions(+), 4 deletions(-)\n>\n> diff --git a/utils/tuning/rkisp1.py b/utils/tuning/rkisp1.py\n> index fae35783c656..0d279a39ab1b 100755\n> --- a/utils/tuning/rkisp1.py\n> +++ b/utils/tuning/rkisp1.py\n> @@ -15,11 +15,24 @@ from libtuning.generators import YamlOutput\n>   from libtuning.modules.lsc import LSCRkISP1\n>   from libtuning.modules.agc import AGCRkISP1\n>   from libtuning.modules.ccm import CCMRkISP1\n> -\n> +from libtuning.modules.static import StaticModule\n>   \n>   coloredlogs.install(level=logging.INFO, fmt='%(name)s %(levelname)s %(message)s')\n>   \n> +awb = StaticModule('Awb')\n> +blc = StaticModule('BlackLevelCorrection')\n> +color_processing = StaticModule('ColorProcessing')\n> +filter = StaticModule('Filter')\n> +gamma_out = StaticModule('GammaOutCorrection', {'gamma': 2.2})\n> +\n>   tuner = lt.Tuner('RkISP1')\n> +tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))\n> +tuner.add(awb)\n> +tuner.add(blc)\n> +tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))\n> +tuner.add(color_processing)\n> +tuner.add(filter)\n> +tuner.add(gamma_out)\n>   tuner.add(LSCRkISP1(\n\nThis is nitpicking so feel free to ignore me, but I think it'd be better to be consistent in using \nvariables for the modules or not.\n\nReviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n\n>             debug=[lt.Debug.Plot],\n>             # This is for the actual LSC tuning, and is part of the base LSC\n> @@ -39,11 +52,11 @@ tuner.add(LSCRkISP1(\n>             # values.  This can also be a custom function.\n>             smoothing_function=lt.smoothing.MedianBlur(3),\n>             ))\n> -tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))\n> -tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))\n> +\n>   tuner.set_input_parser(YamlParser())\n>   tuner.set_output_formatter(YamlOutput())\n> -tuner.set_output_order([AGCRkISP1, CCMRkISP1, LSCRkISP1])\n> +tuner.set_output_order([AGCRkISP1, awb, blc, CCMRkISP1, color_processing,\n> +                        filter, gamma_out, LSCRkISP1])\n>   \n>   if __name__ == '__main__':\n>       sys.exit(tuner.run(sys.argv))","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7D515BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Jul 2024 19:19:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7854F62E23;\n\tFri,  5 Jul 2024 21:19:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3568F619BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Jul 2024 21:19:08 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 565694CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Jul 2024 21:18:38 +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=\"T0QfeKln\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720207118;\n\tbh=szScDmHYV+CJaL46jrxjD9be8onuMXN7H24PzLswnuI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=T0QfeKln6zEK+bsmBwJiX3z3Jbf3/YOCqfXyxTbkts+m6JHCEMfIo7NRUj6cbPR1T\n\t2ySVIQuJKcaqu8m9KazfbDKrOrSnxCJQe5ukPr3+pQEN87RuQT971f7ygbg5F/LNyl\n\tC9Ul7vJ5+1tUgC7seU4ENrHsTw1xY3NfAnlERvxg=","Message-ID":"<299694f1-39e3-4594-b6af-c7764a1eb077@ideasonboard.com>","Date":"Fri, 5 Jul 2024 20:19:04 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 20/23] tuning: rkisp1: Add some static modules","To":"libcamera-devel@lists.libcamera.org","References":"<20240705144209.418906-1-stefan.klug@ideasonboard.com>\n\t<20240705144209.418906-21-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240705144209.418906-21-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30371,"web_url":"https://patchwork.libcamera.org/comment/30371/","msgid":"<va4sv6tdaf5ita7qbrukz7m5qxhvj4av5nbhzqwxv2jjvjhopr@igc3hjkqax4v>","date":"2024-07-05T19:54:12","subject":"Re: [PATCH v4 20/23] tuning: rkisp1: Add some static modules","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Dan,\n\nThank you for the review.\n\nOn Fri, Jul 05, 2024 at 08:19:04PM +0100, Dan Scally wrote:\n> Hi Stefan\n> \n> On 05/07/2024 15:41, Stefan Klug wrote:\n> > Add awb, blc, cproc, filter, and gamma to the tuning file.  These don't\n> > need any configuration.\n> > \n> > At the moment there are no inter-module dependencies in the tuning\n> > process. We can therefore safely sort them alphabetically. As soon as\n> > the first dependency gets introduced (most likely lsc -> ccm) we will\n> > see how to solve that.\n> > \n> > The output order controls the order of processing in the IPA. It is now\n> > also in alphabetical order which happens to be no change for the modules\n> > that existed previously. For the others, there is no need for a specific\n> > order.\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> > ---\n> >   utils/tuning/rkisp1.py | 21 +++++++++++++++++----\n> >   1 file changed, 17 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/utils/tuning/rkisp1.py b/utils/tuning/rkisp1.py\n> > index fae35783c656..0d279a39ab1b 100755\n> > --- a/utils/tuning/rkisp1.py\n> > +++ b/utils/tuning/rkisp1.py\n> > @@ -15,11 +15,24 @@ from libtuning.generators import YamlOutput\n> >   from libtuning.modules.lsc import LSCRkISP1\n> >   from libtuning.modules.agc import AGCRkISP1\n> >   from libtuning.modules.ccm import CCMRkISP1\n> > -\n> > +from libtuning.modules.static import StaticModule\n> >   coloredlogs.install(level=logging.INFO, fmt='%(name)s %(levelname)s %(message)s')\n> > +awb = StaticModule('Awb')\n> > +blc = StaticModule('BlackLevelCorrection')\n> > +color_processing = StaticModule('ColorProcessing')\n> > +filter = StaticModule('Filter')\n> > +gamma_out = StaticModule('GammaOutCorrection', {'gamma': 2.2})\n> > +\n> >   tuner = lt.Tuner('RkISP1')\n> > +tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))\n> > +tuner.add(awb)\n> > +tuner.add(blc)\n> > +tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))\n> > +tuner.add(color_processing)\n> > +tuner.add(filter)\n> > +tuner.add(gamma_out)\n> >   tuner.add(LSCRkISP1(\n> \n> This is nitpicking so feel free to ignore me, but I think it'd be better to\n> be consistent in using variables for the modules or not.\n\nHonestly I don't know why I didn't do that. At first I wanted to add the\nstatic modules the same way as the others, but I couldn't do that\nbecause multiple entries of the StaticModule class in the output_order\nwould be detected as error. I then (unwillingly) had to go for the\nintermediate variables. But I never thought about doing the same with\nthe others even though I really disliked the asymmetry.\n\nSo thanks for the hint :-). I'll add that as a separate patch if that's\nok for you.\n\nCheers,\nStefan\n\n> \n> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n> \n> >             debug=[lt.Debug.Plot],\n> >             # This is for the actual LSC tuning, and is part of the base LSC\n> > @@ -39,11 +52,11 @@ tuner.add(LSCRkISP1(\n> >             # values.  This can also be a custom function.\n> >             smoothing_function=lt.smoothing.MedianBlur(3),\n> >             ))\n> > -tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))\n> > -tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))\n> > +\n> >   tuner.set_input_parser(YamlParser())\n> >   tuner.set_output_formatter(YamlOutput())\n> > -tuner.set_output_order([AGCRkISP1, CCMRkISP1, LSCRkISP1])\n> > +tuner.set_output_order([AGCRkISP1, awb, blc, CCMRkISP1, color_processing,\n> > +                        filter, gamma_out, LSCRkISP1])\n> >   if __name__ == '__main__':\n> >       sys.exit(tuner.run(sys.argv))","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EE885BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Jul 2024 19:54:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18790619BF;\n\tFri,  5 Jul 2024 21:54:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 825B9619BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Jul 2024 21:54:15 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:60b6:33a3:3a20:6030])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 852854CC;\n\tFri,  5 Jul 2024 21:53:45 +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=\"UvDe5GsS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720209225;\n\tbh=5Oh3sdOE7c1fpPMeHNNrBGW6xvnAMAztiL7zo4SaLuk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UvDe5GsSoZKl3cyud25vK3mglkIqQJWEi6MhvRBwbo3zn5FaqTEYD4hsHHvbMNyeL\n\t6+qLJ9KqIwslRp+DwPZGdmNzt1J8SEowh8GQio6OGK+0Pg0lKcHF4nH1MYZShylOCP\n\tWtOTfXumXolbcfbfMp2ReriOT8mQwjSXJhUO2pTs=","Date":"Fri, 5 Jul 2024 21:54:12 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Dan Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 20/23] tuning: rkisp1: Add some static modules","Message-ID":"<va4sv6tdaf5ita7qbrukz7m5qxhvj4av5nbhzqwxv2jjvjhopr@igc3hjkqax4v>","References":"<20240705144209.418906-1-stefan.klug@ideasonboard.com>\n\t<20240705144209.418906-21-stefan.klug@ideasonboard.com>\n\t<299694f1-39e3-4594-b6af-c7764a1eb077@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<299694f1-39e3-4594-b6af-c7764a1eb077@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30385,"web_url":"https://patchwork.libcamera.org/comment/30385/","msgid":"<Zo9iV6GsYaf0V3El@pyrite.rasen.tech>","date":"2024-07-11T04:40:55","subject":"Re: [PATCH v4 20/23] tuning: rkisp1: Add some static modules","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, Jul 05, 2024 at 09:54:12PM +0200, Stefan Klug wrote:\n> Hi Dan,\n> \n> Thank you for the review.\n> \n> On Fri, Jul 05, 2024 at 08:19:04PM +0100, Dan Scally wrote:\n> > Hi Stefan\n> > \n> > On 05/07/2024 15:41, Stefan Klug wrote:\n> > > Add awb, blc, cproc, filter, and gamma to the tuning file.  These don't\n> > > need any configuration.\n> > > \n> > > At the moment there are no inter-module dependencies in the tuning\n> > > process. We can therefore safely sort them alphabetically. As soon as\n> > > the first dependency gets introduced (most likely lsc -> ccm) we will\n> > > see how to solve that.\n> > > \n> > > The output order controls the order of processing in the IPA. It is now\n> > > also in alphabetical order which happens to be no change for the modules\n> > > that existed previously. For the others, there is no need for a specific\n> > > order.\n> > > \n> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> > > ---\n> > >   utils/tuning/rkisp1.py | 21 +++++++++++++++++----\n> > >   1 file changed, 17 insertions(+), 4 deletions(-)\n> > > \n> > > diff --git a/utils/tuning/rkisp1.py b/utils/tuning/rkisp1.py\n> > > index fae35783c656..0d279a39ab1b 100755\n> > > --- a/utils/tuning/rkisp1.py\n> > > +++ b/utils/tuning/rkisp1.py\n> > > @@ -15,11 +15,24 @@ from libtuning.generators import YamlOutput\n> > >   from libtuning.modules.lsc import LSCRkISP1\n> > >   from libtuning.modules.agc import AGCRkISP1\n> > >   from libtuning.modules.ccm import CCMRkISP1\n> > > -\n> > > +from libtuning.modules.static import StaticModule\n> > >   coloredlogs.install(level=logging.INFO, fmt='%(name)s %(levelname)s %(message)s')\n> > > +awb = StaticModule('Awb')\n> > > +blc = StaticModule('BlackLevelCorrection')\n> > > +color_processing = StaticModule('ColorProcessing')\n> > > +filter = StaticModule('Filter')\n> > > +gamma_out = StaticModule('GammaOutCorrection', {'gamma': 2.2})\n> > > +\n> > >   tuner = lt.Tuner('RkISP1')\n> > > +tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))\n> > > +tuner.add(awb)\n> > > +tuner.add(blc)\n> > > +tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))\n> > > +tuner.add(color_processing)\n> > > +tuner.add(filter)\n> > > +tuner.add(gamma_out)\n> > >   tuner.add(LSCRkISP1(\n> > \n> > This is nitpicking so feel free to ignore me, but I think it'd be better to\n> > be consistent in using variables for the modules or not.\n> \n> Honestly I don't know why I didn't do that. At first I wanted to add the\n> static modules the same way as the others, but I couldn't do that\n> because multiple entries of the StaticModule class in the output_order\n> would be detected as error. I then (unwillingly) had to go for the\n> intermediate variables. But I never thought about doing the same with\n> the others even though I really disliked the asymmetry.\n\nIt would be a lot of duplicate text even if you changed output_order\nvalidation since you'd have:\n\ntuner.add(StaticModule('BlackLevelCorrection'))\n...\ntuner.set_output_order([... , StaticModule('BlackLevelCorrection'), ...])\n\nWhereas for the others there are no extra parameters so it's less of an\nissue just duplicating the class.\n\n\nPaul\n\n> \n> So thanks for the hint :-). I'll add that as a separate patch if that's\n> ok for you.\n> \n> Cheers,\n> Stefan\n> \n> > \n> > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n> > \n> > >             debug=[lt.Debug.Plot],\n> > >             # This is for the actual LSC tuning, and is part of the base LSC\n> > > @@ -39,11 +52,11 @@ tuner.add(LSCRkISP1(\n> > >             # values.  This can also be a custom function.\n> > >             smoothing_function=lt.smoothing.MedianBlur(3),\n> > >             ))\n> > > -tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))\n> > > -tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))\n> > > +\n> > >   tuner.set_input_parser(YamlParser())\n> > >   tuner.set_output_formatter(YamlOutput())\n> > > -tuner.set_output_order([AGCRkISP1, CCMRkISP1, LSCRkISP1])\n> > > +tuner.set_output_order([AGCRkISP1, awb, blc, CCMRkISP1, color_processing,\n> > > +                        filter, gamma_out, LSCRkISP1])\n> > >   if __name__ == '__main__':\n> > >       sys.exit(tuner.run(sys.argv))","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7A988BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 11 Jul 2024 04:41:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68FDF6336F;\n\tThu, 11 Jul 2024 06:41: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 9A70F619AB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 11 Jul 2024 06:41:02 +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 CBCC18D0;\n\tThu, 11 Jul 2024 06:40:27 +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=\"K0l2sOeI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720672828;\n\tbh=hXsO5etMYUWmhww/Bcb9PmzEY6r/9SUo9fYNdFG6+Js=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K0l2sOeIKRSq8r4g2TLgEBVqxJt8wshwX1lg2A+S5IxUQyvqFF3wjm+o/+z8eauQE\n\tSQz+2wdyIPib73ORsqyJpngEeImXGGYhJWFoPZ0yfsgQO1E32VEns/BWvTK+QLwIn2\n\tePwhhis29looYTYn++DXDQODV8jmnjbmKA/JTmZQ=","Date":"Thu, 11 Jul 2024 13:40:55 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"Dan Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 20/23] tuning: rkisp1: Add some static modules","Message-ID":"<Zo9iV6GsYaf0V3El@pyrite.rasen.tech>","References":"<20240705144209.418906-1-stefan.klug@ideasonboard.com>\n\t<20240705144209.418906-21-stefan.klug@ideasonboard.com>\n\t<299694f1-39e3-4594-b6af-c7764a1eb077@ideasonboard.com>\n\t<va4sv6tdaf5ita7qbrukz7m5qxhvj4av5nbhzqwxv2jjvjhopr@igc3hjkqax4v>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<va4sv6tdaf5ita7qbrukz7m5qxhvj4av5nbhzqwxv2jjvjhopr@igc3hjkqax4v>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]