[libcamera-devel] [PATCH] ipa: Convert the IPA API to plain C
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Sep 19 08:43:57 CEST 2019
Hi Laurent,
On 2019-09-18 21:05:52 +0300, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Tue, Sep 17, 2019 at 11:36:03AM +0200, Niklas Söderlund wrote:
> > On 2019-09-15 22:04:08 +0300, Laurent Pinchart wrote:
> > > From: Jacopo Mondi <jacopo at jmondi.org>
> > >
> > > The C++ objects that are expected to convey data through the IPA API
> > > will have associated methods that would require IPAs to link to
> > > libcamera. Even though the libcamera license allows this, suppliers of
> > > closed-source IPAs may have a different interpretation. To ease their
> > > mind and clearly separate vendor code and libcamera code, turn the IPA
> > > API to plain C. The corresponding C objects will be stored in plain C
> > > structures or have their binary format documented, removing the need for
> > > linking to libcamera code on the IPA side.
> > >
> > > This is implemented by adding two new C structures, ipa_context and
> > > ipa_operations. The ipa_operations contains function pointers for all
> > > the IPA API operations. The ipa_context represents a context of
> > > operation for the IPA, and is passed to the IPA oparations. The
> > > IPAInterface class is retained as it is easier to use than a plain C API
> > > for pipeline handlers, with a new IPAWrapper class that wraps the
> > > ipa_context and ipa_operations into and IPAInterface.
> > >
> > > On the IPA module side usage of IPAInterface may be desired for IPAs
> > > implemented in C++ that want to link to libcamera. For those IPAs, a new
> > > IPAWrapperContext helper class is introduce to wrap the IPAInterface
> > > implemented internally by the IPA module into an ipa_context and
> > > ipa_operations.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > The fix in vimc.cpp should really be it's own patch as it's not related
> > to the C API ;-) Other than that,
>
> Agreed, it could have been split. It was the result of a previous API
> version, and when undoing that I've left the != nullptr to ! change.
> Would you like me to send a new version, or can I leave it as-is this
> time ?
I'm fine either way ;-)
>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> > > ---
> > > Documentation/Doxyfile.in | 1 +
> > > Documentation/meson.build | 2 +
> > > include/ipa/ipa_interface.h | 16 ++++
> > > src/ipa/ipa_dummy.cpp | 6 +-
> > > src/ipa/libipa/ipa_interface_wrapper.cpp | 74 ++++++++++++++++++
> > > src/ipa/libipa/ipa_interface_wrapper.h | 29 +++++++
> > > src/ipa/libipa/meson.build | 10 +++
> > > src/ipa/meson.build | 3 +
> > > src/libcamera/include/ipa_context_wrapper.h | 26 +++++++
> > > src/libcamera/include/ipa_module.h | 5 +-
> > > src/libcamera/include/meson.build | 1 +
> > > src/libcamera/ipa_context_wrapper.cpp | 52 +++++++++++++
> > > src/libcamera/ipa_interface.cpp | 77 ++++++++++++++++++-
> > > src/libcamera/ipa_manager.cpp | 67 +++++++++++++++-
> > > src/libcamera/ipa_module.cpp | 23 +++---
> > > src/libcamera/meson.build | 1 +
> > > src/libcamera/pipeline/vimc.cpp | 2 +-
> > > .../proxy/worker/ipa_proxy_linux_worker.cpp | 8 +-
> > > 18 files changed, 381 insertions(+), 22 deletions(-)
> > > create mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp
> > > create mode 100644 src/ipa/libipa/ipa_interface_wrapper.h
> > > create mode 100644 src/ipa/libipa/meson.build
> > > create mode 100644 src/libcamera/include/ipa_context_wrapper.h
> > > create mode 100644 src/libcamera/ipa_context_wrapper.cpp
> > >
> > > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> > > index ecc058eec683..2ab93687755f 100644
> > > --- a/Documentation/Doxyfile.in
> > > +++ b/Documentation/Doxyfile.in
> > > @@ -793,6 +793,7 @@ WARN_LOGFILE =
> > >
> > > INPUT = "@TOP_SRCDIR@/include/ipa" \
> > > "@TOP_SRCDIR@/include/libcamera" \
> > > + "@TOP_SRCDIR@/src/ipa/libipa" \
> > > "@TOP_SRCDIR@/src/libcamera"
> > >
> > > # This tag can be used to specify the character encoding of the source files
> > > diff --git a/Documentation/meson.build b/Documentation/meson.build
> > > index 4ff3fbeb0674..9136506f5d9c 100644
> > > --- a/Documentation/meson.build
> > > +++ b/Documentation/meson.build
> > > @@ -24,6 +24,8 @@ if doxygen.found()
> > > libcamera_ipa_api,
> > > libcamera_headers,
> > > libcamera_sources,
> > > + libipa_headers,
> > > + libipa_sources,
> > > ],
> > > output : 'api-html',
> > > command : [doxygen, doxyfile],
> > > diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> > > index 9bbc4cf58ec6..f1ebac20f151 100644
> > > --- a/include/ipa/ipa_interface.h
> > > +++ b/include/ipa/ipa_interface.h
> > > @@ -7,6 +7,21 @@
> > > #ifndef __LIBCAMERA_IPA_INTERFACE_H__
> > > #define __LIBCAMERA_IPA_INTERFACE_H__
> > >
> > > +#ifdef __cplusplus
> > > +extern "C" {
> > > +#endif
> > > +
> > > +struct ipa_context {
> > > + const struct ipa_operations *ops;
> > > +};
> > > +
> > > +struct ipa_operations {
> > > + void (*destroy)(struct ipa_context *ctx);
> > > +};
> > > +
> > > +#ifdef __cplusplus
> > > +}
> > > +
> > > namespace libcamera {
> > >
> > > class IPAInterface
> > > @@ -16,5 +31,6 @@ public:
> > > };
> > >
> > > } /* namespace libcamera */
> > > +#endif
> > >
> > > #endif /* __LIBCAMERA_IPA_INTERFACE_H__ */
> > > diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp
> > > index c833e5fb0b2d..6dc9448a3f56 100644
> > > --- a/src/ipa/ipa_dummy.cpp
> > > +++ b/src/ipa/ipa_dummy.cpp
> > > @@ -8,6 +8,8 @@
> > > #include <ipa/ipa_interface.h>
> > > #include <ipa/ipa_module_info.h>
> > >
> > > +#include "libipa/ipa_interface_wrapper.h"
> > > +
> > > namespace libcamera {
> > >
> > > class IPADummy : public IPAInterface
> > > @@ -27,9 +29,9 @@ const struct IPAModuleInfo ipaModuleInfo = {
> > > LICENSE,
> > > };
> > >
> > > -IPAInterface *ipaCreate()
> > > +struct ipa_context *ipaCreate()
> > > {
> > > - return new IPADummy();
> > > + return new IPAInterfaceWrapper(new IPADummy());
> > > }
> > > };
> > >
> > > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
> > > new file mode 100644
> > > index 000000000000..aacd189851c3
> > > --- /dev/null
> > > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
> > > @@ -0,0 +1,74 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2019, Google Inc.
> > > + *
> > > + * ipa_interface_wrapper.cpp - Image Processing Algorithm interface wrapper
> > > + */
> > > +
> > > +#include "ipa_interface_wrapper.h"
> > > +
> > > +#include <ipa/ipa_interface.h>
> > > +
> > > +/**
> > > + * \file ipa_interface_wrapper.h
> > > + * \brief Image Processing Algorithm interface wrapper
> > > + */
> > > +
> > > +namespace libcamera {
> > > +
> > > +/**
> > > + * \class IPAInterfaceWrapper
> > > + * \brief Wrap an IPAInterface and expose it as an ipa_context
> > > + *
> > > + * This class implements the ipa_context API based on a provided IPAInterface.
> > > + * It helps IPAs that implement the IPAInterface API to provide the external
> > > + * ipa_context API.
> > > + *
> > > + * To use the wrapper, an IPA module simple creates a new instance of its
> > > + * IPAInterface implementation, and passes it to the constructor of the
> > > + * IPAInterfaceWrapper. As IPAInterfaceWrapper inherits from ipa_context, the
> > > + * constructed wrapper can then be directly returned from the IPA module's
> > > + * ipaCreate() function.
> > > + *
> > > + * \code{.cpp}
> > > + * class MyIPA : public IPAInterface
> > > + * {
> > > + * ...
> > > + * };
> > > + *
> > > + * struct ipa_context *ipaCreate()
> > > + * {
> > > + * return new IPAInterfaceWrapper(new MyIPA());
> > > + * }
> > > + * \endcode
> > > + */
> > > +
> > > +/**
> > > + * \brief Construct an IPAInterfaceWrapper wrapping \a interface
> > > + * \param[in] interface The interface to wrap
> > > + */
> > > +IPAInterfaceWrapper::IPAInterfaceWrapper(IPAInterface *interface)
> > > + : ipa(interface)
> > > +{
> > > + ops = &operations;
> > > +}
> > > +
> > > +void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx)
> > > +{
> > > + IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
> > > +
> > > + delete ctx->ipa;
> > > + delete ctx;
> > > +}
> > > +
> > > +#ifndef __DOXYGEN__
> > > +/*
> > > + * This construct confuses Doygen and makes it believe that all members of the
> > > + * operations is a member of IPAInterfaceWrapper. It must thus be hidden.
> > > + */
> > > +const struct ipa_operations IPAInterfaceWrapper::operations = {
> > > + .destroy = &IPAInterfaceWrapper::destroy,
> > > +};
> > > +#endif
> > > +
> > > +}; /* namespace libcamera */
> > > diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h
> > > new file mode 100644
> > > index 000000000000..d2ab46f50d3c
> > > --- /dev/null
> > > +++ b/src/ipa/libipa/ipa_interface_wrapper.h
> > > @@ -0,0 +1,29 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2019, Google Inc.
> > > + *
> > > + * ipa_interface_wrapper.h - Image Processing Algorithm interface wrapper
> > > + */
> > > +#ifndef __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__
> > > +#define __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__
> > > +
> > > +#include <ipa/ipa_interface.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +class IPAInterfaceWrapper : public ipa_context
> > > +{
> > > +public:
> > > + IPAInterfaceWrapper(IPAInterface *interface);
> > > +
> > > +private:
> > > + static void destroy(struct ipa_context *ctx);
> > > +
> > > + static const struct ipa_operations operations;
> > > +
> > > + IPAInterface *ipa;
> > > +};
> > > +
> > > +} /* namespace libcamera */
> > > +
> > > +#endif /* __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ */
> > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> > > new file mode 100644
> > > index 000000000000..ab3515ed2021
> > > --- /dev/null
> > > +++ b/src/ipa/libipa/meson.build
> > > @@ -0,0 +1,10 @@
> > > +libipa_headers = files([
> > > + 'ipa_interface_wrapper.h',
> > > +])
> > > +
> > > +libipa_sources = files([
> > > + 'ipa_interface_wrapper.cpp',
> > > +])
> > > +
> > > +libipa = static_library('libipa', libipa_sources,
> > > + dependencies : libcamera_dep)
> > > diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> > > index f09915bc1388..6483ea66a478 100644
> > > --- a/src/ipa/meson.build
> > > +++ b/src/ipa/meson.build
> > > @@ -1,3 +1,5 @@
> > > +subdir('libipa')
> > > +
> > > ipa_dummy_sources = [
> > > ['ipa_dummy', 'LGPL-2.1-or-later'],
> > > ['ipa_dummy_isolate', 'Proprietary'],
> > > @@ -9,6 +11,7 @@ foreach t : ipa_dummy_sources
> > > ipa = shared_module(t[0], 'ipa_dummy.cpp',
> > > name_prefix : '',
> > > include_directories : libcamera_includes,
> > > + link_with : libipa,
> > > install : true,
> > > install_dir : ipa_install_dir,
> > > cpp_args : '-DLICENSE="' + t[1] + '"')
> > > diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h
> > > new file mode 100644
> > > index 000000000000..12894ac6885e
> > > --- /dev/null
> > > +++ b/src/libcamera/include/ipa_context_wrapper.h
> > > @@ -0,0 +1,26 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2019, Google Inc.
> > > + *
> > > + * ipa_context_wrapper.h - Image Processing Algorithm context wrapper
> > > + */
> > > +#ifndef __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__
> > > +#define __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__
> > > +
> > > +#include <ipa/ipa_interface.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +class IPAContextWrapper final : public IPAInterface
> > > +{
> > > +public:
> > > + IPAContextWrapper(struct ipa_context *context);
> > > + ~IPAContextWrapper();
> > > +
> > > +private:
> > > + struct ipa_context *ctx_;
> > > +};
> > > +
> > > +} /* namespace libcamera */
> > > +
> > > +#endif /* __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__ */
> > > diff --git a/src/libcamera/include/ipa_module.h b/src/libcamera/include/ipa_module.h
> > > index 97737587ab3a..2028b76a1913 100644
> > > --- a/src/libcamera/include/ipa_module.h
> > > +++ b/src/libcamera/include/ipa_module.h
> > > @@ -7,7 +7,6 @@
> > > #ifndef __LIBCAMERA_IPA_MODULE_H__
> > > #define __LIBCAMERA_IPA_MODULE_H__
> > >
> > > -#include <memory>
> > > #include <string>
> > >
> > > #include <ipa/ipa_interface.h>
> > > @@ -30,7 +29,7 @@ public:
> > >
> > > bool load();
> > >
> > > - std::unique_ptr<IPAInterface> createInstance();
> > > + struct ipa_context *createContext();
> > >
> > > bool match(PipelineHandler *pipe,
> > > uint32_t minVersion, uint32_t maxVersion) const;
> > > @@ -45,7 +44,7 @@ private:
> > > bool loaded_;
> > >
> > > void *dlHandle_;
> > > - typedef IPAInterface *(*IPAIntfFactory)(void);
> > > + typedef struct ipa_context *(*IPAIntfFactory)(void);
> > > IPAIntfFactory ipaCreate_;
> > >
> > > int loadIPAModuleInfo();
> > > diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build
> > > index 933be8543a8d..0abb978a389a 100644
> > > --- a/src/libcamera/include/meson.build
> > > +++ b/src/libcamera/include/meson.build
> > > @@ -5,6 +5,7 @@ libcamera_headers = files([
> > > 'device_enumerator_udev.h',
> > > 'event_dispatcher_poll.h',
> > > 'formats.h',
> > > + 'ipa_context_wrapper.h',
> > > 'ipa_manager.h',
> > > 'ipa_module.h',
> > > 'ipa_proxy.h',
> > > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
> > > new file mode 100644
> > > index 000000000000..87ff98d45c99
> > > --- /dev/null
> > > +++ b/src/libcamera/ipa_context_wrapper.cpp
> > > @@ -0,0 +1,52 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2019, Google Inc.
> > > + *
> > > + * ipa_context_wrapper.cpp - Image Processing Algorithm context wrapper
> > > + */
> > > +
> > > +#include "ipa_context_wrapper.h"
> > > +
> > > +#include <libcamera/controls.h>
> > > +
> > > +/**
> > > + * \file ipa_context_wrapper.h
> > > + * \brief Image Processing Algorithm context wrapper
> > > + */
> > > +
> > > +namespace libcamera {
> > > +
> > > +/**
> > > + * \class IPAContextWrapper
> > > + * \brief Wrap an ipa_context and expose it as an IPAInterface
> > > + *
> > > + * The IPAContextWrapper class wraps an ipa_context, provided by an IPA module, and
> > > + * exposes an IPAInterface. This mechanism is used for IPAs that are not
> > > + * isolated in a separate process to allow direct calls from pipeline handler
> > > + * using the IPAInterface API instead of the lower-level ipa_context API.
> > > + *
> > > + * The IPAInterface methods are converted to the ipa_context API by serialising
> > > + * all C++ arguments into plain C structures or byte arrays that contain no
> > > + * pointer, as required by the ipa_context API.
> > > + */
> > > +
> > > +/**
> > > + * \brief Construct an IPAContextWrapper instance that wraps the \a context
> > > + * \param[in] context The IPA module context
> > > + *
> > > + * Ownership of the \a context is passed to the IPAContextWrapper. The context remains
> > > + * valid for the whole lifetime of the wrapper and is destroyed automatically
> > > + * with it.
> > > + */
> > > +IPAContextWrapper::IPAContextWrapper(struct ipa_context *context)
> > > + : ctx_(context)
> > > +{
> > > +}
> > > +
> > > +IPAContextWrapper::~IPAContextWrapper()
> > > +{
> > > + if (ctx_)
> > > + ctx_->ops->destroy(ctx_);
> > > +}
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> > > index f70d91ded1ab..6e83aab0fb73 100644
> > > --- a/src/libcamera/ipa_interface.cpp
> > > +++ b/src/libcamera/ipa_interface.cpp
> > > @@ -10,13 +10,88 @@
> > > /**
> > > * \file ipa_interface.h
> > > * \brief Image Processing Algorithm interface
> > > + *
> > > + * libcamera interfaces with IPA modules through a plain C interface. IPA
> > > + * modules shall expose a public function name ipaCreate() with the following
> > > + * prototype.
> > > + *
> > > + * \code{.c}
> > > + * struct ipa_context *ipaCreate();
> > > + * \endcode
> > > + *
> > > + * The ipaCreate() function creates an instance of an IPA context, which models
> > > + * a context of execution for the IPA. IPA modules shall support creating one
> > > + * or multiple contexts, as required by their associated pipeline handler.
> > > + *
> > > + * The IPA module operations are defined in the struct ipa_operations. An IPA
> > > + * module stores a pointer to the operations corresponding to its context in
> > > + * the ipa_context::ops field. That pointer is immutable for the lifetime of
> > > + * the context, and may be different for multiple contexts created by the same
> > > + * IPA module.
> > > + *
> > > + * All argument to ipa_operations members are Plain Old Data and are documented
> > > + * either in the form of C data types, or as a textual description for byte
> > > + * arrays that can't be expressed using C data types (such as variable-length
> > > + * arrays). IPA modules can thus use the C API without calling into libcamera
> > > + * to access the data passed to the IPA operations.
> > > + *
> > > + * The IPAInterface class is a C++ representation of the ipa_operations, using
> > > + * C++ data classes provided by libcamera. This is the API exposed to pipeline
> > > + * handlers to communicate with IPA modules. IPA modules may use the
> > > + * IPAInterface API internally if they want to benefit from the data and helper
> > > + * classes offered by libcamera.
> > > + */
> > > +
> > > +/**
> > > + * \struct ipa_context
> > > + * \brief IPA module context of execution
> > > + *
> > > + * This structure models a context of execution for an IPA module. It is
> > > + * instantiated by the IPA module ipaCreate() function. IPA modules allocate
> > > + * context instances in an implementation-defined way, contexts shall thus be
> > > + * destroyed using the ipa_operation::destroy operation only.
> > > + *
> > > + * The ipa_context structure provides a pointer to the IPA operations. It shall
> > > + * otherwise be treated as a constant black-box cookie and passed unmodified to
> > > + * the operations defined in struct ipa_operations.
> > > + *
> > > + * IPA modules are expected to extend struct ipa_context by inheriting from it,
> > > + * either through structure embedding to model inheritance in plain C, or
> > > + * through C++ class inheritance. A simple example of the latter is available
> > > + * in the IPAContextWrapper class implementation.
> > > + *
> > > + * \var ipa_context::ops
> > > + * \brief The IPA context operations
> > > + */
> > > +
> > > +/**
> > > + * \struct ipa_operations
> > > + * \brief IPA context operations as a set of function pointers
> > > + *
> > > + * To allow for isolation of IPA modules in separate processes, the functions
> > > + * defined in the ipa_operations structure return only data related to the
> > > + * libcamera side of the operations. In particular, error related to the
> > > + * libcamera side of the IPC may be returned. Data returned by the IPA,
> > > + * including status information, shall be provided through callbacks from the
> > > + * IPA to libcamera.
> > > + *
> > > + * \var ipa_operations::destroy
> > > + * \brief Destroy the ipa_context created by the module's ipaCreate() function
> > > */
> > >
> > > namespace libcamera {
> > >
> > > /**
> > > * \class IPAInterface
> > > - * \brief Interface for IPA implementation
> > > + * \brief IPA module interface
> > > + *
> > > + * This pure virtual class defines a C++ API corresponding to the ipa_context
> > > + * and ipa_operations API. It is used by pipeline handlers to interact with IPA
> > > + * modules, and may be used internally in IPA modules if desired to benefit
> > > + * from the data and helper classes provided by libcamera.
> > > + *
> > > + * As for the operations defined in struct ipa_operations, the methods defined
> > > + * by this class shall not return data from the IPA.
> > > */
> > >
> > > } /* namespace libcamera */
> > > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> > > index 4276d995bdd5..0a908eae6261 100644
> > > --- a/src/libcamera/ipa_manager.cpp
> > > +++ b/src/libcamera/ipa_manager.cpp
> > > @@ -11,6 +11,7 @@
> > > #include <string.h>
> > > #include <sys/types.h>
> > >
> > > +#include "ipa_context_wrapper.h"
> > > #include "ipa_module.h"
> > > #include "ipa_proxy.h"
> > > #include "log.h"
> > > @@ -29,6 +30,66 @@ LOG_DEFINE_CATEGORY(IPAManager)
> > > /**
> > > * \class IPAManager
> > > * \brief Manager for IPA modules
> > > + *
> > > + * The IPA module manager discovers IPA modules from disk, queries and loads
> > > + * them, and creates IPA contexts. It supports isolation of the modules in a
> > > + * separate process with IPC communication and offers a unified IPAInterface
> > > + * view of the IPA contexts to pipeline handlers regardless of whether the
> > > + * modules are isolated or loaded in the same process.
> > > + *
> > > + * Module isolation is based on the module licence. Open-source modules are
> > > + * loaded without isolation, while closed-source module are forcefully isolated.
> > > + * The isolation mechanism ensures that no code from a closed-source module is
> > > + * ever run in the libcamera process.
> > > + *
> > > + * To create an IPA context, pipeline handlers call the IPAManager::ipaCreate()
> > > + * method. For a directly loaded module, the manager calls the module's
> > > + * ipaCreate() function directly and wraps the returned context in an
> > > + * IPAContextWrapper that exposes an IPAInterface.
> > > + *
> > > + * ~~~~
> > > + * +---------------+
> > > + * | Pipeline |
> > > + * | Handler |
> > > + * +---------------+
> > > + * |
> > > + * v
> > > + * +---------------+ +---------------+
> > > + * | IPA | | Open Source |
> > > + * | Interface | | IPA Module |
> > > + * | - - - - - - - | | - - - - - - - |
> > > + * | IPA Context | ipa_operations | ipa_context |
> > > + * | Wrapper | ----------------> | |
> > > + * +---------------+ +---------------+
> > > + * ~~~~
> > > + *
> > > + * For an isolated module, the manager instantiates an IPAProxy which spawns a
> > > + * new process for an IPA proxy worker. The worker loads the IPA module and
> > > + * creates the IPA context. The IPAProxy alse exposes an IPAInterface.
> > > + *
> > > + * ~~~~
> > > + * +---------------+ +---------------+
> > > + * | Pipeline | | Closed Source |
> > > + * | Handler | | IPA Module |
> > > + * +---------------+ | - - - - - - - |
> > > + * | | ipa_context |
> > > + * v | |
> > > + * +---------------+ +---------------+
> > > + * | IPA | ipa_operations ^
> > > + * | Interface | |
> > > + * | - - - - - - - | +---------------+
> > > + * | IPA Proxy | operations | IPA Proxy |
> > > + * | | ----------------> | Worker |
> > > + * +---------------+ over IPC +---------------+
> > > + * ~~~~
> > > + *
> > > + * The IPAInterface implemented by the IPAContextWrapper or IPAProxy is
> > > + * returned to the pipeline handler, and all interactions with the IPA context
> > > + * go the same interface regarless of process isolation.
> > > + *
> > > + * In all cases the data passed to the IPAInterface methods is serialised to
> > > + * Plain Old Data, either for the purpose of passing it to the IPA context
> > > + * plain C API, or to transmit the data to the isolated process through IPC.
> > > */
> > >
> > > IPAManager::IPAManager()
> > > @@ -177,7 +238,11 @@ std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
> > > if (!m->load())
> > > return nullptr;
> > >
> > > - return m->createInstance();
> > > + struct ipa_context *ctx = m->createContext();
> > > + if (!ctx)
> > > + return nullptr;
> > > +
> > > + return utils::make_unique<IPAContextWrapper>(ctx);
> > > }
> > >
> > > } /* namespace libcamera */
> > > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> > > index 99d308efd47b..9f5a01418f73 100644
> > > --- a/src/libcamera/ipa_module.cpp
> > > +++ b/src/libcamera/ipa_module.cpp
> > > @@ -385,13 +385,13 @@ const std::string &IPAModule::path() const
> > > /**
> > > * \brief Load the IPA implementation factory from the shared object
> > > *
> > > - * The IPA module shared object implements an IPAInterface class to be used
> > > + * The IPA module shared object implements an ipa_context object to be used
> > > * by pipeline handlers. This method loads the factory function from the
> > > - * shared object. Later, createInstance() can be called to instantiate the
> > > - * IPAInterface.
> > > + * shared object. Later, createContext() can be called to instantiate the
> > > + * ipa_context.
> > > *
> > > * This method only needs to be called successfully once, after which
> > > - * createInstance() can be called as many times as IPAInterface instances are
> > > + * createContext() can be called as many times as ipa_context instances are
> > > * needed.
> > > *
> > > * Calling this function on an invalid module (as returned by isValid()) is
> > > @@ -433,24 +433,25 @@ bool IPAModule::load()
> > > }
> > >
> > > /**
> > > - * \brief Instantiate an IPAInterface
> > > + * \brief Instantiate an IPA context
> > > *
> > > - * After loading the IPA module with load(), this method creates an
> > > - * instance of the IPA module interface.
> > > + * After loading the IPA module with load(), this method creates an instance of
> > > + * the IPA module context. Ownership of the context is passed to the caller, and
> > > + * the context shall be destroyed by calling the \ref ipa_operations::destroy
> > > + * "ipa_context::ops::destroy()" operation.
> > > *
> > > * Calling this function on a module that has not yet been loaded, or an
> > > * invalid module (as returned by load() and isValid(), respectively) is
> > > * an error.
> > > *
> > > - * \return The IPA implementation as a new IPAInterface instance on success,
> > > - * or nullptr on error
> > > + * \return The IPA context on success, or nullptr on error
> > > */
> > > -std::unique_ptr<IPAInterface> IPAModule::createInstance()
> > > +struct ipa_context *IPAModule::createContext()
> > > {
> > > if (!valid_ || !loaded_)
> > > return nullptr;
> > >
> > > - return std::unique_ptr<IPAInterface>(ipaCreate_());
> > > + return ipaCreate_();
> > > }
> > >
> > > /**
> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > index 755149c55c7b..d3ae305e5655 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -12,6 +12,7 @@ libcamera_sources = files([
> > > 'event_notifier.cpp',
> > > 'formats.cpp',
> > > 'geometry.cpp',
> > > + 'ipa_context_wrapper.cpp',
> > > 'ipa_interface.cpp',
> > > 'ipa_manager.cpp',
> > > 'ipa_module.cpp',
> > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > > index a401b1099f3c..612edcda69d6 100644
> > > --- a/src/libcamera/pipeline/vimc.cpp
> > > +++ b/src/libcamera/pipeline/vimc.cpp
> > > @@ -362,7 +362,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> > > return false;
> > >
> > > ipa_ = IPAManager::instance()->createIPA(this, 0, 0);
> > > - if (ipa_ == nullptr)
> > > + if (!ipa_)
> > > LOG(VIMC, Warning) << "no matching IPA found";
> > >
> > > std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this);
> > > diff --git a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp
> > > index a10761e52b32..07380c16e2d5 100644
> > > --- a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp
> > > +++ b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp
> > > @@ -72,9 +72,9 @@ int main(int argc, char **argv)
> > > }
> > > socket.readyRead.connect(&readyRead);
> > >
> > > - std::unique_ptr<IPAInterface> ipa = ipam->createInstance();
> > > - if (!ipa) {
> > > - LOG(IPAProxyLinuxWorker, Error) << "Failed to create IPA interface";
> > > + struct ipa_context *ipac = ipam->createContext();
> > > + if (!ipac) {
> > > + LOG(IPAProxyLinuxWorker, Error) << "Failed to create IPA context";
> > > return EXIT_FAILURE;
> > > }
> > >
> > > @@ -85,5 +85,7 @@ int main(int argc, char **argv)
> > > while (1)
> > > dispatcher->processEvents();
> > >
> > > + ipac->ops->destroy(ipac);
> > > +
> > > return 0;
> > > }
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list