[libcamera-devel] [PATCH v1 2/5] controls: build: Allow separate vendor control YAML files

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Nov 20 05:09:53 CET 2023


Hi Naush,

Thank you for the patch.

On Fri, Nov 10, 2023 at 10:59:59AM +0000, Naushir Patuck via libcamera-devel wrote:
> Add support for using separate YAML files for vendor specific controls.
> This simplifies management of vendor control definitions and avoids
> possible merge conflicts when changing the control_ids.yaml file for
> core and draft controls. The mapping of vendor/pipeline handler to
> control file is done through the vendor_controls_mapping variable in
> include/libcamera/meson.build.
> 
> The command line argument handling for the gen-py-controls.py and
> gen-controls.py have now changed to allow multiple input files to be
> provided. This means that all positional arguments have been replaced
> by non-positional equivalents.

This could have been done in a separate patch to ease review.

> Add a new control_ids_rpi.yaml file to hold the Raspberry Pi specific
> vendor controls. This contains a single control PispConfigDumpFile that
> will be used in the Pi 5 pipeline handler as a trigger to dump the
> Backend configuration as a JSON file.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/meson.build       | 48 ++++++++++++++++++++++++++---
>  meson.build                         |  2 ++
>  src/libcamera/control_ids_rpi.yaml  | 17 ++++++++++
>  src/libcamera/meson.build           | 19 ++++++++++--
>  src/py/libcamera/gen-py-controls.py | 12 +++++---
>  src/py/libcamera/meson.build        | 26 ++++++++++------
>  utils/gen-controls.py               | 15 +++++----
>  7 files changed, 111 insertions(+), 28 deletions(-)
>  create mode 100644 src/libcamera/control_ids_rpi.yaml
> 
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index f736cca07228..f8af51174f08 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -32,21 +32,61 @@ install_headers(libcamera_public_headers,
>  
>  libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir
>  
> -# control_ids.h and property_ids.h
> +# control_ids.h and property_ids.h and associated modes

I think this belongs to the previous patch.

>  control_source_files = {
>      'control_ids': 'controls',
>      'property_ids': 'properties',
>  }
>  
> +vendor_mappings = {

The commit message names this vendor_controls_mapping, which I think is
a better name as it's related to controls.

> +    # Mapping of vendor (pipeline handler) specific controls files
> +    'controls':
> +    {
> +        'rpi/pisp': 'control_ids_rpi.yaml',
> +        'rpi/vc4': 'control_ids_rpi.yaml'
> +    },
> +    # Mapping of vendor (pipeline handler) specific properties files
> +    'properties':
> +    {
> +
> +    }
> +}
> +
>  control_headers = []
> +vendor_ctrl_files = []
> +vendor_prop_files = []
>  
>  foreach header, mode : control_source_files
> -    input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')
> +    # Start by populating the vendor specific mappings into vendor_ctrl_files
> +    # vendor_prop_files. These will be cached for later use.

s/vendor_prop_files/and vendor_prop_files/

> +    vendor_files = []
> +    foreach pipeline, file : vendor_mappings[mode]
> +        if pipeline not in pipelines
> +            continue
> +        endif
> +        if file not in vendor_files
> +            vendor_files += file
> +        endif
> +    endforeach
> +
> +    if mode == 'controls'
> +        vendor_ctrl_files = vendor_files
> +    else
> +        vendor_prop_files = vendor_files
> +    endif
> +
> +    input_files = files('../../src/libcamera/' + header +'.yaml')

While at it, add the missing space before the '.yaml'.

> +
> +    foreach file : vendor_files
> +        input_files += files('../../src/libcamera/' + file)
> +    endforeach
> +
> +    template_file = files(header + '.h.in')
>      control_headers += custom_target(header + '_h',
>                                       input : input_files,
>                                       output : header + '.h',
> -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> -                                                '--mode', mode],
> +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',
> +                                                '--mode', mode, '-t', template_file],
>                                       install : true,
>                                       install_dir : libcamera_headers_install_dir)
>  endforeach
> diff --git a/meson.build b/meson.build
> index e9a1c7e360ce..1423abf16c77 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,
> +            'Vendor controls': vendor_ctrl_files,
> +            'Vendor properties': vendor_prop_files,
>              'Hotplug support': libudev.found(),
>              'Tracing support': tracing_enabled,
>              'Android support': android_enabled,
> diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml
> new file mode 100644
> index 000000000000..239904a008a8
> --- /dev/null
> +++ b/src/libcamera/control_ids_rpi.yaml
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +#
> +# Copyright (C) 2023, Raspberry Pi Ltd
> +#
> +%YAML 1.1
> +---
> +controls:
> +
> +  - PispConfigDumpFile:
> +      type: string
> +      vendor: rpi
> +      description: |
> +        Triggers the Raspberry Pi PiSP pipeline handler to generate a JSON
> +        formatted dump of the Backend configuration to the filename given by the
> +        value of the control.
> +
> +...
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index e49bf850b355..8891052a3316 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -128,12 +128,25 @@ endif
>  control_sources = []
>  
>  foreach source, mode : control_source_files
> -    input_files = files(source +'.yaml', source + '.cpp.in')
> +    input_files = files(source +'.yaml')
> +
> +    # Add the vendor specific files to the input.
> +    if mode == 'controls'
> +        vendor_files = vendor_ctrl_files
> +    else
> +        vendor_files = vendor_prop_files
> +    endif
> +
> +    foreach file : vendor_files
> +        input_files += files(file)
> +    endforeach
> +
> +    template_file = files(source + '.cpp.in')
>      control_sources += custom_target(source + '_cpp',
>                                       input : input_files,
>                                       output : source + '.cpp',
> -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> -                                                '--mode', mode])
> +                                     command : [gen_controls, '-o', '@OUTPUT@', '-i', '@INPUT@',

meson will not expand this to multiple -i arguments, I'm actually
surprised it works as expected. Could we keep the input files as
positional arguments ?

> +                                                '--mode', mode, '-t', template_file])
>  endforeach
>  
>  libcamera_sources += control_sources
> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> index e89de674966a..a32ef09c2f48 100755
> --- a/src/py/libcamera/gen-py-controls.py
> +++ b/src/py/libcamera/gen-py-controls.py
> @@ -91,9 +91,9 @@ def main(argv):
>      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,
> +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,
> +                        help='Input file name(s).')
> +    parser.add_argument('-t', dest='template', type=str, required=True,
>                          help='Template file name.')
>      parser.add_argument('--mode', type=str, required=True,
>                          help='Mode is either "controls" or "properties"')
> @@ -103,8 +103,10 @@ def main(argv):
>          print(f'Invalid mode option "{args.mode}"', file=sys.stderr)
>          return -1
>  
> -    data = open(args.input, 'rb').read()
> -    controls = yaml.safe_load(data)['controls']
> +    controls = []
> +    for input in args.inputs:
> +        data = open(input, 'rb').read()
> +        controls += 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 f58c7198ee9e..e4bc208b4ac1 100644
> --- a/src/py/libcamera/meson.build
> +++ b/src/py/libcamera/meson.build
> @@ -28,29 +28,35 @@ pycamera_sources = files([
>  
>  # Generate controls
>  
> -gen_py_controls_input_files = files([
> -    '../../libcamera/control_ids.yaml',
> -    'py_controls_generated.cpp.in',
> -])
> +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')
> +
> +foreach file : vendor_ctrl_files
> +    gen_py_controls_input_files += files('../../libcamera/' + file)
> +endforeach
>  
>  gen_py_controls = files('gen-py-controls.py')
> +gen_py_controls_template = files('py_controls_generated.cpp.in')
>  
>  pycamera_sources += custom_target('py_gen_controls',
>                                    input : gen_py_controls_input_files,
>                                    output : ['py_controls_generated.cpp'],
> -                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])
> +                                  command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '-i', '@INPUT@',
> +                                             '-t', gen_py_controls_template])
>  
>  # Generate properties
>  
> -gen_py_property_enums_input_files = files([
> -    '../../libcamera/property_ids.yaml',
> -    'py_properties_generated.cpp.in',
> -])
> +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')
> +gen_py_property_template = files('py_properties_generated.cpp.in')
> +
> +foreach file : vendor_prop_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'],
> -                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])
> +                                  command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '-i', '@INPUT@',
> +                                             '-t', gen_py_property_template])
>  
>  # Generate formats
>  
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index dd55753e792c..3c490a562676 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -362,18 +362,21 @@ def main(argv):
>  
>      # Parse command line arguments
>      parser = argparse.ArgumentParser()
> -    parser.add_argument('-o', dest='output', metavar='file', type=str,
> +    parser.add_argument('-o', dest='output', metavar='file', type=str, required=True,
>                          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,
> +    parser.add_argument('-i', dest='inputs', type=str, nargs='+', required=True,
> +                        help='Input file name(s).')
> +    parser.add_argument('-t', dest='template', type=str, required=True,
>                          help='Template file name.')
>      parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],
>                          help='Mode of operation')
>      args = parser.parse_args(argv[1:])
>  
> -    data = open(args.input, 'rb').read()
> -    controls = yaml.safe_load(data)['controls']
> +    controls = []
> +    for input in args.inputs:
> +        data = open(input, 'rb').read()
> +        controls = controls + yaml.safe_load(data)['controls']
> +
>      controls = [Control(*ctrl.popitem()) for ctrl in controls]
>  
>      if args.template.endswith('.cpp.in'):

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list