[libcamera-devel] [PATCH v2 1/6] controls: Add vendor control/property support to generation scripts

Naushir Patuck naush at raspberrypi.com
Thu Nov 23 13:53:44 CET 2023


Hi Laurent,

Thank you for the feedback!

On Thu, 23 Nov 2023 at 12:00, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Naush,
>
> Thank you for the patch.
>
> On Tue, Nov 21, 2023 at 02:53:45PM +0000, Naushir Patuck via libcamera-devel wrote:
> > Add support for vendor specific controls and properties to libcamera.
> > The controls/properties are defined by a "vendor" tag in the YAML
> > control description file, for example:
> >
> > vendor: rpi
> > controls:
> >   - MyExampleControl:
> >       type: string
> >       description: |
> >         Test for libcamera vendor specific controls.
> >
> > This will now generate a control id in the libcamera::controls::rpi
> > namespace, ensuring no id conflict between different vendors, core or
> > draft libcamera controls. Similarly, a ControlIdMap control is generated
> > in the libcamera::controls::rpi namespace.
>
> I think we'll have a problem here. There are many places in libcamera
> that use controls::controls today, and all of a sudden they won't have
> draft controls included there anymore. This includes the cam application
> or the Python bindings when they process metadata, the control
> serialization code, the Request constructor when it creates the
> ControlList for controls stored in the request, or various pipeline
> handlers and IPA modules when processing control lists.

Yes, this will indeed cause problems with some existing code.

>
> One option would be to keep a single ControlIdMap with all the controls,
> but that isn't going to work well long term. The better option would be
> to get rid of controls::controls, replacing it with Camera::controls()
> (conceptually, as they're not the same type, and Camera::controls() gets
> updated at runtime, leading to possible use-after-free). I don't want to
> block vendor controls completely until we get there, so I'm fine with an
> incremental approach, but I think we need to at least sketch the plan
> now to make sure the first step goes in the right direction.

Changing things around in this patch to keep a single ControlIdMap
with all (libcamera/draft/vendor) controls is fairly trivial.  I would
suggest we start there to at least merge this functionality without
any breakages and build on top of that.

