[{"id":25138,"web_url":"https://patchwork.libcamera.org/comment/25138/","msgid":"","date":"2022-09-28T02:13:35","subject":"Re: [libcamera-devel] [RFC PATCH] [RFC] [DNI] utils: tuning: Draft\n\ta new tuning infrastructure","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Mon, Sep 26, 2022 at 02:09:31PM +0900, Paul Elder wrote:\n> This patch introduces the interface of a new tuning infrastructure, with\n> samples of how I imagine different platforms would use it (aptly named\n> \"libtuning\" for now) to create their own tuning scripts.\n> \n> The samples include (in reverse order of how they appear in the patch,\n> but in semantic order of how I want to show them):\n> - rkisp1, which for now only has ALSC, and comes with most of the\n> descriptions in the form of comments\n> - raspberrypi's alsc_only, which is similar to rkisp1's, to show how\n> different platforms could implement similar-but-slightly-different\n> tuning scripts without much duplication\n> - raspberrypi's main one, which only has ALSC and a dummy AWB one for\n> now\n> \n> The two raspberrypi tuning scripts have a shared ALSC component, which\n> is in a separate file to show where platforms could place custom\n> components if libtuning doesn't support exactly what the platform's\n> tuning script wants.\n> \n> Obviously none of this runs (hence the DNI) and none of it is\n> implemented yet, but I wanted to show how it would be used first and to\n> gather comments on it.\n> \n> There are some questions nested in the descriptions comments as well.\n> \n> Signed-off-by: Paul Elder \n> ---\n> utils/tuning/libtuning/__init__.py | 0\n> utils/tuning/libtuning/modules/__init__.py | 0\n> utils/tuning/raspberrypi.py | 37 ++++++++++++\n> utils/tuning/raspberrypi/__init__.py | 0\n> utils/tuning/raspberrypi/alsc.py | 16 ++++++\n> utils/tuning/raspberrypi_alsc_only.py | 16 ++++++\n> utils/tuning/rkisp1.py | 67 ++++++++++++++++++++++\n> 7 files changed, 136 insertions(+)\n> create mode 100644 utils/tuning/libtuning/__init__.py\n> create mode 100644 utils/tuning/libtuning/modules/__init__.py\n> create mode 100644 utils/tuning/raspberrypi.py\n> create mode 100644 utils/tuning/raspberrypi/__init__.py\n> create mode 100644 utils/tuning/raspberrypi/alsc.py\n> create mode 100644 utils/tuning/raspberrypi_alsc_only.py\n> create mode 100644 utils/tuning/rkisp1.py\n> \n> diff --git a/utils/tuning/libtuning/__init__.py b/utils/tuning/libtuning/__init__.py\n> new file mode 100644\n> index 00000000..e69de29b\n> diff --git a/utils/tuning/libtuning/modules/__init__.py b/utils/tuning/libtuning/modules/__init__.py\n> new file mode 100644\n> index 00000000..e69de29b\n> diff --git a/utils/tuning/raspberrypi.py b/utils/tuning/raspberrypi.py\n> new file mode 100644\n> index 00000000..322b9c36\n> --- /dev/null\n> +++ b/utils/tuning/raspberrypi.py\n> @@ -0,0 +1,37 @@\n> +import sys\n> +\n> +import libtuning as lt\n> +from libtuning.modules import AWB\n> +from libtuning.parsers import RaspberryPiParser\n> +from libtuning.generators import RaspberryPiOutput\n> +\n> +from raspberrypi.alsc import ALSC as RaspberryPiALSC\n> +\n> +tuner = lt.Camera()\n\nNitpicking on naming, it feels weird to name a variable \"tuner\" when it\nis of type \"Camera\".\n\n> +\n> +# These modules can also be custom modules. libtuning will come with utilities\n> +# for handling stuff like images, so there shouldn't be too much boilerplate\n> +# involved in creating custom modules, though I don't yet have a concrete\n> +# vision on how custom implementations of modules would look.\n> +tuner.add(RaspberryPiALSC)\n> +\n> +# Other tuning modules can be added like so.\n> +# The order that the tuning modules will be executed is determined by the order\n> +# that they're added.\n> +# This is kind of an implementation detail, but the \"context\" is saved\n> +# internally in lt.Camera, so modules that are added (and therefore executed)\n> +# later can use the output of the previous modules. I'm thinking that a module\n> +# that depends on values from another module has two modes of execution, for\n> +# when those values are available and another for when they're not. Not quite\n> +# sure concretely how to handle this yet.\n> +tuner.add(AWB( # module parameters\n> + ))\n> +\n> +tuner.setInputType(RaspberryPiParser)\n> +tuner.setOutputType(RaspberryPiOutput)\n> +\n> +# The order of the output doesn't necessarily have to be the same as the order\n> +# of input, which is specified by the order of adding the modules above.\n> +tuner.setOutputOrder(AWB, ALSC)\n> +\n> +tuner.run(sys.argv)\n> diff --git a/utils/tuning/raspberrypi/__init__.py b/utils/tuning/raspberrypi/__init__.py\n> new file mode 100644\n> index 00000000..e69de29b\n> diff --git a/utils/tuning/raspberrypi/alsc.py b/utils/tuning/raspberrypi/alsc.py\n> new file mode 100644\n> index 00000000..ff8a02fd\n> --- /dev/null\n> +++ b/utils/tuning/raspberrypi/alsc.py\n> @@ -0,0 +1,16 @@\n> +ALSC(do_color = lt.paramsIfUnset(True), \\\n> + debug = [lt.debug.Plot], \\\n> + luminance_strength = lt.params.IfUnSet(0.5) \\\n> + sector_shape = (16, 12), \\\n> + sector_x_gradient = lt.gradients.Linear, \\\n> + sector_y_gradient = lt.gradients.Linear, \\\n> + sector_x_remainder = lt.remainder.Append, \\\n> + sector_y_remainder = lt.remainder.Append, \\\n> + sector_average_function = lt.average_functions.Mean, \\\n> + smoothing_function = lt.smoothing.MedianBlur, \\\n> + smoothing_parameters = [3], \\\n> + output_type = lt.type.Float, \\\n> + output_color_channels = [lt.color.R, lt.color.G, lt.color.B], \\\n> + output_luminance_channels = [lt.color.G] \\\n> + output_range = [0, 3.999] \\\n> + )\n> diff --git a/utils/tuning/raspberrypi_alsc_only.py b/utils/tuning/raspberrypi_alsc_only.py\n> new file mode 100644\n> index 00000000..11a1fc23\n> --- /dev/null\n> +++ b/utils/tuning/raspberrypi_alsc_only.py\n> @@ -0,0 +1,16 @@\n> +import sys\n> +\n> +import libtuning as lt\n> +from libtuning.modules import ALSC\n> +from libtuning.parsers import RaspberryPiParser\n> +from libtuning.generators import RaspberryPiOutput\n> +\n> +from raspberrypi.alsc import ALSC as RaspberryPiALSC\n> +\n> +tuner = lt.Camera()\n> +tuner.add(RaspberryPiALSC)\n> +tuner.setInputType(RaspberryPiParser)\n> +tuner.setOutputType(RaspberryPiOutput)\n> +tuner.setOutputOrder(ALSC)\n> +\n> +tuner.run(sys.argv)\n> diff --git a/utils/tuning/rkisp1.py b/utils/tuning/rkisp1.py\n> new file mode 100644\n> index 00000000..8e26beba\n> --- /dev/null\n> +++ b/utils/tuning/rkisp1.py\n> @@ -0,0 +1,67 @@\n> +import sys\n> +\n> +import libtuning as lt\n> +from libtuning.modules import ALSC\n> +from libtuning.parsers import YamlParser\n> +from libtuning.generators import YamlOutput\n> +\n> +tuner = lt.Camera()\n> +tuner.add(ALSC(do_color = lt.paramsIfUnset(True), \\\n> +\n> + # This can support other debug options (I can't think of any rn\n> + # but for future-proofing)\n> + debug = [lt.debug.Plot], \\\n> +\n> + # The name of IfUnSet can obviously be changed, but really I\n> + # just want something that says \"it must be specified in the\n> + # configuration, and get the value from there\" or \"if the value\n> + # is not in the configuration, use this\"\n> + luminance_strength = lt.params.IfUnSet(0.5) \\\n> +\n> + sector_shape = (16, 16), \\\n> +\n> + # Other functions might include Circular, Hyperbolic, Gaussian,\n> + # Linear, Exponential, Logarithmic, etc\n> + # Of course, both need not be the same function\n> + # Some functions would need a sector_x_parameter (eg. sigma for Gaussian)\n> + # Alternatively: sector_x_sizes = [] ? I don't think it would work tho\n> + sector_x_gradient = lt.gradients.Parabolic, \\\n> + sector_y_gradient = lt.gradients.Parabolic, \\\n> +\n> + # This is the function that will be used to average the pixels in each sector\n> + # This can also be a custom function.\n> + sector_average_function = lt.average_functions.Mean, \\\n> +\n> + # This is the function that will be used to smooth the color ratio values\n> + # This can also be a custom function.\n> + smoothing_function = lt.smoothing.MedianBlur, \\\n> + smoothing_parameters = [3], \\\n> +\n> + # Are there any platforms that use integer values for their lsc table?\n> + output_type = lt.type.Float, \\\n> +\n> + # Required if and only if do_color can be or is True\n> + output_color_channels = [lt.color.R, lt.color.GR, lt.color.GB, lt.color.B], \\\n> +\n> + # Required if and only if do_color can be or is False\n> + output_luminance_channels = [lt.color.GR, lt.color.GB], \\\n> +\n> + # Automatically get the precision from this\n> + output_range = [0, 3.999] \\\n> +\n> + # Do we need a flag to enable/disable calculating sigmas? afaik\n> + # the rkisp1 IPA doesn't use it? But we could output it anyway\n> + # and algorithms can decide whether or not if they want to use\n> + # it. Oh well, no flag for now, we can always add it later if\n> + # there's a big demand, plus it's only two to three values and\n> + # not an entire table.\n> + ))\n> +tuner.setInputType(YamlParser)\n> +tuner.setOutputType(YamlOutput)\n> +tuner.setOutputOrder(ALSC)\n\nI may be missing something obvious, but isn't tuning supposed to use\nimages ? :-)\n\n> +\n> +# Maybe this should be wrapped in an if __main__ = '__main__' ? That way the\n\n__name__ == '__main__'\n\n> +# developer can control which tuner they want to be executed based on another\n> +# layer of arguments? But I was thinking that this would handle *all* arguments\n> +# based on the modules' and the input/output configurations.\n> +tuner.run(sys.argv)\n\nIt looks like you have quite a few ideas on how to implement all this,\nbut based on this skeleton only, I have a hard time understanding them\n:-) It would make sense to me to move forward a bit more and then\ncontinue the discussion.","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 7B6F7C0DA4\n\tfor ;\n\tWed, 28 Sep 2022 02:13:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD69F6227A;\n\tWed, 28 Sep 2022 04:13:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AAF1B61F78\n\tfor ;\n\tWed, 28 Sep 2022 04:13:38 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C55B847C;\n\tWed, 28 Sep 2022 04:13:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1664331219;\n\tbh=YoRlK0Sf69TTyJcqOIARO5M6CAUL9LN4DHtztIeCiMc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=R1H8tn/z4WQefMru1/7paHqZebB39sLt5sd2BX9ndJWDWQVxy6ngHds6NllwlHL4O\n\t97IaecHgnqskFkcqN9U11nt4mr/9kc2dZn/3RX5UFNwEr8pOU7yKkSSov9sg638yH7\n\tSymF7xh6gkLdCrC2aftOM2s60drD8FHo6i5j+8bgJo9b/Qsp6QrWmN/DknL8QCqJyb\n\tNqkxG1OzzlUwqGwOi5NzmWLwlDvCIxe+AAVv4r1Aw6sZ1VGB5wNPYn/zUCG+fTSjOj\n\t/LB6aK+QrUW5l45Vd6xDzthS8xJwzunW2rKfoqaTr+PaHKTY7AR4qnsYvRjjJNPIyN\n\tgGvwSbtQdEAKw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1664331218;\n\tbh=YoRlK0Sf69TTyJcqOIARO5M6CAUL9LN4DHtztIeCiMc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=N7rIwq+d4qz/JTgsKHGKNWhyQXjdqiDjnvUGSqLuIkU0nMKgjIk+Q9vTbnLZqSp8G\n\tt3yiq3woB3OwiRydzbtG9JVzKv31BBrRvHcgHp6XlKTwvkLEeNdno+lcN3WCnH5r5q\n\tssGg2HTBW4ZHDKq15Q8IyzCsFmwvQhJuq9sUINvw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"N7rIwq+d\"; dkim-atps=neutral","Date":"Wed, 28 Sep 2022 05:13:35 +0300","To":"Paul Elder ","Message-ID":"","References":"<20220926050931.686428-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220926050931.686428-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH] [RFC] [DNI] utils: tuning: Draft\n\ta new tuning infrastructure","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]