[libcamera-devel,v4,24/37] ipa: Add core.mojom
diff mbox series

Message ID 20201106103707.49660-25-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • IPA isolation implementation
Related show

Commit Message

Paul Elder Nov. 6, 2020, 10:36 a.m. UTC 
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

Comments

Laurent Pinchart Nov. 26, 2020, 2:33 p.m. UTC | #1 
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',

Patch
diff mbox series 

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',