| Message ID | 20231124123713.22519-4-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote: > Add support for using separate YAML files for controls and properties > generation. The mapping of vendor/pipeline handler to control file is > done through the controls_map variable in include/libcamera/meson.build. > > This simplifies management of vendor control definitions and avoids > possible merge conflicts when changing the control_ids.yaml file for > core and draft controls. With this change, libcamera and draft controls > and properties files are designated the 'libcamera' vendor tag. > > In this change, we also rename control_ids.yaml -> control_ids_core.yaml > and property_ids.yaml -> property_ids_core.yaml to designate these as > core libcamera controls. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > Documentation/guides/pipeline-handler.rst | 8 +-- > include/libcamera/meson.build | 52 +++++++++++++++---- > meson.build | 2 + > ...control_ids.yaml => control_ids_core.yaml} | 0 > src/libcamera/meson.build | 21 ++++++-- > ...operty_ids.yaml => property_ids_core.yaml} | 0 > src/py/libcamera/gen-py-controls.py | 10 ++-- > src/py/libcamera/meson.build | 12 ++++- > utils/gen-controls.py | 13 +++-- > 9 files changed, 88 insertions(+), 30 deletions(-) > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%) > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%) > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > index 10b9c75c2a7f..66d428a19c4f 100644 > --- a/Documentation/guides/pipeline-handler.rst > +++ b/Documentation/guides/pipeline-handler.rst > @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device. > > The libcamera controls and properties are defined in YAML form which is > processed to automatically generate documentation and interfaces. Controls are > -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties > -are defined by src/libcamera/`properties_ids.yaml`_. > +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties > +are defined by src/libcamera/`properties_ids_core.yaml`_. > > .. _controls framework: https://libcamera.org/api-html/controls_8h.html > -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html > -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html > +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html > +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html > > Pipeline handlers can optionally register the list of controls an application > can set as well as a list of immutable camera properties. Being both > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 5fb772e6dd14..a763c8ff4344 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers, > > libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir > > -# control_ids.h and property_ids.h and associated modes > -control_source_files = { > - 'control_ids': 'controls', > - 'property_ids': 'properties', > +controls_map = { > + 'controls': { > + 'core': 'control_ids_core.yaml', > + }, > + > + 'properties': { > + 'core': 'property_ids_core.yaml', > + } > } > > control_headers = [] > +controls_files = [] > +properties_files = [] > + > +foreach mode, entry : controls_map > + files_list = [] > + input_files = [] > + foreach vendor, header : entry > + if vendor != 'core' and vendor != 'draft' > + if vendor not in pipelines > + message('vendor not in pipelines') This gives Message: vendor not in pipelines Message: vendor not in pipelines in the meson output log, that's not really useful imo > + continue > + endif > + endif > + > + if header in files_list > + message('header in files list') > + continue > + endif > + > + files_list += header > + input_files += files('../../src/libcamera/' + header) > + endforeach > + > + outfile = '' > + if mode == 'controls' > + outfile = 'control_ids.h' > + controls_files += files_list > + else > + outfile = 'property_ids.h' > + properties_files += files_list > + endif > > -foreach header, mode : control_source_files > - input_files = files('../../src/libcamera/' + header +'.yaml') > - template_file = files(header + '.h.in') > + template_file = files(outfile + '.in') > control_headers += custom_target(header + '_h', > input : input_files, > - output : header + '.h', > + output : outfile, > command : [gen_controls, '-o', '@OUTPUT@', > - '--mode', mode, '-t', template_file, '@INPUT@'], > + '--mode', mode, '-t', template_file, > + '@INPUT@'], > install : true, > install_dir : libcamera_headers_install_dir) > endforeach > diff --git a/meson.build b/meson.build > index e9a1c7e360ce..ee57cb780149 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, > + 'Controls files': controls_files, > + 'Properties files': properties_files, Controls files : control_ids_draft.yaml control_ids_core.yaml Properties files : property_ids_draft.yaml property_ids_core.yaml This (not from this patch but with the whole series applied) is more useful indeed! > 'Hotplug support': libudev.found(), > 'Tracing support': tracing_enabled, > 'Android support': android_enabled, > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml > similarity index 100% > rename from src/libcamera/control_ids.yaml > rename to src/libcamera/control_ids_core.yaml > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 05ee38daf22b..6d9902e6ffd1 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -127,12 +127,23 @@ endif > > control_sources = [] > > -foreach source, mode : control_source_files > - input_files = files(source +'.yaml') > - template_file = files(source + '.cpp.in') > - control_sources += custom_target(source + '_cpp', > +controls_mode_files = { > + 'controls' : controls_files, > + 'properties' : properties_files, > +} > + > +foreach mode, input_files : controls_mode_files > + input_files = files(input_files) > + > + if mode == 'controls' > + template_file = files('control_ids.cpp.in') > + else > + template_file = files('property_ids.cpp.in') > + endif > + > + control_sources += custom_target(mode + '_cpp', > input : input_files, > - output : source + '.cpp', > + output : mode + '_ids.cpp', > command : [gen_controls, '-o', '@OUTPUT@', > '--mode', mode, '-t', template_file, '@INPUT@']) > endforeach > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml > similarity index 100% > rename from src/libcamera/property_ids.yaml > rename to src/libcamera/property_ids_core.yaml > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py > index cfcfd4d16acf..8ae8d5126e39 100755 > --- a/src/py/libcamera/gen-py-controls.py > +++ b/src/py/libcamera/gen-py-controls.py > @@ -95,7 +95,7 @@ def main(argv): > help='Output file name. Defaults to standard output if not specified.') > parser.add_argument('--template', '-t', type=str, required=True, > help='Template file name.') > - parser.add_argument('input', type=str, > + parser.add_argument('input', type=str, nargs='+', > help='Input file name.') > args = parser.parse_args(argv[1:]) > > @@ -103,11 +103,11 @@ def main(argv): > print(f'Invalid mode option "{args.mode}"', file=sys.stderr) > return -1 > > - data = open(args.input, 'rb').read() > - > controls = {} > - vendor = yaml.safe_load(data)['vendor'] > - controls[vendor] = yaml.safe_load(data)['controls'] > + for input in args.input: > + data = open(input, 'rb').read() > + 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/meson.build b/src/py/libcamera/meson.build > index 1c3ea1843ac0..31af63ec0dc6 100644 > --- a/src/py/libcamera/meson.build > +++ b/src/py/libcamera/meson.build > @@ -28,11 +28,15 @@ pycamera_sources = files([ > > # Generate controls > > -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml') > +gen_py_controls_input_files = [] > gen_py_controls_template = files('py_controls_generated.cpp.in') > > gen_py_controls = files('gen-py-controls.py') > > +foreach file : controls_files > + gen_py_controls_input_files += files('../../libcamera/' + file) > +endforeach > + > pycamera_sources += custom_target('py_gen_controls', > input : gen_py_controls_input_files, > output : ['py_controls_generated.cpp'], > @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls', > > # Generate properties > > -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml') > +gen_py_property_enums_input_files = [] > gen_py_properties_template = files('py_properties_generated.cpp.in') > > +foreach file : properties_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'], > diff --git a/utils/gen-controls.py b/utils/gen-controls.py > index 6680ecf84acb..30c58f35473c 100755 > --- a/utils/gen-controls.py > +++ b/utils/gen-controls.py > @@ -12,6 +12,7 @@ import operator > import string > import sys > import yaml > +import os > > > class ControlEnum(object): > @@ -339,15 +340,17 @@ def main(argv): > help='Output file name. Defaults to standard output if not specified.') > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > help='Template file name.') > - parser.add_argument('input', type=str, > + parser.add_argument('input', type=str, nargs='+', > help='Input file name.') > > 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(), vendor) for ctrl in controls] > + controls = [] > + for input in args.input: > + data = open(input, 'rb').read() > + vendor = yaml.safe_load(data)['vendor'] > + ctrls = yaml.safe_load(data)['controls'] > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls] How does thi work in Python, should data be closed ? Anyway, this all looks good, with the above 'message' dropped Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > > if args.template.endswith('.cpp.in'): > data = generate_cpp(controls) > -- > 2.34.1 >
Hi Jacopo, Thank you for the feedback. On Mon, 27 Nov 2023 at 15:38, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Naush > > On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote: > > Add support for using separate YAML files for controls and properties > > generation. The mapping of vendor/pipeline handler to control file is > > done through the controls_map variable in include/libcamera/meson.build. > > > > This simplifies management of vendor control definitions and avoids > > possible merge conflicts when changing the control_ids.yaml file for > > core and draft controls. With this change, libcamera and draft controls > > and properties files are designated the 'libcamera' vendor tag. > > > > In this change, we also rename control_ids.yaml -> control_ids_core.yaml > > and property_ids.yaml -> property_ids_core.yaml to designate these as > > core libcamera controls. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > Documentation/guides/pipeline-handler.rst | 8 +-- > > include/libcamera/meson.build | 52 +++++++++++++++---- > > meson.build | 2 + > > ...control_ids.yaml => control_ids_core.yaml} | 0 > > src/libcamera/meson.build | 21 ++++++-- > > ...operty_ids.yaml => property_ids_core.yaml} | 0 > > src/py/libcamera/gen-py-controls.py | 10 ++-- > > src/py/libcamera/meson.build | 12 ++++- > > utils/gen-controls.py | 13 +++-- > > 9 files changed, 88 insertions(+), 30 deletions(-) > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%) > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%) > > > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > > index 10b9c75c2a7f..66d428a19c4f 100644 > > --- a/Documentation/guides/pipeline-handler.rst > > +++ b/Documentation/guides/pipeline-handler.rst > > @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device. > > > > The libcamera controls and properties are defined in YAML form which is > > processed to automatically generate documentation and interfaces. Controls are > > -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties > > -are defined by src/libcamera/`properties_ids.yaml`_. > > +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties > > +are defined by src/libcamera/`properties_ids_core.yaml`_. > > > > .. _controls framework: https://libcamera.org/api-html/controls_8h.html > > -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html > > -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html > > +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html > > +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html > > > > Pipeline handlers can optionally register the list of controls an application > > can set as well as a list of immutable camera properties. Being both > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > index 5fb772e6dd14..a763c8ff4344 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers, > > > > libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir > > > > -# control_ids.h and property_ids.h and associated modes > > -control_source_files = { > > - 'control_ids': 'controls', > > - 'property_ids': 'properties', > > +controls_map = { > > + 'controls': { > > + 'core': 'control_ids_core.yaml', > > + }, > > + > > + 'properties': { > > + 'core': 'property_ids_core.yaml', > > + } > > } > > > > control_headers = [] > > +controls_files = [] > > +properties_files = [] > > + > > +foreach mode, entry : controls_map > > + files_list = [] > > + input_files = [] > > + foreach vendor, header : entry > > + if vendor != 'core' and vendor != 'draft' > > + if vendor not in pipelines > > + message('vendor not in pipelines') > > This gives > > Message: vendor not in pipelines > Message: vendor not in pipelines > > in the meson output log, that's not really useful imo Indeed. This was my debug message and should really not be in there! > > > + continue > > + endif > > + endif > > + > > + if header in files_list > > + message('header in files list') > > + continue > > + endif > > + > > + files_list += header > > + input_files += files('../../src/libcamera/' + header) > > + endforeach > > + > > + outfile = '' > > + if mode == 'controls' > > + outfile = 'control_ids.h' > > + controls_files += files_list > > + else > > + outfile = 'property_ids.h' > > + properties_files += files_list > > + endif > > > > -foreach header, mode : control_source_files > > - input_files = files('../../src/libcamera/' + header +'.yaml') > > - template_file = files(header + '.h.in') > > + template_file = files(outfile + '.in') > > control_headers += custom_target(header + '_h', > > input : input_files, > > - output : header + '.h', > > + output : outfile, > > command : [gen_controls, '-o', '@OUTPUT@', > > - '--mode', mode, '-t', template_file, '@INPUT@'], > > + '--mode', mode, '-t', template_file, > > + '@INPUT@'], > > install : true, > > install_dir : libcamera_headers_install_dir) > > endforeach > > diff --git a/meson.build b/meson.build > > index e9a1c7e360ce..ee57cb780149 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, > > + 'Controls files': controls_files, > > + 'Properties files': properties_files, > > Controls files : control_ids_draft.yaml > control_ids_core.yaml > Properties files : property_ids_draft.yaml > property_ids_core.yaml > This (not from this patch but with the whole series applied) is more > useful indeed! > > > 'Hotplug support': libudev.found(), > > 'Tracing support': tracing_enabled, > > 'Android support': android_enabled, > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml > > similarity index 100% > > rename from src/libcamera/control_ids.yaml > > rename to src/libcamera/control_ids_core.yaml > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 05ee38daf22b..6d9902e6ffd1 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -127,12 +127,23 @@ endif > > > > control_sources = [] > > > > -foreach source, mode : control_source_files > > - input_files = files(source +'.yaml') > > - template_file = files(source + '.cpp.in') > > - control_sources += custom_target(source + '_cpp', > > +controls_mode_files = { > > + 'controls' : controls_files, > > + 'properties' : properties_files, > > +} > > + > > +foreach mode, input_files : controls_mode_files > > + input_files = files(input_files) > > + > > + if mode == 'controls' > > + template_file = files('control_ids.cpp.in') > > + else > > + template_file = files('property_ids.cpp.in') > > + endif > > + > > + control_sources += custom_target(mode + '_cpp', > > input : input_files, > > - output : source + '.cpp', > > + output : mode + '_ids.cpp', > > command : [gen_controls, '-o', '@OUTPUT@', > > '--mode', mode, '-t', template_file, '@INPUT@']) > > endforeach > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml > > similarity index 100% > > rename from src/libcamera/property_ids.yaml > > rename to src/libcamera/property_ids_core.yaml > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py > > index cfcfd4d16acf..8ae8d5126e39 100755 > > --- a/src/py/libcamera/gen-py-controls.py > > +++ b/src/py/libcamera/gen-py-controls.py > > @@ -95,7 +95,7 @@ def main(argv): > > help='Output file name. Defaults to standard output if not specified.') > > parser.add_argument('--template', '-t', type=str, required=True, > > help='Template file name.') > > - parser.add_argument('input', type=str, > > + parser.add_argument('input', type=str, nargs='+', > > help='Input file name.') > > args = parser.parse_args(argv[1:]) > > > > @@ -103,11 +103,11 @@ def main(argv): > > print(f'Invalid mode option "{args.mode}"', file=sys.stderr) > > return -1 > > > > - data = open(args.input, 'rb').read() > > - > > controls = {} > > - vendor = yaml.safe_load(data)['vendor'] > > - controls[vendor] = yaml.safe_load(data)['controls'] > > + for input in args.input: > > + data = open(input, 'rb').read() > > + 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/meson.build b/src/py/libcamera/meson.build > > index 1c3ea1843ac0..31af63ec0dc6 100644 > > --- a/src/py/libcamera/meson.build > > +++ b/src/py/libcamera/meson.build > > @@ -28,11 +28,15 @@ pycamera_sources = files([ > > > > # Generate controls > > > > -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml') > > +gen_py_controls_input_files = [] > > gen_py_controls_template = files('py_controls_generated.cpp.in') > > > > gen_py_controls = files('gen-py-controls.py') > > > > +foreach file : controls_files > > + gen_py_controls_input_files += files('../../libcamera/' + file) > > +endforeach > > + > > pycamera_sources += custom_target('py_gen_controls', > > input : gen_py_controls_input_files, > > output : ['py_controls_generated.cpp'], > > @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls', > > > > # Generate properties > > > > -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml') > > +gen_py_property_enums_input_files = [] > > gen_py_properties_template = files('py_properties_generated.cpp.in') > > > > +foreach file : properties_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'], > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py > > index 6680ecf84acb..30c58f35473c 100755 > > --- a/utils/gen-controls.py > > +++ b/utils/gen-controls.py > > @@ -12,6 +12,7 @@ import operator > > import string > > import sys > > import yaml > > +import os > > > > > > class ControlEnum(object): > > @@ -339,15 +340,17 @@ def main(argv): > > help='Output file name. Defaults to standard output if not specified.') > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > help='Template file name.') > > - parser.add_argument('input', type=str, > > + parser.add_argument('input', type=str, nargs='+', > > help='Input file name.') > > > > 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(), vendor) for ctrl in controls] > > + controls = [] > > + for input in args.input: > > + data = open(input, 'rb').read() > > + vendor = yaml.safe_load(data)['vendor'] > > + ctrls = yaml.safe_load(data)['controls'] > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls] > > How does thi work in Python, should data be closed ? I think it does get closed since we don't hold onto a reference to the open() return value. But please correct me if that is wrong since I was (blindly) following what the existing code was doing. Regards, Naush > > Anyway, this all looks good, with the above 'message' dropped > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Thanks > j > > > > > if args.template.endswith('.cpp.in'): > > data = generate_cpp(controls) > > -- > > 2.34.1 > >
Quoting Naushir Patuck via libcamera-devel (2023-11-27 15:45:35) > Hi Jacopo, > > Thank you for the feedback. > > On Mon, 27 Nov 2023 at 15:38, Jacopo Mondi > <jacopo.mondi@ideasonboard.com> wrote: > > > > Hi Naush > > > > On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote: > > > Add support for using separate YAML files for controls and properties > > > generation. The mapping of vendor/pipeline handler to control file is > > > done through the controls_map variable in include/libcamera/meson.build. > > > > > > This simplifies management of vendor control definitions and avoids > > > possible merge conflicts when changing the control_ids.yaml file for > > > core and draft controls. With this change, libcamera and draft controls > > > and properties files are designated the 'libcamera' vendor tag. > > > > > > In this change, we also rename control_ids.yaml -> control_ids_core.yaml > > > and property_ids.yaml -> property_ids_core.yaml to designate these as > > > core libcamera controls. > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > Documentation/guides/pipeline-handler.rst | 8 +-- > > > include/libcamera/meson.build | 52 +++++++++++++++---- > > > meson.build | 2 + > > > ...control_ids.yaml => control_ids_core.yaml} | 0 > > > src/libcamera/meson.build | 21 ++++++-- > > > ...operty_ids.yaml => property_ids_core.yaml} | 0 > > > src/py/libcamera/gen-py-controls.py | 10 ++-- > > > src/py/libcamera/meson.build | 12 ++++- > > > utils/gen-controls.py | 13 +++-- > > > 9 files changed, 88 insertions(+), 30 deletions(-) > > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%) > > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%) > > > > > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > > > index 10b9c75c2a7f..66d428a19c4f 100644 > > > --- a/Documentation/guides/pipeline-handler.rst > > > +++ b/Documentation/guides/pipeline-handler.rst > > > @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device. > > > > > > The libcamera controls and properties are defined in YAML form which is > > > processed to automatically generate documentation and interfaces. Controls are > > > -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties > > > -are defined by src/libcamera/`properties_ids.yaml`_. > > > +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties > > > +are defined by src/libcamera/`properties_ids_core.yaml`_. > > > > > > .. _controls framework: https://libcamera.org/api-html/controls_8h.html > > > -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html > > > -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html > > > +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html > > > +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html > > > > > > Pipeline handlers can optionally register the list of controls an application > > > can set as well as a list of immutable camera properties. Being both > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > > index 5fb772e6dd14..a763c8ff4344 100644 > > > --- a/include/libcamera/meson.build > > > +++ b/include/libcamera/meson.build > > > @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers, > > > > > > libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir > > > > > > -# control_ids.h and property_ids.h and associated modes > > > -control_source_files = { > > > - 'control_ids': 'controls', > > > - 'property_ids': 'properties', > > > +controls_map = { > > > + 'controls': { > > > + 'core': 'control_ids_core.yaml', > > > + }, > > > + > > > + 'properties': { > > > + 'core': 'property_ids_core.yaml', > > > + } > > > } > > > > > > control_headers = [] > > > +controls_files = [] > > > +properties_files = [] > > > + > > > +foreach mode, entry : controls_map > > > + files_list = [] > > > + input_files = [] > > > + foreach vendor, header : entry > > > + if vendor != 'core' and vendor != 'draft' > > > + if vendor not in pipelines > > > + message('vendor not in pipelines') > > > > This gives > > > > Message: vendor not in pipelines > > Message: vendor not in pipelines > > > > in the meson output log, that's not really useful imo > > Indeed. This was my debug message and should really not be in there! > > > > > > + continue > > > + endif > > > + endif > > > + > > > + if header in files_list > > > + message('header in files list') > > > + continue > > > + endif > > > + > > > + files_list += header > > > + input_files += files('../../src/libcamera/' + header) > > > + endforeach > > > + > > > + outfile = '' > > > + if mode == 'controls' > > > + outfile = 'control_ids.h' > > > + controls_files += files_list > > > + else > > > + outfile = 'property_ids.h' > > > + properties_files += files_list > > > + endif > > > > > > -foreach header, mode : control_source_files > > > - input_files = files('../../src/libcamera/' + header +'.yaml') > > > - template_file = files(header + '.h.in') > > > + template_file = files(outfile + '.in') > > > control_headers += custom_target(header + '_h', > > > input : input_files, > > > - output : header + '.h', > > > + output : outfile, > > > command : [gen_controls, '-o', '@OUTPUT@', > > > - '--mode', mode, '-t', template_file, '@INPUT@'], > > > + '--mode', mode, '-t', template_file, > > > + '@INPUT@'], > > > install : true, > > > install_dir : libcamera_headers_install_dir) > > > endforeach > > > diff --git a/meson.build b/meson.build > > > index e9a1c7e360ce..ee57cb780149 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, > > > + 'Controls files': controls_files, > > > + 'Properties files': properties_files, > > > > Controls files : control_ids_draft.yaml > > control_ids_core.yaml > > Properties files : property_ids_draft.yaml > > property_ids_core.yaml > > This (not from this patch but with the whole series applied) is more > > useful indeed! > > > > > 'Hotplug support': libudev.found(), > > > 'Tracing support': tracing_enabled, > > > 'Android support': android_enabled, > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml > > > similarity index 100% > > > rename from src/libcamera/control_ids.yaml > > > rename to src/libcamera/control_ids_core.yaml > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > > index 05ee38daf22b..6d9902e6ffd1 100644 > > > --- a/src/libcamera/meson.build > > > +++ b/src/libcamera/meson.build > > > @@ -127,12 +127,23 @@ endif > > > > > > control_sources = [] > > > > > > -foreach source, mode : control_source_files > > > - input_files = files(source +'.yaml') > > > - template_file = files(source + '.cpp.in') > > > - control_sources += custom_target(source + '_cpp', > > > +controls_mode_files = { > > > + 'controls' : controls_files, > > > + 'properties' : properties_files, > > > +} > > > + > > > +foreach mode, input_files : controls_mode_files > > > + input_files = files(input_files) > > > + > > > + if mode == 'controls' > > > + template_file = files('control_ids.cpp.in') > > > + else > > > + template_file = files('property_ids.cpp.in') > > > + endif > > > + > > > + control_sources += custom_target(mode + '_cpp', > > > input : input_files, > > > - output : source + '.cpp', > > > + output : mode + '_ids.cpp', > > > command : [gen_controls, '-o', '@OUTPUT@', > > > '--mode', mode, '-t', template_file, '@INPUT@']) > > > endforeach > > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml > > > similarity index 100% > > > rename from src/libcamera/property_ids.yaml > > > rename to src/libcamera/property_ids_core.yaml > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py > > > index cfcfd4d16acf..8ae8d5126e39 100755 > > > --- a/src/py/libcamera/gen-py-controls.py > > > +++ b/src/py/libcamera/gen-py-controls.py > > > @@ -95,7 +95,7 @@ def main(argv): > > > help='Output file name. Defaults to standard output if not specified.') > > > parser.add_argument('--template', '-t', type=str, required=True, > > > help='Template file name.') > > > - parser.add_argument('input', type=str, > > > + parser.add_argument('input', type=str, nargs='+', > > > help='Input file name.') > > > args = parser.parse_args(argv[1:]) > > > > > > @@ -103,11 +103,11 @@ def main(argv): > > > print(f'Invalid mode option "{args.mode}"', file=sys.stderr) > > > return -1 > > > > > > - data = open(args.input, 'rb').read() > > > - > > > controls = {} > > > - vendor = yaml.safe_load(data)['vendor'] > > > - controls[vendor] = yaml.safe_load(data)['controls'] > > > + for input in args.input: > > > + data = open(input, 'rb').read() > > > + 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/meson.build b/src/py/libcamera/meson.build > > > index 1c3ea1843ac0..31af63ec0dc6 100644 > > > --- a/src/py/libcamera/meson.build > > > +++ b/src/py/libcamera/meson.build > > > @@ -28,11 +28,15 @@ pycamera_sources = files([ > > > > > > # Generate controls > > > > > > -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml') > > > +gen_py_controls_input_files = [] > > > gen_py_controls_template = files('py_controls_generated.cpp.in') > > > > > > gen_py_controls = files('gen-py-controls.py') > > > > > > +foreach file : controls_files > > > + gen_py_controls_input_files += files('../../libcamera/' + file) > > > +endforeach > > > + > > > pycamera_sources += custom_target('py_gen_controls', > > > input : gen_py_controls_input_files, > > > output : ['py_controls_generated.cpp'], > > > @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls', > > > > > > # Generate properties > > > > > > -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml') > > > +gen_py_property_enums_input_files = [] > > > gen_py_properties_template = files('py_properties_generated.cpp.in') > > > > > > +foreach file : properties_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'], > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py > > > index 6680ecf84acb..30c58f35473c 100755 > > > --- a/utils/gen-controls.py > > > +++ b/utils/gen-controls.py > > > @@ -12,6 +12,7 @@ import operator > > > import string > > > import sys > > > import yaml > > > +import os > > > > > > > > > class ControlEnum(object): > > > @@ -339,15 +340,17 @@ def main(argv): > > > help='Output file name. Defaults to standard output if not specified.') > > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > > help='Template file name.') > > > - parser.add_argument('input', type=str, > > > + parser.add_argument('input', type=str, nargs='+', > > > help='Input file name.') > > > > > > 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(), vendor) for ctrl in controls] > > > + controls = [] > > > + for input in args.input: > > > + data = open(input, 'rb').read() > > > + vendor = yaml.safe_load(data)['vendor'] > > > + ctrls = yaml.safe_load(data)['controls'] > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls] > > > > How does thi work in Python, should data be closed ? > > I think it does get closed since we don't hold onto a reference to the > open() return value. But please correct me if that is wrong since I > was (blindly) following what the existing code was doing. > Not a python expert enough here, but I know I often use " with open(input, 'rb') as file: data = file.read etc ... which automatically closes the file at the end of that scope. With the style above - there is no handle to the opened file - so we don't have anything to call close on ... so it's a bit of refactoring either way - unless we can be sure python closes automatically. But ... we're certainly going to close when the file exits... With that resolved in a suitable way (which could just be confirming that it autocloses if that works) and the debug message removed: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Regards, > Naush > > > > > Anyway, this all looks good, with the above 'message' dropped > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > Thanks > > j > > > > > > > > if args.template.endswith('.cpp.in'): > > > data = generate_cpp(controls) > > > -- > > > 2.34.1 > > >
On Tue, Nov 28, 2023 at 03:09:10PM +0000, Kieran Bingham via libcamera-devel wrote: > Quoting Naushir Patuck via libcamera-devel (2023-11-27 15:45:35) > > Hi Jacopo, > > > > Thank you for the feedback. > > > > On Mon, 27 Nov 2023 at 15:38, Jacopo Mondi > > <jacopo.mondi@ideasonboard.com> wrote: > > > > > > Hi Naush > > > > > > On Fri, Nov 24, 2023 at 12:37:09PM +0000, Naushir Patuck via libcamera-devel wrote: > > > > Add support for using separate YAML files for controls and properties > > > > generation. The mapping of vendor/pipeline handler to control file is > > > > done through the controls_map variable in include/libcamera/meson.build. > > > > > > > > This simplifies management of vendor control definitions and avoids > > > > possible merge conflicts when changing the control_ids.yaml file for > > > > core and draft controls. With this change, libcamera and draft controls > > > > and properties files are designated the 'libcamera' vendor tag. > > > > > > > > In this change, we also rename control_ids.yaml -> control_ids_core.yaml > > > > and property_ids.yaml -> property_ids_core.yaml to designate these as > > > > core libcamera controls. > > > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > > --- > > > > Documentation/guides/pipeline-handler.rst | 8 +-- > > > > include/libcamera/meson.build | 52 +++++++++++++++---- > > > > meson.build | 2 + > > > > ...control_ids.yaml => control_ids_core.yaml} | 0 > > > > src/libcamera/meson.build | 21 ++++++-- > > > > ...operty_ids.yaml => property_ids_core.yaml} | 0 > > > > src/py/libcamera/gen-py-controls.py | 10 ++-- > > > > src/py/libcamera/meson.build | 12 ++++- > > > > utils/gen-controls.py | 13 +++-- > > > > 9 files changed, 88 insertions(+), 30 deletions(-) > > > > rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%) > > > > rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%) > > > > > > > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst > > > > index 10b9c75c2a7f..66d428a19c4f 100644 > > > > --- a/Documentation/guides/pipeline-handler.rst > > > > +++ b/Documentation/guides/pipeline-handler.rst > > > > @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device. > > > > > > > > The libcamera controls and properties are defined in YAML form which is > > > > processed to automatically generate documentation and interfaces. Controls are > > > > -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties > > > > -are defined by src/libcamera/`properties_ids.yaml`_. > > > > +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties > > > > +are defined by src/libcamera/`properties_ids_core.yaml`_. > > > > > > > > .. _controls framework: https://libcamera.org/api-html/controls_8h.html > > > > -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html > > > > -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html > > > > +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html > > > > +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html > > > > > > > > Pipeline handlers can optionally register the list of controls an application > > > > can set as well as a list of immutable camera properties. Being both > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > > > index 5fb772e6dd14..a763c8ff4344 100644 > > > > --- a/include/libcamera/meson.build > > > > +++ b/include/libcamera/meson.build > > > > @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers, > > > > > > > > libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir > > > > > > > > -# control_ids.h and property_ids.h and associated modes > > > > -control_source_files = { > > > > - 'control_ids': 'controls', > > > > - 'property_ids': 'properties', > > > > +controls_map = { > > > > + 'controls': { > > > > + 'core': 'control_ids_core.yaml', > > > > + }, > > > > + > > > > + 'properties': { > > > > + 'core': 'property_ids_core.yaml', > > > > + } > > > > } > > > > > > > > control_headers = [] > > > > +controls_files = [] > > > > +properties_files = [] > > > > + > > > > +foreach mode, entry : controls_map > > > > + files_list = [] > > > > + input_files = [] > > > > + foreach vendor, header : entry > > > > + if vendor != 'core' and vendor != 'draft' > > > > + if vendor not in pipelines > > > > + message('vendor not in pipelines') > > > > > > This gives > > > > > > Message: vendor not in pipelines > > > Message: vendor not in pipelines > > > > > > in the meson output log, that's not really useful imo > > > > Indeed. This was my debug message and should really not be in there! > > > > > > > > > + continue > > > > + endif > > > > + endif > > > > + > > > > + if header in files_list > > > > + message('header in files list') > > > > + continue > > > > + endif > > > > + > > > > + files_list += header > > > > + input_files += files('../../src/libcamera/' + header) > > > > + endforeach > > > > + > > > > + outfile = '' > > > > + if mode == 'controls' > > > > + outfile = 'control_ids.h' > > > > + controls_files += files_list > > > > + else > > > > + outfile = 'property_ids.h' > > > > + properties_files += files_list > > > > + endif > > > > > > > > -foreach header, mode : control_source_files > > > > - input_files = files('../../src/libcamera/' + header +'.yaml') > > > > - template_file = files(header + '.h.in') > > > > + template_file = files(outfile + '.in') > > > > control_headers += custom_target(header + '_h', > > > > input : input_files, > > > > - output : header + '.h', > > > > + output : outfile, > > > > command : [gen_controls, '-o', '@OUTPUT@', > > > > - '--mode', mode, '-t', template_file, '@INPUT@'], > > > > + '--mode', mode, '-t', template_file, > > > > + '@INPUT@'], > > > > install : true, > > > > install_dir : libcamera_headers_install_dir) > > > > endforeach > > > > diff --git a/meson.build b/meson.build > > > > index e9a1c7e360ce..ee57cb780149 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, > > > > + 'Controls files': controls_files, > > > > + 'Properties files': properties_files, > > > > > > Controls files : control_ids_draft.yaml > > > control_ids_core.yaml > > > Properties files : property_ids_draft.yaml > > > property_ids_core.yaml > > > This (not from this patch but with the whole series applied) is more > > > useful indeed! > > > > > > > 'Hotplug support': libudev.found(), > > > > 'Tracing support': tracing_enabled, > > > > 'Android support': android_enabled, > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml > > > > similarity index 100% > > > > rename from src/libcamera/control_ids.yaml > > > > rename to src/libcamera/control_ids_core.yaml > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > > > index 05ee38daf22b..6d9902e6ffd1 100644 > > > > --- a/src/libcamera/meson.build > > > > +++ b/src/libcamera/meson.build > > > > @@ -127,12 +127,23 @@ endif > > > > > > > > control_sources = [] > > > > > > > > -foreach source, mode : control_source_files > > > > - input_files = files(source +'.yaml') > > > > - template_file = files(source + '.cpp.in') > > > > - control_sources += custom_target(source + '_cpp', > > > > +controls_mode_files = { > > > > + 'controls' : controls_files, > > > > + 'properties' : properties_files, > > > > +} > > > > + > > > > +foreach mode, input_files : controls_mode_files > > > > + input_files = files(input_files) > > > > + > > > > + if mode == 'controls' > > > > + template_file = files('control_ids.cpp.in') > > > > + else > > > > + template_file = files('property_ids.cpp.in') > > > > + endif > > > > + > > > > + control_sources += custom_target(mode + '_cpp', > > > > input : input_files, > > > > - output : source + '.cpp', > > > > + output : mode + '_ids.cpp', > > > > command : [gen_controls, '-o', '@OUTPUT@', > > > > '--mode', mode, '-t', template_file, '@INPUT@']) > > > > endforeach > > > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml > > > > similarity index 100% > > > > rename from src/libcamera/property_ids.yaml > > > > rename to src/libcamera/property_ids_core.yaml > > > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py > > > > index cfcfd4d16acf..8ae8d5126e39 100755 > > > > --- a/src/py/libcamera/gen-py-controls.py > > > > +++ b/src/py/libcamera/gen-py-controls.py > > > > @@ -95,7 +95,7 @@ def main(argv): > > > > help='Output file name. Defaults to standard output if not specified.') > > > > parser.add_argument('--template', '-t', type=str, required=True, > > > > help='Template file name.') > > > > - parser.add_argument('input', type=str, > > > > + parser.add_argument('input', type=str, nargs='+', > > > > help='Input file name.') > > > > args = parser.parse_args(argv[1:]) > > > > > > > > @@ -103,11 +103,11 @@ def main(argv): > > > > print(f'Invalid mode option "{args.mode}"', file=sys.stderr) > > > > return -1 > > > > > > > > - data = open(args.input, 'rb').read() > > > > - > > > > controls = {} > > > > - vendor = yaml.safe_load(data)['vendor'] > > > > - controls[vendor] = yaml.safe_load(data)['controls'] > > > > + for input in args.input: > > > > + data = open(input, 'rb').read() > > > > + 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/meson.build b/src/py/libcamera/meson.build > > > > index 1c3ea1843ac0..31af63ec0dc6 100644 > > > > --- a/src/py/libcamera/meson.build > > > > +++ b/src/py/libcamera/meson.build > > > > @@ -28,11 +28,15 @@ pycamera_sources = files([ > > > > > > > > # Generate controls > > > > > > > > -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml') > > > > +gen_py_controls_input_files = [] > > > > gen_py_controls_template = files('py_controls_generated.cpp.in') > > > > > > > > gen_py_controls = files('gen-py-controls.py') > > > > > > > > +foreach file : controls_files > > > > + gen_py_controls_input_files += files('../../libcamera/' + file) > > > > +endforeach > > > > + > > > > pycamera_sources += custom_target('py_gen_controls', > > > > input : gen_py_controls_input_files, > > > > output : ['py_controls_generated.cpp'], > > > > @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls', > > > > > > > > # Generate properties > > > > > > > > -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml') > > > > +gen_py_property_enums_input_files = [] > > > > gen_py_properties_template = files('py_properties_generated.cpp.in') > > > > > > > > +foreach file : properties_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'], > > > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py > > > > index 6680ecf84acb..30c58f35473c 100755 > > > > --- a/utils/gen-controls.py > > > > +++ b/utils/gen-controls.py > > > > @@ -12,6 +12,7 @@ import operator > > > > import string > > > > import sys > > > > import yaml > > > > +import os > > > > > > > > > > > > class ControlEnum(object): > > > > @@ -339,15 +340,17 @@ def main(argv): > > > > help='Output file name. Defaults to standard output if not specified.') > > > > parser.add_argument('--template', '-t', dest='template', type=str, required=True, > > > > help='Template file name.') > > > > - parser.add_argument('input', type=str, > > > > + parser.add_argument('input', type=str, nargs='+', > > > > help='Input file name.') > > > > > > > > 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(), vendor) for ctrl in controls] > > > > + controls = [] > > > > + for input in args.input: > > > > + data = open(input, 'rb').read() > > > > + vendor = yaml.safe_load(data)['vendor'] > > > > + ctrls = yaml.safe_load(data)['controls'] > > > > + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls] > > > > > > How does thi work in Python, should data be closed ? > > > > I think it does get closed since we don't hold onto a reference to the > > open() return value. But please correct me if that is wrong since I > > was (blindly) following what the existing code was doing. > > > > Not a python expert enough here, but I know I often use " > with open(input, 'rb') as file: > data = file.read > > etc ... which automatically closes the file at the end of that scope. That's the most pythonic way to handle scoping for a file object indeed, when explicit management of the scope is needed (i.e. when you need to know exactly when the file will be closed). > With the style above - there is no handle to the opened file - so we > don't have anything to call close on ... so it's a bit of refactoring > either way - unless we can be sure python closes automatically. Python will close the file. The reference count of object returned by open() reaches 0 when the read() function returns, so the file will get closed immediately as far as I understand: >>> class Foo(object): ... def __init__(self): ... print('__init__') ... def __del__(self): ... print('__del__') ... >>> f = Foo() __init__ >>> f = None ; print('deleted') __del__ deleted > But ... we're certainly going to close when the file exits... And there are few files anyway, so even if the objects stayed around until a garbage collection pass it wouldn't matter that much. > With that resolved in a suitable way (which could just be confirming > that it autocloses if that works) and the debug message removed: > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > Anyway, this all looks good, with the above 'message' dropped > > > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > > > > > > if args.template.endswith('.cpp.in'): > > > > data = generate_cpp(controls)
diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 10b9c75c2a7f..66d428a19c4f 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -587,12 +587,12 @@ immutable properties of the ``Camera`` device. The libcamera controls and properties are defined in YAML form which is processed to automatically generate documentation and interfaces. Controls are -defined by the src/libcamera/`control_ids.yaml`_ file and camera properties -are defined by src/libcamera/`properties_ids.yaml`_. +defined by the src/libcamera/`control_ids_core.yaml`_ file and camera properties +are defined by src/libcamera/`properties_ids_core.yaml`_. .. _controls framework: https://libcamera.org/api-html/controls_8h.html -.. _control_ids.yaml: https://libcamera.org/api-html/control__ids_8h.html -.. _properties_ids.yaml: https://libcamera.org/api-html/property__ids_8h.html +.. _control_ids_core.yaml: https://libcamera.org/api-html/control__ids_8h.html +.. _properties_ids_core.yaml: https://libcamera.org/api-html/property__ids_8h.html Pipeline handlers can optionally register the list of controls an application can set as well as a list of immutable camera properties. Being both diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 5fb772e6dd14..a763c8ff4344 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -32,22 +32,56 @@ install_headers(libcamera_public_headers, libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir -# control_ids.h and property_ids.h and associated modes -control_source_files = { - 'control_ids': 'controls', - 'property_ids': 'properties', +controls_map = { + 'controls': { + 'core': 'control_ids_core.yaml', + }, + + 'properties': { + 'core': 'property_ids_core.yaml', + } } control_headers = [] +controls_files = [] +properties_files = [] + +foreach mode, entry : controls_map + files_list = [] + input_files = [] + foreach vendor, header : entry + if vendor != 'core' and vendor != 'draft' + if vendor not in pipelines + message('vendor not in pipelines') + continue + endif + endif + + if header in files_list + message('header in files list') + continue + endif + + files_list += header + input_files += files('../../src/libcamera/' + header) + endforeach + + outfile = '' + if mode == 'controls' + outfile = 'control_ids.h' + controls_files += files_list + else + outfile = 'property_ids.h' + properties_files += files_list + endif -foreach header, mode : control_source_files - input_files = files('../../src/libcamera/' + header +'.yaml') - template_file = files(header + '.h.in') + template_file = files(outfile + '.in') control_headers += custom_target(header + '_h', input : input_files, - output : header + '.h', + output : outfile, command : [gen_controls, '-o', '@OUTPUT@', - '--mode', mode, '-t', template_file, '@INPUT@'], + '--mode', mode, '-t', template_file, + '@INPUT@'], install : true, install_dir : libcamera_headers_install_dir) endforeach diff --git a/meson.build b/meson.build index e9a1c7e360ce..ee57cb780149 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, + 'Controls files': controls_files, + 'Properties files': properties_files, 'Hotplug support': libudev.found(), 'Tracing support': tracing_enabled, 'Android support': android_enabled, diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids_core.yaml similarity index 100% rename from src/libcamera/control_ids.yaml rename to src/libcamera/control_ids_core.yaml diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 05ee38daf22b..6d9902e6ffd1 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -127,12 +127,23 @@ endif control_sources = [] -foreach source, mode : control_source_files - input_files = files(source +'.yaml') - template_file = files(source + '.cpp.in') - control_sources += custom_target(source + '_cpp', +controls_mode_files = { + 'controls' : controls_files, + 'properties' : properties_files, +} + +foreach mode, input_files : controls_mode_files + input_files = files(input_files) + + if mode == 'controls' + template_file = files('control_ids.cpp.in') + else + template_file = files('property_ids.cpp.in') + endif + + control_sources += custom_target(mode + '_cpp', input : input_files, - output : source + '.cpp', + output : mode + '_ids.cpp', command : [gen_controls, '-o', '@OUTPUT@', '--mode', mode, '-t', template_file, '@INPUT@']) endforeach diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids_core.yaml similarity index 100% rename from src/libcamera/property_ids.yaml rename to src/libcamera/property_ids_core.yaml diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py index cfcfd4d16acf..8ae8d5126e39 100755 --- a/src/py/libcamera/gen-py-controls.py +++ b/src/py/libcamera/gen-py-controls.py @@ -95,7 +95,7 @@ def main(argv): help='Output file name. Defaults to standard output if not specified.') parser.add_argument('--template', '-t', type=str, required=True, help='Template file name.') - parser.add_argument('input', type=str, + parser.add_argument('input', type=str, nargs='+', help='Input file name.') args = parser.parse_args(argv[1:]) @@ -103,11 +103,11 @@ def main(argv): print(f'Invalid mode option "{args.mode}"', file=sys.stderr) return -1 - data = open(args.input, 'rb').read() - controls = {} - vendor = yaml.safe_load(data)['vendor'] - controls[vendor] = yaml.safe_load(data)['controls'] + for input in args.input: + data = open(input, 'rb').read() + 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/meson.build b/src/py/libcamera/meson.build index 1c3ea1843ac0..31af63ec0dc6 100644 --- a/src/py/libcamera/meson.build +++ b/src/py/libcamera/meson.build @@ -28,11 +28,15 @@ pycamera_sources = files([ # Generate controls -gen_py_controls_input_files = files('../../libcamera/control_ids.yaml') +gen_py_controls_input_files = [] gen_py_controls_template = files('py_controls_generated.cpp.in') gen_py_controls = files('gen-py-controls.py') +foreach file : controls_files + gen_py_controls_input_files += files('../../libcamera/' + file) +endforeach + pycamera_sources += custom_target('py_gen_controls', input : gen_py_controls_input_files, output : ['py_controls_generated.cpp'], @@ -41,9 +45,13 @@ pycamera_sources += custom_target('py_gen_controls', # Generate properties -gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml') +gen_py_property_enums_input_files = [] gen_py_properties_template = files('py_properties_generated.cpp.in') +foreach file : properties_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'], diff --git a/utils/gen-controls.py b/utils/gen-controls.py index 6680ecf84acb..30c58f35473c 100755 --- a/utils/gen-controls.py +++ b/utils/gen-controls.py @@ -12,6 +12,7 @@ import operator import string import sys import yaml +import os class ControlEnum(object): @@ -339,15 +340,17 @@ def main(argv): help='Output file name. Defaults to standard output if not specified.') parser.add_argument('--template', '-t', dest='template', type=str, required=True, help='Template file name.') - parser.add_argument('input', type=str, + parser.add_argument('input', type=str, nargs='+', help='Input file name.') 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(), vendor) for ctrl in controls] + controls = [] + for input in args.input: + data = open(input, 'rb').read() + vendor = yaml.safe_load(data)['vendor'] + ctrls = yaml.safe_load(data)['controls'] + controls = controls + [Control(*ctrl.popitem(), vendor) for ctrl in ctrls] if args.template.endswith('.cpp.in'): data = generate_cpp(controls)
Add support for using separate YAML files for controls and properties generation. The mapping of vendor/pipeline handler to control file is done through the controls_map variable in include/libcamera/meson.build. This simplifies management of vendor control definitions and avoids possible merge conflicts when changing the control_ids.yaml file for core and draft controls. With this change, libcamera and draft controls and properties files are designated the 'libcamera' vendor tag. In this change, we also rename control_ids.yaml -> control_ids_core.yaml and property_ids.yaml -> property_ids_core.yaml to designate these as core libcamera controls. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- Documentation/guides/pipeline-handler.rst | 8 +-- include/libcamera/meson.build | 52 +++++++++++++++---- meson.build | 2 + ...control_ids.yaml => control_ids_core.yaml} | 0 src/libcamera/meson.build | 21 ++++++-- ...operty_ids.yaml => property_ids_core.yaml} | 0 src/py/libcamera/gen-py-controls.py | 10 ++-- src/py/libcamera/meson.build | 12 ++++- utils/gen-controls.py | 13 +++-- 9 files changed, 88 insertions(+), 30 deletions(-) rename src/libcamera/{control_ids.yaml => control_ids_core.yaml} (100%) rename src/libcamera/{property_ids.yaml => property_ids_core.yaml} (100%)