[libcamera-devel] [PATCH v2 4/6] libcamera: control: Add vendor control id range reservation

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Tue, Nov 21, 2023 at 02:53:48PM +0000, Naushir Patuck via libcamera-devel wrote:
> Add a new control_ranges.yaml file that is used to reserve control id
> ranges/offsets for libcamera and vendor specific controls. This file is
> used by the gen-controls.py script to generate control id values for
> each control.
> 
> Draft controls now have a separate range from core libcamera controls,
> breaking the existing numbering behaviour.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/meson.build     |  4 +++-
>  src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++
>  src/libcamera/meson.build         |  4 +++-
>  utils/gen-controls.py             | 15 ++++++++++++---
>  4 files changed, 36 insertions(+), 5 deletions(-)
>  create mode 100644 src/libcamera/control_ranges.yaml
> 
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 54123c5b858d..6cac385fdee4 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -83,11 +83,13 @@ foreach header, mode : control_source_files
>      endforeach
>  
>      template_file = files(header + '.h.in')
> +    ranges_file = files('../../src/libcamera/control_ranges.yaml')
>      control_headers += custom_target(header + '_h',
>                                       input : input_files,
>                                       output : header + '.h',
>                                       command : [gen_controls, '-o', '@OUTPUT@',
> -                                                '--mode', mode, '-t', template_file, '@INPUT@'],
> +                                                '--mode', mode, '-t', template_file,
> +                                                '-r', ranges_file, '@INPUT@'],
>                                       install : true,
>                                       install_dir : libcamera_headers_install_dir)
>  endforeach
> diff --git a/src/libcamera/control_ranges.yaml b/src/libcamera/control_ranges.yaml
> new file mode 100644
> index 000000000000..150ae7fa5efd
> --- /dev/null
> +++ b/src/libcamera/control_ranges.yaml
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +#
> +# Copyright (C) 2023, Raspberry Pi Ltd
> +#
> +%YAML 1.1
> +---
> +# Specifies the control id ranges/offsets for core libcamera and vendor specific
> +# controls.
> +ranges:
> +  # Core libcamera controls
> +  libcamera: 0
> +  # Draft designated libcamera controls
> +  draft: 10000
> +  # Raspberry Pi vendor controls
> +  rpi: 20000
> +  # Next range starts at 30000
> +
> +...
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 7eb26ccbb7f5..303eee84a77e 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -142,11 +142,13 @@ foreach source, mode : control_source_files
>      endforeach
>  
>      template_file = files(source + '.cpp.in')
> +    ranges_file = files('control_ranges.yaml')
>      control_sources += custom_target(source + '_cpp',
>                                       input : input_files,
>                                       output : source + '.cpp',
>                                       command : [gen_controls, '-o', '@OUTPUT@',
> -                                                '--mode', mode, '-t', template_file, '@INPUT@'])
> +                                                '--mode', mode, '-t', template_file,
> +                                                '-r', ranges_file, '@INPUT@'])
>  endforeach
>  
>  libcamera_sources += control_sources
> diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> index d7862142b8c1..9eede8940f24 100755
> --- a/utils/gen-controls.py
> +++ b/utils/gen-controls.py
> @@ -257,7 +257,7 @@ ${vendor_controls_map}
>      }
>  
>  
> -def generate_h(controls, mode):
> +def generate_h(controls, mode, ranges):
>      enum_template_start = string.Template('''enum ${name}Enum {''')
>      enum_value_template = string.Template('''\t${name} = ${value},''')
>      enum_values_template = string.Template('''extern const std::array<const ControlValue, ${size}> ${name}Values;''')
> @@ -272,8 +272,10 @@ def generate_h(controls, mode):
>  
>          vendor = 'draft' if ctrl.is_draft else ctrl.vendor
>          if vendor not in ctrls:
> +            if vendor not in ranges.keys():
> +                raise RuntimeError(f'Control id range is not defined for vendor {vendor}')
> +            id_value[vendor] = ranges[vendor] + 1
>              ids[vendor] = []
> -            id_value[vendor] = 1
>              ctrls[vendor] = []
>  
>          # Core and draft controls use the same ID value
> @@ -365,8 +367,15 @@ def main(argv):
>                          help='Template file name.')
>      parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],
>                          help='Mode of operation')
> +    parser.add_argument('-r', dest='ranges', type=str, required=True,
> +                        help='Control id range reservation file.')
>      args = parser.parse_args(argv[1:])
>  
> +    ranges = []
> +    if args.ranges:

As the argument is required, I would drop the check.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +        data = open(args.ranges, 'rb').read()
> +        ranges = yaml.safe_load(data)['ranges']
> +
>      controls = []
>      for input in args.input:
>          data = open(input, 'rb').read()
> @@ -377,7 +386,7 @@ def main(argv):
>      if args.template.endswith('.cpp.in'):
>          data = generate_cpp(controls)
>      elif args.template.endswith('.h.in'):
> -        data = generate_h(controls, args.mode)
> +        data = generate_h(controls, args.mode, ranges)
>      else:
>          raise RuntimeError('Unknown template type')
>  

-- 
Regards,

Laurent Pinchart