[libcamera-devel] [PATCH v3 4/7] libcamera: control: Add vendor control id range reservation
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Nov 28 10:58:39 CET 2023
Quoting Naushir Patuck via libcamera-devel (2023-11-28 09:28:08)
> On Tue, 28 Nov 2023 at 09:24, Jacopo Mondi
> <jacopo.mondi at 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 at 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 at 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 at raspberrypi.com>
> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart at 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
> > > > >
More information about the libcamera-devel
mailing list