[{"id":30753,"web_url":"https://patchwork.libcamera.org/comment/30753/","msgid":"<881671db-eea7-4796-a108-5f316a78a87b@ideasonboard.com>","date":"2024-08-12T14:45:32","subject":"Re: [PATCH 10/10] py: gen-py-controls: Convert to jinja2 templates","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 09/08/2024 03:59, Laurent Pinchart wrote:\n> Jinja2 templates help separating the logic related to the template from\n> the generation of the data. The python code gets much clearer as a\n> result.\n> \n> As an added bonus, we can use a single template file for both controls\n> and properties.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>   src/py/libcamera/gen-py-controls.py           | 114 ++++++++----------\n>   src/py/libcamera/meson.build                  |   8 +-\n>   src/py/libcamera/py_controls_generated.cpp.in |  35 ++++--\n>   .../libcamera/py_properties_generated.cpp.in  |  30 -----\n>   4 files changed, 78 insertions(+), 109 deletions(-)\n>   delete mode 100644 src/py/libcamera/py_properties_generated.cpp.in\n> \n> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> index a18dc5337090..cf09c146084d 100755\n> --- a/src/py/libcamera/gen-py-controls.py\n> +++ b/src/py/libcamera/gen-py-controls.py\n> @@ -4,7 +4,7 @@\n>   # Generate Python bindings controls from YAML\n>   \n>   import argparse\n> -import string\n> +import jinja2\n>   import sys\n>   import yaml\n>   \n> @@ -23,67 +23,39 @@ def find_common_prefix(strings):\n>       return prefix\n>   \n>   \n> -def generate_py(controls, mode):\n> -    out = ''\n> +def extend_control(ctrl, mode):\n> +    if ctrl.vendor != 'libcamera':\n> +        ctrl.klass = ctrl.vendor\n> +        ctrl.namespace = f'{ctrl.vendor}::'\n> +    else:\n> +        ctrl.klass = mode\n> +        ctrl.namespace = ''\n>   \n> -    vendors_class_def = []\n> -    vendor_defs = []\n> -    vendors = []\n> -    for vendor, ctrl_list in controls.items():\n> -        for ctrl in ctrl_list:\n> -            if vendor not in vendors and vendor != 'libcamera':\n> -                vendor_mode_str = f'{vendor.capitalize()}{mode.capitalize()}'\n> -                vendors_class_def.append('class Py{}\\n{{\\n}};\\n'.format(vendor_mode_str))\n> -                vendor_defs.append('\\tauto {} = py::class_<Py{}>(controls, \\\"{}\\\");'.format(vendor, vendor_mode_str, vendor))\n> -                vendors.append(vendor)\n> +    if not ctrl.is_enum:\n> +        return ctrl\n>   \n> -            if vendor != 'libcamera':\n> -                ns = 'libcamera::{}::{}::'.format(mode, vendor)\n> -                container = vendor\n> -            else:\n> -                ns = 'libcamera::{}::'.format(mode)\n> -                container = 'controls'\n> +    if mode == 'controls':\n> +        # Adjustments for controls\n> +        if ctrl.name == 'LensShadingMapMode':\n> +            prefix = 'LensShadingMapMode'\n> +        else:\n> +            prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n> +    else:\n> +        # Adjustments for properties\n> +        prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n>   \n> -            out += f'\\t{container}.def_readonly_static(\"{ctrl.name}\", static_cast<const libcamera::ControlId *>(&{ns}{ctrl.name}));\\n\\n'\n> +    for enum in ctrl.enum_values:\n> +        enum.py_name = enum.name[len(prefix):]\n>   \n> -            if not ctrl.is_enum:\n> -                continue\n> -\n> -            cpp_enum = ctrl.name + 'Enum'\n> -\n> -            out += '\\tpy::enum_<{}{}>({}, \\\"{}\\\")\\n'.format(ns, cpp_enum, container, cpp_enum)\n> -\n> -            if mode == 'controls':\n> -                # Adjustments for controls\n> -                if ctrl.name == 'LensShadingMapMode':\n> -                    prefix = 'LensShadingMapMode'\n> -                else:\n> -                    prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n> -            else:\n> -                # Adjustments for properties\n> -                prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n> -\n> -            for entry in ctrl.enum_values:\n> -                cpp_enum = entry.name\n> -                py_enum = entry.name[len(prefix):]\n> -\n> -                out += '\\t\\t.value(\\\"{}\\\", {}{})\\n'.format(py_enum, ns, cpp_enum)\n> -\n> -            out += '\\t;\\n\\n'\n> -\n> -    return {'controls': out,\n> -            'vendors_class_def': '\\n'.join(vendors_class_def),\n> -            'vendors_defs': '\\n'.join(vendor_defs)}\n> -\n> -\n> -def fill_template(template, data):\n> -    template = open(template, 'rb').read()\n> -    template = template.decode('utf-8')\n> -    template = string.Template(template)\n> -    return template.substitute(data)\n> +    return ctrl\n>   \n>   \n>   def main(argv):\n> +    headers = {\n> +        'controls': 'control_ids.h',\n> +        'properties': 'property_ids.h',\n> +    }\n> +\n>       # Parse command line arguments\n>       parser = argparse.ArgumentParser()\n>       parser.add_argument('--mode', '-m', type=str, required=True,\n> @@ -96,27 +68,41 @@ def main(argv):\n>                           help='Input file name.')\n>       args = parser.parse_args(argv[1:])\n>   \n> -    if args.mode not in ['controls', 'properties']:\n> +    if not headers.get(args.mode):\n>           print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n>           return -1\n>   \n> -    controls = {}\n> +    controls = []\n> +    vendors = []\n> +\n>       for input in args.input:\n>           data = yaml.safe_load(open(input, 'rb').read())\n> +\n>           vendor = data['vendor']\n> -        ctrls = data['controls']\n> -        controls[vendor] = [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> +        if vendor != 'libcamera':\n> +            vendors.append(vendor)\n>   \n> -    data = generate_py(controls, args.mode)\n> +        for ctrl in data['controls']:\n> +            ctrl = Control(*ctrl.popitem(), vendor)\n> +            controls.append(extend_control(ctrl, args.mode))\n>   \n> -    data = fill_template(args.template, data)\n> +    data = {\n> +        'mode': args.mode,\n> +        'header': headers[args.mode],\n> +        'vendors': vendors,\n> +        'controls': controls,\n> +    }\n> +\n> +    env = jinja2.Environment()\n> +    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())\n> +    string = template.render(data)\n>   \n>       if args.output:\n> -        output = open(args.output, 'wb')\n> -        output.write(data.encode('utf-8'))\n> +        output = open(args.output, 'w', encoding='utf-8')\n> +        output.write(string)\n>           output.close()\n>       else:\n> -        sys.stdout.write(data)\n> +        sys.stdout.write(string)\n>   \n>       return 0\n>   \n> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> index 6ad2d7713e4d..596a203ca4cc 100644\n> --- a/src/py/libcamera/meson.build\n> +++ b/src/py/libcamera/meson.build\n> @@ -26,7 +26,7 @@ pycamera_sources = files([\n>       'py_transform.cpp',\n>   ])\n>   \n> -# Generate controls\n> +# Generate controls and properties\n>   \n>   gen_py_controls_template = files('py_controls_generated.cpp.in')\n>   gen_py_controls = files('gen-py-controls.py')\n> @@ -38,15 +38,11 @@ pycamera_sources += custom_target('py_gen_controls',\n>                                                '-t', gen_py_controls_template, '@INPUT@'],\n>                                     env : py_build_env)\n>   \n> -# Generate properties\n> -\n> -gen_py_properties_template = files('py_properties_generated.cpp.in')\n> -\n>   pycamera_sources += custom_target('py_gen_properties',\n>                                     input : properties_files,\n>                                     output : ['py_properties_generated.cpp'],\n>                                     command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@',\n> -                                             '-t', gen_py_properties_template, '@INPUT@'],\n> +                                             '-t', gen_py_controls_template, '@INPUT@'],\n>                                     env : py_build_env)\n>   \n>   # Generate formats\n> diff --git a/src/py/libcamera/py_controls_generated.cpp.in b/src/py/libcamera/py_controls_generated.cpp.in\n> index 26d5a104f209..22a132d19ea9 100644\n> --- a/src/py/libcamera/py_controls_generated.cpp.in\n> +++ b/src/py/libcamera/py_controls_generated.cpp.in\n> @@ -2,12 +2,12 @@\n>   /*\n>    * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n>    *\n> - * Python bindings - Auto-generated controls\n> + * Python bindings - Auto-generated {{mode}}\n>    *\n>    * This file is auto-generated. Do not edit.\n>    */\n>   \n> -#include <libcamera/control_ids.h>\n> +#include <libcamera/{{header}}>\n>   \n>   #include <pybind11/pybind11.h>\n>   \n> @@ -15,16 +15,33 @@\n>   \n>   namespace py = pybind11;\n>   \n> -class PyControls\n> +class Py{{mode|capitalize}}\n>   {\n>   };\n>   \n> -${vendors_class_def}\n> -\n> -void init_py_controls_generated(py::module& m)\n> +{% for vendor in vendors -%}\n> +class Py{{vendor|capitalize}}{{mode|capitalize}}\n>   {\n> -\tauto controls = py::class_<PyControls>(m, \"controls\");\n> -${vendors_defs}\n> +};\n>   \n> -${controls}\n> +{% endfor -%}\n> +\n> +void init_py_{{mode}}_generated(py::module& m)\n> +{\n> +\tauto {{mode}} = py::class_<Py{{mode|capitalize}}>(m, \"{{mode}}\");\n> +{%- for vendor in vendors %}\n> +\tauto {{vendor}} = py::class_<Py{{vendor|capitalize}}{{mode|capitalize}}>({{mode}}, \"{{vendor}}\");\n> +{%- endfor %}\n> +\n> +{% for ctrl in controls %}\n> +        {{ctrl.klass}}.def_readonly_static(\"{{ctrl.name}}\", static_cast<const libcamera::ControlId *>(&libcamera::{{mode}}::{{ctrl.namespace}}{{ctrl.name}}));\n> +{%- if ctrl.is_enum %}\n> +\n> +        py::enum_<libcamera::{{mode}}::{{ctrl.namespace}}{{ctrl.name}}Enum>({{ctrl.klass}}, \"{{ctrl.name}}Enum\")\n> +{%- for enum in ctrl.enum_values %}\n> +                .value(\"{{enum.py_name}}\", libcamera::{{mode}}::{{ctrl.namespace}}{{enum.name}})\n> +{%- endfor %}\n> +        ;\n> +{%- endif %}\n> +{% endfor -%}\n>   }\n> diff --git a/src/py/libcamera/py_properties_generated.cpp.in b/src/py/libcamera/py_properties_generated.cpp.in\n> deleted file mode 100644\n> index d28f1ab8b61a..000000000000\n> --- a/src/py/libcamera/py_properties_generated.cpp.in\n> +++ /dev/null\n> @@ -1,30 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n> - *\n> - * Python bindings - Auto-generated properties\n> - *\n> - * This file is auto-generated. Do not edit.\n> - */\n> -\n> -#include <libcamera/property_ids.h>\n> -\n> -#include <pybind11/pybind11.h>\n> -\n> -#include \"py_main.h\"\n> -\n> -namespace py = pybind11;\n> -\n> -class PyProperties\n> -{\n> -};\n> -\n> -${vendors_class_def}\n> -\n> -void init_py_properties_generated(py::module& m)\n> -{\n> -\tauto controls = py::class_<PyProperties>(m, \"properties\");\n> -${vendors_defs}\n> -\n> -${controls}\n> -}\n\nI'm not familiar with jinja2, but looks fine to me and works fine.\n\nReviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n\n  Tomi","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 C338ABE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Aug 2024 14:45:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE0DD633B9;\n\tMon, 12 Aug 2024 16:45:35 +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 B10D963398\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Aug 2024 16:45:34 +0200 (CEST)","from [192.168.88.20] (91-156-87-48.elisa-laajakaista.fi\n\t[91.156.87.48])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F85C6B5;\n\tMon, 12 Aug 2024 16:44: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=\"fabacteK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723473878;\n\tbh=czld1n2kgtBO5scLhw0C+9621v8Su9sudF3XMrEy/vQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=fabacteKf7IC3FwlyYGgwEBncZml68H97/194oqQoXlBhOgQOkjm/RjfTy53nh674\n\t47E+zZb/uCv2GFtTC6kHSrhPqWNPWQrRKNNIUYck04MMJGwXPZYthHUbthn+a2tLr7\n\t5j12YPFmwWQ8x0Vm5diEMQWC957RLUB+0opfEVac=","Message-ID":"<881671db-eea7-4796-a108-5f316a78a87b@ideasonboard.com>","Date":"Mon, 12 Aug 2024 17:45:32 +0300","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 10/10] py: gen-py-controls: Convert to jinja2 templates","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240809005914.20662-1-laurent.pinchart@ideasonboard.com>\n\t<20240809005914.20662-11-laurent.pinchart@ideasonboard.com>","Content-Language":"en-US","From":"Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>","Autocrypt":"addr=tomi.valkeinen@ideasonboard.com; keydata=\n\txsFNBE6ms0cBEACyizowecZqXfMZtnBniOieTuFdErHAUyxVgtmr0f5ZfIi9Z4l+uUN4Zdw2\n\twCEZjx3o0Z34diXBaMRJ3rAk9yB90UJAnLtb8A97Oq64DskLF81GCYB2P1i0qrG7UjpASgCA\n\tRu0lVvxsWyIwSfoYoLrazbT1wkWRs8YBkkXQFfL7Mn3ZMoGPcpfwYH9O7bV1NslbmyJzRCMO\n\teYV258gjCcwYlrkyIratlHCek4GrwV8Z9NQcjD5iLzrONjfafrWPwj6yn2RlL0mQEwt1lOvn\n\tLnI7QRtB3zxA3yB+FLsT1hx0va6xCHpX3QO2gBsyHCyVafFMrg3c/7IIWkDLngJxFgz6DLiA\n\tG4ld1QK/jsYqfP2GIMH1mFdjY+iagG4DqOsjip479HCWAptpNxSOCL6z3qxCU8MCz8iNOtZk\n\tDYXQWVscM5qgYSn+fmMM2qN+eoWlnCGVURZZLDjg387S2E1jT/dNTOsM/IqQj+ZROUZuRcF7\n\t0RTtuU5q1HnbRNwy+23xeoSGuwmLQ2UsUk7Q5CnrjYfiPo3wHze8avK95JBoSd+WIRmV3uoO\n\trXCoYOIRlDhg9XJTrbnQ3Ot5zOa0Y9c4IpyAlut6mDtxtKXr4+8OzjSVFww7tIwadTK3wDQv\n\tBus4jxHjS6dz1g2ypT65qnHen6mUUH63lhzewqO9peAHJ0SLrQARAQABzTBUb21pIFZhbGtl\n\taW5lbiA8dG9taS52YWxrZWluZW5AaWRlYXNvbmJvYXJkLmNvbT7CwY4EEwEIADgWIQTEOAw+\n\tll79gQef86f6PaqMvJYe9QUCX/HruAIbAwULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAKCRD6\n\tPaqMvJYe9WmFD/99NGoD5lBJhlFDHMZvO+Op8vCwnIRZdTsyrtGl72rVh9xRfcSgYPZUvBuT\n\tVDxE53mY9HaZyu1eGMccYRBaTLJSfCXl/g317CrMNdY0k40b9YeIX10feiRYEWoDIPQ3tMmA\n\t0nHDygzcnuPiPT68JYZ6tUOvAt7r6OX/litM+m2/E9mtp8xCoWOo/kYO4mOAIoMNvLB8vufi\n\tuBB4e/AvAjtny4ScuNV5c5q8MkfNIiOyag9QCiQ/JfoAqzXRjVb4VZG72AKaElwipiKCWEcU\n\tR4+Bu5Qbaxj7Cd36M/bI54OrbWWETJkVVSV1i0tghCd6HHyquTdFl7wYcz6cL1hn/6byVnD+\n\tsR3BLvSBHYp8WSwv0TCuf6tLiNgHAO1hWiQ1pOoXyMEsxZlgPXT+wb4dbNVunckwqFjGxRbl\n\tRz7apFT/ZRwbazEzEzNyrBOfB55xdipG/2+SmFn0oMFqFOBEszXLQVslh64lI0CMJm2OYYe3\n\tPxHqYaztyeXsx13Bfnq9+bUynAQ4uW1P5DJ3OIRZWKmbQd/Me3Fq6TU57LsvwRgE0Le9PFQs\n\tdcP2071rMTpqTUteEgODJS4VDf4lXJfY91u32BJkiqM7/62Cqatcz5UWWHq5xeF03MIUTqdE\n\tqHWk3RJEoWHWQRzQfcx6Fn2fDAUKhAddvoopfcjAHfpAWJ+ENc7BTQROprNHARAAx0aat8GU\n\thsusCLc4MIxOQwidecCTRc9Dz/7U2goUwhw2O5j9TPqLtp57VITmHILnvZf6q3QAho2QMQyE\n\tDDvHubrdtEoqaaSKxKkFie1uhWNNvXPhwkKLYieyL9m2JdU+b88HaDnpzdyTTR4uH7wk0bBa\n\tKbTSgIFDDe5lXInypewPO30TmYNkFSexnnM3n1PBCqiJXsJahE4ZQ+WnV5FbPUj8T2zXS2xk\n\t0LZ0+DwKmZ0ZDovvdEWRWrz3UzJ8DLHb7blPpGhmqj3ANXQXC7mb9qJ6J/VSl61GbxIO2Dwb\n\txPNkHk8fwnxlUBCOyBti/uD2uSTgKHNdabhVm2dgFNVuS1y3bBHbI/qjC3J7rWE0WiaHWEqy\n\tUVPk8rsph4rqITsj2RiY70vEW0SKePrChvET7D8P1UPqmveBNNtSS7In+DdZ5kUqLV7rJnM9\n\t/4cwy+uZUt8cuCZlcA5u8IsBCNJudxEqBG10GHg1B6h1RZIz9Q9XfiBdaqa5+CjyFs8ua01c\n\t9HmyfkuhXG2OLjfQuK+Ygd56mV3lq0aFdwbaX16DG22c6flkkBSjyWXYepFtHz9KsBS0DaZb\n\t4IkLmZwEXpZcIOQjQ71fqlpiXkXSIaQ6YMEs8WjBbpP81h7QxWIfWtp+VnwNGc6nq5IQDESH\n\tmvQcsFS7d3eGVI6eyjCFdcAO8eMAEQEAAcLBXwQYAQIACQUCTqazRwIbDAAKCRD6PaqMvJYe\n\t9fA7EACS6exUedsBKmt4pT7nqXBcRsqm6YzT6DeCM8PWMTeaVGHiR4TnNFiT3otD5UpYQI7S\n\tsuYxoTdHrrrBzdlKe5rUWpzoZkVK6p0s9OIvGzLT0lrb0HC9iNDWT3JgpYDnk4Z2mFi6tTbq\n\txKMtpVFRA6FjviGDRsfkfoURZI51nf2RSAk/A8BEDDZ7lgJHskYoklSpwyrXhkp9FHGMaYII\n\tm9EKuUTX9JPDG2FTthCBrdsgWYPdJQvM+zscq09vFMQ9Fykbx5N8z/oFEUy3ACyPqW2oyfvU\n\tCH5WDpWBG0s5BALp1gBJPytIAd/pY/5ZdNoi0Cx3+Z7jaBFEyYJdWy1hGddpkgnMjyOfLI7B\n\tCFrdecTZbR5upjNSDvQ7RG85SnpYJTIin+SAUazAeA2nS6gTZzumgtdw8XmVXZwdBfF+ICof\n\t92UkbYcYNbzWO/GHgsNT1WnM4sa9lwCSWH8Fw1o/3bX1VVPEsnESOfxkNdu+gAF5S6+I6n3a\n\tueeIlwJl5CpT5l8RpoZXEOVtXYn8zzOJ7oGZYINRV9Pf8qKGLf3Dft7zKBP832I3PQjeok7F\n\tyjt+9S+KgSFSHP3Pa4E7lsSdWhSlHYNdG/czhoUkSCN09C0rEK93wxACx3vtxPLjXu6RptBw\n\t3dRq7n+mQChEB1am0BueV1JZaBboIL0AGlSJkm23kw==","In-Reply-To":"<20240809005914.20662-11-laurent.pinchart@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":30836,"web_url":"https://patchwork.libcamera.org/comment/30836/","msgid":"<Zr2NO13rNe8VrtBf@pyrite.rasen.tech>","date":"2024-08-15T05:08:11","subject":"Re: [PATCH 10/10] py: gen-py-controls: Convert to jinja2 templates","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, Aug 09, 2024 at 03:59:14AM +0300, Laurent Pinchart wrote:\n> Jinja2 templates help separating the logic related to the template from\n> the generation of the data. The python code gets much clearer as a\n> result.\n> \n> As an added bonus, we can use a single template file for both controls\n> and properties.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nLooks good to me.\n\nAcked-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> ---\n>  src/py/libcamera/gen-py-controls.py           | 114 ++++++++----------\n>  src/py/libcamera/meson.build                  |   8 +-\n>  src/py/libcamera/py_controls_generated.cpp.in |  35 ++++--\n>  .../libcamera/py_properties_generated.cpp.in  |  30 -----\n>  4 files changed, 78 insertions(+), 109 deletions(-)\n>  delete mode 100644 src/py/libcamera/py_properties_generated.cpp.in\n> \n> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> index a18dc5337090..cf09c146084d 100755\n> --- a/src/py/libcamera/gen-py-controls.py\n> +++ b/src/py/libcamera/gen-py-controls.py\n> @@ -4,7 +4,7 @@\n>  # Generate Python bindings controls from YAML\n>  \n>  import argparse\n> -import string\n> +import jinja2\n>  import sys\n>  import yaml\n>  \n> @@ -23,67 +23,39 @@ def find_common_prefix(strings):\n>      return prefix\n>  \n>  \n> -def generate_py(controls, mode):\n> -    out = ''\n> +def extend_control(ctrl, mode):\n> +    if ctrl.vendor != 'libcamera':\n> +        ctrl.klass = ctrl.vendor\n> +        ctrl.namespace = f'{ctrl.vendor}::'\n> +    else:\n> +        ctrl.klass = mode\n> +        ctrl.namespace = ''\n>  \n> -    vendors_class_def = []\n> -    vendor_defs = []\n> -    vendors = []\n> -    for vendor, ctrl_list in controls.items():\n> -        for ctrl in ctrl_list:\n> -            if vendor not in vendors and vendor != 'libcamera':\n> -                vendor_mode_str = f'{vendor.capitalize()}{mode.capitalize()}'\n> -                vendors_class_def.append('class Py{}\\n{{\\n}};\\n'.format(vendor_mode_str))\n> -                vendor_defs.append('\\tauto {} = py::class_<Py{}>(controls, \\\"{}\\\");'.format(vendor, vendor_mode_str, vendor))\n> -                vendors.append(vendor)\n> +    if not ctrl.is_enum:\n> +        return ctrl\n>  \n> -            if vendor != 'libcamera':\n> -                ns = 'libcamera::{}::{}::'.format(mode, vendor)\n> -                container = vendor\n> -            else:\n> -                ns = 'libcamera::{}::'.format(mode)\n> -                container = 'controls'\n> +    if mode == 'controls':\n> +        # Adjustments for controls\n> +        if ctrl.name == 'LensShadingMapMode':\n> +            prefix = 'LensShadingMapMode'\n> +        else:\n> +            prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n> +    else:\n> +        # Adjustments for properties\n> +        prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n>  \n> -            out += f'\\t{container}.def_readonly_static(\"{ctrl.name}\", static_cast<const libcamera::ControlId *>(&{ns}{ctrl.name}));\\n\\n'\n> +    for enum in ctrl.enum_values:\n> +        enum.py_name = enum.name[len(prefix):]\n>  \n> -            if not ctrl.is_enum:\n> -                continue\n> -\n> -            cpp_enum = ctrl.name + 'Enum'\n> -\n> -            out += '\\tpy::enum_<{}{}>({}, \\\"{}\\\")\\n'.format(ns, cpp_enum, container, cpp_enum)\n> -\n> -            if mode == 'controls':\n> -                # Adjustments for controls\n> -                if ctrl.name == 'LensShadingMapMode':\n> -                    prefix = 'LensShadingMapMode'\n> -                else:\n> -                    prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n> -            else:\n> -                # Adjustments for properties\n> -                prefix = find_common_prefix([e.name for e in ctrl.enum_values])\n> -\n> -            for entry in ctrl.enum_values:\n> -                cpp_enum = entry.name\n> -                py_enum = entry.name[len(prefix):]\n> -\n> -                out += '\\t\\t.value(\\\"{}\\\", {}{})\\n'.format(py_enum, ns, cpp_enum)\n> -\n> -            out += '\\t;\\n\\n'\n> -\n> -    return {'controls': out,\n> -            'vendors_class_def': '\\n'.join(vendors_class_def),\n> -            'vendors_defs': '\\n'.join(vendor_defs)}\n> -\n> -\n> -def fill_template(template, data):\n> -    template = open(template, 'rb').read()\n> -    template = template.decode('utf-8')\n> -    template = string.Template(template)\n> -    return template.substitute(data)\n> +    return ctrl\n>  \n>  \n>  def main(argv):\n> +    headers = {\n> +        'controls': 'control_ids.h',\n> +        'properties': 'property_ids.h',\n> +    }\n> +\n>      # Parse command line arguments\n>      parser = argparse.ArgumentParser()\n>      parser.add_argument('--mode', '-m', type=str, required=True,\n> @@ -96,27 +68,41 @@ def main(argv):\n>                          help='Input file name.')\n>      args = parser.parse_args(argv[1:])\n>  \n> -    if args.mode not in ['controls', 'properties']:\n> +    if not headers.get(args.mode):\n>          print(f'Invalid mode option \"{args.mode}\"', file=sys.stderr)\n>          return -1\n>  \n> -    controls = {}\n> +    controls = []\n> +    vendors = []\n> +\n>      for input in args.input:\n>          data = yaml.safe_load(open(input, 'rb').read())\n> +\n>          vendor = data['vendor']\n> -        ctrls = data['controls']\n> -        controls[vendor] = [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]\n> +        if vendor != 'libcamera':\n> +            vendors.append(vendor)\n>  \n> -    data = generate_py(controls, args.mode)\n> +        for ctrl in data['controls']:\n> +            ctrl = Control(*ctrl.popitem(), vendor)\n> +            controls.append(extend_control(ctrl, args.mode))\n>  \n> -    data = fill_template(args.template, data)\n> +    data = {\n> +        'mode': args.mode,\n> +        'header': headers[args.mode],\n> +        'vendors': vendors,\n> +        'controls': controls,\n> +    }\n> +\n> +    env = jinja2.Environment()\n> +    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())\n> +    string = template.render(data)\n>  \n>      if args.output:\n> -        output = open(args.output, 'wb')\n> -        output.write(data.encode('utf-8'))\n> +        output = open(args.output, 'w', encoding='utf-8')\n> +        output.write(string)\n>          output.close()\n>      else:\n> -        sys.stdout.write(data)\n> +        sys.stdout.write(string)\n>  \n>      return 0\n>  \n> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> index 6ad2d7713e4d..596a203ca4cc 100644\n> --- a/src/py/libcamera/meson.build\n> +++ b/src/py/libcamera/meson.build\n> @@ -26,7 +26,7 @@ pycamera_sources = files([\n>      'py_transform.cpp',\n>  ])\n>  \n> -# Generate controls\n> +# Generate controls and properties\n>  \n>  gen_py_controls_template = files('py_controls_generated.cpp.in')\n>  gen_py_controls = files('gen-py-controls.py')\n> @@ -38,15 +38,11 @@ pycamera_sources += custom_target('py_gen_controls',\n>                                               '-t', gen_py_controls_template, '@INPUT@'],\n>                                    env : py_build_env)\n>  \n> -# Generate properties\n> -\n> -gen_py_properties_template = files('py_properties_generated.cpp.in')\n> -\n>  pycamera_sources += custom_target('py_gen_properties',\n>                                    input : properties_files,\n>                                    output : ['py_properties_generated.cpp'],\n>                                    command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@',\n> -                                             '-t', gen_py_properties_template, '@INPUT@'],\n> +                                             '-t', gen_py_controls_template, '@INPUT@'],\n>                                    env : py_build_env)\n>  \n>  # Generate formats\n> diff --git a/src/py/libcamera/py_controls_generated.cpp.in b/src/py/libcamera/py_controls_generated.cpp.in\n> index 26d5a104f209..22a132d19ea9 100644\n> --- a/src/py/libcamera/py_controls_generated.cpp.in\n> +++ b/src/py/libcamera/py_controls_generated.cpp.in\n> @@ -2,12 +2,12 @@\n>  /*\n>   * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n>   *\n> - * Python bindings - Auto-generated controls\n> + * Python bindings - Auto-generated {{mode}}\n>   *\n>   * This file is auto-generated. Do not edit.\n>   */\n>  \n> -#include <libcamera/control_ids.h>\n> +#include <libcamera/{{header}}>\n>  \n>  #include <pybind11/pybind11.h>\n>  \n> @@ -15,16 +15,33 @@\n>  \n>  namespace py = pybind11;\n>  \n> -class PyControls\n> +class Py{{mode|capitalize}}\n>  {\n>  };\n>  \n> -${vendors_class_def}\n> -\n> -void init_py_controls_generated(py::module& m)\n> +{% for vendor in vendors -%}\n> +class Py{{vendor|capitalize}}{{mode|capitalize}}\n>  {\n> -\tauto controls = py::class_<PyControls>(m, \"controls\");\n> -${vendors_defs}\n> +};\n>  \n> -${controls}\n> +{% endfor -%}\n> +\n> +void init_py_{{mode}}_generated(py::module& m)\n> +{\n> +\tauto {{mode}} = py::class_<Py{{mode|capitalize}}>(m, \"{{mode}}\");\n> +{%- for vendor in vendors %}\n> +\tauto {{vendor}} = py::class_<Py{{vendor|capitalize}}{{mode|capitalize}}>({{mode}}, \"{{vendor}}\");\n> +{%- endfor %}\n> +\n> +{% for ctrl in controls %}\n> +        {{ctrl.klass}}.def_readonly_static(\"{{ctrl.name}}\", static_cast<const libcamera::ControlId *>(&libcamera::{{mode}}::{{ctrl.namespace}}{{ctrl.name}}));\n> +{%- if ctrl.is_enum %}\n> +\n> +        py::enum_<libcamera::{{mode}}::{{ctrl.namespace}}{{ctrl.name}}Enum>({{ctrl.klass}}, \"{{ctrl.name}}Enum\")\n> +{%- for enum in ctrl.enum_values %}\n> +                .value(\"{{enum.py_name}}\", libcamera::{{mode}}::{{ctrl.namespace}}{{enum.name}})\n> +{%- endfor %}\n> +        ;\n> +{%- endif %}\n> +{% endfor -%}\n>  }\n> diff --git a/src/py/libcamera/py_properties_generated.cpp.in b/src/py/libcamera/py_properties_generated.cpp.in\n> deleted file mode 100644\n> index d28f1ab8b61a..000000000000\n> --- a/src/py/libcamera/py_properties_generated.cpp.in\n> +++ /dev/null\n> @@ -1,30 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n> - *\n> - * Python bindings - Auto-generated properties\n> - *\n> - * This file is auto-generated. Do not edit.\n> - */\n> -\n> -#include <libcamera/property_ids.h>\n> -\n> -#include <pybind11/pybind11.h>\n> -\n> -#include \"py_main.h\"\n> -\n> -namespace py = pybind11;\n> -\n> -class PyProperties\n> -{\n> -};\n> -\n> -${vendors_class_def}\n> -\n> -void init_py_properties_generated(py::module& m)\n> -{\n> -\tauto controls = py::class_<PyProperties>(m, \"properties\");\n> -${vendors_defs}\n> -\n> -${controls}\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 C481CC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Aug 2024 05:08:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF148633BE;\n\tThu, 15 Aug 2024 07:08: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 AA6F463382\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Aug 2024 07:08:17 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-024.catv02.itscom.jp\n\t[175.177.49.24])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 72D11827;\n\tThu, 15 Aug 2024 07:07:18 +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=\"hRNN6Vod\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723698439;\n\tbh=ak3dRKZphuGFT3lXWVvSF4iYKpsKY72yJwzqmn3Oq14=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hRNN6Vod4zKxfp4nfiW+he/6yYWBfJhlIGcJT8mjbHNcf5f9Gihim2tRcBqpv8C4l\n\tOsfSOm2vD+UVEmAUvXqaktm2M2ialKdEd3sBjs6roObhqx834ylVxrrzdRBGebw7FD\n\t/5kwYTHejwrlayc4o2VRHwy6UX9d+1CpIQDqbLNw=","Date":"Thu, 15 Aug 2024 14:08:11 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 10/10] py: gen-py-controls: Convert to jinja2 templates","Message-ID":"<Zr2NO13rNe8VrtBf@pyrite.rasen.tech>","References":"<20240809005914.20662-1-laurent.pinchart@ideasonboard.com>\n\t<20240809005914.20662-11-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240809005914.20662-11-laurent.pinchart@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>"}}]