[libcamera-devel] [PATCH 01/13] meson: ipa: Add mapping for pipeline handler to mojom interface file
Naushir Patuck
naush at raspberrypi.com
Thu May 4 18:26:24 CEST 2023
Hi Laurent,
On Thu, 4 May 2023 at 16:38, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> 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 ?
Oops, no I didn't. But we do need to replace pipeline_name with
interface_name. Finger trouble on my part.
>
> > 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.
Thanks for that!
Regards,
Naush
>
> 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