[libcamera-devel] [PATCH 01/13] meson: ipa: Add mapping for pipeline handler to mojom interface file

[Jacopo Mondi]

Hi Naush

On Wed, Apr 26, 2023 at 02:10:45PM +0100, Naushir Patuck via libcamera-devel wrote:
> Allow an arbitrary mapping between the pipeline handler and IPA mojom
> interface file in the build system. This removes the 1:1 mapping of
> pipeline handler name to mojom filename, and allows more flexibility to
> pipeline developers.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

> ---
>  Documentation/guides/ipa.rst      | 19 ++++++++--------
>  include/libcamera/ipa/meson.build | 36 ++++++++++++++++++++-----------
>  2 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
> index fc0317451e24..89839408672a 100644
> --- a/Documentation/guides/ipa.rst
> +++ b/Documentation/guides/ipa.rst
> @@ -269,35 +269,36 @@ The following is an example of an event interface definition:
>  Compiling the IPA interface
>  ---------------------------
>
> -After the IPA interface is defined in include/libcamera/ipa/{pipeline_name}.mojom,
> +After the IPA interface is defined in include/libcamera/ipa/{interface_name}.mojom,
>  an entry for it must be added in meson so that it can be compiled. The filename
> -must be added to the ipa_mojom_files object in include/libcamera/ipa/meson.build.
> +must be added to the pipeline_ipa_mojom_mapping object in include/libcamera/ipa/meson.build.
> +This object maps the pipeline handler name with an ipa interface file.
>
>  For example, adding the raspberrypi.mojom file to meson:
>
>  .. code-block:: none
>
> -        ipa_mojom_files = [
> -            'raspberrypi.mojom',
> +        pipeline_ipa_mojom_mapping = [
> +            'raspberrypi': 'raspberrypi.mojom',
>          ]
>
>  This will cause the mojo data definition file to be compiled. Specifically, it
>  generates five files:
>
>  - a header describing the custom data structures, and the complete IPA
> -  interface (at {$build_dir}/include/libcamera/ipa/{pipeline}_ipa_interface.h)
> +  interface (at {$build_dir}/include/libcamera/ipa/{interface}_ipa_interface.h)
>
>  - a serializer implementing de/serialization for the custom data structures (at
> -  {$build_dir}/include/libcamera/ipa/{pipeline}_ipa_serializer.h)
> +  {$build_dir}/include/libcamera/ipa/{interface}_ipa_serializer.h)
>
>  - a proxy header describing a specialized IPA proxy (at
> -  {$build_dir}/include/libcamera/ipa/{pipeline}_ipa_proxy.h)
> +  {$build_dir}/include/libcamera/ipa/{interface}_ipa_proxy.h)
>
>  - a proxy source implementing the IPA proxy (at
> -  {$build_dir}/src/libcamera/proxy/{pipeline}_ipa_proxy.cpp)
> +  {$build_dir}/src/libcamera/proxy/{interface}_ipa_proxy.cpp)
>
>  - a proxy worker source implementing the other end of the IPA proxy (at
> -  {$build_dir}/src/libcamera/proxy/worker/{pipeline}_ipa_proxy_worker.cpp)
> +  {$build_dir}/src/libcamera/proxy/worker/{interface}_ipa_proxy_worker.cpp)
>
>  The IPA proxy serves as the layer between the pipeline handler and the IPA, and
>  handles threading vs isolation transparently. The pipeline handler and the IPA
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index 442ca3dd7e1c..67c31cb04ccf 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -60,13 +60,15 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
>                        './' +'@INPUT@'
>                    ])
>
> -ipa_mojom_files = [
> -    'ipu3.mojom',
> -    'raspberrypi.mojom',
> -    'rkisp1.mojom',
> -    'vimc.mojom',
> -]
> -
> +# Mapping from pipeline handler name to mojom file
> +pipeline_ipa_mojom_mapping = {
> +    'ipu3': 'ipu3.mojom',
> +    'rkisp1': 'rkisp1.mojom',
> +    'raspberrypi': 'raspberrypi.mojom',
> +    'vimc': 'vimc.mojom',
> +}
> +
> +ipa_mojom_files = []
>  ipa_mojoms = []
>
>  #
> @@ -75,14 +77,22 @@ ipa_mojoms = []
>
>  # TODO Define per-pipeline ControlInfoMap with yaml?
>
> -foreach file : ipa_mojom_files
> +foreach pipeline, file : pipeline_ipa_mojom_mapping
> +
>      name = file.split('.')[0]
>
> -    if name not in pipelines
> +    # Ensure we do not build duplicate mojom modules
> +    if (file in ipa_mojom_files)
> +        continue
> +    endif
> +
> +    ipa_mojom_files += file
> +
> +    if pipeline not in pipelines
>          continue
>      endif
>
> -    # {pipeline}.mojom-module
> +    # {interface}.mojom-module
>      mojom = custom_target(name + '_mojom_module',
>                            input : file,
>                            output : file + '-module',
> @@ -94,7 +104,7 @@ foreach file : ipa_mojom_files
>                                '--mojoms', '@INPUT@'
>                            ])
>
> -    # {pipeline}_ipa_interface.h
> +    # {interface}_ipa_interface.h
>      header = custom_target(name + '_ipa_interface_h',
>                             input : mojom,
>                             output : name + '_ipa_interface.h',
> @@ -110,7 +120,7 @@ foreach file : ipa_mojom_files
>                                 './' +'@INPUT@'
>                             ])
>
> -    # {pipeline}_ipa_serializer.h
> +    # {interface}_ipa_serializer.h
>      serializer = custom_target(name + '_ipa_serializer_h',
>                                 input : mojom,
>                                 output : name + '_ipa_serializer.h',
> @@ -124,7 +134,7 @@ foreach file : ipa_mojom_files
>                                     './' +'@INPUT@'
>                                 ])
>
> -    # {pipeline}_ipa_proxy.h
> +    # {interface}_ipa_proxy.h
>      proxy_header = custom_target(name + '_proxy_h',
>                                   input : mojom,
>                                   output : name + '_ipa_proxy.h',
> --
> 2.34.1
>