| Message ID | 20231124123713.22519-5-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
I wonder if it's worth passing this as param, as I suspect the enumeration of controls will be something that user won't change.. On Fri, Nov 24, 2023 at 12:37:10PM +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@raspberrypi.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/meson.build | 3 ++- > src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++ > src/libcamera/meson.build | 4 +++- > utils/gen-controls.py | 14 ++++++++++---- > 4 files changed, 33 insertions(+), 6 deletions(-) > create mode 100644 src/libcamera/control_ranges.yaml > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index a763c8ff4344..79187d3fdfc9 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -76,12 +76,13 @@ foreach mode, entry : controls_map > endif > > template_file = files(outfile + '.in') > + ranges_file = files('../../src/libcamera/control_ranges.yaml') > control_headers += custom_target(header + '_h', > input : input_files, > output : outfile, > command : [gen_controls, '-o', '@OUTPUT@', > '--mode', mode, '-t', template_file, > - '@INPUT@'], > + '-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..d42447d04647 > --- /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/draft libcamera and vendor > +# controls and properties. > +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 6d9902e6ffd1..45f63e932e4f 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -141,11 +141,13 @@ foreach mode, input_files : controls_mode_files > template_file = files('property_ids.cpp.in') > endif > > + ranges_file = files('control_ranges.yaml') > control_sources += custom_target(mode + '_cpp', > input : input_files, > output : mode + '_ids.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 30c58f35473c..04c63098b19b 100755 > --- a/utils/gen-controls.py > +++ b/utils/gen-controls.py > @@ -236,7 +236,7 @@ ${vendor_controls_str} > } > > > -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;''') > @@ -251,8 +251,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 > @@ -338,13 +340,17 @@ def main(argv): > help='Mode of operation') > parser.add_argument('--output', '-o', metavar='file', type=str, > help='Output file name. Defaults to standard output if not specified.') > + parser.add_argument('--ranges', '-r', type=str, required=True, > + help='Control id range reservation file.') > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > help='Template file name.') > parser.add_argument('input', type=str, nargs='+', > help='Input file name.') > - > args = parser.parse_args(argv[1:]) > > + data = open(args.ranges, 'rb').read() > + ranges = yaml.safe_load(data)['ranges'] > + > controls = [] > for input in args.input: > data = open(input, 'rb').read() > @@ -355,7 +361,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') > > -- > 2.34.1 >
Hi Jacopo, On Mon, 27 Nov 2023 at 16:54, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > I wonder if it's worth passing this as param, as I suspect the > enumeration of controls will be something that user won't change.. I could do that, but that involves setting the vendor ranges in the meson.build file somewhere and fetching it when calling the script. Personally I prefer this living in a YAML file as this patch does, but if folks want, I can move it into the build scripts. Regards, Naush > > On Fri, Nov 24, 2023 at 12:37:10PM +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@raspberrypi.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/meson.build | 3 ++- > > src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++ > > src/libcamera/meson.build | 4 +++- > > utils/gen-controls.py | 14 ++++++++++---- > > 4 files changed, 33 insertions(+), 6 deletions(-) > > create mode 100644 src/libcamera/control_ranges.yaml > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > index a763c8ff4344..79187d3fdfc9 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -76,12 +76,13 @@ foreach mode, entry : controls_map > > endif > > > > template_file = files(outfile + '.in') > > + ranges_file = files('../../src/libcamera/control_ranges.yaml') > > control_headers += custom_target(header + '_h', > > input : input_files, > > output : outfile, > > command : [gen_controls, '-o', '@OUTPUT@', > > '--mode', mode, '-t', template_file, > > - '@INPUT@'], > > + '-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..d42447d04647 > > --- /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/draft libcamera and vendor > > +# controls and properties. > > +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 6d9902e6ffd1..45f63e932e4f 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -141,11 +141,13 @@ foreach mode, input_files : controls_mode_files > > template_file = files('property_ids.cpp.in') > > endif > > > > + ranges_file = files('control_ranges.yaml') > > control_sources += custom_target(mode + '_cpp', > > input : input_files, > > output : mode + '_ids.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 30c58f35473c..04c63098b19b 100755 > > --- a/utils/gen-controls.py > > +++ b/utils/gen-controls.py > > @@ -236,7 +236,7 @@ ${vendor_controls_str} > > } > > > > > > -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;''') > > @@ -251,8 +251,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 > > @@ -338,13 +340,17 @@ def main(argv): > > help='Mode of operation') > > parser.add_argument('--output', '-o', metavar='file', type=str, > > help='Output file name. Defaults to standard output if not specified.') > > + parser.add_argument('--ranges', '-r', type=str, required=True, > > + help='Control id range reservation file.') > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > help='Template file name.') > > parser.add_argument('input', type=str, nargs='+', > > help='Input file name.') > > - > > args = parser.parse_args(argv[1:]) > > > > + data = open(args.ranges, 'rb').read() > > + ranges = yaml.safe_load(data)['ranges'] > > + > > controls = [] > > for input in args.input: > > data = open(input, 'rb').read() > > @@ -355,7 +361,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') > > > > -- > > 2.34.1 > >
Hi Naush On Tue, Nov 28, 2023 at 09:21:05AM +0000, Naushir Patuck wrote: > Hi Jacopo, > > On Mon, 27 Nov 2023 at 16:54, Jacopo Mondi > <jacopo.mondi@ideasonboard.com> wrote: > > > > I wonder if it's worth passing this as param, as I suspect the > > enumeration of controls will be something that user won't change.. > > I could do that, but that involves setting the vendor ranges in the > meson.build file somewhere and fetching it when calling the script. > Personally I prefer this living in a YAML file as this patch does, but > if folks want, I can move it into the build scripts. > I was not questioning the usage of the yaml file, that's fine. I was wondering if it's worth passing the ranges file name and location as parameter, or gen-controls.py can assume it knows where it lives.. > Regards, > Naush > > > > > On Fri, Nov 24, 2023 at 12:37:10PM +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@raspberrypi.com> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > include/libcamera/meson.build | 3 ++- > > > src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++ > > > src/libcamera/meson.build | 4 +++- > > > utils/gen-controls.py | 14 ++++++++++---- > > > 4 files changed, 33 insertions(+), 6 deletions(-) > > > create mode 100644 src/libcamera/control_ranges.yaml > > > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > > index a763c8ff4344..79187d3fdfc9 100644 > > > --- a/include/libcamera/meson.build > > > +++ b/include/libcamera/meson.build > > > @@ -76,12 +76,13 @@ foreach mode, entry : controls_map > > > endif > > > > > > template_file = files(outfile + '.in') > > > + ranges_file = files('../../src/libcamera/control_ranges.yaml') > > > control_headers += custom_target(header + '_h', > > > input : input_files, > > > output : outfile, > > > command : [gen_controls, '-o', '@OUTPUT@', > > > '--mode', mode, '-t', template_file, > > > - '@INPUT@'], > > > + '-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..d42447d04647 > > > --- /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/draft libcamera and vendor > > > +# controls and properties. > > > +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 6d9902e6ffd1..45f63e932e4f 100644 > > > --- a/src/libcamera/meson.build > > > +++ b/src/libcamera/meson.build > > > @@ -141,11 +141,13 @@ foreach mode, input_files : controls_mode_files > > > template_file = files('property_ids.cpp.in') > > > endif > > > > > > + ranges_file = files('control_ranges.yaml') > > > control_sources += custom_target(mode + '_cpp', > > > input : input_files, > > > output : mode + '_ids.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 30c58f35473c..04c63098b19b 100755 > > > --- a/utils/gen-controls.py > > > +++ b/utils/gen-controls.py > > > @@ -236,7 +236,7 @@ ${vendor_controls_str} > > > } > > > > > > > > > -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;''') > > > @@ -251,8 +251,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 > > > @@ -338,13 +340,17 @@ def main(argv): > > > help='Mode of operation') > > > parser.add_argument('--output', '-o', metavar='file', type=str, > > > help='Output file name. Defaults to standard output if not specified.') > > > + parser.add_argument('--ranges', '-r', type=str, required=True, > > > + help='Control id range reservation file.') > > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > > help='Template file name.') > > > parser.add_argument('input', type=str, nargs='+', > > > help='Input file name.') > > > - > > > args = parser.parse_args(argv[1:]) > > > > > > + data = open(args.ranges, 'rb').read() > > > + ranges = yaml.safe_load(data)['ranges'] > > > + > > > controls = [] > > > for input in args.input: > > > data = open(input, 'rb').read() > > > @@ -355,7 +361,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') > > > > > > -- > > > 2.34.1 > > >
On Tue, 28 Nov 2023 at 09:24, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Naush > > On Tue, Nov 28, 2023 at 09:21:05AM +0000, Naushir Patuck wrote: > > Hi Jacopo, > > > > On Mon, 27 Nov 2023 at 16:54, Jacopo Mondi > > <jacopo.mondi@ideasonboard.com> wrote: > > > > > > I wonder if it's worth passing this as param, as I suspect the > > > enumeration of controls will be something that user won't change.. > > > > I could do that, but that involves setting the vendor ranges in the > > meson.build file somewhere and fetching it when calling the script. > > Personally I prefer this living in a YAML file as this patch does, but > > if folks want, I can move it into the build scripts. > > > > I was not questioning the usage of the yaml file, that's fine. > I was wondering if it's worth passing the ranges file name and > location as parameter, or gen-controls.py can assume it knows where it > lives.. Ah, sorry I misunderstood! I did it as a parameter as all the inputs seem to go through parameters, but again, I am happy to keep the ranges file hardcoded in the script if folks think that's better. Naush > > > Regards, > > Naush > > > > > > > > On Fri, Nov 24, 2023 at 12:37:10PM +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@raspberrypi.com> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > include/libcamera/meson.build | 3 ++- > > > > src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++ > > > > src/libcamera/meson.build | 4 +++- > > > > utils/gen-controls.py | 14 ++++++++++---- > > > > 4 files changed, 33 insertions(+), 6 deletions(-) > > > > create mode 100644 src/libcamera/control_ranges.yaml > > > > > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > > > index a763c8ff4344..79187d3fdfc9 100644 > > > > --- a/include/libcamera/meson.build > > > > +++ b/include/libcamera/meson.build > > > > @@ -76,12 +76,13 @@ foreach mode, entry : controls_map > > > > endif > > > > > > > > template_file = files(outfile + '.in') > > > > + ranges_file = files('../../src/libcamera/control_ranges.yaml') > > > > control_headers += custom_target(header + '_h', > > > > input : input_files, > > > > output : outfile, > > > > command : [gen_controls, '-o', '@OUTPUT@', > > > > '--mode', mode, '-t', template_file, > > > > - '@INPUT@'], > > > > + '-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..d42447d04647 > > > > --- /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/draft libcamera and vendor > > > > +# controls and properties. > > > > +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 6d9902e6ffd1..45f63e932e4f 100644 > > > > --- a/src/libcamera/meson.build > > > > +++ b/src/libcamera/meson.build > > > > @@ -141,11 +141,13 @@ foreach mode, input_files : controls_mode_files > > > > template_file = files('property_ids.cpp.in') > > > > endif > > > > > > > > + ranges_file = files('control_ranges.yaml') > > > > control_sources += custom_target(mode + '_cpp', > > > > input : input_files, > > > > output : mode + '_ids.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 30c58f35473c..04c63098b19b 100755 > > > > --- a/utils/gen-controls.py > > > > +++ b/utils/gen-controls.py > > > > @@ -236,7 +236,7 @@ ${vendor_controls_str} > > > > } > > > > > > > > > > > > -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;''') > > > > @@ -251,8 +251,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 > > > > @@ -338,13 +340,17 @@ def main(argv): > > > > help='Mode of operation') > > > > parser.add_argument('--output', '-o', metavar='file', type=str, > > > > help='Output file name. Defaults to standard output if not specified.') > > > > + parser.add_argument('--ranges', '-r', type=str, required=True, > > > > + help='Control id range reservation file.') > > > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > > > help='Template file name.') > > > > parser.add_argument('input', type=str, nargs='+', > > > > help='Input file name.') > > > > - > > > > args = parser.parse_args(argv[1:]) > > > > > > > > + data = open(args.ranges, 'rb').read() > > > > + ranges = yaml.safe_load(data)['ranges'] > > > > + > > > > controls = [] > > > > for input in args.input: > > > > data = open(input, 'rb').read() > > > > @@ -355,7 +361,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') > > > > > > > > -- > > > > 2.34.1 > > > >
Quoting Naushir Patuck via libcamera-devel (2023-11-28 09:28:08) > On Tue, 28 Nov 2023 at 09:24, Jacopo Mondi > <jacopo.mondi@ideasonboard.com> wrote: > > > > Hi Naush > > > > On Tue, Nov 28, 2023 at 09:21:05AM +0000, Naushir Patuck wrote: > > > Hi Jacopo, > > > > > > On Mon, 27 Nov 2023 at 16:54, Jacopo Mondi > > > <jacopo.mondi@ideasonboard.com> wrote: > > > > > > > > I wonder if it's worth passing this as param, as I suspect the > > > > enumeration of controls will be something that user won't change.. > > > > > > I could do that, but that involves setting the vendor ranges in the > > > meson.build file somewhere and fetching it when calling the script. > > > Personally I prefer this living in a YAML file as this patch does, but > > > if folks want, I can move it into the build scripts. > > > > > > > I was not questioning the usage of the yaml file, that's fine. > > I was wondering if it's worth passing the ranges file name and > > location as parameter, or gen-controls.py can assume it knows where it > > lives.. > > Ah, sorry I misunderstood! I did it as a parameter as all the inputs > seem to go through parameters, but again, I am happy to keep the > ranges file hardcoded in the script if folks think that's better. I likely wouldn't have asked to change it to be a parameter specified externally, but as it's already there I think it's ok as is. This script is specifically custom anyway, it doesn't have to be generic. Having it specified in the meson build means we can likely handle things easier if we move the files around too. I anticipate once this lands we'll want to move and collate all controls into a subdir to group the likely expansions here, but we don't need to do that as part of this series. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Naush > > > > > > > Regards, > > > Naush > > > > > > > > > > > On Fri, Nov 24, 2023 at 12:37:10PM +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@raspberrypi.com> > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > --- > > > > > include/libcamera/meson.build | 3 ++- > > > > > src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++ > > > > > src/libcamera/meson.build | 4 +++- > > > > > utils/gen-controls.py | 14 ++++++++++---- > > > > > 4 files changed, 33 insertions(+), 6 deletions(-) > > > > > create mode 100644 src/libcamera/control_ranges.yaml > > > > > > > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > > > > index a763c8ff4344..79187d3fdfc9 100644 > > > > > --- a/include/libcamera/meson.build > > > > > +++ b/include/libcamera/meson.build > > > > > @@ -76,12 +76,13 @@ foreach mode, entry : controls_map > > > > > endif > > > > > > > > > > template_file = files(outfile + '.in') > > > > > + ranges_file = files('../../src/libcamera/control_ranges.yaml') > > > > > control_headers += custom_target(header + '_h', > > > > > input : input_files, > > > > > output : outfile, > > > > > command : [gen_controls, '-o', '@OUTPUT@', > > > > > '--mode', mode, '-t', template_file, > > > > > - '@INPUT@'], > > > > > + '-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..d42447d04647 > > > > > --- /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/draft libcamera and vendor > > > > > +# controls and properties. > > > > > +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 6d9902e6ffd1..45f63e932e4f 100644 > > > > > --- a/src/libcamera/meson.build > > > > > +++ b/src/libcamera/meson.build > > > > > @@ -141,11 +141,13 @@ foreach mode, input_files : controls_mode_files > > > > > template_file = files('property_ids.cpp.in') > > > > > endif > > > > > > > > > > + ranges_file = files('control_ranges.yaml') > > > > > control_sources += custom_target(mode + '_cpp', > > > > > input : input_files, > > > > > output : mode + '_ids.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 30c58f35473c..04c63098b19b 100755 > > > > > --- a/utils/gen-controls.py > > > > > +++ b/utils/gen-controls.py > > > > > @@ -236,7 +236,7 @@ ${vendor_controls_str} > > > > > } > > > > > > > > > > > > > > > -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;''') > > > > > @@ -251,8 +251,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 > > > > > @@ -338,13 +340,17 @@ def main(argv): > > > > > help='Mode of operation') > > > > > parser.add_argument('--output', '-o', metavar='file', type=str, > > > > > help='Output file name. Defaults to standard output if not specified.') > > > > > + parser.add_argument('--ranges', '-r', type=str, required=True, > > > > > + help='Control id range reservation file.') > > > > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > > > > help='Template file name.') > > > > > parser.add_argument('input', type=str, nargs='+', > > > > > help='Input file name.') > > > > > - > > > > > args = parser.parse_args(argv[1:]) > > > > > > > > > > + data = open(args.ranges, 'rb').read() > > > > > + ranges = yaml.safe_load(data)['ranges'] > > > > > + > > > > > controls = [] > > > > > for input in args.input: > > > > > data = open(input, 'rb').read() > > > > > @@ -355,7 +361,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') > > > > > > > > > > -- > > > > > 2.34.1 > > > > >
Hi On Tue, Nov 28, 2023 at 09:58:39AM +0000, Kieran Bingham via libcamera-devel wrote: > Quoting Naushir Patuck via libcamera-devel (2023-11-28 09:28:08) > > On Tue, 28 Nov 2023 at 09:24, Jacopo Mondi > > <jacopo.mondi@ideasonboard.com> wrote: > > > > > > Hi Naush > > > > > > On Tue, Nov 28, 2023 at 09:21:05AM +0000, Naushir Patuck wrote: > > > > Hi Jacopo, > > > > > > > > On Mon, 27 Nov 2023 at 16:54, Jacopo Mondi > > > > <jacopo.mondi@ideasonboard.com> wrote: > > > > > > > > > > I wonder if it's worth passing this as param, as I suspect the > > > > > enumeration of controls will be something that user won't change.. > > > > > > > > I could do that, but that involves setting the vendor ranges in the > > > > meson.build file somewhere and fetching it when calling the script. > > > > Personally I prefer this living in a YAML file as this patch does, but > > > > if folks want, I can move it into the build scripts. > > > > > > > > > > I was not questioning the usage of the yaml file, that's fine. > > > I was wondering if it's worth passing the ranges file name and > > > location as parameter, or gen-controls.py can assume it knows where it > > > lives.. > > > > Ah, sorry I misunderstood! I did it as a parameter as all the inputs > > seem to go through parameters, but again, I am happy to keep the > > ranges file hardcoded in the script if folks think that's better. > > I likely wouldn't have asked to change it to be a parameter specified > externally, but as it's already there I think it's ok as is. > > This script is specifically custom anyway, it doesn't have to be > generic. > > Having it specified in the meson build means we can likely handle things > easier if we move the files around too. > Yeah, mine was defintely more a question more than a requirement, I think it's fine too > I anticipate once this lands we'll want to move and collate all > controls into a subdir to group the likely expansions here, but we don't > need to do that as part of this series. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > And seems I forgot my tag Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > > > > Naush > > > > > > > > > > > Regards, > > > > Naush > > > > > > > > > > > > > > On Fri, Nov 24, 2023 at 12:37:10PM +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@raspberrypi.com> > > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > --- > > > > > > include/libcamera/meson.build | 3 ++- > > > > > > src/libcamera/control_ranges.yaml | 18 ++++++++++++++++++ > > > > > > src/libcamera/meson.build | 4 +++- > > > > > > utils/gen-controls.py | 14 ++++++++++---- > > > > > > 4 files changed, 33 insertions(+), 6 deletions(-) > > > > > > create mode 100644 src/libcamera/control_ranges.yaml > > > > > > > > > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > > > > > index a763c8ff4344..79187d3fdfc9 100644 > > > > > > --- a/include/libcamera/meson.build > > > > > > +++ b/include/libcamera/meson.build > > > > > > @@ -76,12 +76,13 @@ foreach mode, entry : controls_map > > > > > > endif > > > > > > > > > > > > template_file = files(outfile + '.in') > > > > > > + ranges_file = files('../../src/libcamera/control_ranges.yaml') > > > > > > control_headers += custom_target(header + '_h', > > > > > > input : input_files, > > > > > > output : outfile, > > > > > > command : [gen_controls, '-o', '@OUTPUT@', > > > > > > '--mode', mode, '-t', template_file, > > > > > > - '@INPUT@'], > > > > > > + '-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..d42447d04647 > > > > > > --- /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/draft libcamera and vendor > > > > > > +# controls and properties. > > > > > > +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 6d9902e6ffd1..45f63e932e4f 100644 > > > > > > --- a/src/libcamera/meson.build > > > > > > +++ b/src/libcamera/meson.build > > > > > > @@ -141,11 +141,13 @@ foreach mode, input_files : controls_mode_files > > > > > > template_file = files('property_ids.cpp.in') > > > > > > endif > > > > > > > > > > > > + ranges_file = files('control_ranges.yaml') > > > > > > control_sources += custom_target(mode + '_cpp', > > > > > > input : input_files, > > > > > > output : mode + '_ids.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 30c58f35473c..04c63098b19b 100755 > > > > > > --- a/utils/gen-controls.py > > > > > > +++ b/utils/gen-controls.py > > > > > > @@ -236,7 +236,7 @@ ${vendor_controls_str} > > > > > > } > > > > > > > > > > > > > > > > > > -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;''') > > > > > > @@ -251,8 +251,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 > > > > > > @@ -338,13 +340,17 @@ def main(argv): > > > > > > help='Mode of operation') > > > > > > parser.add_argument('--output', '-o', metavar='file', type=str, > > > > > > help='Output file name. Defaults to standard output if not specified.') > > > > > > + parser.add_argument('--ranges', '-r', type=str, required=True, > > > > > > + help='Control id range reservation file.') > > > > > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > > > > > help='Template file name.') > > > > > > parser.add_argument('input', type=str, nargs='+', > > > > > > help='Input file name.') > > > > > > - > > > > > > args = parser.parse_args(argv[1:]) > > > > > > > > > > > > + data = open(args.ranges, 'rb').read() > > > > > > + ranges = yaml.safe_load(data)['ranges'] > > > > > > + > > > > > > controls = [] > > > > > > for input in args.input: > > > > > > data = open(input, 'rb').read() > > > > > > @@ -355,7 +361,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') > > > > > > > > > > > > -- > > > > > > 2.34.1 > > > > > >
diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index a763c8ff4344..79187d3fdfc9 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -76,12 +76,13 @@ foreach mode, entry : controls_map endif template_file = files(outfile + '.in') + ranges_file = files('../../src/libcamera/control_ranges.yaml') control_headers += custom_target(header + '_h', input : input_files, output : outfile, command : [gen_controls, '-o', '@OUTPUT@', '--mode', mode, '-t', template_file, - '@INPUT@'], + '-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..d42447d04647 --- /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/draft libcamera and vendor +# controls and properties. +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 6d9902e6ffd1..45f63e932e4f 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -141,11 +141,13 @@ foreach mode, input_files : controls_mode_files template_file = files('property_ids.cpp.in') endif + ranges_file = files('control_ranges.yaml') control_sources += custom_target(mode + '_cpp', input : input_files, output : mode + '_ids.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 30c58f35473c..04c63098b19b 100755 --- a/utils/gen-controls.py +++ b/utils/gen-controls.py @@ -236,7 +236,7 @@ ${vendor_controls_str} } -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;''') @@ -251,8 +251,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 @@ -338,13 +340,17 @@ def main(argv): help='Mode of operation') parser.add_argument('--output', '-o', metavar='file', type=str, help='Output file name. Defaults to standard output if not specified.') + parser.add_argument('--ranges', '-r', type=str, required=True, + help='Control id range reservation file.') parser.add_argument('--template', '-t', dest='template', type=str, required=True, help='Template file name.') parser.add_argument('input', type=str, nargs='+', help='Input file name.') - args = parser.parse_args(argv[1:]) + data = open(args.ranges, 'rb').read() + ranges = yaml.safe_load(data)['ranges'] + controls = [] for input in args.input: data = open(input, 'rb').read() @@ -355,7 +361,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')