>
> > A #define LIBCAMERA_HAS_RPI_VENDOR_CONTROLS is also generated to allow
> > applications to conditionally compile code if the specific vendor
> > controls are present. For the python bindings, the control is available
> > with libcamera.controls.rpi.MyExampleControl. The above controls
> > example applies similarly to properties.
> >
> > Existing libcamera controls defined in control_ids.yaml are given the
> > "libcamera" vendor tag.
> >
> > A new --mode flag is added to gen-controls.py to specify the mode of
> > operation, either 'controls' or 'properties' to allow the code generator
> > to correctly set the #define string.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  include/libcamera/control_ids.h.in            |   2 +
> >  include/libcamera/meson.build                 |  15 +-
> >  include/libcamera/property_ids.h.in           |   2 +
> >  src/libcamera/control_ids.cpp.in              |   6 +
> >  src/libcamera/control_ids.yaml                |   1 +
> >  src/libcamera/meson.build                     |   5 +-
> >  src/libcamera/property_ids.cpp.in             |   6 +
> >  src/libcamera/property_ids.yaml               |   1 +
> >  src/py/libcamera/gen-py-controls.py           |  75 +++++----
> >  src/py/libcamera/py_controls_generated.cpp.in |   3 +
> >  .../libcamera/py_properties_generated.cpp.in  |   3 +
> >  utils/gen-controls.py                         | 144 ++++++++++++++----
> >  12 files changed, 194 insertions(+), 69 deletions(-)
> >
> > diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in
> > index 0718a8886f6c..c97b09a82450 100644
> > --- a/include/libcamera/control_ids.h.in
> > +++ b/include/libcamera/control_ids.h.in
> > @@ -32,6 +32,8 @@ ${draft_controls}
> >
> >  } /* namespace draft */
> >
> > +${vendor_controls}
> > +
>
> If we had to redo it, it could have been nicer to turn support for draft
> controls into vendor controls, instead of adding vendor controls and
> then using them to handle draft controls. I think that would have been
> easier to review. No need to rewrite the series, just a comment for the
> future.
>
> >  } /* namespace controls */
> >
> >  } /* namespace libcamera */
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index a24c50d66a82..2c8c0258c95e 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -32,20 +32,21 @@ install_headers(libcamera_public_headers,
> >
> >  libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir
> >
> > -# control_ids.h and property_ids.h
> > -control_source_files = [
> > -    'control_ids',
> > -    'property_ids',
> > -]
> > +# control_ids.h and property_ids.h and associated modes
> > +control_source_files = {
> > +    'control_ids': 'controls',
> > +    'property_ids': 'properties',
> > +}
> >
> >  control_headers = []
> >
> > -foreach header : control_source_files
> > +foreach header, mode : control_source_files
> >      input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')
> >      control_headers += custom_target(header + '_h',
> >                                       input : input_files,
> >                                       output : header + '.h',
> > -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],
> > +                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> > +                                                '--mode', mode],
> >                                       install : true,
> >                                       install_dir : libcamera_headers_install_dir)
> >  endforeach
> > diff --git a/include/libcamera/property_ids.h.in b/include/libcamera/property_ids.h.in
> > index ff0194083af0..47c5d6bf2e28 100644
> > --- a/include/libcamera/property_ids.h.in
> > +++ b/include/libcamera/property_ids.h.in
> > @@ -31,6 +31,8 @@ ${draft_controls}
> >
> >  extern const ControlIdMap properties;
> >
> > +${vendor_controls}
> > +
> >  } /* namespace properties */
> >
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> > index 5fb1c2c30558..d26eb930b30c 100644
> > --- a/src/libcamera/control_ids.cpp.in
> > +++ b/src/libcamera/control_ids.cpp.in
> > @@ -33,6 +33,8 @@ ${draft_controls_doc}
> >
> >  } /* namespace draft */
> >
> > +${vendor_controls_doc}
> > +
> >  #ifndef __DOXYGEN__
> >  /*
> >   * Keep the controls definitions hidden from doxygen as it incorrectly parses
> > @@ -45,6 +47,8 @@ namespace draft {
> >  ${draft_controls_def}
> >
> >  } /* namespace draft */
> > +
> > +${vendor_controls_def}
> >  #endif
> >
> >  /**
> > @@ -57,6 +61,8 @@ extern const ControlIdMap controls {
> >  ${controls_map}
> >  };
> >
> > +${vendor_controls_map}
> > +
> >  } /* namespace controls */
> >
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 5827d7ecef49..ff74ce1deedb 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -6,6 +6,7 @@
> >  ---
> >  # Unless otherwise stated, all controls are bi-directional, i.e. they can be
> >  # set through Request::controls() and returned out through Request::metadata().
> > +vendor: libcamera
> >  controls:
> >    - AeEnable:
> >        type: bool
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index d0e26f6b4141..e49bf850b355 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -127,12 +127,13 @@ endif
> >
> >  control_sources = []
> >
> > -foreach source : control_source_files
> > +foreach source, mode : control_source_files
> >      input_files = files(source +'.yaml', source + '.cpp.in')
> >      control_sources += custom_target(source + '_cpp',
> >                                       input : input_files,
> >                                       output : source + '.cpp',
> > -                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])
> > +                                     command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',
> > +                                                '--mode', mode])
> >  endforeach
> >
> >  libcamera_sources += control_sources
> > diff --git a/src/libcamera/property_ids.cpp.in b/src/libcamera/property_ids.cpp.in
> > index f917e3349766..ddbe714c3f00 100644
> > --- a/src/libcamera/property_ids.cpp.in
> > +++ b/src/libcamera/property_ids.cpp.in
> > @@ -32,6 +32,8 @@ ${draft_controls_doc}
> >
> >  } /* namespace draft */
> >
> > +${vendor_controls_doc}
> > +
> >  #ifndef __DOXYGEN__
> >  /*
> >   * Keep the properties definitions hidden from doxygen as it incorrectly parses
> > @@ -44,6 +46,8 @@ namespace draft {
> >  ${draft_controls_def}
> >
> >  } /* namespace draft */
> > +
> > +${vendor_controls_def}
> >  #endif
> >
> >  /**
> > @@ -53,6 +57,8 @@ extern const ControlIdMap properties {
> >  ${controls_map}
> >  };
> >
> > +${vendor_controls_map}
> > +
> >  } /* namespace properties */
> >
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index f35563842a5a..45f3609b4236 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -4,6 +4,7 @@
> >  #
> >  %YAML 1.1
> >  ---
> > +vendor: libcamera
> >  controls:
> >    - Location:
> >        type: int32_t
> > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> > index 9948c41e42b1..8f14cdafe313 100755
> > --- a/src/py/libcamera/gen-py-controls.py
> > +++ b/src/py/libcamera/gen-py-controls.py
> > @@ -24,45 +24,59 @@ def find_common_prefix(strings):
> >  def generate_py(controls, mode):
> >      out = ''
> >
> > -    for ctrl in controls:
> > -        name, ctrl = ctrl.popitem()
> > -
> > -        if ctrl.get('draft'):
> > -            ns = 'libcamera::{}::draft::'.format(mode)
> > -            container = 'draft'
> > -        else:
> > -            ns = 'libcamera::{}::'.format(mode)
> > -            container = 'controls'
> > +    vendors_class_def = []
> > +    vendor_defs = []
> > +    vendors = []
> > +    for vendor, ctrl_list in controls.items():
> > +        for ctrls in ctrl_list:
> > +            name, ctrl = ctrls.popitem()
> > +
> > +            if vendor not in vendors:
> > +                vendors_class_def.append('class Py{}Controls\n{{\n}};\n'.format(vendor))
> > +                vendor_defs.append('\tauto {} = py::class_<Py{}Controls>(controls, \"{}\");'.format(vendor, vendor, vendor))
> > +                vendors.append(vendor)
> > +
> > +            if ctrl.get('draft'):
> > +                ns = 'libcamera::{}::draft::'.format(mode)
> > +                container = 'draft'
> > +            elif vendor != 'libcamera':
> > +                ns = 'libcamera::{}::{}::'.format(mode, vendor)
> > +                container = vendor
> > +            else:
> > +                ns = 'libcamera::{}::'.format(mode)
> > +                container = 'controls'
> >
> > -        out += f'\t{container}.def_readonly_static("{name}", static_cast<const libcamera::ControlId *>(&{ns}{name}));\n\n'
> > +            out += f'\t{container}.def_readonly_static("{name}", static_cast<const libcamera::ControlId *>(&{ns}{name}));\n\n'
> >
> > -        enum = ctrl.get('enum')
> > -        if not enum:
> > -            continue
> > +            enum = ctrl.get('enum')
> > +            if not enum:
> > +                continue
> >
> > -        cpp_enum = name + 'Enum'
> > +            cpp_enum = name + 'Enum'
> >
> > -        out += '\tpy::enum_<{}{}>({}, \"{}\")\n'.format(ns, cpp_enum, container, cpp_enum)
> > +            out += '\tpy::enum_<{}{}>({}, \"{}\")\n'.format(ns, cpp_enum, container, cpp_enum)
> >
> > -        if mode == 'controls':
> > -            # Adjustments for controls
> > -            if name == 'LensShadingMapMode':
> > -                prefix = 'LensShadingMapMode'
> > +            if mode == 'controls':
> > +                # Adjustments for controls
> > +                if name == 'LensShadingMapMode':
> > +                    prefix = 'LensShadingMapMode'
> > +                else:
> > +                    prefix = find_common_prefix([e['name'] for e in enum])
> >              else:
> > +                # Adjustments for properties
> >                  prefix = find_common_prefix([e['name'] for e in enum])
> > -        else:
> > -            # Adjustments for properties
> > -            prefix = find_common_prefix([e['name'] for e in enum])
> >
> > -        for entry in enum:
> > -            cpp_enum = entry['name']
> > -            py_enum = entry['name'][len(prefix):]
> > +            for entry in enum:
> > +                cpp_enum = entry['name']
> > +                py_enum = entry['name'][len(prefix):]
> >
> > -            out += '\t\t.value(\"{}\", {}{})\n'.format(py_enum, ns, cpp_enum)
> > +                out += '\t\t.value(\"{}\", {}{})\n'.format(py_enum, ns, cpp_enum)
> >
> > -        out += '\t;\n\n'
> > +            out += '\t;\n\n'
>
> I think we've reached a major milestone: making Python code as
> unreadable as Perl :-) Not your fault, I'll try to find time to convert
> this to jinja templates.

