[{"id":25749,"web_url":"https://patchwork.libcamera.org/comment/25749/","msgid":"","date":"2022-11-09T10:17:09","subject":"Re: [libcamera-devel] [PATCH v2 05/11] utils: libtuning: parsers:\n\tAdd raspberrypi parser","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sat, Oct 22, 2022 at 03:23:04PM +0900, Paul Elder via libcamera-devel wrote:\n> Add a parser to libtuning for parsing configuration files that are the\n> same format as raspberrypi's ctt's configuration files.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v2:\n> - fix python errors\n> - fix style\n> - add SPDX and copyright\n> - don't put black level in the processed config, as raspberrypi ctt's\n> config format uses -1 as a magical value to tell ctt to use the value\n> from the image, but in our Image loading function it already does, and\n> uses this config value to override it if its present\n> - Don't validate module config in parser, instead libtuning will do it;\n> parser just converts the key from string to module instance\n> ---\n> utils/tuning/libtuning/parsers/__init__.py | 5 +\n> .../libtuning/parsers/raspberrypi_parser.py | 91 +++++++++++++++++++\n> 2 files changed, 96 insertions(+)\n> create mode 100644 utils/tuning/libtuning/parsers/raspberrypi_parser.py\n> \n> diff --git a/utils/tuning/libtuning/parsers/__init__.py b/utils/tuning/libtuning/parsers/__init__.py\n> index e69de29b..9d20d2fc 100644\n> --- a/utils/tuning/libtuning/parsers/__init__.py\n> +++ b/utils/tuning/libtuning/parsers/__init__.py\n> @@ -0,0 +1,5 @@\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +#\n> +# Copyright (C) 2022, Paul Elder \n> +\n> +from libtuning.parsers.raspberrypi_parser import RaspberryPiParser\n> diff --git a/utils/tuning/libtuning/parsers/raspberrypi_parser.py b/utils/tuning/libtuning/parsers/raspberrypi_parser.py\n> new file mode 100644\n> index 00000000..196c5b36\n> --- /dev/null\n> +++ b/utils/tuning/libtuning/parsers/raspberrypi_parser.py\n> @@ -0,0 +1,91 @@\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +#\n> +# Copyright (C) 2022, Paul Elder \n> +\n> +from .parser import Parser\n> +\n> +import json\n> +import numbers\n> +\n> +import libtuning.utils as utils\n> +\n> +\n> +class RaspberryPiParser(Parser):\n> + def __init__(self):\n> + super().__init__()\n> +\n> + # The string in the 'disable' and 'plot' lists are formatted as\n> + # 'rpi.{module_name}'.\n> + # @brief Enumerate, as a module, @a listt if its value exists in @a dictt\n> + # and it is the name of a valid module in @a modules\n> + def enumerate_rpi_modules(self, listt, dictt, modules):\n\nFollowing your naming convention, shouldn't this start with an _ ?\n\n> + for x in listt:\n> + name = x.replace('rpi.', '')\n> + if name not in dictt:\n> + continue\n> + module = utils.get_module_by_typename(modules, name)\n> + if module is not None:\n> + yield module\n> +\n> + def _validMacbethOption(self, value):\n\nsnake_case\n\n> + if not isinstance(value, dict):\n> + return False\n> +\n> + if list(value.keys()) != ['small', 'show']:\n> + return False\n> +\n> + for val in value.values():\n> + if not isinstance(val, numbers.Number):\n> + return False\n> +\n> + return True\n> +\n> + def _parse(self, config_file, modules):\n> + with open(config_file, 'r') as config_json:\n> + config = json.load(config_json)\n> +\n> + disable = []\n> + for module in self.enumerate_rpi_modules(config['disable'], config, modules):\n> + disable.append(module)\n> + # Remove the disabled module's config too\n> + config.pop(module.name)\n> + config.pop('disable')\n> +\n> + # The raspberrypi config format has 'plot' map to a list of module\n> + # names which should be plotted. libtuning has each module contain the\n\ns/ libtuning/ libtuning/\n\nReviewed-by: Laurent Pinchart \n\n> + # plot information in itself so do this conversion.\n> +\n> + for module in self.enumerate_rpi_modules(config['plot'], config, modules):\n> + # It's fine to set the value of a potentially disabled module, as\n> + # the object still exists at this point\n> + module.appendValue('debug', 'plot')\n> + config.pop('plot')\n> +\n> + # Convert the keys from module name to module instance\n> +\n> + new_config = {}\n> +\n> + for module_name in config:\n> + module = utils.get_module_by_typename(modules, module_name)\n> + if module is not None:\n> + new_config[module] = config[module_name]\n> +\n> + new_config['general'] = {}\n> +\n> + if 'blacklevel' in config:\n> + if not isinstance(config['blacklevel'], numbers.Number):\n> + raise TypeError('Config \"blacklevel\" must be a number')\n> + # Raspberry Pi's ctt config has magic blacklevel value -1 to mean\n> + # \"get it from the image metadata\". Since we already do that in\n> + # Image, don't save it to the config here.\n> + if config['blacklevel'] >= 0:\n> + new_config['general']['blacklevel'] = config['blacklevel']\n> +\n> + if 'macbeth' in config:\n> + if not self._validMacbethOption(config['macbeth']):\n> + raise TypeError('Config \"macbeth\" must be a dict: {\"small\": number, \"show\": number}')\n> + new_config['general']['macbeth'] = config['macbeth']\n> + else:\n> + new_config['general']['macbeth'] = {'small': 0, 'show': 0}\n> +\n> + return new_config, disable","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 A768BBD16B\n\tfor ;\n\tWed, 9 Nov 2022 10:17:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 665186307B;\n\tWed, 9 Nov 2022 11:17:30 +0100 (CET)","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 A0AB661F3F\n\tfor ;\n\tWed, 9 Nov 2022 11:17:28 +0100 (CET)","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 2C9FD896;\n\tWed, 9 Nov 2022 11:17:28 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667989050;\n\tbh=vNrBdhpc697hP+MO5LNpxc2esMrYcByDtFy/+HlL4SQ=;\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=2ym/jOo4IFxmbgO8rs+uf3TzDOzl8wPj9VR023HwXHUKJ+LaNTW/TNKbLzPaJrop8\n\tS3kAesnvYW+gbM90Z6biBhxqA7/+EeLbBX2BSFLZkX58vBqL9kMeVI6tfDwEKFdQ48\n\tKv533FbkGnwG7WWJH/CtBPFAphTSRhK/G1b+CkmXbbW1AquHpMSmqcdNXHd4xwxLzq\n\t6BwcF6PWVZZskjeV0JpK6Z2liZKjYffl+TQZfZYE5SCMxshSdpXHOjOLvxcHfZBnKM\n\teRQYBkGlakrQZ/fVSzfQ5MTdK7VmwvRUlUaLAZYvhxcjMqoOmAH30EinlIZxOCTd19\n\tFKg6BBIIX495g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1667989048;\n\tbh=vNrBdhpc697hP+MO5LNpxc2esMrYcByDtFy/+HlL4SQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p8ufIZeX8TvEwCH6kcOAYSiXTZz7sJIJh86CNF3KnR1qCY+vFD4SnRKcK8AHDJLdb\n\tZAztp5at8O0QvT1gDutj0Ufkr4vEDEwpLXIgcuUcrifeSI6tmaNfFUa7NzaPAGgEeL\n\tNRDG98cYCS0s+hi5QxJDlIebnGAm3jxku8k7dM3o="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"p8ufIZeX\"; dkim-atps=neutral","Date":"Wed, 9 Nov 2022 12:17:09 +0200","To":"Paul Elder ","Message-ID":"","References":"<20221022062310.2545463-1-paul.elder@ideasonboard.com>\n\t<20221022062310.2545463-6-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221022062310.2545463-6-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 05/11] utils: libtuning: parsers:\n\tAdd raspberrypi parser","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\" "}}]