[{"id":2753,"web_url":"https://patchwork.libcamera.org/comment/2753/","msgid":"<20191003193114.GH1322@bigcity.dyn.berto.se>","date":"2019-10-03T19:31:14","subject":"Re: [libcamera-devel] [PATCH v2 05/13] libcamera: controls:\n\tAuto-generate control_ids.h and control_ids.cpp","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your patch.\n\nOn 2019-09-29 22:02:46 +0300, Laurent Pinchart wrote:\n> Bring back auto-generation of control ids. In this version, both the\n> header and the source files are generated from a single YAML file that\n> stores all control definitions. This allows centralising controls in a\n> single file, while the previous version required keeping both\n> declarations (in a header) and documentation (in a the source) in sync\n> manually.\n> \n> Using YAML as a format to store control definitions is a trade-off\n> between ease of use (there are many YAML parsers available) and\n> simplicity (XML was considered, but would have lead to more complex\n> processing). A new build time dependency is added on python3-yaml, which\n> should be available as a package in all distributions and build\n> environments.\n> \n> The YAML format is likely to change over time as we improve\n> documentation of controls, the first version simply copies the\n> information currently available. Future improvements should also include\n> a YAML schema to validate the YAML source file.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  Documentation/Doxyfile.in                     |   4 +-\n>  README.rst                                    |   2 +-\n>  .../{control_ids.h => control_ids.h.in}       |  16 +--\n>  include/libcamera/gen-header.sh               |   2 +-\n>  include/libcamera/meson.build                 |  18 ++-\n>  .../libcamera/libcamera-9999.ebuild           |   9 +-\n>  src/libcamera/control_ids.cpp                 |  52 --------\n>  src/libcamera/control_ids.cpp.in              |  25 ++++\n>  src/libcamera/control_ids.yaml                |  35 ++++++\n>  src/libcamera/gen-controls.py                 | 114 ++++++++++++++++++\n>  src/libcamera/meson.build                     |  12 +-\n>  11 files changed, 215 insertions(+), 74 deletions(-)\n>  rename include/libcamera/{control_ids.h => control_ids.h.in} (53%)\n>  delete mode 100644 src/libcamera/control_ids.cpp\n>  create mode 100644 src/libcamera/control_ids.cpp.in\n>  create mode 100644 src/libcamera/control_ids.yaml\n>  create mode 100755 src/libcamera/gen-controls.py\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index 28a9c2da1ad4..5237cf60854f 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -793,7 +793,9 @@ WARN_LOGFILE           =\n>  \n>  INPUT                  = \"@TOP_SRCDIR@/include/ipa\" \\\n>  \t\t\t \"@TOP_SRCDIR@/include/libcamera\" \\\n> -\t\t\t \"@TOP_SRCDIR@/src/libcamera\"\n> +\t\t\t \"@TOP_SRCDIR@/src/libcamera\" \\\n> +\t\t\t \"@TOP_BUILDDIR@/include/libcamera\" \\\n> +\t\t\t \"@TOP_BUILDDIR@/src/libcamera\"\n>  \n>  # This tag can be used to specify the character encoding of the source files\n>  # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses\n> diff --git a/README.rst b/README.rst\n> index 169837e41a4e..2ccf7cbec40a 100644\n> --- a/README.rst\n> +++ b/README.rst\n> @@ -40,7 +40,7 @@ A C++ toolchain: [required]\n>  \tEither {g++, clang}\n>  \n>  for libcamera: [required]\n> -\tmeson ninja-build\n> +\tmeson ninja-build python3-yaml\n>  \n>  for device hotplug enumeration: [optional]\n>  \tpkg-config libudev-dev\n> diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h.in\n> similarity index 53%\n> rename from include/libcamera/control_ids.h\n> rename to include/libcamera/control_ids.h.in\n> index 54235f1aea95..1d0bc791e559 100644\n> --- a/include/libcamera/control_ids.h\n> +++ b/include/libcamera/control_ids.h.in\n> @@ -3,6 +3,8 @@\n>   * Copyright (C) 2019, Google Inc.\n>   *\n>   * control_ids.h : Control ID list\n> + *\n> + * This file is auto-generated. Do not edit.\n>   */\n>  \n>  #ifndef __LIBCAMERA_CONTROL_IDS_H__\n> @@ -17,20 +19,10 @@ namespace libcamera {\n>  namespace controls {\n>  \n>  enum {\n> -\tAWB_ENABLE = 1,\n> -\tBRIGHTNESS = 2,\n> -\tCONTRAST = 3,\n> -\tSATURATION = 4,\n> -\tMANUAL_EXPOSURE = 5,\n> -\tMANUAL_GAIN = 6,\n> +${ids}\n>  };\n>  \n> -extern const Control<bool> AwbEnable;\n> -extern const Control<int32_t> Brightness;\n> -extern const Control<int32_t> Contrast;\n> -extern const Control<int32_t> Saturation;\n> -extern const Control<int32_t> ManualExposure;\n> -extern const Control<int32_t> ManualGain;\n> +${controls}\n>  \n>  } /* namespace controls */\n>  \n> diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh\n> index a69fe8e982a1..7f7816c9f879 100755\n> --- a/include/libcamera/gen-header.sh\n> +++ b/include/libcamera/gen-header.sh\n> @@ -19,7 +19,7 @@ EOF\n>  headers=$(for header in \"$src_dir\"/*.h ; do\n>  \theader=$(basename \"$header\")\n>  \techo \"$header\"\n> -done ; echo \"version.h\" | sort)\n> +done ; echo \"control_ids.h\" ; echo \"version.h\" | sort)\n>  \n>  for header in $headers ; do\n>  \techo \"#include <libcamera/$header>\" >> \"$dst_file\"\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 868f1a6bf1ab..4ffbdab3b173 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -3,7 +3,6 @@ libcamera_api = files([\n>      'buffer.h',\n>      'camera.h',\n>      'camera_manager.h',\n> -    'control_ids.h',\n>      'controls.h',\n>      'event_dispatcher.h',\n>      'event_notifier.h',\n> @@ -18,6 +17,20 @@ libcamera_api = files([\n>  \n>  include_dir = join_paths(libcamera_include_dir, 'libcamera')\n>  \n> +install_headers(libcamera_api,\n> +                subdir : include_dir)\n> +\n> +gen_controls = files('../../src/libcamera/gen-controls.py')\n> +\n> +control_ids_h = custom_target('control_ids_h',\n> +                              input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'),\n> +                              output : 'control_ids.h',\n> +                              depend_files : gen_controls,\n> +                              command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],\n> +                              install_dir : join_paths('include', include_dir))\n> +\n> +libcamera_api += control_ids_h\n> +\n>  gen_header = files('gen-header.sh')\n>  \n>  libcamera_h = custom_target('gen-header',\n> @@ -37,6 +50,3 @@ configure_file(input : 'version.h.in',\n>                 output : 'version.h',\n>                 configuration : libcamera_version_config,\n>                 install_dir : join_paths('include', include_dir))\n> -\n> -install_headers(libcamera_api,\n> -                subdir : include_dir)\n> diff --git a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> index fed2b409a91b..fc241b1f5584 100644\n> --- a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> +++ b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> @@ -2,7 +2,9 @@\n>  # Distributed under the terms of the GNU General Public License v2\n>  \n>  EAPI=6\n> -inherit git-r3 meson\n> +PYTHON_COMPAT=( python3_{5,6,7} )\n> +\n> +inherit git-r3 meson python-any-r1\n>  \n>  DESCRIPTION=\"Camera support library for Linux\"\n>  HOMEPAGE=\"http://libcamera.org\"\n> @@ -15,7 +17,10 @@ KEYWORDS=\"*\"\n>  IUSE=\"udev\"\n>  \n>  RDEPEND=\"udev? ( virtual/libudev )\"\n> -DEPEND=\"${RDEPEND}\"\n> +DEPEND=\"\n> +\t${RDEPEND}\n> +\t$(python_gen_any_dep 'dev-python/pyyaml[${PYTHON_USEDEP}]')\n> +\"\n>  \n>  src_configure() {\n>  \tlocal emesonargs=(\n> diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp\n> deleted file mode 100644\n> index 3af23a458862..000000000000\n> --- a/src/libcamera/control_ids.cpp\n> +++ /dev/null\n> @@ -1,52 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * control_ids.cpp : Control ID list\n> - */\n> -\n> -#include <libcamera/control_ids.h>\n> -\n> -/**\n> - * \\file control_ids.h\n> - * \\brief Camera control identifiers\n> - */\n> -\n> -namespace libcamera {\n> -\n> -namespace controls {\n> -\n> -/**\n> - * \\brief Enables or disables the AWB.\n> - * \\sa ManualGain\n> - */\n> -extern const Control<bool> AwbEnable(AWB_ENABLE, \"AwbEnable\");\n> -\n> -/**\n> - * \\brief Specify a fixed brightness parameter.\n> - */\n> -extern const Control<int32_t> Brightness(BRIGHTNESS, \"Brightness\");\n> -\n> -/**\n> - * \\brief Specify a fixed contrast parameter.\n> - */\n> -extern const Control<int32_t> Contrast(CONTRAST, \"Contrast\");\n> -\n> -/**\n> - * \\brief Specify a fixed saturation parameter.\n> - */\n> -extern const Control<int32_t> Saturation(SATURATION, \"Saturation\");\n> -\n> -/**\n> - * \\brief Specify a fixed exposure time in milli-seconds\n> - */\n> -extern const Control<int32_t> ManualExposure(MANUAL_EXPOSURE, \"ManualExposure\");\n> -\n> -/**\n> - * \\brief Specify a fixed gain parameter\n> - */\n> -extern const Control<int32_t> ManualGain(MANUAL_GAIN, \"ManualGain\");\n> -\n> -} /* namespace controls */\n> -\n> -} /* namespace libcamera */\n> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\n> new file mode 100644\n> index 000000000000..f699ac9eea54\n> --- /dev/null\n> +++ b/src/libcamera/control_ids.cpp.in\n> @@ -0,0 +1,25 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * control_ids.cpp : Control ID list\n> + *\n> + * This file is auto-generated. Do not edit.\n> + */\n> +\n> +#include <libcamera/control_ids.h>\n> +\n> +/**\n> + * \\file control_ids.h\n> + * \\brief Camera control identifiers\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace controls {\n> +\n> +${controls}\n> +\n> +} /* namespace controls */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> new file mode 100644\n> index 000000000000..819a5967a2fc\n> --- /dev/null\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -0,0 +1,35 @@\n> +# SPDX-License-Identifier: LGPL-2.1-or-later\n> +#\n> +# Copyright (C) 2019, Google Inc.\n> +#\n> +%YAML 1.2\n> +---\n> +controls:\n> +  - AwbEnable:\n> +      type: bool\n> +      description: |\n> +        Enables or disables the AWB.\n> +\n> +        \\sa ManualGain\n> +\n> +  - Brightness:\n> +      type: int32_t\n> +      description: Specify a fixed brightness parameter\n> +\n> +  - Contrast:\n> +      type: int32_t\n> +      description: Specify a fixed contrast parameter\n> +\n> +  - Saturation:\n> +      type: int32_t\n> +      description: Specify a fixed saturation parameter\n> +\n> +  - ManualExposure:\n> +      type: int32_t\n> +      description: Specify a fixed exposure time in milli-seconds\n> +\n> +  - ManualGain:\n> +      type: int32_t\n> +      description: Specify a fixed gain parameter\n> +\n> +...\n> diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py\n> new file mode 100755\n> index 000000000000..0899e40b4080\n> --- /dev/null\n> +++ b/src/libcamera/gen-controls.py\n> @@ -0,0 +1,114 @@\n> +#!/usr/bin/python3\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +# Copyright (C) 2019, Google Inc.\n> +#\n> +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> +#\n> +# gen-controls.py - Generate control definitions from YAML\n> +\n> +import argparse\n> +import string\n> +import sys\n> +import yaml\n> +\n> +\n> +def snake_case(s):\n> +    return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')\n> +\n> +\n> +def generate_cpp(controls):\n> +    template = string.Template('''/**\n> +${description}\n> + */\n> +extern const Control<${type}> ${name}(${id_name}, \"${name}\");''')\n> +\n> +    ctrls = []\n> +\n> +    for ctrl in controls:\n> +        name, ctrl = ctrl.popitem()\n> +        id_name = snake_case(name).upper()\n> +\n> +        description = ctrl['description'].strip('\\n').split('\\n')\n> +        description[0] = '\\\\brief ' + description[0]\n> +        description = '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n> +\n> +        info = {\n> +            'name': name,\n> +            'type': ctrl['type'],\n> +            'description': description,\n> +            'id_name': id_name,\n> +        }\n> +\n> +        ctrls.append(template.substitute(info))\n> +\n> +    return {'controls': '\\n\\n'.join(ctrls)}\n> +\n> +\n> +def generate_h(controls):\n> +    template = string.Template('''extern const Control<${type}> ${name};''')\n> +\n> +    ctrls = []\n> +    ids = []\n> +    id_value = 1\n> +\n> +    for ctrl in controls:\n> +        name, ctrl = ctrl.popitem()\n> +        id_name = snake_case(name).upper()\n> +\n> +        ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> +\n> +        info = {\n> +            'name': name,\n> +            'type': ctrl['type'],\n> +        }\n> +\n> +        ctrls.append(template.substitute(info))\n> +        id_value += 1\n> +\n> +    return {'ids': '\\n'.join(ids), 'controls': '\\n'.join(ctrls)}\n> +\n> +\n> +def fill_template(template, data):\n> +\n> +    template = open(template, 'rb').read()\n> +    template = template.decode('utf-8')\n> +    template = string.Template(template)\n> +    return template.substitute(data)\n> +\n> +\n> +def main(argv):\n> +\n> +    # Parse command line arguments\n> +    parser = argparse.ArgumentParser()\n> +    parser.add_argument('-o', dest='output', metavar='file', type=str,\n> +                        help='Output file name. Defaults to standard output if not specified.')\n> +    parser.add_argument('input', type=str,\n> +                        help='Input file name.')\n> +    parser.add_argument('template', type=str,\n> +                        help='Template file name.')\n> +    args = parser.parse_args(argv[1:])\n> +\n> +    data = open(args.input, 'rb').read()\n> +    controls = yaml.safe_load(data)['controls']\n> +\n> +    if args.template.endswith('.cpp.in'):\n> +        data = generate_cpp(controls)\n> +    elif args.template.endswith('.h.in'):\n> +        data = generate_h(controls)\n> +    else:\n> +        raise RuntimeError('Unknown template type')\n> +\n> +    data = fill_template(args.template, data)\n> +\n> +    if args.output:\n> +        output = open(args.output, 'wb')\n> +        output.write(data.encode('utf-8'))\n> +        output.close()\n> +    else:\n> +        sys.stdout.write(data)\n> +\n> +    return 0\n> +\n> +\n> +if __name__ == '__main__':\n> +    sys.exit(main(sys.argv))\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 8123d1d5bee9..6df48365266d 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -5,7 +5,6 @@ libcamera_sources = files([\n>      'camera_manager.cpp',\n>      'camera_sensor.cpp',\n>      'controls.cpp',\n> -    'control_ids.cpp',\n>      'device_enumerator.cpp',\n>      'device_enumerator_sysfs.cpp',\n>      'event_dispatcher.cpp',\n> @@ -58,6 +57,17 @@ if libudev.found()\n>      ])\n>  endif\n>  \n> +gen_controls = files('gen-controls.py')\n> +\n> +control_ids_cpp = custom_target('control_ids_cpp',\n> +                                input : files('control_ids.yaml', 'control_ids.cpp.in'),\n> +                                output : 'control_ids.cpp',\n> +                                depend_files : gen_controls,\n> +                                command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])\n> +\n> +libcamera_sources += control_ids_cpp\n> +#libcamera_sources += control_ids_h\n\nI assume this comment out line should be removed right?\n\nWith that fixed,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> +\n>  gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n>  \n>  version_cpp = vcs_tag(command : [gen_version, meson.build_root()],\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E7F260BE8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  3 Oct 2019 21:31:16 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id b20so4052194ljj.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 03 Oct 2019 12:31:16 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tz16sm885710ljz.72.2019.10.03.12.31.14\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 03 Oct 2019 12:31:15 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=iOTLQ/d85sgjBrhjd0cC1mC+lGoX8BNbr4743Nf4xzA=;\n\tb=zGhkvuwyIahuDlvLAUVj/P+SWNO04ZmqkHqyPP3tg4PdSKLDnzFqTjSJC9NCimJtio\n\t76C53K6j40VNoV96L+eZ+EUDZHgSYhJ4TuvYtMGMPVLYzJly/T2EoQsYarovnT2QZiO8\n\tKx4JhP6esHbtekMZ1GYGWlTOKG7xBwA6fOwyrA7vZdhMSEP/X1uTAL9dKxcZJamB0UU3\n\tH6DJUuinB0LG4in3dq4dpVLJHqLHDnktMqn1eEWsdPQWenF0q4xhICYLWLvkrAXfTrRg\n\tPEkX4W0NxrguJ/4Crd43o4uXw4ZGjAN57NJNWOo0sUhV/gRlKH7PDp2sorOdOuIWcyu8\n\tVYVQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=iOTLQ/d85sgjBrhjd0cC1mC+lGoX8BNbr4743Nf4xzA=;\n\tb=tJw6wFpwbVOlsbCWAsDAcu3Z3+SJR4odHPI0ONXi+DQjN/TX6O02nSFh1cKgMwMYeg\n\tHh0wnF5bkGvSOYvOlR0cqUgMLyzIeR0m/nNiO0lllAMJHkm6I+718y4MkeCRXT+dtG2p\n\tkTrhdlC1BnzJSqsK/68HeeEKQ8pzwczxwxUwITtZG1EbedUwIAreDVKuvyh8irV0+k9G\n\t3hmafrXOdO/B0jn8jkaoDZ+zXm6JgG8tALbLqu2yGf8xFKMybvZM24+Ovyqq0eSMDCq/\n\t/kXo93Vrm29CNDea1BvF/mA/tLQo8/FNWjzCae0zc1l0s3QuA3F8IOcrNGxlrSzMcrUe\n\t/Z/g==","X-Gm-Message-State":"APjAAAXJTKlNfhguQZRvOZFR5S5Kztj4ns6nJS0uXF/Hcrnrhwf9uOt3\n\tI5nmvqL871JSjJz9SjePuCwVfA==","X-Google-Smtp-Source":"APXvYqz9MHA7aDBO/W4W51GDQnVguYpIujQ8I1afp/VQUczN7/Yqd9dIfVSeX49xRiFf9x+ir55E4w==","X-Received":"by 2002:a2e:9a0c:: with SMTP id\n\to12mr7154949lji.204.1570131075689; \n\tThu, 03 Oct 2019 12:31:15 -0700 (PDT)","Date":"Thu, 3 Oct 2019 21:31:14 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191003193114.GH1322@bigcity.dyn.berto.se>","References":"<20190929190254.18920-1-laurent.pinchart@ideasonboard.com>\n\t<20190929190254.18920-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190929190254.18920-6-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 05/13] libcamera: controls:\n\tAuto-generate control_ids.h and control_ids.cpp","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>","X-List-Received-Date":"Thu, 03 Oct 2019 19:31:16 -0000"}},{"id":2768,"web_url":"https://patchwork.libcamera.org/comment/2768/","msgid":"<20191003211853.GE4737@pendragon.ideasonboard.com>","date":"2019-10-03T21:18:53","subject":"Re: [libcamera-devel] [PATCH v2 05/13] libcamera: controls:\n\tAuto-generate control_ids.h and control_ids.cpp","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Oct 03, 2019 at 09:31:14PM +0200, Niklas Söderlund wrote:\n> On 2019-09-29 22:02:46 +0300, Laurent Pinchart wrote:\n> > Bring back auto-generation of control ids. In this version, both the\n> > header and the source files are generated from a single YAML file that\n> > stores all control definitions. This allows centralising controls in a\n> > single file, while the previous version required keeping both\n> > declarations (in a header) and documentation (in a the source) in sync\n> > manually.\n> > \n> > Using YAML as a format to store control definitions is a trade-off\n> > between ease of use (there are many YAML parsers available) and\n> > simplicity (XML was considered, but would have lead to more complex\n> > processing). A new build time dependency is added on python3-yaml, which\n> > should be available as a package in all distributions and build\n> > environments.\n> > \n> > The YAML format is likely to change over time as we improve\n> > documentation of controls, the first version simply copies the\n> > information currently available. Future improvements should also include\n> > a YAML schema to validate the YAML source file.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  Documentation/Doxyfile.in                     |   4 +-\n> >  README.rst                                    |   2 +-\n> >  .../{control_ids.h => control_ids.h.in}       |  16 +--\n> >  include/libcamera/gen-header.sh               |   2 +-\n> >  include/libcamera/meson.build                 |  18 ++-\n> >  .../libcamera/libcamera-9999.ebuild           |   9 +-\n> >  src/libcamera/control_ids.cpp                 |  52 --------\n> >  src/libcamera/control_ids.cpp.in              |  25 ++++\n> >  src/libcamera/control_ids.yaml                |  35 ++++++\n> >  src/libcamera/gen-controls.py                 | 114 ++++++++++++++++++\n> >  src/libcamera/meson.build                     |  12 +-\n> >  11 files changed, 215 insertions(+), 74 deletions(-)\n> >  rename include/libcamera/{control_ids.h => control_ids.h.in} (53%)\n> >  delete mode 100644 src/libcamera/control_ids.cpp\n> >  create mode 100644 src/libcamera/control_ids.cpp.in\n> >  create mode 100644 src/libcamera/control_ids.yaml\n> >  create mode 100755 src/libcamera/gen-controls.py\n> > \n> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > index 28a9c2da1ad4..5237cf60854f 100644\n> > --- a/Documentation/Doxyfile.in\n> > +++ b/Documentation/Doxyfile.in\n> > @@ -793,7 +793,9 @@ WARN_LOGFILE           =\n> >  \n> >  INPUT                  = \"@TOP_SRCDIR@/include/ipa\" \\\n> >  \t\t\t \"@TOP_SRCDIR@/include/libcamera\" \\\n> > -\t\t\t \"@TOP_SRCDIR@/src/libcamera\"\n> > +\t\t\t \"@TOP_SRCDIR@/src/libcamera\" \\\n> > +\t\t\t \"@TOP_BUILDDIR@/include/libcamera\" \\\n> > +\t\t\t \"@TOP_BUILDDIR@/src/libcamera\"\n> >  \n> >  # This tag can be used to specify the character encoding of the source files\n> >  # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses\n> > diff --git a/README.rst b/README.rst\n> > index 169837e41a4e..2ccf7cbec40a 100644\n> > --- a/README.rst\n> > +++ b/README.rst\n> > @@ -40,7 +40,7 @@ A C++ toolchain: [required]\n> >  \tEither {g++, clang}\n> >  \n> >  for libcamera: [required]\n> > -\tmeson ninja-build\n> > +\tmeson ninja-build python3-yaml\n> >  \n> >  for device hotplug enumeration: [optional]\n> >  \tpkg-config libudev-dev\n> > diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h.in\n> > similarity index 53%\n> > rename from include/libcamera/control_ids.h\n> > rename to include/libcamera/control_ids.h.in\n> > index 54235f1aea95..1d0bc791e559 100644\n> > --- a/include/libcamera/control_ids.h\n> > +++ b/include/libcamera/control_ids.h.in\n> > @@ -3,6 +3,8 @@\n> >   * Copyright (C) 2019, Google Inc.\n> >   *\n> >   * control_ids.h : Control ID list\n> > + *\n> > + * This file is auto-generated. Do not edit.\n> >   */\n> >  \n> >  #ifndef __LIBCAMERA_CONTROL_IDS_H__\n> > @@ -17,20 +19,10 @@ namespace libcamera {\n> >  namespace controls {\n> >  \n> >  enum {\n> > -\tAWB_ENABLE = 1,\n> > -\tBRIGHTNESS = 2,\n> > -\tCONTRAST = 3,\n> > -\tSATURATION = 4,\n> > -\tMANUAL_EXPOSURE = 5,\n> > -\tMANUAL_GAIN = 6,\n> > +${ids}\n> >  };\n> >  \n> > -extern const Control<bool> AwbEnable;\n> > -extern const Control<int32_t> Brightness;\n> > -extern const Control<int32_t> Contrast;\n> > -extern const Control<int32_t> Saturation;\n> > -extern const Control<int32_t> ManualExposure;\n> > -extern const Control<int32_t> ManualGain;\n> > +${controls}\n> >  \n> >  } /* namespace controls */\n> >  \n> > diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh\n> > index a69fe8e982a1..7f7816c9f879 100755\n> > --- a/include/libcamera/gen-header.sh\n> > +++ b/include/libcamera/gen-header.sh\n> > @@ -19,7 +19,7 @@ EOF\n> >  headers=$(for header in \"$src_dir\"/*.h ; do\n> >  \theader=$(basename \"$header\")\n> >  \techo \"$header\"\n> > -done ; echo \"version.h\" | sort)\n> > +done ; echo \"control_ids.h\" ; echo \"version.h\" | sort)\n> >  \n> >  for header in $headers ; do\n> >  \techo \"#include <libcamera/$header>\" >> \"$dst_file\"\n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index 868f1a6bf1ab..4ffbdab3b173 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -3,7 +3,6 @@ libcamera_api = files([\n> >      'buffer.h',\n> >      'camera.h',\n> >      'camera_manager.h',\n> > -    'control_ids.h',\n> >      'controls.h',\n> >      'event_dispatcher.h',\n> >      'event_notifier.h',\n> > @@ -18,6 +17,20 @@ libcamera_api = files([\n> >  \n> >  include_dir = join_paths(libcamera_include_dir, 'libcamera')\n> >  \n> > +install_headers(libcamera_api,\n> > +                subdir : include_dir)\n> > +\n> > +gen_controls = files('../../src/libcamera/gen-controls.py')\n> > +\n> > +control_ids_h = custom_target('control_ids_h',\n> > +                              input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'),\n> > +                              output : 'control_ids.h',\n> > +                              depend_files : gen_controls,\n> > +                              command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],\n> > +                              install_dir : join_paths('include', include_dir))\n> > +\n> > +libcamera_api += control_ids_h\n> > +\n> >  gen_header = files('gen-header.sh')\n> >  \n> >  libcamera_h = custom_target('gen-header',\n> > @@ -37,6 +50,3 @@ configure_file(input : 'version.h.in',\n> >                 output : 'version.h',\n> >                 configuration : libcamera_version_config,\n> >                 install_dir : join_paths('include', include_dir))\n> > -\n> > -install_headers(libcamera_api,\n> > -                subdir : include_dir)\n> > diff --git a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> > index fed2b409a91b..fc241b1f5584 100644\n> > --- a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> > +++ b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> > @@ -2,7 +2,9 @@\n> >  # Distributed under the terms of the GNU General Public License v2\n> >  \n> >  EAPI=6\n> > -inherit git-r3 meson\n> > +PYTHON_COMPAT=( python3_{5,6,7} )\n> > +\n> > +inherit git-r3 meson python-any-r1\n> >  \n> >  DESCRIPTION=\"Camera support library for Linux\"\n> >  HOMEPAGE=\"http://libcamera.org\"\n> > @@ -15,7 +17,10 @@ KEYWORDS=\"*\"\n> >  IUSE=\"udev\"\n> >  \n> >  RDEPEND=\"udev? ( virtual/libudev )\"\n> > -DEPEND=\"${RDEPEND}\"\n> > +DEPEND=\"\n> > +\t${RDEPEND}\n> > +\t$(python_gen_any_dep 'dev-python/pyyaml[${PYTHON_USEDEP}]')\n> > +\"\n> >  \n> >  src_configure() {\n> >  \tlocal emesonargs=(\n> > diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp\n> > deleted file mode 100644\n> > index 3af23a458862..000000000000\n> > --- a/src/libcamera/control_ids.cpp\n> > +++ /dev/null\n> > @@ -1,52 +0,0 @@\n> > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > -/*\n> > - * Copyright (C) 2019, Google Inc.\n> > - *\n> > - * control_ids.cpp : Control ID list\n> > - */\n> > -\n> > -#include <libcamera/control_ids.h>\n> > -\n> > -/**\n> > - * \\file control_ids.h\n> > - * \\brief Camera control identifiers\n> > - */\n> > -\n> > -namespace libcamera {\n> > -\n> > -namespace controls {\n> > -\n> > -/**\n> > - * \\brief Enables or disables the AWB.\n> > - * \\sa ManualGain\n> > - */\n> > -extern const Control<bool> AwbEnable(AWB_ENABLE, \"AwbEnable\");\n> > -\n> > -/**\n> > - * \\brief Specify a fixed brightness parameter.\n> > - */\n> > -extern const Control<int32_t> Brightness(BRIGHTNESS, \"Brightness\");\n> > -\n> > -/**\n> > - * \\brief Specify a fixed contrast parameter.\n> > - */\n> > -extern const Control<int32_t> Contrast(CONTRAST, \"Contrast\");\n> > -\n> > -/**\n> > - * \\brief Specify a fixed saturation parameter.\n> > - */\n> > -extern const Control<int32_t> Saturation(SATURATION, \"Saturation\");\n> > -\n> > -/**\n> > - * \\brief Specify a fixed exposure time in milli-seconds\n> > - */\n> > -extern const Control<int32_t> ManualExposure(MANUAL_EXPOSURE, \"ManualExposure\");\n> > -\n> > -/**\n> > - * \\brief Specify a fixed gain parameter\n> > - */\n> > -extern const Control<int32_t> ManualGain(MANUAL_GAIN, \"ManualGain\");\n> > -\n> > -} /* namespace controls */\n> > -\n> > -} /* namespace libcamera */\n> > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\n> > new file mode 100644\n> > index 000000000000..f699ac9eea54\n> > --- /dev/null\n> > +++ b/src/libcamera/control_ids.cpp.in\n> > @@ -0,0 +1,25 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * control_ids.cpp : Control ID list\n> > + *\n> > + * This file is auto-generated. Do not edit.\n> > + */\n> > +\n> > +#include <libcamera/control_ids.h>\n> > +\n> > +/**\n> > + * \\file control_ids.h\n> > + * \\brief Camera control identifiers\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace controls {\n> > +\n> > +${controls}\n> > +\n> > +} /* namespace controls */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > new file mode 100644\n> > index 000000000000..819a5967a2fc\n> > --- /dev/null\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -0,0 +1,35 @@\n> > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > +#\n> > +# Copyright (C) 2019, Google Inc.\n> > +#\n> > +%YAML 1.2\n> > +---\n> > +controls:\n> > +  - AwbEnable:\n> > +      type: bool\n> > +      description: |\n> > +        Enables or disables the AWB.\n> > +\n> > +        \\sa ManualGain\n> > +\n> > +  - Brightness:\n> > +      type: int32_t\n> > +      description: Specify a fixed brightness parameter\n> > +\n> > +  - Contrast:\n> > +      type: int32_t\n> > +      description: Specify a fixed contrast parameter\n> > +\n> > +  - Saturation:\n> > +      type: int32_t\n> > +      description: Specify a fixed saturation parameter\n> > +\n> > +  - ManualExposure:\n> > +      type: int32_t\n> > +      description: Specify a fixed exposure time in milli-seconds\n> > +\n> > +  - ManualGain:\n> > +      type: int32_t\n> > +      description: Specify a fixed gain parameter\n> > +\n> > +...\n> > diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py\n> > new file mode 100755\n> > index 000000000000..0899e40b4080\n> > --- /dev/null\n> > +++ b/src/libcamera/gen-controls.py\n> > @@ -0,0 +1,114 @@\n> > +#!/usr/bin/python3\n> > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > +# Copyright (C) 2019, Google Inc.\n> > +#\n> > +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > +#\n> > +# gen-controls.py - Generate control definitions from YAML\n> > +\n> > +import argparse\n> > +import string\n> > +import sys\n> > +import yaml\n> > +\n> > +\n> > +def snake_case(s):\n> > +    return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')\n> > +\n> > +\n> > +def generate_cpp(controls):\n> > +    template = string.Template('''/**\n> > +${description}\n> > + */\n> > +extern const Control<${type}> ${name}(${id_name}, \"${name}\");''')\n> > +\n> > +    ctrls = []\n> > +\n> > +    for ctrl in controls:\n> > +        name, ctrl = ctrl.popitem()\n> > +        id_name = snake_case(name).upper()\n> > +\n> > +        description = ctrl['description'].strip('\\n').split('\\n')\n> > +        description[0] = '\\\\brief ' + description[0]\n> > +        description = '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n> > +\n> > +        info = {\n> > +            'name': name,\n> > +            'type': ctrl['type'],\n> > +            'description': description,\n> > +            'id_name': id_name,\n> > +        }\n> > +\n> > +        ctrls.append(template.substitute(info))\n> > +\n> > +    return {'controls': '\\n\\n'.join(ctrls)}\n> > +\n> > +\n> > +def generate_h(controls):\n> > +    template = string.Template('''extern const Control<${type}> ${name};''')\n> > +\n> > +    ctrls = []\n> > +    ids = []\n> > +    id_value = 1\n> > +\n> > +    for ctrl in controls:\n> > +        name, ctrl = ctrl.popitem()\n> > +        id_name = snake_case(name).upper()\n> > +\n> > +        ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> > +\n> > +        info = {\n> > +            'name': name,\n> > +            'type': ctrl['type'],\n> > +        }\n> > +\n> > +        ctrls.append(template.substitute(info))\n> > +        id_value += 1\n> > +\n> > +    return {'ids': '\\n'.join(ids), 'controls': '\\n'.join(ctrls)}\n> > +\n> > +\n> > +def fill_template(template, data):\n> > +\n> > +    template = open(template, 'rb').read()\n> > +    template = template.decode('utf-8')\n> > +    template = string.Template(template)\n> > +    return template.substitute(data)\n> > +\n> > +\n> > +def main(argv):\n> > +\n> > +    # Parse command line arguments\n> > +    parser = argparse.ArgumentParser()\n> > +    parser.add_argument('-o', dest='output', metavar='file', type=str,\n> > +                        help='Output file name. Defaults to standard output if not specified.')\n> > +    parser.add_argument('input', type=str,\n> > +                        help='Input file name.')\n> > +    parser.add_argument('template', type=str,\n> > +                        help='Template file name.')\n> > +    args = parser.parse_args(argv[1:])\n> > +\n> > +    data = open(args.input, 'rb').read()\n> > +    controls = yaml.safe_load(data)['controls']\n> > +\n> > +    if args.template.endswith('.cpp.in'):\n> > +        data = generate_cpp(controls)\n> > +    elif args.template.endswith('.h.in'):\n> > +        data = generate_h(controls)\n> > +    else:\n> > +        raise RuntimeError('Unknown template type')\n> > +\n> > +    data = fill_template(args.template, data)\n> > +\n> > +    if args.output:\n> > +        output = open(args.output, 'wb')\n> > +        output.write(data.encode('utf-8'))\n> > +        output.close()\n> > +    else:\n> > +        sys.stdout.write(data)\n> > +\n> > +    return 0\n> > +\n> > +\n> > +if __name__ == '__main__':\n> > +    sys.exit(main(sys.argv))\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 8123d1d5bee9..6df48365266d 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -5,7 +5,6 @@ libcamera_sources = files([\n> >      'camera_manager.cpp',\n> >      'camera_sensor.cpp',\n> >      'controls.cpp',\n> > -    'control_ids.cpp',\n> >      'device_enumerator.cpp',\n> >      'device_enumerator_sysfs.cpp',\n> >      'event_dispatcher.cpp',\n> > @@ -58,6 +57,17 @@ if libudev.found()\n> >      ])\n> >  endif\n> >  \n> > +gen_controls = files('gen-controls.py')\n> > +\n> > +control_ids_cpp = custom_target('control_ids_cpp',\n> > +                                input : files('control_ids.yaml', 'control_ids.cpp.in'),\n> > +                                output : 'control_ids.cpp',\n> > +                                depend_files : gen_controls,\n> > +                                command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])\n> > +\n> > +libcamera_sources += control_ids_cpp\n> > +#libcamera_sources += control_ids_h\n> \n> I assume this comment out line should be removed right?\n\nOops, yes, I'll fix that.\n\n> With that fixed,\n> \n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> \n> > +\n> >  gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> >  \n> >  version_cpp = vcs_tag(command : [gen_version, meson.build_root()],","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 2D4C260BE8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  3 Oct 2019 23:19:08 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [132.205.229.214])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1D78F2E5;\n\tThu,  3 Oct 2019 23:19:06 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570137547;\n\tbh=Nd6dbsmmDP6FhuREpZ6oJCPK4XhbAW5NiThJXlD6LhQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=j3qAIUhYtbP8XmyoaqOxmVDAGpzD0ZHYG+sx9GtDNKGiokDrQrPli7/NVUtxehi0/\n\tSNjmzmhrgOZRPWz/z3h868Pu72veZt0u93lcuO5Zm6wTYqPQkgCKEkIXGAxqmbp7c4\n\t5c9/UM5aUPzmPw/mMYTHQoFdOMXQksG6aIaLs5Dc=","Date":"Fri, 4 Oct 2019 00:18:53 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191003211853.GE4737@pendragon.ideasonboard.com>","References":"<20190929190254.18920-1-laurent.pinchart@ideasonboard.com>\n\t<20190929190254.18920-6-laurent.pinchart@ideasonboard.com>\n\t<20191003193114.GH1322@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191003193114.GH1322@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 05/13] libcamera: controls:\n\tAuto-generate control_ids.h and control_ids.cpp","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>","X-List-Received-Date":"Thu, 03 Oct 2019 21:19:08 -0000"}},{"id":2792,"web_url":"https://patchwork.libcamera.org/comment/2792/","msgid":"<20191005170150.GD11154@pendragon.ideasonboard.com>","date":"2019-10-05T17:01:50","subject":"Re: [libcamera-devel] [PATCH v2 05/13] libcamera: controls:\n\tAuto-generate control_ids.h and control_ids.cpp","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Fri, Oct 04, 2019 at 12:18:53AM +0300, Laurent Pinchart wrote:\n> On Thu, Oct 03, 2019 at 09:31:14PM +0200, Niklas Söderlund wrote:\n> > On 2019-09-29 22:02:46 +0300, Laurent Pinchart wrote:\n> > > Bring back auto-generation of control ids. In this version, both the\n> > > header and the source files are generated from a single YAML file that\n> > > stores all control definitions. This allows centralising controls in a\n> > > single file, while the previous version required keeping both\n> > > declarations (in a header) and documentation (in a the source) in sync\n> > > manually.\n> > > \n> > > Using YAML as a format to store control definitions is a trade-off\n> > > between ease of use (there are many YAML parsers available) and\n> > > simplicity (XML was considered, but would have lead to more complex\n> > > processing). A new build time dependency is added on python3-yaml, which\n> > > should be available as a package in all distributions and build\n> > > environments.\n> > > \n> > > The YAML format is likely to change over time as we improve\n> > > documentation of controls, the first version simply copies the\n> > > information currently available. Future improvements should also include\n> > > a YAML schema to validate the YAML source file.\n> > > \n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > >  Documentation/Doxyfile.in                     |   4 +-\n> > >  README.rst                                    |   2 +-\n> > >  .../{control_ids.h => control_ids.h.in}       |  16 +--\n> > >  include/libcamera/gen-header.sh               |   2 +-\n> > >  include/libcamera/meson.build                 |  18 ++-\n> > >  .../libcamera/libcamera-9999.ebuild           |   9 +-\n> > >  src/libcamera/control_ids.cpp                 |  52 --------\n> > >  src/libcamera/control_ids.cpp.in              |  25 ++++\n> > >  src/libcamera/control_ids.yaml                |  35 ++++++\n> > >  src/libcamera/gen-controls.py                 | 114 ++++++++++++++++++\n> > >  src/libcamera/meson.build                     |  12 +-\n> > >  11 files changed, 215 insertions(+), 74 deletions(-)\n> > >  rename include/libcamera/{control_ids.h => control_ids.h.in} (53%)\n> > >  delete mode 100644 src/libcamera/control_ids.cpp\n> > >  create mode 100644 src/libcamera/control_ids.cpp.in\n> > >  create mode 100644 src/libcamera/control_ids.yaml\n> > >  create mode 100755 src/libcamera/gen-controls.py\n> > > \n> > > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > > index 28a9c2da1ad4..5237cf60854f 100644\n> > > --- a/Documentation/Doxyfile.in\n> > > +++ b/Documentation/Doxyfile.in\n> > > @@ -793,7 +793,9 @@ WARN_LOGFILE           =\n> > >  \n> > >  INPUT                  = \"@TOP_SRCDIR@/include/ipa\" \\\n> > >  \t\t\t \"@TOP_SRCDIR@/include/libcamera\" \\\n> > > -\t\t\t \"@TOP_SRCDIR@/src/libcamera\"\n> > > +\t\t\t \"@TOP_SRCDIR@/src/libcamera\" \\\n> > > +\t\t\t \"@TOP_BUILDDIR@/include/libcamera\" \\\n> > > +\t\t\t \"@TOP_BUILDDIR@/src/libcamera\"\n> > >  \n> > >  # This tag can be used to specify the character encoding of the source files\n> > >  # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses\n> > > diff --git a/README.rst b/README.rst\n> > > index 169837e41a4e..2ccf7cbec40a 100644\n> > > --- a/README.rst\n> > > +++ b/README.rst\n> > > @@ -40,7 +40,7 @@ A C++ toolchain: [required]\n> > >  \tEither {g++, clang}\n> > >  \n> > >  for libcamera: [required]\n> > > -\tmeson ninja-build\n> > > +\tmeson ninja-build python3-yaml\n> > >  \n> > >  for device hotplug enumeration: [optional]\n> > >  \tpkg-config libudev-dev\n> > > diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h.in\n> > > similarity index 53%\n> > > rename from include/libcamera/control_ids.h\n> > > rename to include/libcamera/control_ids.h.in\n> > > index 54235f1aea95..1d0bc791e559 100644\n> > > --- a/include/libcamera/control_ids.h\n> > > +++ b/include/libcamera/control_ids.h.in\n> > > @@ -3,6 +3,8 @@\n> > >   * Copyright (C) 2019, Google Inc.\n> > >   *\n> > >   * control_ids.h : Control ID list\n> > > + *\n> > > + * This file is auto-generated. Do not edit.\n> > >   */\n> > >  \n> > >  #ifndef __LIBCAMERA_CONTROL_IDS_H__\n> > > @@ -17,20 +19,10 @@ namespace libcamera {\n> > >  namespace controls {\n> > >  \n> > >  enum {\n> > > -\tAWB_ENABLE = 1,\n> > > -\tBRIGHTNESS = 2,\n> > > -\tCONTRAST = 3,\n> > > -\tSATURATION = 4,\n> > > -\tMANUAL_EXPOSURE = 5,\n> > > -\tMANUAL_GAIN = 6,\n> > > +${ids}\n> > >  };\n> > >  \n> > > -extern const Control<bool> AwbEnable;\n> > > -extern const Control<int32_t> Brightness;\n> > > -extern const Control<int32_t> Contrast;\n> > > -extern const Control<int32_t> Saturation;\n> > > -extern const Control<int32_t> ManualExposure;\n> > > -extern const Control<int32_t> ManualGain;\n> > > +${controls}\n> > >  \n> > >  } /* namespace controls */\n> > >  \n> > > diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh\n> > > index a69fe8e982a1..7f7816c9f879 100755\n> > > --- a/include/libcamera/gen-header.sh\n> > > +++ b/include/libcamera/gen-header.sh\n> > > @@ -19,7 +19,7 @@ EOF\n> > >  headers=$(for header in \"$src_dir\"/*.h ; do\n> > >  \theader=$(basename \"$header\")\n> > >  \techo \"$header\"\n> > > -done ; echo \"version.h\" | sort)\n> > > +done ; echo \"control_ids.h\" ; echo \"version.h\" | sort)\n> > >  \n> > >  for header in $headers ; do\n> > >  \techo \"#include <libcamera/$header>\" >> \"$dst_file\"\n> > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > > index 868f1a6bf1ab..4ffbdab3b173 100644\n> > > --- a/include/libcamera/meson.build\n> > > +++ b/include/libcamera/meson.build\n> > > @@ -3,7 +3,6 @@ libcamera_api = files([\n> > >      'buffer.h',\n> > >      'camera.h',\n> > >      'camera_manager.h',\n> > > -    'control_ids.h',\n> > >      'controls.h',\n> > >      'event_dispatcher.h',\n> > >      'event_notifier.h',\n> > > @@ -18,6 +17,20 @@ libcamera_api = files([\n> > >  \n> > >  include_dir = join_paths(libcamera_include_dir, 'libcamera')\n> > >  \n> > > +install_headers(libcamera_api,\n> > > +                subdir : include_dir)\n> > > +\n> > > +gen_controls = files('../../src/libcamera/gen-controls.py')\n> > > +\n> > > +control_ids_h = custom_target('control_ids_h',\n> > > +                              input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'),\n> > > +                              output : 'control_ids.h',\n> > > +                              depend_files : gen_controls,\n> > > +                              command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],\n> > > +                              install_dir : join_paths('include', include_dir))\n> > > +\n> > > +libcamera_api += control_ids_h\n> > > +\n> > >  gen_header = files('gen-header.sh')\n> > >  \n> > >  libcamera_h = custom_target('gen-header',\n> > > @@ -37,6 +50,3 @@ configure_file(input : 'version.h.in',\n> > >                 output : 'version.h',\n> > >                 configuration : libcamera_version_config,\n> > >                 install_dir : join_paths('include', include_dir))\n> > > -\n> > > -install_headers(libcamera_api,\n> > > -                subdir : include_dir)\n> > > diff --git a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> > > index fed2b409a91b..fc241b1f5584 100644\n> > > --- a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> > > +++ b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild\n> > > @@ -2,7 +2,9 @@\n> > >  # Distributed under the terms of the GNU General Public License v2\n> > >  \n> > >  EAPI=6\n> > > -inherit git-r3 meson\n> > > +PYTHON_COMPAT=( python3_{5,6,7} )\n> > > +\n> > > +inherit git-r3 meson python-any-r1\n> > >  \n> > >  DESCRIPTION=\"Camera support library for Linux\"\n> > >  HOMEPAGE=\"http://libcamera.org\"\n> > > @@ -15,7 +17,10 @@ KEYWORDS=\"*\"\n> > >  IUSE=\"udev\"\n> > >  \n> > >  RDEPEND=\"udev? ( virtual/libudev )\"\n> > > -DEPEND=\"${RDEPEND}\"\n> > > +DEPEND=\"\n> > > +\t${RDEPEND}\n> > > +\t$(python_gen_any_dep 'dev-python/pyyaml[${PYTHON_USEDEP}]')\n> > > +\"\n> > >  \n> > >  src_configure() {\n> > >  \tlocal emesonargs=(\n> > > diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp\n> > > deleted file mode 100644\n> > > index 3af23a458862..000000000000\n> > > --- a/src/libcamera/control_ids.cpp\n> > > +++ /dev/null\n> > > @@ -1,52 +0,0 @@\n> > > -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > -/*\n> > > - * Copyright (C) 2019, Google Inc.\n> > > - *\n> > > - * control_ids.cpp : Control ID list\n> > > - */\n> > > -\n> > > -#include <libcamera/control_ids.h>\n> > > -\n> > > -/**\n> > > - * \\file control_ids.h\n> > > - * \\brief Camera control identifiers\n> > > - */\n> > > -\n> > > -namespace libcamera {\n> > > -\n> > > -namespace controls {\n> > > -\n> > > -/**\n> > > - * \\brief Enables or disables the AWB.\n> > > - * \\sa ManualGain\n> > > - */\n> > > -extern const Control<bool> AwbEnable(AWB_ENABLE, \"AwbEnable\");\n> > > -\n> > > -/**\n> > > - * \\brief Specify a fixed brightness parameter.\n> > > - */\n> > > -extern const Control<int32_t> Brightness(BRIGHTNESS, \"Brightness\");\n> > > -\n> > > -/**\n> > > - * \\brief Specify a fixed contrast parameter.\n> > > - */\n> > > -extern const Control<int32_t> Contrast(CONTRAST, \"Contrast\");\n> > > -\n> > > -/**\n> > > - * \\brief Specify a fixed saturation parameter.\n> > > - */\n> > > -extern const Control<int32_t> Saturation(SATURATION, \"Saturation\");\n> > > -\n> > > -/**\n> > > - * \\brief Specify a fixed exposure time in milli-seconds\n> > > - */\n> > > -extern const Control<int32_t> ManualExposure(MANUAL_EXPOSURE, \"ManualExposure\");\n> > > -\n> > > -/**\n> > > - * \\brief Specify a fixed gain parameter\n> > > - */\n> > > -extern const Control<int32_t> ManualGain(MANUAL_GAIN, \"ManualGain\");\n> > > -\n> > > -} /* namespace controls */\n> > > -\n> > > -} /* namespace libcamera */\n> > > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\n> > > new file mode 100644\n> > > index 000000000000..f699ac9eea54\n> > > --- /dev/null\n> > > +++ b/src/libcamera/control_ids.cpp.in\n> > > @@ -0,0 +1,25 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * control_ids.cpp : Control ID list\n> > > + *\n> > > + * This file is auto-generated. Do not edit.\n> > > + */\n> > > +\n> > > +#include <libcamera/control_ids.h>\n> > > +\n> > > +/**\n> > > + * \\file control_ids.h\n> > > + * \\brief Camera control identifiers\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace controls {\n> > > +\n> > > +${controls}\n> > > +\n> > > +} /* namespace controls */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > new file mode 100644\n> > > index 000000000000..819a5967a2fc\n> > > --- /dev/null\n> > > +++ b/src/libcamera/control_ids.yaml\n> > > @@ -0,0 +1,35 @@\n> > > +# SPDX-License-Identifier: LGPL-2.1-or-later\n> > > +#\n> > > +# Copyright (C) 2019, Google Inc.\n> > > +#\n> > > +%YAML 1.2\n> > > +---\n> > > +controls:\n> > > +  - AwbEnable:\n> > > +      type: bool\n> > > +      description: |\n> > > +        Enables or disables the AWB.\n> > > +\n> > > +        \\sa ManualGain\n> > > +\n> > > +  - Brightness:\n> > > +      type: int32_t\n> > > +      description: Specify a fixed brightness parameter\n> > > +\n> > > +  - Contrast:\n> > > +      type: int32_t\n> > > +      description: Specify a fixed contrast parameter\n> > > +\n> > > +  - Saturation:\n> > > +      type: int32_t\n> > > +      description: Specify a fixed saturation parameter\n> > > +\n> > > +  - ManualExposure:\n> > > +      type: int32_t\n> > > +      description: Specify a fixed exposure time in milli-seconds\n> > > +\n> > > +  - ManualGain:\n> > > +      type: int32_t\n> > > +      description: Specify a fixed gain parameter\n> > > +\n> > > +...\n> > > diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py\n> > > new file mode 100755\n> > > index 000000000000..0899e40b4080\n> > > --- /dev/null\n> > > +++ b/src/libcamera/gen-controls.py\n> > > @@ -0,0 +1,114 @@\n> > > +#!/usr/bin/python3\n> > > +# SPDX-License-Identifier: GPL-2.0-or-later\n> > > +# Copyright (C) 2019, Google Inc.\n> > > +#\n> > > +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > +#\n> > > +# gen-controls.py - Generate control definitions from YAML\n> > > +\n> > > +import argparse\n> > > +import string\n> > > +import sys\n> > > +import yaml\n> > > +\n> > > +\n> > > +def snake_case(s):\n> > > +    return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')\n> > > +\n> > > +\n> > > +def generate_cpp(controls):\n> > > +    template = string.Template('''/**\n> > > +${description}\n> > > + */\n> > > +extern const Control<${type}> ${name}(${id_name}, \"${name}\");''')\n> > > +\n> > > +    ctrls = []\n> > > +\n> > > +    for ctrl in controls:\n> > > +        name, ctrl = ctrl.popitem()\n> > > +        id_name = snake_case(name).upper()\n> > > +\n> > > +        description = ctrl['description'].strip('\\n').split('\\n')\n> > > +        description[0] = '\\\\brief ' + description[0]\n> > > +        description = '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n> > > +\n> > > +        info = {\n> > > +            'name': name,\n> > > +            'type': ctrl['type'],\n> > > +            'description': description,\n> > > +            'id_name': id_name,\n> > > +        }\n> > > +\n> > > +        ctrls.append(template.substitute(info))\n> > > +\n> > > +    return {'controls': '\\n\\n'.join(ctrls)}\n> > > +\n> > > +\n> > > +def generate_h(controls):\n> > > +    template = string.Template('''extern const Control<${type}> ${name};''')\n> > > +\n> > > +    ctrls = []\n> > > +    ids = []\n> > > +    id_value = 1\n> > > +\n> > > +    for ctrl in controls:\n> > > +        name, ctrl = ctrl.popitem()\n> > > +        id_name = snake_case(name).upper()\n> > > +\n> > > +        ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> > > +\n> > > +        info = {\n> > > +            'name': name,\n> > > +            'type': ctrl['type'],\n> > > +        }\n> > > +\n> > > +        ctrls.append(template.substitute(info))\n> > > +        id_value += 1\n> > > +\n> > > +    return {'ids': '\\n'.join(ids), 'controls': '\\n'.join(ctrls)}\n> > > +\n> > > +\n> > > +def fill_template(template, data):\n> > > +\n> > > +    template = open(template, 'rb').read()\n> > > +    template = template.decode('utf-8')\n> > > +    template = string.Template(template)\n> > > +    return template.substitute(data)\n> > > +\n> > > +\n> > > +def main(argv):\n> > > +\n> > > +    # Parse command line arguments\n> > > +    parser = argparse.ArgumentParser()\n> > > +    parser.add_argument('-o', dest='output', metavar='file', type=str,\n> > > +                        help='Output file name. Defaults to standard output if not specified.')\n> > > +    parser.add_argument('input', type=str,\n> > > +                        help='Input file name.')\n> > > +    parser.add_argument('template', type=str,\n> > > +                        help='Template file name.')\n> > > +    args = parser.parse_args(argv[1:])\n> > > +\n> > > +    data = open(args.input, 'rb').read()\n> > > +    controls = yaml.safe_load(data)['controls']\n> > > +\n> > > +    if args.template.endswith('.cpp.in'):\n> > > +        data = generate_cpp(controls)\n> > > +    elif args.template.endswith('.h.in'):\n> > > +        data = generate_h(controls)\n> > > +    else:\n> > > +        raise RuntimeError('Unknown template type')\n> > > +\n> > > +    data = fill_template(args.template, data)\n> > > +\n> > > +    if args.output:\n> > > +        output = open(args.output, 'wb')\n> > > +        output.write(data.encode('utf-8'))\n> > > +        output.close()\n> > > +    else:\n> > > +        sys.stdout.write(data)\n> > > +\n> > > +    return 0\n> > > +\n> > > +\n> > > +if __name__ == '__main__':\n> > > +    sys.exit(main(sys.argv))\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index 8123d1d5bee9..6df48365266d 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -5,7 +5,6 @@ libcamera_sources = files([\n> > >      'camera_manager.cpp',\n> > >      'camera_sensor.cpp',\n> > >      'controls.cpp',\n> > > -    'control_ids.cpp',\n> > >      'device_enumerator.cpp',\n> > >      'device_enumerator_sysfs.cpp',\n> > >      'event_dispatcher.cpp',\n> > > @@ -58,6 +57,17 @@ if libudev.found()\n> > >      ])\n> > >  endif\n> > >  \n> > > +gen_controls = files('gen-controls.py')\n> > > +\n> > > +control_ids_cpp = custom_target('control_ids_cpp',\n> > > +                                input : files('control_ids.yaml', 'control_ids.cpp.in'),\n> > > +                                output : 'control_ids.cpp',\n> > > +                                depend_files : gen_controls,\n> > > +                                command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])\n> > > +\n> > > +libcamera_sources += control_ids_cpp\n> > > +#libcamera_sources += control_ids_h\n> > \n> > I assume this comment out line should be removed right?\n> \n> Oops, yes, I'll fix that.\n\nActually it's the comment that needs to be removed, the line itself is\nneeded, otherwise the build gets racy.\n\n> > With that fixed,\n> > \n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > \n> > > +\n> > >  gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')\n> > >  \n> > >  version_cpp = vcs_tag(command : [gen_version, meson.build_root()],","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 089F560BC6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  5 Oct 2019 19:02:05 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(modemcable151.96-160-184.mc.videotron.ca [184.160.96.151])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CBD14DD;\n\tSat,  5 Oct 2019 19:02:03 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570294924;\n\tbh=FbZicR7YL7KsGPyK5bfWz9tnUtV1ONJZ4hWMHKSmU8E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gWZU8BuhJIsK+3b34TjGR1AJc4TtiC9PO3iX02wGu9U1ZDgO81NLl2S7GRPa7WyHi\n\tyYRv3vjwg5Mf2VbvrLMi6FnGIG9OOsEjzP2WVc/3GDUw+/1cq0zdHgZWtZLKNhSVZp\n\tLmtbjyn34frzgOjcm5x1QWm/w+AVjYH4Q314CvT8=","Date":"Sat, 5 Oct 2019 20:01:50 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191005170150.GD11154@pendragon.ideasonboard.com>","References":"<20190929190254.18920-1-laurent.pinchart@ideasonboard.com>\n\t<20190929190254.18920-6-laurent.pinchart@ideasonboard.com>\n\t<20191003193114.GH1322@bigcity.dyn.berto.se>\n\t<20191003211853.GE4737@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191003211853.GE4737@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 05/13] libcamera: controls:\n\tAuto-generate control_ids.h and control_ids.cpp","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>","X-List-Received-Date":"Sat, 05 Oct 2019 17:02:05 -0000"}}]