I know... :)

>
> >
> > -    return {'controls': out}
> > +    return {'controls': out,
> > +            'vendors_class_def': '\n'.join(vendors_class_def),
> > +            'vendors_defs': '\n'.join(vendor_defs)}
> >
> >
> >  def fill_template(template, data):
> > @@ -90,7 +104,10 @@ def main(argv):
> >          return -1
> >
> >      data = open(args.input, 'rb').read()
> > -    controls = yaml.safe_load(data)['controls']
> > +
> > +    controls = {}
> > +    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/py_controls_generated.cpp.in b/src/py/libcamera/py_controls_generated.cpp.in
> > index 18fa57d948ea..ec4b55ef2011 100644
> > --- a/src/py/libcamera/py_controls_generated.cpp.in
> > +++ b/src/py/libcamera/py_controls_generated.cpp.in
> > @@ -21,10 +21,13 @@ class PyDraftControls
> >  {
> >  };
> >
> > +${vendors_class_def}
> > +
> >  void init_py_controls_generated(py::module& m)
> >  {
> >       auto controls = py::class_<PyControls>(m, "controls");
> >       auto draft = py::class_<PyDraftControls>(controls, "draft");
> > +${vendors_defs}
> >
> >  ${controls}
> >  }
> > diff --git a/src/py/libcamera/py_properties_generated.cpp.in b/src/py/libcamera/py_properties_generated.cpp.in
> > index e49b6e91bb83..f7b5ec8c635d 100644
> > --- a/src/py/libcamera/py_properties_generated.cpp.in
> > +++ b/src/py/libcamera/py_properties_generated.cpp.in
> > @@ -21,10 +21,13 @@ class PyDraftProperties
> >  {
> >  };
> >
> > +${vendors_class_def}
> > +
> >  void init_py_properties_generated(py::module& m)
> >  {
> >       auto controls = py::class_<PyProperties>(m, "properties");
> >       auto draft = py::class_<PyDraftProperties>(controls, "draft");
> > +${vendors_defs}
> >
> >  ${controls}
> >  }
> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> > index 1075ae302ce1..8f2f8fdb02c3 100755
> > --- a/utils/gen-controls.py
> > +++ b/utils/gen-controls.py
> > @@ -35,11 +35,12 @@ class ControlEnum(object):
> >
> >
> >  class Control(object):
> > -    def __init__(self, name, data):
> > +    def __init__(self, name, data, vendor):
> >          self.__name = name
> >          self.__data = data
> >          self.__enum_values = None
> >          self.__size = None
> > +        self.__vendor = vendor
> >
> >          enum_values = data.get('enum')
> >          if enum_values is not None:
> > @@ -89,6 +90,11 @@ class Control(object):
> >          """Is the control a draft control"""
> >          return self.__data.get('draft') is not None
> >
> > +    @property
> > +    def vendor(self):
> > +        """The vendor string, or None"""
> > +        return self.__vendor
> > +
> >      @property
> >      def name(self):
> >          """The control name (CamelCase)"""
> > @@ -145,15 +151,19 @@ ${description}
> >      enum_values_start = string.Template('''extern const std::array<const ControlValue, ${size}> ${name}Values = {''')
> >      enum_values_values = string.Template('''\tstatic_cast<int32_t>(${name}),''')
> >
> > -    ctrls_doc = []
> > -    ctrls_def = []
> > -    draft_ctrls_doc = []
> > -    draft_ctrls_def = []
> > -    ctrls_map = []
> > +    ctrls_doc = {}
> > +    ctrls_def = {}
> > +    ctrls_map = {}
> >
> >      for ctrl in controls:
> >          id_name = snake_case(ctrl.name).upper()
> >
> > +        vendor = 'draft' if ctrl.is_draft else ctrl.vendor
> > +        if vendor not in ctrls_doc:
> > +            ctrls_doc[vendor] = []
> > +            ctrls_def[vendor] = []
> > +            ctrls_map[vendor] = []
> > +
> >          info = {
> >              'name': ctrl.name,
> >              'type': ctrl.type,
> > @@ -161,11 +171,9 @@ ${description}
> >              'id_name': id_name,
> >          }
> >
> > -        target_doc = ctrls_doc
> > -        target_def = ctrls_def
> > -        if ctrl.is_draft:
> > -            target_doc = draft_ctrls_doc
> > -            target_def = draft_ctrls_def
> > +        target_doc = ctrls_doc[vendor]
> > +        target_def = ctrls_def[vendor]
> > +        target_ctrls_map = ctrls_map[vendor]
> >
> >          if ctrl.is_enum:
> >              enum_doc = []
> > @@ -201,41 +209,87 @@ ${description}
> >          target_doc.append(doc_template.substitute(info))
> >          target_def.append(def_template.substitute(info))
> >
> > -        ctrls_map.append('\t{ ' + id_name + ', &' + ctrl.q_name + ' },')
> > +        target_ctrls_map.append('\t{ ' + id_name + ', &' + ctrl.q_name + ' },')
> > +
> > +    vendor_ctrl_doc_sub = []
> > +    vendor_ctrl_template = string.Template('''
> > +namespace ${vendor} {
> > +
> > +${vendor_controls_str}
> > +
> > +} /* namespace ${vendor} */''')
> > +
> > +    for vendor in [v for v in ctrls_map.keys() if v not in ['libcamera', 'draft']]:
> > +        vendor_ctrl_doc_sub.append(vendor_ctrl_template.substitute({'vendor': vendor, 'vendor_controls_str': '\n\n'.join(ctrls_doc[vendor])}))
> > +
> > +    vendor_ctrl_def_sub = []
> > +    for vendor in [v for v in ctrls_map.keys() if v not in ['libcamera', 'draft']]:
> > +        vendor_ctrl_def_sub.append(vendor_ctrl_template.substitute({'vendor': vendor, 'vendor_controls_str': '\n'.join(ctrls_def[vendor])}))
> > +
> > +    vendor_ctrl_map_sub = []
> > +    vendor_ctrl_template = string.Template('''namespace ${vendor} {
> > +/**
> > + * \\brief List of all supported ${vendor} vendor controls
> > + *
> > + * Unless otherwise stated, all controls are bi-directional, i.e. they can be
> > + * set through Request::controls() and returned out through Request::metadata().
> > + */
> > +extern const ControlIdMap controls {
> > +${vendor_controls_map}
> > +};
> > +
> > +} /* namespace ${vendor} */
> > +''')
> > +
> > +    for vendor in [v for v in ctrls_map.keys() if v not in ['libcamera', 'draft']]:
> > +        vendor_ctrl_map_sub.append(vendor_ctrl_template.substitute({'vendor': vendor,
> > +                                                                    'vendor_controls_map': '\n'.join(ctrls_map[vendor])}))
> >
> >      return {
> > -        'controls_doc': '\n\n'.join(ctrls_doc),
> > -        'controls_def': '\n'.join(ctrls_def),
> > -        'draft_controls_doc': '\n\n'.join(draft_ctrls_doc),
> > -        'draft_controls_def': '\n\n'.join(draft_ctrls_def),
> > -        'controls_map': '\n'.join(ctrls_map),
> > +        'controls_doc': '\n\n'.join(ctrls_doc['libcamera']),
> > +        'controls_def': '\n'.join(ctrls_def['libcamera']),
> > +        'draft_controls_doc': '\n\n'.join(ctrls_doc['draft']),
> > +        'draft_controls_def': '\n\n'.join(ctrls_def['draft']),
> > +        'controls_map': '\n'.join(ctrls_map['libcamera'] + ctrls_map['draft']),
> > +        'vendor_controls_doc': '\n'.join(vendor_ctrl_doc_sub),
> > +        'vendor_controls_def': '\n'.join(vendor_ctrl_def_sub),
> > +        'vendor_controls_map': '\n'.join(vendor_ctrl_map_sub),
> >      }
> >
> >
> > -def generate_h(controls):
> > +def generate_h(controls, mode):
> >      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;''')
> >      template = string.Template('''extern const Control<${type}> ${name};''')
> >
> > -    ctrls = []
> > -    draft_ctrls = []
> > -    ids = []
> > -    id_value = 1
> > +    ctrls = {}
> > +    ids = {}
> > +    id_value = {}
> >
> >      for ctrl in controls:
> >          id_name = snake_case(ctrl.name).upper()
> >
> > -        ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
> > +        vendor = 'draft' if ctrl.is_draft else ctrl.vendor
> > +        if vendor not in ctrls:
> > +            ids[vendor] = []
> > +            id_value[vendor] = 1
> > +            ctrls[vendor] = []
> > +
> > +        # Core and draft controls use the same ID value
> > +        target_ids = ids['libcamera'] if vendor in ['libcamera', 'draft'] else ids[vendor]
> > +        target_ids.append('\t' + id_name + ' = ' + str(id_value[vendor]) + ',')
> >
> >          info = {
> >              'name': ctrl.name,
> >              'type': ctrl.type,
> >          }
> >
> > -        target_ctrls = ctrls
> > +        target_ctrls = ctrls['libcamera']
> >          if ctrl.is_draft:
> > -            target_ctrls = draft_ctrls
> > +            target_ctrls = ctrls['draft']
> > +        elif vendor != 'libcamera':
> > +            target_ctrls = ctrls[vendor]
> >
> >          if ctrl.is_enum:
> >              target_ctrls.append(enum_template_start.substitute(info))
> > @@ -257,12 +311,37 @@ def generate_h(controls):
> >              target_ctrls.append(enum_values_template.substitute(values_info))
> >
> >          target_ctrls.append(template.substitute(info))
> > -        id_value += 1
> > +        id_value[vendor] += 1
> > +
> > +    vendor_template = string.Template('''
> > +namespace ${vendor} {
> > +
> > +#define LIBCAMERA_HAS_${vendor_def}_VENDOR_${mode}
> > +
> > +enum {
> > +${vendor_enums}
> > +};
> > +
> > +extern const ControlIdMap controls;
> > +
> > +${vendor_controls}
> > +
> > +} /* namespace ${vendor} */
> > +''')
> > +
> > +    vendor_sub = []
> > +    for vendor in [v for v in ctrls.keys() if v not in ['libcamera', 'draft']]:
> > +        vendor_sub.append(vendor_template.substitute({'mode': mode.upper(),
> > +                                                      'vendor': vendor,
> > +                                                      'vendor_def': vendor.upper(),
> > +                                                      'vendor_enums': '\n'.join(ids[vendor]),
> > +                                                      'vendor_controls': '\n'.join(ctrls[vendor])}))
> >
> >      return {
> > -        'ids': '\n'.join(ids),
> > -        'controls': '\n'.join(ctrls),
> > -        'draft_controls': '\n'.join(draft_ctrls)
> > +        'ids': '\n'.join(ids['libcamera']),
> > +        'controls': '\n'.join(ctrls['libcamera']),
> > +        'draft_controls': '\n'.join(ctrls['draft']),
> > +        'vendor_controls': '\n'.join(vendor_sub)
> >      }
> >
> >
> > @@ -284,16 +363,19 @@ def main(argv):
> >                          help='Input file name.')
> >      parser.add_argument('template', type=str,
> >                          help='Template file name.')
> > +    parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],
> > +                        help='Mode of operation')
>
> Let's keep arguments sorted alphabetically, this should go before the
> '-o' option. I would also name if '--mode', '-m' to have a short
> argument, and change '-o' to '--output', '-o', for consistency.
>
> >      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()) for ctrl in controls]
> > +    controls = [Control(*ctrl.popitem(), vendor) for ctrl in controls]
> >
> >      if args.template.endswith('.cpp.in'):
> >          data = generate_cpp(controls)
> >      elif args.template.endswith('.h.in'):
> > -        data = generate_h(controls)
> > +        data = generate_h(controls, args.mode)
> >      else:
> >          raise RuntimeError('Unknown template type')
> >
>
> --
> Regards,
>
> Laurent Pinchart


More information about the libcamera-devel mailing list