Message ID | 20201106103707.49660-25-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Paul, Thank you for the patch. On Fri, Nov 06, 2020 at 07:36:54PM +0900, Paul Elder wrote: > Add a base mojom file to contain empty mojom definitions of libcamera > objects, as well as documentation for the IPA interfaces that need to be > defined in the mojom files. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v4: > - move docs to IPA guide > > Changes in v3: > - add doc that structs need to be defined > - add doc to recommend namespacing > - change indentation > - add requirement that base controls *must* be defined in > libcamera::{pipeline_name}::Controls > > New in v2 > --- > include/libcamera/ipa/core.mojom | 18 ++++++++++++++++++ > include/libcamera/ipa/meson.build | 18 +++++++++++++++--- > src/libcamera/proxy/meson.build | 2 +- > src/libcamera/proxy/worker/meson.build | 2 +- > 4 files changed, 35 insertions(+), 5 deletions(-) > create mode 100644 include/libcamera/ipa/core.mojom > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom > new file mode 100644 > index 00000000..ed26aaad > --- /dev/null > +++ b/include/libcamera/ipa/core.mojom > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > + > +/* > + * Any libcamera objects that are used by any interfaces must be declared > + * here, and a de/serializer be implemented in IPADataSerializer. In addition, s/de\/serializer/(de)serializer/ ? s/in IPADataSerializer/as a template specialization of IPADataSerializer/ ? > + * the corresponding header file (or forward-declarations) must be placed in s/placed/included/ > + * {pipeline_name}.h. > + * > + * For libcamera types, the [hasFd] attribute is needed to notify the compiler > + * that the struct embeds a FileDescriptor. > + */ > +struct CameraSensorInfo {}; > +struct ControlInfoMap {}; > +struct ControlList {}; > +struct FileDescriptor {}; > +[hasFd] struct IPABuffer {}; > +struct IPASettings {}; > +struct IPAStream {}; I think the last three structures can be defined fully in core.mojom. The comment above will need to be updated, to reflect the fact that specializations of IPADataSerializer are only needed for opaque structures, not structures defined in mojom. > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build > index d7a2b6b2..b7caec95 100644 > --- a/include/libcamera/ipa/meson.build > +++ b/include/libcamera/ipa/meson.build > @@ -13,6 +13,17 @@ install_headers(libcamera_ipa_headers, > # Prepare IPA/IPC generation components > # > > +core_mojom_file = 'core.mojom' > +ipa_mojom_core = custom_target(core_mojom_file.split('.')[0] + '_mojom_module', > + input : core_mojom_file, > + output : core_mojom_file + '-module', > + command : [ > + mojom_parser, > + '--output-root', meson.build_root(), > + '--input-root', meson.source_root(), > + '--mojoms', '@INPUT@' > + ]) It would be nice if the mojo parser was able to handle imports automatically :-S > + > ipa_mojom_files = [] > > ipa_mojoms = [] > @@ -30,6 +41,7 @@ foreach file : ipa_mojom_files > mojom = custom_target(file.split('.')[0] + '_mojom_module', > input : file, > output : file + '-module', > + depends : ipa_mojom_core, > command : [ > mojom_parser, > '--output-root', meson.build_root(), > @@ -41,7 +53,7 @@ foreach file : ipa_mojom_files > header = custom_target(name + '_ipa_interface_h', > input : mojom, > output : name + '_ipa_interface.h', > - depends : mojom_templates, > + depends : [mojom_templates, ipa_mojom_core], Do we need this, given that input is set to mojom, which depends on ipa_mojom_core already ? The dependency is for the parsing step, not the generation step. I think all the dependencies below can be dropped. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > command : [ > mojom_generator, 'generate', > '-g', 'libcamera', > @@ -55,7 +67,7 @@ foreach file : ipa_mojom_files > serializer = custom_target(name + '_ipa_serializer_h', > input : mojom, > output : name + '_ipa_serializer.h', > - depends : mojom_templates, > + depends : [mojom_templates, ipa_mojom_core], > command : [ > mojom_generator, 'generate', > '-g', 'libcamera', > @@ -69,7 +81,7 @@ foreach file : ipa_mojom_files > proxy_header = custom_target(name + '_proxy_h', > input : mojom, > output : 'ipa_proxy_' + name + '.h', > - depends : mojom_templates, > + depends : [mojom_templates, ipa_mojom_core], > command : [ > mojom_generator, 'generate', > '-g', 'libcamera', > diff --git a/src/libcamera/proxy/meson.build b/src/libcamera/proxy/meson.build > index ac68ad08..f27b453a 100644 > --- a/src/libcamera/proxy/meson.build > +++ b/src/libcamera/proxy/meson.build > @@ -10,7 +10,7 @@ foreach mojom : ipa_mojoms > proxy = custom_target(mojom['name'] + '_proxy_cpp', > input : mojom['mojom'], > output : 'ipa_proxy_' + mojom['name'] + '.cpp', > - depends : [mojom_templates], > + depends : [mojom_templates, ipa_mojom_core], > command : [ > mojom_generator, 'generate', > '-g', 'libcamera', > diff --git a/src/libcamera/proxy/worker/meson.build b/src/libcamera/proxy/worker/meson.build > index 8e0b978a..628bb050 100644 > --- a/src/libcamera/proxy/worker/meson.build > +++ b/src/libcamera/proxy/worker/meson.build > @@ -11,7 +11,7 @@ foreach mojom : ipa_mojoms > worker = custom_target(mojom['name'] + '_proxy_worker', > input : mojom['mojom'], > output : 'ipa_proxy_' + mojom['name'] + '_worker.cpp', > - depends : [mojom_templates], > + depends : [mojom_templates, ipa_mojom_core], > command : [ > mojom_generator, 'generate', > '-g', 'libcamera',
diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom new file mode 100644 index 00000000..ed26aaad --- /dev/null +++ b/include/libcamera/ipa/core.mojom @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +/* + * Any libcamera objects that are used by any interfaces must be declared + * here, and a de/serializer be implemented in IPADataSerializer. In addition, + * the corresponding header file (or forward-declarations) must be placed in + * {pipeline_name}.h. + * + * For libcamera types, the [hasFd] attribute is needed to notify the compiler + * that the struct embeds a FileDescriptor. + */ +struct CameraSensorInfo {}; +struct ControlInfoMap {}; +struct ControlList {}; +struct FileDescriptor {}; +[hasFd] struct IPABuffer {}; +struct IPASettings {}; +struct IPAStream {}; diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build index d7a2b6b2..b7caec95 100644 --- a/include/libcamera/ipa/meson.build +++ b/include/libcamera/ipa/meson.build @@ -13,6 +13,17 @@ install_headers(libcamera_ipa_headers, # Prepare IPA/IPC generation components # +core_mojom_file = 'core.mojom' +ipa_mojom_core = custom_target(core_mojom_file.split('.')[0] + '_mojom_module', + input : core_mojom_file, + output : core_mojom_file + '-module', + command : [ + mojom_parser, + '--output-root', meson.build_root(), + '--input-root', meson.source_root(), + '--mojoms', '@INPUT@' + ]) + ipa_mojom_files = [] ipa_mojoms = [] @@ -30,6 +41,7 @@ foreach file : ipa_mojom_files mojom = custom_target(file.split('.')[0] + '_mojom_module', input : file, output : file + '-module', + depends : ipa_mojom_core, command : [ mojom_parser, '--output-root', meson.build_root(), @@ -41,7 +53,7 @@ foreach file : ipa_mojom_files header = custom_target(name + '_ipa_interface_h', input : mojom, output : name + '_ipa_interface.h', - depends : mojom_templates, + depends : [mojom_templates, ipa_mojom_core], command : [ mojom_generator, 'generate', '-g', 'libcamera', @@ -55,7 +67,7 @@ foreach file : ipa_mojom_files serializer = custom_target(name + '_ipa_serializer_h', input : mojom, output : name + '_ipa_serializer.h', - depends : mojom_templates, + depends : [mojom_templates, ipa_mojom_core], command : [ mojom_generator, 'generate', '-g', 'libcamera', @@ -69,7 +81,7 @@ foreach file : ipa_mojom_files proxy_header = custom_target(name + '_proxy_h', input : mojom, output : 'ipa_proxy_' + name + '.h', - depends : mojom_templates, + depends : [mojom_templates, ipa_mojom_core], command : [ mojom_generator, 'generate', '-g', 'libcamera', diff --git a/src/libcamera/proxy/meson.build b/src/libcamera/proxy/meson.build index ac68ad08..f27b453a 100644 --- a/src/libcamera/proxy/meson.build +++ b/src/libcamera/proxy/meson.build @@ -10,7 +10,7 @@ foreach mojom : ipa_mojoms proxy = custom_target(mojom['name'] + '_proxy_cpp', input : mojom['mojom'], output : 'ipa_proxy_' + mojom['name'] + '.cpp', - depends : [mojom_templates], + depends : [mojom_templates, ipa_mojom_core], command : [ mojom_generator, 'generate', '-g', 'libcamera', diff --git a/src/libcamera/proxy/worker/meson.build b/src/libcamera/proxy/worker/meson.build index 8e0b978a..628bb050 100644 --- a/src/libcamera/proxy/worker/meson.build +++ b/src/libcamera/proxy/worker/meson.build @@ -11,7 +11,7 @@ foreach mojom : ipa_mojoms worker = custom_target(mojom['name'] + '_proxy_worker', input : mojom['mojom'], output : 'ipa_proxy_' + mojom['name'] + '_worker.cpp', - depends : [mojom_templates], + depends : [mojom_templates, ipa_mojom_core], command : [ mojom_generator, 'generate', '-g', 'libcamera',
Add a base mojom file to contain empty mojom definitions of libcamera objects, as well as documentation for the IPA interfaces that need to be defined in the mojom files. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v4: - move docs to IPA guide Changes in v3: - add doc that structs need to be defined - add doc to recommend namespacing - change indentation - add requirement that base controls *must* be defined in libcamera::{pipeline_name}::Controls New in v2 --- include/libcamera/ipa/core.mojom | 18 ++++++++++++++++++ include/libcamera/ipa/meson.build | 18 +++++++++++++++--- src/libcamera/proxy/meson.build | 2 +- src/libcamera/proxy/worker/meson.build | 2 +- 4 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 include/libcamera/ipa/core.mojom