[{"id":23895,"web_url":"https://patchwork.libcamera.org/comment/23895/","msgid":"","date":"2022-07-14T23:40:59","subject":"Re: [libcamera-devel] [PATCH v5 6/8] utils: raspberrypi: Add tuning\n\tfile conversion script","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Thu, Jul 14, 2022 at 04:24:07PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Add a script to convert the Raspberry Pi camera tuning file format from version\n> 1.0 to 2.0.\n> \n> The version 1.0 format was originally used with the boost JSON parser that\n> happen to provided algorithm ordering based on the ordering in the file. The new\n\ns/provided/provide/\n\n> format provides implicit ordering by having the algorithms listed in an array.\n> \n> This script also adds a root level version key set to 2.0 to the config file,\n> allowing the controller to distinguish between the two formats.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> utils/raspberrypi/ctt/convert_tuning.py | 117 ++++++++++++++++++++++++\n> 1 file changed, 117 insertions(+)\n> create mode 100755 utils/raspberrypi/ctt/convert_tuning.py\n> \n> diff --git a/utils/raspberrypi/ctt/convert_tuning.py b/utils/raspberrypi/ctt/convert_tuning.py\n> new file mode 100755\n> index 000000000000..c915bcb46f64\n> --- /dev/null\n> +++ b/utils/raspberrypi/ctt/convert_tuning.py\n> @@ -0,0 +1,117 @@\n> +#!/bin/python3\n\nPlease add an SPDX header to specify the license.\n\n> +# Script to convert version 1.0 Raspberry Pi camera tuning files to version 2.0\n> +# and later.\n> +#\n> +# Copyright 2022 Raspberry Pi Ltd.\n> +\n> +import argparse\n> +import json\n> +import textwrap\n> +\n> +\n> +class Encoder(json.JSONEncoder):\n> +\n> + def __init__(self, *args, **kwargs):\n> + super().__init__(*args, **kwargs)\n> + self.indentation_level = 0\n> + self.hard_break = 120\n> + self.custom_elems = {\n> + 'table': 16,\n> + 'luminance_lut': 16,\n> + 'ct_curve': 3,\n> + 'ccm': 3\n> + }\n> +\n> + def encode(self, o, node_key=None):\n> + if isinstance(o, (list, tuple)):\n> + # Check if we are a flat list of numbers.\n> + if not any(isinstance(el, (list, tuple, dict)) for el in o):\n> + s = ', '.join(json.dumps(el) for el in o)\n> + if node_key in self.custom_elems.keys():\n> + # Special case handling to specify number of elements in a row for tables, ccm, etc.\n> + self.indentation_level += 1\n> + sl = s.split(', ')\n> + num = self.custom_elems[node_key]\n> + chunk = [self.indent_str + ', '.join(sl[x:x + num]) for x in range(0, len(sl), num)]\n> + t = ',\\n'.join(chunk)\n> + self.indentation_level -= 1\n> + output = f'\\n{self.indent_str}[\\n{t}\\n{self.indent_str}]'\n> + elif len(s) > self.hard_break - len(self.indent_str):\n> + # Break a long list with wraps.\n> + self.indentation_level += 1\n> + t = textwrap.fill(s, self.hard_break, break_long_words=False,\n> + initial_indent=self.indent_str, subsequent_indent=self.indent_str)\n> + self.indentation_level -= 1\n> + output = f'\\n{self.indent_str}[\\n{t}\\n{self.indent_str}]'\n> + else:\n> + # Smaller lists can remain on a single line.\n> + output = f' [ {s} ]'\n> + return output\n> + else:\n> + # Sub-structures in the list case.\n> + self.indentation_level += 1\n> + output = [self.indent_str + self.encode(el) for el in o]\n> + self.indentation_level -= 1\n> + output = ',\\n'.join(output)\n> + return f' [\\n{output}\\n{self.indent_str}]'\n> +\n> + elif isinstance(o, dict):\n> + self.indentation_level += 1\n> + output = []\n> + for k, v in o.items():\n> + if isinstance(v, dict) and len(v) == 0:\n> + # Empty config block special case.\n> + output.append(self.indent_str + f'{json.dumps(k)}: {{ }}')\n> + else:\n> + # Only linebreak if the next node is a config block.\n> + sep = f'\\n{self.indent_str}' if isinstance(v, dict) else ''\n> + output.append(self.indent_str + f'{json.dumps(k)}:{sep}{self.encode(v, k)}')\n> + output = ',\\n'.join(output)\n> + self.indentation_level -= 1\n> + return f'{{\\n{output}\\n{self.indent_str}}}'\n> +\n> + else:\n> + return ' ' + json.dumps(o)\n> +\n> + @property\n> + def indent_str(self) -> str:\n> + return ' ' * self.indentation_level * self.indent\n> +\n> + def iterencode(self, o, **kwargs):\n> + return self.encode(o)\n\nThat's nice, I like the output.\n\n> +\n> +\n> +def convert_v2(in_json):\n> +\n> + ver = 1.0 if 'version' not in in_json.keys() else in_json['version']\n> +\n> + if ver == 1.0:\n> + converted = {}\n> + converted['version'] = 2.0\n> + converted['algorithms'] = []\n> +\n> + for k, v in in_json.items():\n> + if k == 'version':\n> + continue\n\nI'd drop this check for the same reason as explained in the previous\npatch.\n\n> + converted['algorithms'].append(dict(zip([k], [v])))\n> + else:\n> + converted = in_json\n> +\n> + return json.dumps(converted, cls=Encoder, indent=4, sort_keys=False)\n> +\n> +\n> +if __name__ == \"__main__\":\n> + parser = argparse.ArgumentParser(formatter_class=argparse.RawTextHelpFormatter, description=\n> + 'Convert the format of the Raspberry Pi camera tuning file from v1.0 to v2.0.\\n'\n> + 'If a v2.0 format file is provided, the tool prettifies its contents.')\n> + parser.add_argument('input', type=str, nargs=1, help='Input tuning file')\n\nIf you drop nargs, you can replace args.input[0] with args.input below.\n\n> + parser.add_argument('output', type=str, nargs='?', help='Output converted tuning file', default=None)\n\nIt would be useful to indicate that if the output argument is not\nprovided, the tool updates the input file in-place.\n\n> + args = parser.parse_args()\n> +\n> + with open(args.input[0], 'r') as f:\n> + in_json = json.load(f)\n> +\n> + out_json = convert_v2(in_json)\n> +\n> + with open(args.output if args.output is not None else args.input[0], 'w') as f:\n\n with open(args.output or args.input, 'w') as f:\n\nI also wonder if it could be useful to support outputting to stdout by\nspecifying '-' as the output file (and maybe the same thing for stdin),\nbut that can always be done later if needed.\n\nReviewed-by: Laurent Pinchart \n\n> + f.write(out_json)","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 435DFBE173\n\tfor ;\n\tThu, 14 Jul 2022 23:41:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A25D563312;\n\tFri, 15 Jul 2022 01:41:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0499D60489\n\tfor ;\n\tFri, 15 Jul 2022 01:41:30 +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 6E2029DA;\n\tFri, 15 Jul 2022 01:41:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657842092;\n\tbh=NbNMLOpoamALgVOKR1Kitqz/wEn5pBTFEn9+5ETIkik=;\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=pqVSZeFpyydad0AGTpp4doAxnZmGbA+9dDGCLV8a9U3VqciOj0OeA2RG+hU+t3oQA\n\tHfBjMLUdt4Cgoo/WYVvhBHtZZ79NoW/SXylWIQ6m3eETtdfTclQHDBo1kub8w1jxzj\n\tFaPfxsylfgix+Nf7jnzXxcEmKukEz0cQv0BGs+sNWP/hh/iafXGfsIPZsWrubjM6VJ\n\tlhi1+CyCqidrrVKsFG5POnPQdV7I9MkfxQFh6o0U0hT65oVpTn1IZduDKGTPMWjlgJ\n\t7S9n+6xHpoG2lMh8UCLwQERuWtXBJ/ZJ2xORxL0i+ZvIUGWyskfeD0NvT6I9CMxhwc\n\tw3+oi8TV4ZwGQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657842090;\n\tbh=NbNMLOpoamALgVOKR1Kitqz/wEn5pBTFEn9+5ETIkik=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bKWmXw1dPn7znjap2U8MMDntOc+h6xPn40KokLQ9Q+uDsOqVu+HnjA0cm9BfdhhT1\n\tWmmCU7QSJiYRuEWg64UwLsJNp3OJjrYPXKMD9gTy0RuGeMsPjrfmo/fMPsLNO5/UEy\n\t8rX4L/k6HZ9hzrIjCs4Ix9+7+IXREMEoE0dhD1CM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"bKWmXw1d\"; dkim-atps=neutral","Date":"Fri, 15 Jul 2022 02:40:59 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20220714152409.9780-1-naush@raspberrypi.com>\n\t<20220714152409.9780-7-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220714152409.9780-7-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v5 6/8] utils: raspberrypi: Add tuning\n\tfile conversion script","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\" "}},{"id":23898,"web_url":"https://patchwork.libcamera.org/comment/23898/","msgid":"","date":"2022-07-14T23:58:13","subject":"Re: [libcamera-devel] [PATCH v5 6/8] utils: raspberrypi: Add tuning\n\tfile conversion script","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"One more comment.\n\nOn Fri, Jul 15, 2022 at 02:40:59AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Hi Naush,\n> \n> Thank you for the patch.\n> \n> On Thu, Jul 14, 2022 at 04:24:07PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > Add a script to convert the Raspberry Pi camera tuning file format from version\n> > 1.0 to 2.0.\n> > \n> > The version 1.0 format was originally used with the boost JSON parser that\n> > happen to provided algorithm ordering based on the ordering in the file. The new\n> \n> s/provided/provide/\n> \n> > format provides implicit ordering by having the algorithms listed in an array.\n> > \n> > This script also adds a root level version key set to 2.0 to the config file,\n> > allowing the controller to distinguish between the two formats.\n> > \n> > Signed-off-by: Naushir Patuck \n> > ---\n> > utils/raspberrypi/ctt/convert_tuning.py | 117 ++++++++++++++++++++++++\n> > 1 file changed, 117 insertions(+)\n> > create mode 100755 utils/raspberrypi/ctt/convert_tuning.py\n> > \n> > diff --git a/utils/raspberrypi/ctt/convert_tuning.py b/utils/raspberrypi/ctt/convert_tuning.py\n> > new file mode 100755\n> > index 000000000000..c915bcb46f64\n> > --- /dev/null\n> > +++ b/utils/raspberrypi/ctt/convert_tuning.py\n> > @@ -0,0 +1,117 @@\n> > +#!/bin/python3\n\nThis should be\n\n#!/usr/bin/env python3\n\nor possibly\n\n#!/usr/bin/python3\n\n> Please add an SPDX header to specify the license.\n> \n> > +# Script to convert version 1.0 Raspberry Pi camera tuning files to version 2.0\n> > +# and later.\n> > +#\n> > +# Copyright 2022 Raspberry Pi Ltd.\n> > +\n> > +import argparse\n> > +import json\n> > +import textwrap\n> > +\n> > +\n> > +class Encoder(json.JSONEncoder):\n> > +\n> > + def __init__(self, *args, **kwargs):\n> > + super().__init__(*args, **kwargs)\n> > + self.indentation_level = 0\n> > + self.hard_break = 120\n> > + self.custom_elems = {\n> > + 'table': 16,\n> > + 'luminance_lut': 16,\n> > + 'ct_curve': 3,\n> > + 'ccm': 3\n> > + }\n> > +\n> > + def encode(self, o, node_key=None):\n> > + if isinstance(o, (list, tuple)):\n> > + # Check if we are a flat list of numbers.\n> > + if not any(isinstance(el, (list, tuple, dict)) for el in o):\n> > + s = ', '.join(json.dumps(el) for el in o)\n> > + if node_key in self.custom_elems.keys():\n> > + # Special case handling to specify number of elements in a row for tables, ccm, etc.\n> > + self.indentation_level += 1\n> > + sl = s.split(', ')\n> > + num = self.custom_elems[node_key]\n> > + chunk = [self.indent_str + ', '.join(sl[x:x + num]) for x in range(0, len(sl), num)]\n> > + t = ',\\n'.join(chunk)\n> > + self.indentation_level -= 1\n> > + output = f'\\n{self.indent_str}[\\n{t}\\n{self.indent_str}]'\n> > + elif len(s) > self.hard_break - len(self.indent_str):\n> > + # Break a long list with wraps.\n> > + self.indentation_level += 1\n> > + t = textwrap.fill(s, self.hard_break, break_long_words=False,\n> > + initial_indent=self.indent_str, subsequent_indent=self.indent_str)\n> > + self.indentation_level -= 1\n> > + output = f'\\n{self.indent_str}[\\n{t}\\n{self.indent_str}]'\n> > + else:\n> > + # Smaller lists can remain on a single line.\n> > + output = f' [ {s} ]'\n> > + return output\n> > + else:\n> > + # Sub-structures in the list case.\n> > + self.indentation_level += 1\n> > + output = [self.indent_str + self.encode(el) for el in o]\n> > + self.indentation_level -= 1\n> > + output = ',\\n'.join(output)\n> > + return f' [\\n{output}\\n{self.indent_str}]'\n> > +\n> > + elif isinstance(o, dict):\n> > + self.indentation_level += 1\n> > + output = []\n> > + for k, v in o.items():\n> > + if isinstance(v, dict) and len(v) == 0:\n> > + # Empty config block special case.\n> > + output.append(self.indent_str + f'{json.dumps(k)}: {{ }}')\n> > + else:\n> > + # Only linebreak if the next node is a config block.\n> > + sep = f'\\n{self.indent_str}' if isinstance(v, dict) else ''\n> > + output.append(self.indent_str + f'{json.dumps(k)}:{sep}{self.encode(v, k)}')\n> > + output = ',\\n'.join(output)\n> > + self.indentation_level -= 1\n> > + return f'{{\\n{output}\\n{self.indent_str}}}'\n> > +\n> > + else:\n> > + return ' ' + json.dumps(o)\n> > +\n> > + @property\n> > + def indent_str(self) -> str:\n> > + return ' ' * self.indentation_level * self.indent\n> > +\n> > + def iterencode(self, o, **kwargs):\n> > + return self.encode(o)\n> \n> That's nice, I like the output.\n> \n> > +\n> > +\n> > +def convert_v2(in_json):\n> > +\n> > + ver = 1.0 if 'version' not in in_json.keys() else in_json['version']\n> > +\n> > + if ver == 1.0:\n> > + converted = {}\n> > + converted['version'] = 2.0\n> > + converted['algorithms'] = []\n> > +\n> > + for k, v in in_json.items():\n> > + if k == 'version':\n> > + continue\n> \n> I'd drop this check for the same reason as explained in the previous\n> patch.\n> \n> > + converted['algorithms'].append(dict(zip([k], [v])))\n> > + else:\n> > + converted = in_json\n> > +\n> > + return json.dumps(converted, cls=Encoder, indent=4, sort_keys=False)\n> > +\n> > +\n> > +if __name__ == \"__main__\":\n> > + parser = argparse.ArgumentParser(formatter_class=argparse.RawTextHelpFormatter, description=\n> > + 'Convert the format of the Raspberry Pi camera tuning file from v1.0 to v2.0.\\n'\n> > + 'If a v2.0 format file is provided, the tool prettifies its contents.')\n> > + parser.add_argument('input', type=str, nargs=1, help='Input tuning file')\n> \n> If you drop nargs, you can replace args.input[0] with args.input below.\n> \n> > + parser.add_argument('output', type=str, nargs='?', help='Output converted tuning file', default=None)\n> \n> It would be useful to indicate that if the output argument is not\n> provided, the tool updates the input file in-place.\n> \n> > + args = parser.parse_args()\n> > +\n> > + with open(args.input[0], 'r') as f:\n> > + in_json = json.load(f)\n> > +\n> > + out_json = convert_v2(in_json)\n> > +\n> > + with open(args.output if args.output is not None else args.input[0], 'w') as f:\n> \n> with open(args.output or args.input, 'w') as f:\n> \n> I also wonder if it could be useful to support outputting to stdout by\n> specifying '-' as the output file (and maybe the same thing for stdin),\n> but that can always be done later if needed.\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > + f.write(out_json)","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 3313EBD1F1\n\tfor ;\n\tThu, 14 Jul 2022 23:58:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E08BB63312;\n\tFri, 15 Jul 2022 01:58:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3B7D60489\n\tfor ;\n\tFri, 15 Jul 2022 01:58:44 +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 5F18D9DA;\n\tFri, 15 Jul 2022 01:58:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657843126;\n\tbh=KbFxOvvimWo9r5UhVgX2OjEV+s7bvNYwIGS0QxHY0PM=;\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:\n\tFrom;\n\tb=sY3RDTQozB/lWVfsTtViyDDHbHP2d4sAaYNG5CIXVozP/HiqLO91KABYq5S+Vye7U\n\tmO4tcw02ni23/3cJotLjCamdAStoPH+6kJlGGrFXTcWuuAGKGOu0c95xwYBkyA0CLc\n\tzTsTgAed3iVRWHbHcnTkOx1GpP6ADE9mwwKRBe5UfSAEZJPs0qDuG6gdv/ZicjdIRX\n\tU7dxnauV8Zkj2WFp8j8MWW5Z15RxPMOihWeJ+060iHUyzQ79iDyEDLIn7ZDV8QgDAJ\n\tBNQqCnWA19iyKO7CWNpo4FcQ6QY8vvqAUArtYspUByNO5SZxnw6uJsidmBfy8ZmLf3\n\tKBHg892hN34lg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657843124;\n\tbh=KbFxOvvimWo9r5UhVgX2OjEV+s7bvNYwIGS0QxHY0PM=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=RPxefut/QbTRV+Txejw8ZZqgX77EI6SSwFrTJjaKgTqFo2KiKql2O0TuifPLDmLeq\n\tOMQf0FsZqVZz/Mqcdx4yqpSjCedO6A5RWbyOZo5OlVhDgiY68c6uzBD3y2HFbqTf38\n\tZH5xylel4ogYI1CkvsOAJbTPs9gqStN3zgknmR6A="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RPxefut/\"; dkim-atps=neutral","Date":"Fri, 15 Jul 2022 02:58:13 +0300","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"","References":"<20220714152409.9780-1-naush@raspberrypi.com>\n\t<20220714152409.9780-7-naush@raspberrypi.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 6/8] utils: raspberrypi: Add tuning\n\tfile conversion script","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]