[libcamera-devel] [PATCH v3 3/7] build: controls: Rework how controls and properties are generated

Jacopo Mondi jacopo.mondi at ideasonboard.com
Mon Nov 27 16:38:44 CET 2023


Hi Naush

On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote:
> Add support for using separate YAML files for controls and properties
> generation. The mapping of vendor/pipeline handler to control file is
> done through the controls_map variable in include/libcamera/meson.build.
>
> This simplifies management of vendor control definitions and avoids
> possible merge conflicts when changing the control_ids.yaml file for
> core and draft controls. With this change, libcamera and draft controls
> and properties files are designated the 'libcamera' vendor tag.
>
> In this change, we also rename control_ids.yaml -> control_ids_core.yaml
> and property_ids.yaml -> property_ids_core.yaml to designate these as
> core libcamera controls.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  Documentation/guides/pipeline-handler.rst     |  8 +--
>  include/libcamera/meson.build                 | 52 +++++++++++++++----
>  meson.build                                   |  2 +
>  ...control_ids.yaml => control_ids_core.yaml} |  0
>  src/libcamera/meson.build                     | 21 ++++++--
>  ...operty_ids.yaml => property_ids_core.yaml} |  0
>  src/py/libcamera/gen-py-controls.py           | 10 ++--
>  src/py/libcamera/meson.build                  | 12 ++++-
>  utils/gen-controls.py                         | 13 +++--
>  9 files changed, 88 insertions(+), 30 deletions(-)
>  rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%)
>  rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%)
>
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 10b9c75c2a7f..66d428a19c4f 100644
> --- a/Documentation/guides/pipeline-handler.rst
> +++ b/Documentation/guides/pipeline-handler.rst
> @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device.
>
>  The libcamera controls and properties are defined in YAML form which is
>  processed to automatically generate documentation and interfaces. Controls are
> -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties
> -are defined by src/libcamera/`properties_ids.yaml`_.
> +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties
> +are defined by src/libcamera/`properties_ids_core.yaml`_.
>
>  .. _controls framework: https://libcamera.org/api-html/controls_8h.html
> -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html
> -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html
> +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html
> +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html
>
>  Pipeline handlers can optionally register the list of controls an application
>  can set as well as a list of immutable camera properties. Being both
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 5fb772e6dd14..a763c8ff4344 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers,
>
>  libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir
>
> -# control_ids.h and property_ids.h and associated modes
> -control_source_files = {
> -    'control_ids': 'controls',
> -    'property_ids': 'properties',
> +controls_map = {
> +    'controls': {
> +        'core': 'control_ids_core.yaml',
> +    },
> +
> +    'properties': {
> +        'core': 'property_ids_core.yaml',
> +    }
>  }
>
>  control_headers = []
> +controls_files = []
> +properties_files = []
> +
> +foreach mode, entry : controls_map
> +    files_list = []
> +    input_files = []
> +    foreach vendor, header : entry
> +        if vendor != 'core' and vendor != 'draft'
> +            if vendor not in pipelines
> +                message('vendor not in pipelines')

This gives

Message: vendor not in pipelines
Message: vendor not in pipelines

in the meson output log, that's not really useful imo

> +                continue
> +            endif
> +        endif
> +
> +        if header in files_list
> +            message('header in files list')
> +            continue
> +        endif
> +
> +        files_list += header
> +        input_files += files('../../src/libcamera/' + header)
> +    endforeach
> +
> +    outfile = ''
> +    if mode == 'controls'
> +        outfile = 'control_ids.h'
> +        controls_files += files_list
> +    else
> +        outfile = 'property_ids.h'
> +        properties_files += files_list
> +    endif
>
> -foreach header, mode : control_source_files
> -    input_files = files('../../src/libcamera/' + header +'.yaml')
> -    template_file = files(header + '.h.in')
> +    template_file = files(outfile + '.in')
>      control_headers += custom_target(header + '_h',
>                                       input : input_files,
> -                                     output : header + '.h',
> +                                     output : outfile,
>                                       command : [gen_controls, '-o', '@OUTPUT@',
> -                                                '--mode', mode, '-t', template_file, '@INPUT@'],
> +                                                '--mode', mode, '-t', template_file,
> +                                                '@INPUT@'],
>                                       install : true,
>                                       install_dir : libcamera_headers_install_dir)
>  endforeach
> diff --git a/meson.build b/meson.build
> index e9a1c7e360ce..ee57cb780149 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -267,6 +267,8 @@ py_mod.find_installation('python3', modules : py_modules)
>  summary({
>              'Enabled pipelines': pipelines,
>              'Enabled IPA modules': enabled_ipa_names,
> +            'Controls files': controls_files,
> +            'Properties files': properties_files,

    Controls files           : control_ids_draft.yaml
                               control_ids_core.yaml
    Properties files         : property_ids_draft.yaml
                               property_ids_core.yaml
This (not from this patch but with the whole series applied) is more
useful indeed!

>              'Hotplug support': libudev.found(),
>              'Tracing support': tracing_enabled,
>              'Android support': android_enabled,
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml
> similarity index 100%
> rename from src/libcamera/control_ids.yaml
> rename to src/libcamera/control_ids_core.yaml
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 05ee38daf22b..6d9902e6ffd1 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -127,12 +127,23 @@ endif
>
>  control_sources = []
>
> -foreach source, mode : control_source_files
> -    input_files = files(source +'.yaml')
> -    template_file = files(source + '.cpp.in')
> -    control_sources += custom_target(source + '_cpp',
> +controls_mode_files = {
> +    'controls' : controls_files,
> +    'properties' : properties_files,
> +}
> +
> +foreach mode, input_files : controls_mode_files
> +    input_files = files(input_files)
> +
> +    if mode == 'controls'
> +        template_file = files('control_ids.cpp.in')
> +    else
> +        template_file = files('property_ids.cpp.in')
> +    endif
> +
> +    control_sources += custom_target(mode + '_cpp',
>                                       input : input_files,
> -                                     output : source + '.cpp',
> +                                     output : mode + '_ids.cpp',
>                                       command : [gen_controls, '-o', '@OUTPUT@',
>                                                  '--mode', mode, '-t', template_file, '@INPUT@'])
>  endforeach
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml
> similarity index 100%
> rename from src/libcamera/property_ids.yaml
> rename to src/libcamera/property_ids_core.yaml
> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> index cfcfd4d16acf..8ae8d5126e39 100755
> --- a/src/py/libcamera/gen-py-controls.py
> +++ b/src/py/libcamera/gen-py-controls.py
> @@ -95,7 +95,7 @@ def main(argv):
>                          help='Output file name. Defaults to standard output if not specified.')
>      parser.add_argument('--template', '-t', type=str, required=True,
>                          help='Template file name.')
> -    parser.add_argument('input', type=str,
> +    parser.add_argument('input', type=str, nargs='+',
>                          help='Input file name.')
>      args = parser.parse_args(argv[1:])
>
> @@ -103,11 +103,11 @@ def main(argv):
>          print(f'Invalid mode option "{args.mode}"', file=sys.stderr)
>          return -1
>
> -    data = open(args.input, 'rb').read()
> -
>      controls = {}
> -    vendor = yaml.safe_load(data)['vendor']
> -    controls[vendor] = yaml.safe_load(data)['controls']
> +    for input in args.input:
> +        data = open(input, 'rb').read()
> +        vendor = yaml.safe_load(data)['vendor']
> +        controls[vendor] = yaml.safe_load(data)['controls']
>
>      data = generate_py(controls, args.mode)
>
> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> index 1c3ea1843ac0..31af63ec0dc6 100644
> --- a/src/py/libcamera/meson.build
> +++ b/src/py/libcamera/meson.build
> @@ -28,11 +28,15 @@ pycamera_sources = files([
>
>  # Generate controls
>
> -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')
> +gen_py_controls_input_files = []
>  gen_py_controls_template = files('py_controls_generated.cpp.in')
>
>  gen_py_controls = files('gen-py-controls.py')
>
> +foreach file : controls_files
> +    gen_py_controls_input_files += files('../../libcamera/' + file)
> +endforeach
> +
>  pycamera_sources += custom_target('py_gen_controls',
>                                    input : gen_py_controls_input_files,
>                                    output : ['py_controls_generated.cpp'],
> @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls',
>
>  # Generate properties
>
> -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')
> +gen_py_property_enums_input_files = []
>  gen_py_properties_template = files('py_properties_generated.cpp.in')
>
> +foreach file : properties_files
> +    gen_py_property_enums_input_files += files('../../libcamera/' + file)
> +endforeach
> +
>  pycamera_sources += custom_target('py_gen_properties',
>                                    input : gen_py_property_enums_input_files,
>                                    output : ['py_properties_generated.cpp'],
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index 6680ecf84acb..30c58f35473c 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -12,6 +12,7 @@ import operator
>  import string
>  import sys
>  import yaml
> +import os
>
>
>  class ControlEnum(object):
> @@ -339,15 +340,17 @@ def main(argv):
>                          help='Output file name. Defaults to standard output if not specified.')
>      parser.add_argument('--template', '-t', dest='template', type=str, required=True,
>                          help='Template file name.')
> -    parser.add_argument('input', type=str,
> +    parser.add_argument('input', type=str, nargs='+',
>                          help='Input file name.')
>
>      args = parser.parse_args(argv[1:])
>
> -    data = open(args.input, 'rb').read()
> -    vendor = yaml.safe_load(data)['vendor']
> -    controls = yaml.safe_load(data)['controls']
> -    controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]
> +    controls = []
> +    for input in args.input:
> +        data = open(input, 'rb').read()
> +        vendor = yaml.safe_load(data)['vendor']
> +        ctrls = yaml.safe_load(data)['controls']
> +        controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls]

How does thi work in Python, should data be closed ?

Anyway, this all looks good, with the above 'message' dropped

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>
>      if args.template.endswith('.cpp.in'):
>          data = generate_cpp(controls)
> --
> 2.34.1
>


More information about the libcamera-devel mailing list