[{"id":30310,"web_url":"https://patchwork.libcamera.org/comment/30310/","msgid":"<ZoZ0ZCSnPl7EieTu@pyrite.rasen.tech>","date":"2024-07-04T10:07:32","subject":"Re: [PATCH v3 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 Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote:\n> Add awb, blc, cproc, filter, and gamma by default to the tuning file. These\n> don't need any configuration.\n> \n> While at it, sort the modules alphabetically.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@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>            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\nThis changes order of executing the tuning. I thought you want lsc to\ntune before ccm and agc?\n\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\nThis determines the order of output and thus the order of executing the\nalgorithms.\n\n\nPaul\n\n>  \n>  if __name__ == '__main__':\n>      sys.exit(tuner.run(sys.argv))\n> -- \n> 2.43.0\n>","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 08C99BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Jul 2024 10:07:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0BCAC62C99;\n\tThu,  4 Jul 2024 12:07:41 +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 3BBFD62C99\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jul 2024 12:07:39 +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 85226502;\n\tThu,  4 Jul 2024 12:07: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=\"cr2z99Mv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720087630;\n\tbh=vmBKc+cpDKF6Y2d3tbONSuay7NCD0TR/aIgKNYJUNp8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cr2z99Mven8YhaNbT54EGemeJvo6sMU+mffHXEe9XOqouTqmBSHtUjZjdhtTqwSHZ\n\tCCEZdNoyMaTyLqV1EjK2l8EkOYnf+Qn9zV8QpWkWRZVHwAMdotqGFh3IrA1Np/D78n\n\tT3OMvAo3vv7zOUrTw7/l4fLq2twK+uIDC+gYT9nc=","Date":"Thu, 4 Jul 2024 19:07:32 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 20/23] tuning: rkisp1: Add some static modules","Message-ID":"<ZoZ0ZCSnPl7EieTu@pyrite.rasen.tech>","References":"<20240703141726.252368-1-stefan.klug@ideasonboard.com>\n\t<20240703141726.252368-21-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240703141726.252368-21-stefan.klug@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":30341,"web_url":"https://patchwork.libcamera.org/comment/30341/","msgid":"<ox6smqedfkywklcitwkhvfnxdqc3lamiyxqlkedmfus7mvsemc@u5xv7euiwzsk>","date":"2024-07-05T07:34:23","subject":"Re: [PATCH v3 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 Paul,\n\nThanks for the review.\n\nOn Thu, Jul 04, 2024 at 07:07:32PM +0900, Paul Elder wrote:\n> On Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote:\n> > Add awb, blc, cproc, filter, and gamma by default to the tuning file. These\n> > don't need any configuration.\n> > \n> > While at it, sort the modules alphabetically.\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@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> >            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> This changes order of executing the tuning. I thought you want lsc to\n> tune before ccm and agc?\n\nThis was discussed briefly here: https://patchwork.libcamera.org/patch/20479/#30202\nRight now we don't have inner module dependencies. But I'm fine with\nkeeping the old order if you prefer.\n\n> \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> This determines the order of output and thus the order of executing the\n> algorithms.\n\nAh ok. I wasn't aware that this also has an influence on the order of\nprocessing inside the IPA. But now that you say it, it seems logical.\nFrom a user point of view I didn't expect that. Should the order of\nprocessing be defined/fixed inside the IPA?\n\nTo get this patch in: Would it be ok for you if I order the modules here\n(and possibly above) in the order of processing inside the actual\nhardware and we postpone the discussion until we get there?\n\nBest regards,\nStefan\n\n> \n> \n> Paul\n> \n> >  \n> >  if __name__ == '__main__':\n> >      sys.exit(tuner.run(sys.argv))\n> > -- \n> > 2.43.0\n> >","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 C9971BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Jul 2024 07:34:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB49F62E22;\n\tFri,  5 Jul 2024 09:34:27 +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 692C462C96\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Jul 2024 09:34:26 +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 D4D214CC;\n\tFri,  5 Jul 2024 09:33: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=\"NDSqXSGa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720164836;\n\tbh=kXFF1O3I6GbWNDDW3UMIHNCm0axcZylpYf3b5jcSpUo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NDSqXSGakjNK5/W06akeF820j34CeHvzf1WSfhjDmaf4DjOmcO+fLwEJzCY+mo5ki\n\tQzMoqhhJnRhsx04wsgdDjy4qpTHJdr6SRd4QlJ/rBItEhArUqe4wtxt3Eg/J9IrFXG\n\tLnsfH8Cjqei2OWTvATh6fsvEkMEVpk9f+dY6lg7s=","Date":"Fri, 5 Jul 2024 09:34:23 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 20/23] tuning: rkisp1: Add some static modules","Message-ID":"<ox6smqedfkywklcitwkhvfnxdqc3lamiyxqlkedmfus7mvsemc@u5xv7euiwzsk>","References":"<20240703141726.252368-1-stefan.klug@ideasonboard.com>\n\t<20240703141726.252368-21-stefan.klug@ideasonboard.com>\n\t<ZoZ0ZCSnPl7EieTu@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<ZoZ0ZCSnPl7EieTu@pyrite.rasen.tech>","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":30342,"web_url":"https://patchwork.libcamera.org/comment/30342/","msgid":"<ZoeorEgAjERq-e3b@pyrite.rasen.tech>","date":"2024-07-05T08:02:52","subject":"Re: [PATCH v3 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:34:23AM +0200, Stefan Klug wrote:\n> Hi Paul,\n> \n> Thanks for the review.\n> \n> On Thu, Jul 04, 2024 at 07:07:32PM +0900, Paul Elder wrote:\n> > On Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote:\n> > > Add awb, blc, cproc, filter, and gamma by default to the tuning file. These\n> > > don't need any configuration.\n> > > \n> > > While at it, sort the modules alphabetically.\n> > > \n> > > Signed-off-by: Stefan Klug <stefan.klug@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> > >            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> > This changes order of executing the tuning. I thought you want lsc to\n> > tune before ccm and agc?\n> \n> This was discussed briefly here: https://patchwork.libcamera.org/patch/20479/#30202\n> Right now we don't have inner module dependencies. But I'm fine with\n> keeping the old order if you prefer.\n\nAh ok I hadn' seen that. If there's no dependency *now* then yeah we can\ncome up with a better dependency mechanism later when you do add it\nlater.\n\n> \n> > \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> > This determines the order of output and thus the order of executing the\n> > algorithms.\n> \n> Ah ok. I wasn't aware that this also has an influence on the order of\n> processing inside the IPA. But now that you say it, it seems logical.\n>\n> From a user point of view I didn't expect that. Should the order of\n> processing be defined/fixed inside the IPA?\n> \n\nI'm not sure. I thought it was fine for users to be able to modify it\nvia the tuning file (that's how algos are enabled/disabled anyway).\n\n> To get this patch in: Would it be ok for you if I order the modules here\n> (and possibly above) in the order of processing inside the actual\n> hardware and we postpone the discussion until we get there?\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> > \n> > >  \n> > >  if __name__ == '__main__':\n> > >      sys.exit(tuner.run(sys.argv))\n> > > -- \n> > > 2.43.0\n> > >","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 22427BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Jul 2024 08:03:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAEB962E22;\n\tFri,  5 Jul 2024 10:03:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D068C62C96\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Jul 2024 10:02:59 +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 52924541;\n\tFri,  5 Jul 2024 10:02: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=\"dLWPO3z4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720166550;\n\tbh=rtV2hJJIM9Ah+oA24NtWM0XCu0HToMRIQv8NByFroxY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dLWPO3z4bjcKla7yiUHvULYAkF2EtyIntXXrfD2LWjrvLv74RfZTlrVv7o6h3fcN4\n\t0m0BYX6MY/EYoMCEl7f+o8d8K8Cngq47z3gldSPX7rlpcr1FIA7Neet2H0L/bXtGvB\n\tUebUhrHZWO+zOYIw6oC7Doyk3K0hCyhlwVYWBmik=","Date":"Fri, 5 Jul 2024 17:02:52 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 20/23] tuning: rkisp1: Add some static modules","Message-ID":"<ZoeorEgAjERq-e3b@pyrite.rasen.tech>","References":"<20240703141726.252368-1-stefan.klug@ideasonboard.com>\n\t<20240703141726.252368-21-stefan.klug@ideasonboard.com>\n\t<ZoZ0ZCSnPl7EieTu@pyrite.rasen.tech>\n\t<ox6smqedfkywklcitwkhvfnxdqc3lamiyxqlkedmfus7mvsemc@u5xv7euiwzsk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<ox6smqedfkywklcitwkhvfnxdqc3lamiyxqlkedmfus7mvsemc@u5xv7euiwzsk>","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":30343,"web_url":"https://patchwork.libcamera.org/comment/30343/","msgid":"<20240705082506.GE24184@pendragon.ideasonboard.com>","date":"2024-07-05T08:25:06","subject":"Re: [PATCH v3 20/23] tuning: rkisp1: Add some static modules","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jul 05, 2024 at 05:02:52PM +0900, Paul Elder wrote:\n> On Fri, Jul 05, 2024 at 09:34:23AM +0200, Stefan Klug wrote:\n> > Hi Paul,\n> > \n> > Thanks for the review.\n> > \n> > On Thu, Jul 04, 2024 at 07:07:32PM +0900, Paul Elder wrote:\n> > > On Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote:\n> > > > Add awb, blc, cproc, filter, and gamma by default to the tuning file. These\n> > > > don't need any configuration.\n> > > > \n> > > > While at it, sort the modules alphabetically.\n> > > > \n> > > > Signed-off-by: Stefan Klug <stefan.klug@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> > > >            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> > > This changes order of executing the tuning. I thought you want lsc to\n> > > tune before ccm and agc?\n> > \n> > This was discussed briefly here: https://patchwork.libcamera.org/patch/20479/#30202\n> > Right now we don't have inner module dependencies. But I'm fine with\n> > keeping the old order if you prefer.\n> \n> Ah ok I hadn' seen that. If there's no dependency *now* then yeah we can\n> come up with a better dependency mechanism later when you do add it\n> later.\n> \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> > > This determines the order of output and thus the order of executing the\n> > > algorithms.\n> > \n> > Ah ok. I wasn't aware that this also has an influence on the order of\n> > processing inside the IPA. But now that you say it, it seems logical.\n> >\n> > From a user point of view I didn't expect that. Should the order of\n> > processing be defined/fixed inside the IPA?\n> > \n> \n> I'm not sure. I thought it was fine for users to be able to modify it\n> via the tuning file (that's how algos are enabled/disabled anyway).\n\nThe IPA design is that algorithms are controlled from tuning files,\nallowing the user to select which algorithms should run, but also the\norder. While this isn't currently doable without recompilation, one goal\nwas to make it possible for users to implement additional algorithms.\nI'd rather keep the ordering controlled by the tuning file for now.\n\n> > To get this patch in: Would it be ok for you if I order the modules here\n> > (and possibly above) in the order of processing inside the actual\n> > hardware and we postpone the discussion until we get there?\n> \n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> \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 5FA44BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Jul 2024 08:25:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 751B362E22;\n\tFri,  5 Jul 2024 10:25:29 +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 304B062C96\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Jul 2024 10:25:28 +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 88B77541;\n\tFri,  5 Jul 2024 10:24:58 +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=\"fUgXuBDk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720167898;\n\tbh=vqTp8AQRRsxBI45z39lTg9Za2x3j8JF1Uz9NNk1GrlA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fUgXuBDk71Jibvx1CfANY5WNRC9BJBCiHis9aeg+EHzDRnntB9sLTX1lvozToVnI1\n\tPFtx1q5BJadZR0dD43Z+lBhg+5xOcqfr0D01rBBN5ZQR9J9z/DOeEWkQQ9AwLu/tkT\n\tozsdCrzk6CPd7jCzgb2Na3c2a3XARxxhW7fxuW7U=","Date":"Fri, 5 Jul 2024 11:25:06 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 20/23] tuning: rkisp1: Add some static modules","Message-ID":"<20240705082506.GE24184@pendragon.ideasonboard.com>","References":"<20240703141726.252368-1-stefan.klug@ideasonboard.com>\n\t<20240703141726.252368-21-stefan.klug@ideasonboard.com>\n\t<ZoZ0ZCSnPl7EieTu@pyrite.rasen.tech>\n\t<ox6smqedfkywklcitwkhvfnxdqc3lamiyxqlkedmfus7mvsemc@u5xv7euiwzsk>\n\t<ZoeorEgAjERq-e3b@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<ZoeorEgAjERq-e3b@pyrite.rasen.tech>","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>"}}]