[libcamera-devel] [PATCH v2 05/13] libcamera: controls: Auto-generate control_ids.h and control_ids.cpp

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sat Oct 5 19:01:50 CEST 2019


Hi Niklas,

On Fri, Oct 04, 2019 at 12:18:53AM +0300, Laurent Pinchart wrote:
> On Thu, Oct 03, 2019 at 09:31:14PM +0200, Niklas Söderlund wrote:
> > On 2019-09-29 22:02:46 +0300, Laurent Pinchart wrote:
> > > Bring back auto-generation of control ids. In this version, both the
> > > header and the source files are generated from a single YAML file that
> > > stores all control definitions. This allows centralising controls in a
> > > single file, while the previous version required keeping both
> > > declarations (in a header) and documentation (in a the source) in sync
> > > manually.
> > > 
> > > Using YAML as a format to store control definitions is a trade-off
> > > between ease of use (there are many YAML parsers available) and
> > > simplicity (XML was considered, but would have lead to more complex
> > > processing). A new build time dependency is added on python3-yaml, which
> > > should be available as a package in all distributions and build
> > > environments.
> > > 
> > > The YAML format is likely to change over time as we improve
> > > documentation of controls, the first version simply copies the
> > > information currently available. Future improvements should also include
> > > a YAML schema to validate the YAML source file.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  Documentation/Doxyfile.in                     |   4 +-
> > >  README.rst                                    |   2 +-
> > >  .../{control_ids.h => control_ids.h.in}       |  16 +--
> > >  include/libcamera/gen-header.sh               |   2 +-
> > >  include/libcamera/meson.build                 |  18 ++-
> > >  .../libcamera/libcamera-9999.ebuild           |   9 +-
> > >  src/libcamera/control_ids.cpp                 |  52 --------
> > >  src/libcamera/control_ids.cpp.in              |  25 ++++
> > >  src/libcamera/control_ids.yaml                |  35 ++++++
> > >  src/libcamera/gen-controls.py                 | 114 ++++++++++++++++++
> > >  src/libcamera/meson.build                     |  12 +-
> > >  11 files changed, 215 insertions(+), 74 deletions(-)
> > >  rename include/libcamera/{control_ids.h => control_ids.h.in} (53%)
> > >  delete mode 100644 src/libcamera/control_ids.cpp
> > >  create mode 100644 src/libcamera/control_ids.cpp.in
> > >  create mode 100644 src/libcamera/control_ids.yaml
> > >  create mode 100755 src/libcamera/gen-controls.py
> > > 
> > > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> > > index 28a9c2da1ad4..5237cf60854f 100644
> > > --- a/Documentation/Doxyfile.in
> > > +++ b/Documentation/Doxyfile.in
> > > @@ -793,7 +793,9 @@ WARN_LOGFILE           =
> > >  
> > >  INPUT                  = "@TOP_SRCDIR@/include/ipa" \
> > >  			 "@TOP_SRCDIR@/include/libcamera" \
> > > -			 "@TOP_SRCDIR@/src/libcamera"
> > > +			 "@TOP_SRCDIR@/src/libcamera" \
> > > +			 "@TOP_BUILDDIR@/include/libcamera" \
> > > +			 "@TOP_BUILDDIR@/src/libcamera"
> > >  
> > >  # This tag can be used to specify the character encoding of the source files
> > >  # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses
> > > diff --git a/README.rst b/README.rst
> > > index 169837e41a4e..2ccf7cbec40a 100644
> > > --- a/README.rst
> > > +++ b/README.rst
> > > @@ -40,7 +40,7 @@ A C++ toolchain: [required]
> > >  	Either {g++, clang}
> > >  
> > >  for libcamera: [required]
> > > -	meson ninja-build
> > > +	meson ninja-build python3-yaml
> > >  
> > >  for device hotplug enumeration: [optional]
> > >  	pkg-config libudev-dev
> > > diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h.in
> > > similarity index 53%
> > > rename from include/libcamera/control_ids.h
> > > rename to include/libcamera/control_ids.h.in
> > > index 54235f1aea95..1d0bc791e559 100644
> > > --- a/include/libcamera/control_ids.h
> > > +++ b/include/libcamera/control_ids.h.in
> > > @@ -3,6 +3,8 @@
> > >   * Copyright (C) 2019, Google Inc.
> > >   *
> > >   * control_ids.h : Control ID list
> > > + *
> > > + * This file is auto-generated. Do not edit.
> > >   */
> > >  
> > >  #ifndef __LIBCAMERA_CONTROL_IDS_H__
> > > @@ -17,20 +19,10 @@ namespace libcamera {
> > >  namespace controls {
> > >  
> > >  enum {
> > > -	AWB_ENABLE = 1,
> > > -	BRIGHTNESS = 2,
> > > -	CONTRAST = 3,
> > > -	SATURATION = 4,
> > > -	MANUAL_EXPOSURE = 5,
> > > -	MANUAL_GAIN = 6,
> > > +${ids}
> > >  };
> > >  
> > > -extern const Control<bool> AwbEnable;
> > > -extern const Control<int32_t> Brightness;
> > > -extern const Control<int32_t> Contrast;
> > > -extern const Control<int32_t> Saturation;
> > > -extern const Control<int32_t> ManualExposure;
> > > -extern const Control<int32_t> ManualGain;
> > > +${controls}
> > >  
> > >  } /* namespace controls */
> > >  
> > > diff --git a/include/libcamera/gen-header.sh b/include/libcamera/gen-header.sh
> > > index a69fe8e982a1..7f7816c9f879 100755
> > > --- a/include/libcamera/gen-header.sh
> > > +++ b/include/libcamera/gen-header.sh
> > > @@ -19,7 +19,7 @@ EOF
> > >  headers=$(for header in "$src_dir"/*.h ; do
> > >  	header=$(basename "$header")
> > >  	echo "$header"
> > > -done ; echo "version.h" | sort)
> > > +done ; echo "control_ids.h" ; echo "version.h" | sort)
> > >  
> > >  for header in $headers ; do
> > >  	echo "#include <libcamera/$header>" >> "$dst_file"
> > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > > index 868f1a6bf1ab..4ffbdab3b173 100644
> > > --- a/include/libcamera/meson.build
> > > +++ b/include/libcamera/meson.build
> > > @@ -3,7 +3,6 @@ libcamera_api = files([
> > >      'buffer.h',
> > >      'camera.h',
> > >      'camera_manager.h',
> > > -    'control_ids.h',
> > >      'controls.h',
> > >      'event_dispatcher.h',
> > >      'event_notifier.h',
> > > @@ -18,6 +17,20 @@ libcamera_api = files([
> > >  
> > >  include_dir = join_paths(libcamera_include_dir, 'libcamera')
> > >  
> > > +install_headers(libcamera_api,
> > > +                subdir : include_dir)
> > > +
> > > +gen_controls = files('../../src/libcamera/gen-controls.py')
> > > +
> > > +control_ids_h = custom_target('control_ids_h',
> > > +                              input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'),
> > > +                              output : 'control_ids.h',
> > > +                              depend_files : gen_controls,
> > > +                              command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],
> > > +                              install_dir : join_paths('include', include_dir))
> > > +
> > > +libcamera_api += control_ids_h
> > > +
> > >  gen_header = files('gen-header.sh')
> > >  
> > >  libcamera_h = custom_target('gen-header',
> > > @@ -37,6 +50,3 @@ configure_file(input : 'version.h.in',
> > >                 output : 'version.h',
> > >                 configuration : libcamera_version_config,
> > >                 install_dir : join_paths('include', include_dir))
> > > -
> > > -install_headers(libcamera_api,
> > > -                subdir : include_dir)
> > > diff --git a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild
> > > index fed2b409a91b..fc241b1f5584 100644
> > > --- a/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild
> > > +++ b/package/gentoo/media-libs/libcamera/libcamera-9999.ebuild
> > > @@ -2,7 +2,9 @@
> > >  # Distributed under the terms of the GNU General Public License v2
> > >  
> > >  EAPI=6
> > > -inherit git-r3 meson
> > > +PYTHON_COMPAT=( python3_{5,6,7} )
> > > +
> > > +inherit git-r3 meson python-any-r1
> > >  
> > >  DESCRIPTION="Camera support library for Linux"
> > >  HOMEPAGE="http://libcamera.org"
> > > @@ -15,7 +17,10 @@ KEYWORDS="*"
> > >  IUSE="udev"
> > >  
> > >  RDEPEND="udev? ( virtual/libudev )"
> > > -DEPEND="${RDEPEND}"
> > > +DEPEND="
> > > +	${RDEPEND}
> > > +	$(python_gen_any_dep 'dev-python/pyyaml[${PYTHON_USEDEP}]')
> > > +"
> > >  
> > >  src_configure() {
> > >  	local emesonargs=(
> > > diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp
> > > deleted file mode 100644
> > > index 3af23a458862..000000000000
> > > --- a/src/libcamera/control_ids.cpp
> > > +++ /dev/null
> > > @@ -1,52 +0,0 @@
> > > -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > -/*
> > > - * Copyright (C) 2019, Google Inc.
> > > - *
> > > - * control_ids.cpp : Control ID list
> > > - */
> > > -
> > > -#include <libcamera/control_ids.h>
> > > -
> > > -/**
> > > - * \file control_ids.h
> > > - * \brief Camera control identifiers
> > > - */
> > > -
> > > -namespace libcamera {
> > > -
> > > -namespace controls {
> > > -
> > > -/**
> > > - * \brief Enables or disables the AWB.
> > > - * \sa ManualGain
> > > - */
> > > -extern const Control<bool> AwbEnable(AWB_ENABLE, "AwbEnable");
> > > -
> > > -/**
> > > - * \brief Specify a fixed brightness parameter.
> > > - */
> > > -extern const Control<int32_t> Brightness(BRIGHTNESS, "Brightness");
> > > -
> > > -/**
> > > - * \brief Specify a fixed contrast parameter.
> > > - */
> > > -extern const Control<int32_t> Contrast(CONTRAST, "Contrast");
> > > -
> > > -/**
> > > - * \brief Specify a fixed saturation parameter.
> > > - */
> > > -extern const Control<int32_t> Saturation(SATURATION, "Saturation");
> > > -
> > > -/**
> > > - * \brief Specify a fixed exposure time in milli-seconds
> > > - */
> > > -extern const Control<int32_t> ManualExposure(MANUAL_EXPOSURE, "ManualExposure");
> > > -
> > > -/**
> > > - * \brief Specify a fixed gain parameter
> > > - */
> > > -extern const Control<int32_t> ManualGain(MANUAL_GAIN, "ManualGain");
> > > -
> > > -} /* namespace controls */
> > > -
> > > -} /* namespace libcamera */
> > > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> > > new file mode 100644
> > > index 000000000000..f699ac9eea54
> > > --- /dev/null
> > > +++ b/src/libcamera/control_ids.cpp.in
> > > @@ -0,0 +1,25 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2019, Google Inc.
> > > + *
> > > + * control_ids.cpp : Control ID list
> > > + *
> > > + * This file is auto-generated. Do not edit.
> > > + */
> > > +
> > > +#include <libcamera/control_ids.h>
> > > +
> > > +/**
> > > + * \file control_ids.h
> > > + * \brief Camera control identifiers
> > > + */
> > > +
> > > +namespace libcamera {
> > > +
> > > +namespace controls {
> > > +
> > > +${controls}
> > > +
> > > +} /* namespace controls */
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > new file mode 100644
> > > index 000000000000..819a5967a2fc
> > > --- /dev/null
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -0,0 +1,35 @@
> > > +# SPDX-License-Identifier: LGPL-2.1-or-later
> > > +#
> > > +# Copyright (C) 2019, Google Inc.
> > > +#
> > > +%YAML 1.2
> > > +---
> > > +controls:
> > > +  - AwbEnable:
> > > +      type: bool
> > > +      description: |
> > > +        Enables or disables the AWB.
> > > +
> > > +        \sa ManualGain
> > > +
> > > +  - Brightness:
> > > +      type: int32_t
> > > +      description: Specify a fixed brightness parameter
> > > +
> > > +  - Contrast:
> > > +      type: int32_t
> > > +      description: Specify a fixed contrast parameter
> > > +
> > > +  - Saturation:
> > > +      type: int32_t
> > > +      description: Specify a fixed saturation parameter
> > > +
> > > +  - ManualExposure:
> > > +      type: int32_t
> > > +      description: Specify a fixed exposure time in milli-seconds
> > > +
> > > +  - ManualGain:
> > > +      type: int32_t
> > > +      description: Specify a fixed gain parameter
> > > +
> > > +...
> > > diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py
> > > new file mode 100755
> > > index 000000000000..0899e40b4080
> > > --- /dev/null
> > > +++ b/src/libcamera/gen-controls.py
> > > @@ -0,0 +1,114 @@
> > > +#!/usr/bin/python3
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +# Copyright (C) 2019, Google Inc.
> > > +#
> > > +# Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > +#
> > > +# gen-controls.py - Generate control definitions from YAML
> > > +
> > > +import argparse
> > > +import string
> > > +import sys
> > > +import yaml
> > > +
> > > +
> > > +def snake_case(s):
> > > +    return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')
> > > +
> > > +
> > > +def generate_cpp(controls):
> > > +    template = string.Template('''/**
> > > +${description}
> > > + */
> > > +extern const Control<${type}> ${name}(${id_name}, "${name}");''')
> > > +
> > > +    ctrls = []
> > > +
> > > +    for ctrl in controls:
> > > +        name, ctrl = ctrl.popitem()
> > > +        id_name = snake_case(name).upper()
> > > +
> > > +        description = ctrl['description'].strip('\n').split('\n')
> > > +        description[0] = '\\brief ' + description[0]
> > > +        description = '\n'.join([(line and ' * ' or ' *') + line for line in description])
> > > +
> > > +        info = {
> > > +            'name': name,
> > > +            'type': ctrl['type'],
> > > +            'description': description,
> > > +            'id_name': id_name,
> > > +        }
> > > +
> > > +        ctrls.append(template.substitute(info))
> > > +
> > > +    return {'controls': '\n\n'.join(ctrls)}
> > > +
> > > +
> > > +def generate_h(controls):
> > > +    template = string.Template('''extern const Control<${type}> ${name};''')
> > > +
> > > +    ctrls = []
> > > +    ids = []
> > > +    id_value = 1
> > > +
> > > +    for ctrl in controls:
> > > +        name, ctrl = ctrl.popitem()
> > > +        id_name = snake_case(name).upper()
> > > +
> > > +        ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
> > > +
> > > +        info = {
> > > +            'name': name,
> > > +            'type': ctrl['type'],
> > > +        }
> > > +
> > > +        ctrls.append(template.substitute(info))
> > > +        id_value += 1
> > > +
> > > +    return {'ids': '\n'.join(ids), 'controls': '\n'.join(ctrls)}
> > > +
> > > +
> > > +def fill_template(template, data):
> > > +
> > > +    template = open(template, 'rb').read()
> > > +    template = template.decode('utf-8')
> > > +    template = string.Template(template)
> > > +    return template.substitute(data)
> > > +
> > > +
> > > +def main(argv):
> > > +
> > > +    # Parse command line arguments
> > > +    parser = argparse.ArgumentParser()
> > > +    parser.add_argument('-o', dest='output', metavar='file', type=str,
> > > +                        help='Output file name. Defaults to standard output if not specified.')
> > > +    parser.add_argument('input', type=str,
> > > +                        help='Input file name.')
> > > +    parser.add_argument('template', type=str,
> > > +                        help='Template file name.')
> > > +    args = parser.parse_args(argv[1:])
> > > +
> > > +    data = open(args.input, 'rb').read()
> > > +    controls = yaml.safe_load(data)['controls']
> > > +
> > > +    if args.template.endswith('.cpp.in'):
> > > +        data = generate_cpp(controls)
> > > +    elif args.template.endswith('.h.in'):
> > > +        data = generate_h(controls)
> > > +    else:
> > > +        raise RuntimeError('Unknown template type')
> > > +
> > > +    data = fill_template(args.template, data)
> > > +
> > > +    if args.output:
> > > +        output = open(args.output, 'wb')
> > > +        output.write(data.encode('utf-8'))
> > > +        output.close()
> > > +    else:
> > > +        sys.stdout.write(data)
> > > +
> > > +    return 0
> > > +
> > > +
> > > +if __name__ == '__main__':
> > > +    sys.exit(main(sys.argv))
> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > index 8123d1d5bee9..6df48365266d 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -5,7 +5,6 @@ libcamera_sources = files([
> > >      'camera_manager.cpp',
> > >      'camera_sensor.cpp',
> > >      'controls.cpp',
> > > -    'control_ids.cpp',
> > >      'device_enumerator.cpp',
> > >      'device_enumerator_sysfs.cpp',
> > >      'event_dispatcher.cpp',
> > > @@ -58,6 +57,17 @@ if libudev.found()
> > >      ])
> > >  endif
> > >  
> > > +gen_controls = files('gen-controls.py')
> > > +
> > > +control_ids_cpp = custom_target('control_ids_cpp',
> > > +                                input : files('control_ids.yaml', 'control_ids.cpp.in'),
> > > +                                output : 'control_ids.cpp',
> > > +                                depend_files : gen_controls,
> > > +                                command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])
> > > +
> > > +libcamera_sources += control_ids_cpp
> > > +#libcamera_sources += control_ids_h
> > 
> > I assume this comment out line should be removed right?
> 
> Oops, yes, I'll fix that.

Actually it's the comment that needs to be removed, the line itself is
needed, otherwise the build gets racy.

> > With that fixed,
> > 
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > 
> > > +
> > >  gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
> > >  
> > >  version_cpp = vcs_tag(command : [gen_version, meson.build_root()],

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list