[libcamera-devel] [PATCH 01/13] meson: ipa: Add mapping for pipeline handler to mojom interface file
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu May 4 17:38:31 CEST 2023
Hi Naush,
Thank you for the patch.
On Wed, May 03, 2023 at 01:20:23PM +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>
> ---
> Documentation/guides/ipa.rst | 39 ++++++++++++++++++++-----------
> include/libcamera/ipa/meson.build | 36 +++++++++++++++++-----------
> 2 files changed, 48 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
> index fc0317451e24..51dbe3aad5f5 100644
> --- a/Documentation/guides/ipa.rst
> +++ b/Documentation/guides/ipa.rst
> @@ -19,6 +19,16 @@ connect to, in order to receive data from the IPA asynchronously. In addition,
> it contains any custom data structures that the pipeline handler and IPA may
> pass to each other.
>
> +It is possible to use the same IPA interface with multiple pipeline handlers
> +on different hardware platforms. Generally in such cases, these platforms would
> +have a common hardware ISP pipeline. For instance, the rkisp1 pipeline handler
> +supports both the RK3399 and the i.MX8MP as they integrate the same ISP.
> +However, the i.MX8MP has a more complex camera pipeline, which may call for a
> +dedicated pipeline handler in the future. As the ISP is the same as for RK3399,
> +the same IPA interface could be used for both pipeline handlers. The build files
> +provide a mapping from pipeline handler to the IPA interface name as detailed in
> +:ref:`compiling-section`.
> +
> The IPA protocol refers to the agreement between the pipeline handler and the
> IPA regarding the expected response(s) from the IPA for given calls to the IPA.
> This protocol doesn't need to be declared anywhere in code, but it shall be
> @@ -43,7 +53,7 @@ interface definition is thus written by the pipeline handler author, based on
> how they design the interactions between the pipeline handler and the IPA.
>
> The entire IPA interface, including the functions, signals, and any custom
> -structs shall be defined in a file named {pipeline_name}.mojom under
> +structs shall be defined in a file named {interface_name}.mojom under
> include/libcamera/ipa/.
>
> .. _mojo Interface Definition Language: https://chromium.googlesource.com/chromium/src.git/+/master/mojo/public/tools/bindings/README.md
> @@ -150,8 +160,6 @@ and the Event IPA interface, which describes the signals received by the
> pipeline handler that the IPA can emit. Both must be defined. This section
> focuses on the Main IPA interface.
>
> -The main interface must be named as IPA{pipeline_name}Interface.
> -
Did you drop this on purpose ?
> The functions that the pipeline handler can call from the IPA may be
> synchronous or asynchronous. Synchronous functions do not return until the IPA
> returns from the function, while asynchronous functions return immediately
> @@ -243,7 +251,7 @@ then it may be empty. These emissions are meant to notify the pipeline handler
> of some event, such as request data is ready, and *must not* be used to drive
> the camera pipeline from the IPA.
>
> -The event interface must be named as IPA{pipeline_name}EventInterface.
> +The event interface must be named as IPA{interface_name}EventInterface.
>
> Functions defined in the event interface are implicitly asynchronous.
> Thus they cannot return any value. Specifying the [async] tag is not
> @@ -266,38 +274,41 @@ The following is an example of an event interface definition:
> setStaggered(libcamera.ControlList controls);
> };
>
> +.. _compiling-section:
> +
> 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.
It's not an object but a variable, I'll fix this while at it.
s/with an ipa/to its IPA/
>
> For example, adding the raspberrypi.mojom file to meson:
>
> .. code-block:: none
>
> - ipa_mojom_files = [
> - 'raspberrypi.mojom',
> + pipeline_ipa_mojom_mapping = [
> + 'rpi/vc4': '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
> @@ -312,7 +323,7 @@ file, the following header must be included:
>
> .. code-block:: C++
>
> - #include <libcamera/ipa/{pipeline_name}_ipa_interface.h>
> + #include <libcamera/ipa/{interface_name}_ipa_interface.h>
>
> The POD types of the structs simply become their C++ counterparts, eg. uint32
> in mojo will become uint32_t in C++. mojo map becomes C++ std::map, and mojo
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index 442ca3dd7e1c..c57b3a5e1570 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
> +
No need for a blank line.
I'll apply all these small changes, no need to resubmit.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 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',
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list