[{"id":2667,"web_url":"https://patchwork.libcamera.org/comment/2667/","msgid":"<20190917093603.GC15610@bigcity.dyn.berto.se>","date":"2019-09-17T09:36:03","subject":"Re: [libcamera-devel] [PATCH] ipa: Convert the IPA API to plain C","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo and Laurent,\n\nThanks for your work.\n\nOn 2019-09-15 22:04:08 +0300, Laurent Pinchart wrote:\n> From: Jacopo Mondi \n> \n> The C++ objects that are expected to convey data through the IPA API\n> will have associated methods that would require IPAs to link to\n> libcamera. Even though the libcamera license allows this, suppliers of\n> closed-source IPAs may have a different interpretation. To ease their\n> mind and clearly separate vendor code and libcamera code, turn the IPA\n> API to plain C. The corresponding C objects will be stored in plain C\n> structures or have their binary format documented, removing the need for\n> linking to libcamera code on the IPA side.\n> \n> This is implemented by adding two new C structures, ipa_context and\n> ipa_operations. The ipa_operations contains function pointers for all\n> the IPA API operations. The ipa_context represents a context of\n> operation for the IPA, and is passed to the IPA oparations. The\n> IPAInterface class is retained as it is easier to use than a plain C API\n> for pipeline handlers, with a new IPAWrapper class that wraps the\n> ipa_context and ipa_operations into and IPAInterface.\n> \n> On the IPA module side usage of IPAInterface may be desired for IPAs\n> implemented in C++ that want to link to libcamera. For those IPAs, a new\n> IPAWrapperContext helper class is introduce to wrap the IPAInterface\n> implemented internally by the IPA module into an ipa_context and\n> ipa_operations.\n> \n> Signed-off-by: Jacopo Mondi \n> Signed-off-by: Laurent Pinchart \n\nThe fix in vimc.cpp should really be it's own patch as it's not related \nto the C API ;-) Other than that,\n\nReviewed-by: Niklas Söderlund \n\n> ---\n> Documentation/Doxyfile.in | 1 +\n> Documentation/meson.build | 2 +\n> include/ipa/ipa_interface.h | 16 ++++\n> src/ipa/ipa_dummy.cpp | 6 +-\n> src/ipa/libipa/ipa_interface_wrapper.cpp | 74 ++++++++++++++++++\n> src/ipa/libipa/ipa_interface_wrapper.h | 29 +++++++\n> src/ipa/libipa/meson.build | 10 +++\n> src/ipa/meson.build | 3 +\n> src/libcamera/include/ipa_context_wrapper.h | 26 +++++++\n> src/libcamera/include/ipa_module.h | 5 +-\n> src/libcamera/include/meson.build | 1 +\n> src/libcamera/ipa_context_wrapper.cpp | 52 +++++++++++++\n> src/libcamera/ipa_interface.cpp | 77 ++++++++++++++++++-\n> src/libcamera/ipa_manager.cpp | 67 +++++++++++++++-\n> src/libcamera/ipa_module.cpp | 23 +++---\n> src/libcamera/meson.build | 1 +\n> src/libcamera/pipeline/vimc.cpp | 2 +-\n> .../proxy/worker/ipa_proxy_linux_worker.cpp | 8 +-\n> 18 files changed, 381 insertions(+), 22 deletions(-)\n> create mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp\n> create mode 100644 src/ipa/libipa/ipa_interface_wrapper.h\n> create mode 100644 src/ipa/libipa/meson.build\n> create mode 100644 src/libcamera/include/ipa_context_wrapper.h\n> create mode 100644 src/libcamera/ipa_context_wrapper.cpp\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index ecc058eec683..2ab93687755f 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -793,6 +793,7 @@ WARN_LOGFILE =\n> \n> INPUT = \"@TOP_SRCDIR@/include/ipa\" \\\n> \t\t\t \"@TOP_SRCDIR@/include/libcamera\" \\\n> +\t\t\t \"@TOP_SRCDIR@/src/ipa/libipa\" \\\n> \t\t\t \"@TOP_SRCDIR@/src/libcamera\"\n> \n> # This tag can be used to specify the character encoding of the source files\n> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> index 4ff3fbeb0674..9136506f5d9c 100644\n> --- a/Documentation/meson.build\n> +++ b/Documentation/meson.build\n> @@ -24,6 +24,8 @@ if doxygen.found()\n> libcamera_ipa_api,\n> libcamera_headers,\n> libcamera_sources,\n> + libipa_headers,\n> + libipa_sources,\n> ],\n> output : 'api-html',\n> command : [doxygen, doxyfile],\n> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\n> index 9bbc4cf58ec6..f1ebac20f151 100644\n> --- a/include/ipa/ipa_interface.h\n> +++ b/include/ipa/ipa_interface.h\n> @@ -7,6 +7,21 @@\n> #ifndef __LIBCAMERA_IPA_INTERFACE_H__\n> #define __LIBCAMERA_IPA_INTERFACE_H__\n> \n> +#ifdef __cplusplus\n> +extern \"C\" {\n> +#endif\n> +\n> +struct ipa_context {\n> +\tconst struct ipa_operations *ops;\n> +};\n> +\n> +struct ipa_operations {\n> +\tvoid (*destroy)(struct ipa_context *ctx);\n> +};\n> +\n> +#ifdef __cplusplus\n> +}\n> +\n> namespace libcamera {\n> \n> class IPAInterface\n> @@ -16,5 +31,6 @@ public:\n> };\n> \n> } /* namespace libcamera */\n> +#endif\n> \n> #endif /* __LIBCAMERA_IPA_INTERFACE_H__ */\n> diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp\n> index c833e5fb0b2d..6dc9448a3f56 100644\n> --- a/src/ipa/ipa_dummy.cpp\n> +++ b/src/ipa/ipa_dummy.cpp\n> @@ -8,6 +8,8 @@\n> #include \n> #include \n> \n> +#include \"libipa/ipa_interface_wrapper.h\"\n> +\n> namespace libcamera {\n> \n> class IPADummy : public IPAInterface\n> @@ -27,9 +29,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \tLICENSE,\n> };\n> \n> -IPAInterface *ipaCreate()\n> +struct ipa_context *ipaCreate()\n> {\n> -\treturn new IPADummy();\n> +\treturn new IPAInterfaceWrapper(new IPADummy());\n> }\n> };\n> \n> diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> new file mode 100644\n> index 000000000000..aacd189851c3\n> --- /dev/null\n> +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> @@ -0,0 +1,74 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipa_interface_wrapper.cpp - Image Processing Algorithm interface wrapper\n> + */\n> +\n> +#include \"ipa_interface_wrapper.h\"\n> +\n> +#include \n> +\n> +/**\n> + * \\file ipa_interface_wrapper.h\n> + * \\brief Image Processing Algorithm interface wrapper\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class IPAInterfaceWrapper\n> + * \\brief Wrap an IPAInterface and expose it as an ipa_context\n> + *\n> + * This class implements the ipa_context API based on a provided IPAInterface.\n> + * It helps IPAs that implement the IPAInterface API to provide the external\n> + * ipa_context API.\n> + *\n> + * To use the wrapper, an IPA module simple creates a new instance of its\n> + * IPAInterface implementation, and passes it to the constructor of the\n> + * IPAInterfaceWrapper. As IPAInterfaceWrapper inherits from ipa_context, the\n> + * constructed wrapper can then be directly returned from the IPA module's\n> + * ipaCreate() function.\n> + *\n> + * \\code{.cpp}\n> + * class MyIPA : public IPAInterface\n> + * {\n> + * \t...\n> + * };\n> + *\n> + * struct ipa_context *ipaCreate()\n> + * {\n> + * \treturn new IPAInterfaceWrapper(new MyIPA());\n> + * }\n> + * \\endcode\n> + */\n> +\n> +/**\n> + * \\brief Construct an IPAInterfaceWrapper wrapping \\a interface\n> + * \\param[in] interface The interface to wrap\n> + */\n> +IPAInterfaceWrapper::IPAInterfaceWrapper(IPAInterface *interface)\n> +\t: ipa(interface)\n> +{\n> +\tops = &operations;\n> +}\n> +\n> +void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx)\n> +{\n> +\tIPAInterfaceWrapper *ctx = static_cast(_ctx);\n> +\n> +\tdelete ctx->ipa;\n> +\tdelete ctx;\n> +}\n> +\n> +#ifndef __DOXYGEN__\n> +/*\n> + * This construct confuses Doygen and makes it believe that all members of the\n> + * operations is a member of IPAInterfaceWrapper. It must thus be hidden.\n> + */\n> +const struct ipa_operations IPAInterfaceWrapper::operations = {\n> +\t.destroy = &IPAInterfaceWrapper::destroy,\n> +};\n> +#endif\n> +\n> +}; /* namespace libcamera */\n> diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h\n> new file mode 100644\n> index 000000000000..d2ab46f50d3c\n> --- /dev/null\n> +++ b/src/ipa/libipa/ipa_interface_wrapper.h\n> @@ -0,0 +1,29 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipa_interface_wrapper.h - Image Processing Algorithm interface wrapper\n> + */\n> +#ifndef __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__\n> +#define __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class IPAInterfaceWrapper : public ipa_context\n> +{\n> +public:\n> +\tIPAInterfaceWrapper(IPAInterface *interface);\n> +\n> +private:\n> +\tstatic void destroy(struct ipa_context *ctx);\n> +\n> +\tstatic const struct ipa_operations operations;\n> +\n> +\tIPAInterface *ipa;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ */\n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> new file mode 100644\n> index 000000000000..ab3515ed2021\n> --- /dev/null\n> +++ b/src/ipa/libipa/meson.build\n> @@ -0,0 +1,10 @@\n> +libipa_headers = files([\n> + 'ipa_interface_wrapper.h',\n> +])\n> +\n> +libipa_sources = files([\n> + 'ipa_interface_wrapper.cpp',\n> +])\n> +\n> +libipa = static_library('libipa', libipa_sources,\n> + dependencies : libcamera_dep)\n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index f09915bc1388..6483ea66a478 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -1,3 +1,5 @@\n> +subdir('libipa')\n> +\n> ipa_dummy_sources = [\n> ['ipa_dummy', 'LGPL-2.1-or-later'],\n> ['ipa_dummy_isolate', 'Proprietary'],\n> @@ -9,6 +11,7 @@ foreach t : ipa_dummy_sources\n> ipa = shared_module(t[0], 'ipa_dummy.cpp',\n> name_prefix : '',\n> include_directories : libcamera_includes,\n> + link_with : libipa,\n> install : true,\n> install_dir : ipa_install_dir,\n> cpp_args : '-DLICENSE=\"' + t[1] + '\"')\n> diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h\n> new file mode 100644\n> index 000000000000..12894ac6885e\n> --- /dev/null\n> +++ b/src/libcamera/include/ipa_context_wrapper.h\n> @@ -0,0 +1,26 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipa_context_wrapper.h - Image Processing Algorithm context wrapper\n> + */\n> +#ifndef __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__\n> +#define __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class IPAContextWrapper final : public IPAInterface\n> +{\n> +public:\n> +\tIPAContextWrapper(struct ipa_context *context);\n> +\t~IPAContextWrapper();\n> +\n> +private:\n> +\tstruct ipa_context *ctx_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__ */\n> diff --git a/src/libcamera/include/ipa_module.h b/src/libcamera/include/ipa_module.h\n> index 97737587ab3a..2028b76a1913 100644\n> --- a/src/libcamera/include/ipa_module.h\n> +++ b/src/libcamera/include/ipa_module.h\n> @@ -7,7 +7,6 @@\n> #ifndef __LIBCAMERA_IPA_MODULE_H__\n> #define __LIBCAMERA_IPA_MODULE_H__\n> \n> -#include \n> #include \n> \n> #include \n> @@ -30,7 +29,7 @@ public:\n> \n> \tbool load();\n> \n> -\tstd::unique_ptr createInstance();\n> +\tstruct ipa_context *createContext();\n> \n> \tbool match(PipelineHandler *pipe,\n> \t\t uint32_t minVersion, uint32_t maxVersion) const;\n> @@ -45,7 +44,7 @@ private:\n> \tbool loaded_;\n> \n> \tvoid *dlHandle_;\n> -\ttypedef IPAInterface *(*IPAIntfFactory)(void);\n> +\ttypedef struct ipa_context *(*IPAIntfFactory)(void);\n> \tIPAIntfFactory ipaCreate_;\n> \n> \tint loadIPAModuleInfo();\n> diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build\n> index 933be8543a8d..0abb978a389a 100644\n> --- a/src/libcamera/include/meson.build\n> +++ b/src/libcamera/include/meson.build\n> @@ -5,6 +5,7 @@ libcamera_headers = files([\n> 'device_enumerator_udev.h',\n> 'event_dispatcher_poll.h',\n> 'formats.h',\n> + 'ipa_context_wrapper.h',\n> 'ipa_manager.h',\n> 'ipa_module.h',\n> 'ipa_proxy.h',\n> diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> new file mode 100644\n> index 000000000000..87ff98d45c99\n> --- /dev/null\n> +++ b/src/libcamera/ipa_context_wrapper.cpp\n> @@ -0,0 +1,52 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipa_context_wrapper.cpp - Image Processing Algorithm context wrapper\n> + */\n> +\n> +#include \"ipa_context_wrapper.h\"\n> +\n> +#include \n> +\n> +/**\n> + * \\file ipa_context_wrapper.h\n> + * \\brief Image Processing Algorithm context wrapper\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class IPAContextWrapper\n> + * \\brief Wrap an ipa_context and expose it as an IPAInterface\n> + *\n> + * The IPAContextWrapper class wraps an ipa_context, provided by an IPA module, and\n> + * exposes an IPAInterface. This mechanism is used for IPAs that are not\n> + * isolated in a separate process to allow direct calls from pipeline handler\n> + * using the IPAInterface API instead of the lower-level ipa_context API.\n> + *\n> + * The IPAInterface methods are converted to the ipa_context API by serialising\n> + * all C++ arguments into plain C structures or byte arrays that contain no\n> + * pointer, as required by the ipa_context API.\n> + */\n> +\n> +/**\n> + * \\brief Construct an IPAContextWrapper instance that wraps the \\a context\n> + * \\param[in] context The IPA module context\n> + *\n> + * Ownership of the \\a context is passed to the IPAContextWrapper. The context remains\n> + * valid for the whole lifetime of the wrapper and is destroyed automatically\n> + * with it.\n> + */\n> +IPAContextWrapper::IPAContextWrapper(struct ipa_context *context)\n> +\t: ctx_(context)\n> +{\n> +}\n> +\n> +IPAContextWrapper::~IPAContextWrapper()\n> +{\n> +\tif (ctx_)\n> +\t\tctx_->ops->destroy(ctx_);\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> index f70d91ded1ab..6e83aab0fb73 100644\n> --- a/src/libcamera/ipa_interface.cpp\n> +++ b/src/libcamera/ipa_interface.cpp\n> @@ -10,13 +10,88 @@\n> /**\n> * \\file ipa_interface.h\n> * \\brief Image Processing Algorithm interface\n> + *\n> + * libcamera interfaces with IPA modules through a plain C interface. IPA\n> + * modules shall expose a public function name ipaCreate() with the following\n> + * prototype.\n> + *\n> + * \\code{.c}\n> + * struct ipa_context *ipaCreate();\n> + * \\endcode\n> + *\n> + * The ipaCreate() function creates an instance of an IPA context, which models\n> + * a context of execution for the IPA. IPA modules shall support creating one\n> + * or multiple contexts, as required by their associated pipeline handler.\n> + *\n> + * The IPA module operations are defined in the struct ipa_operations. An IPA\n> + * module stores a pointer to the operations corresponding to its context in\n> + * the ipa_context::ops field. That pointer is immutable for the lifetime of\n> + * the context, and may be different for multiple contexts created by the same\n> + * IPA module.\n> + *\n> + * All argument to ipa_operations members are Plain Old Data and are documented\n> + * either in the form of C data types, or as a textual description for byte\n> + * arrays that can't be expressed using C data types (such as variable-length\n> + * arrays). IPA modules can thus use the C API without calling into libcamera\n> + * to access the data passed to the IPA operations.\n> + *\n> + * The IPAInterface class is a C++ representation of the ipa_operations, using\n> + * C++ data classes provided by libcamera. This is the API exposed to pipeline\n> + * handlers to communicate with IPA modules. IPA modules may use the\n> + * IPAInterface API internally if they want to benefit from the data and helper\n> + * classes offered by libcamera.\n> + */\n> +\n> +/**\n> + * \\struct ipa_context\n> + * \\brief IPA module context of execution\n> + *\n> + * This structure models a context of execution for an IPA module. It is\n> + * instantiated by the IPA module ipaCreate() function. IPA modules allocate\n> + * context instances in an implementation-defined way, contexts shall thus be\n> + * destroyed using the ipa_operation::destroy operation only.\n> + *\n> + * The ipa_context structure provides a pointer to the IPA operations. It shall\n> + * otherwise be treated as a constant black-box cookie and passed unmodified to\n> + * the operations defined in struct ipa_operations.\n> + *\n> + * IPA modules are expected to extend struct ipa_context by inheriting from it,\n> + * either through structure embedding to model inheritance in plain C, or\n> + * through C++ class inheritance. A simple example of the latter is available\n> + * in the IPAContextWrapper class implementation.\n> + *\n> + * \\var ipa_context::ops\n> + * \\brief The IPA context operations\n> + */\n> +\n> +/**\n> + * \\struct ipa_operations\n> + * \\brief IPA context operations as a set of function pointers\n> + *\n> + * To allow for isolation of IPA modules in separate processes, the functions\n> + * defined in the ipa_operations structure return only data related to the\n> + * libcamera side of the operations. In particular, error related to the\n> + * libcamera side of the IPC may be returned. Data returned by the IPA,\n> + * including status information, shall be provided through callbacks from the\n> + * IPA to libcamera.\n> + *\n> + * \\var ipa_operations::destroy\n> + * \\brief Destroy the ipa_context created by the module's ipaCreate() function\n> */\n> \n> namespace libcamera {\n> \n> /**\n> * \\class IPAInterface\n> - * \\brief Interface for IPA implementation\n> + * \\brief IPA module interface\n> + *\n> + * This pure virtual class defines a C++ API corresponding to the ipa_context\n> + * and ipa_operations API. It is used by pipeline handlers to interact with IPA\n> + * modules, and may be used internally in IPA modules if desired to benefit\n> + * from the data and helper classes provided by libcamera.\n> + *\n> + * As for the operations defined in struct ipa_operations, the methods defined\n> + * by this class shall not return data from the IPA.\n> */\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 4276d995bdd5..0a908eae6261 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> \n> +#include \"ipa_context_wrapper.h\"\n> #include \"ipa_module.h\"\n> #include \"ipa_proxy.h\"\n> #include \"log.h\"\n> @@ -29,6 +30,66 @@ LOG_DEFINE_CATEGORY(IPAManager)\n> /**\n> * \\class IPAManager\n> * \\brief Manager for IPA modules\n> + *\n> + * The IPA module manager discovers IPA modules from disk, queries and loads\n> + * them, and creates IPA contexts. It supports isolation of the modules in a\n> + * separate process with IPC communication and offers a unified IPAInterface\n> + * view of the IPA contexts to pipeline handlers regardless of whether the\n> + * modules are isolated or loaded in the same process.\n> + *\n> + * Module isolation is based on the module licence. Open-source modules are\n> + * loaded without isolation, while closed-source module are forcefully isolated.\n> + * The isolation mechanism ensures that no code from a closed-source module is\n> + * ever run in the libcamera process.\n> + *\n> + * To create an IPA context, pipeline handlers call the IPAManager::ipaCreate()\n> + * method. For a directly loaded module, the manager calls the module's\n> + * ipaCreate() function directly and wraps the returned context in an\n> + * IPAContextWrapper that exposes an IPAInterface.\n> + *\n> + * ~~~~\n> + * +---------------+\n> + * | Pipeline |\n> + * | Handler |\n> + * +---------------+\n> + * |\n> + * v\n> + * +---------------+ +---------------+\n> + * | IPA | | Open Source |\n> + * | Interface | | IPA Module |\n> + * | - - - - - - - | | - - - - - - - |\n> + * | IPA Context | ipa_operations | ipa_context |\n> + * | Wrapper | ----------------> | |\n> + * +---------------+ +---------------+\n> + * ~~~~\n> + *\n> + * For an isolated module, the manager instantiates an IPAProxy which spawns a\n> + * new process for an IPA proxy worker. The worker loads the IPA module and\n> + * creates the IPA context. The IPAProxy alse exposes an IPAInterface.\n> + *\n> + * ~~~~\n> + * +---------------+ +---------------+\n> + * | Pipeline | | Closed Source |\n> + * | Handler | | IPA Module |\n> + * +---------------+ | - - - - - - - |\n> + * | | ipa_context |\n> + * v | |\n> + * +---------------+ +---------------+\n> + * | IPA | ipa_operations ^\n> + * | Interface | |\n> + * | - - - - - - - | +---------------+\n> + * | IPA Proxy | operations | IPA Proxy |\n> + * | | ----------------> | Worker |\n> + * +---------------+ over IPC +---------------+\n> + * ~~~~\n> + *\n> + * The IPAInterface implemented by the IPAContextWrapper or IPAProxy is\n> + * returned to the pipeline handler, and all interactions with the IPA context\n> + * go the same interface regarless of process isolation.\n> + *\n> + * In all cases the data passed to the IPAInterface methods is serialised to\n> + * Plain Old Data, either for the purpose of passing it to the IPA context\n> + * plain C API, or to transmit the data to the isolated process through IPC.\n> */\n> \n> IPAManager::IPAManager()\n> @@ -177,7 +238,11 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe,\n> \tif (!m->load())\n> \t\treturn nullptr;\n> \n> -\treturn m->createInstance();\n> +\tstruct ipa_context *ctx = m->createContext();\n> +\tif (!ctx)\n> +\t\treturn nullptr;\n> +\n> +\treturn utils::make_unique(ctx);\n> }\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index 99d308efd47b..9f5a01418f73 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -385,13 +385,13 @@ const std::string &IPAModule::path() const\n> /**\n> * \\brief Load the IPA implementation factory from the shared object\n> *\n> - * The IPA module shared object implements an IPAInterface class to be used\n> + * The IPA module shared object implements an ipa_context object to be used\n> * by pipeline handlers. This method loads the factory function from the\n> - * shared object. Later, createInstance() can be called to instantiate the\n> - * IPAInterface.\n> + * shared object. Later, createContext() can be called to instantiate the\n> + * ipa_context.\n> *\n> * This method only needs to be called successfully once, after which\n> - * createInstance() can be called as many times as IPAInterface instances are\n> + * createContext() can be called as many times as ipa_context instances are\n> * needed.\n> *\n> * Calling this function on an invalid module (as returned by isValid()) is\n> @@ -433,24 +433,25 @@ bool IPAModule::load()\n> }\n> \n> /**\n> - * \\brief Instantiate an IPAInterface\n> + * \\brief Instantiate an IPA context\n> *\n> - * After loading the IPA module with load(), this method creates an\n> - * instance of the IPA module interface.\n> + * After loading the IPA module with load(), this method creates an instance of\n> + * the IPA module context. Ownership of the context is passed to the caller, and\n> + * the context shall be destroyed by calling the \\ref ipa_operations::destroy\n> + * \"ipa_context::ops::destroy()\" operation.\n> *\n> * Calling this function on a module that has not yet been loaded, or an\n> * invalid module (as returned by load() and isValid(), respectively) is\n> * an error.\n> *\n> - * \\return The IPA implementation as a new IPAInterface instance on success,\n> - * or nullptr on error\n> + * \\return The IPA context on success, or nullptr on error\n> */\n> -std::unique_ptr IPAModule::createInstance()\n> +struct ipa_context *IPAModule::createContext()\n> {\n> \tif (!valid_ || !loaded_)\n> \t\treturn nullptr;\n> \n> -\treturn std::unique_ptr(ipaCreate_());\n> +\treturn ipaCreate_();\n> }\n> \n> /**\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 755149c55c7b..d3ae305e5655 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -12,6 +12,7 @@ libcamera_sources = files([\n> 'event_notifier.cpp',\n> 'formats.cpp',\n> 'geometry.cpp',\n> + 'ipa_context_wrapper.cpp',\n> 'ipa_interface.cpp',\n> 'ipa_manager.cpp',\n> 'ipa_module.cpp',\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index a401b1099f3c..612edcda69d6 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -362,7 +362,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \n> \tipa_ = IPAManager::instance()->createIPA(this, 0, 0);\n> -\tif (ipa_ == nullptr)\n> +\tif (!ipa_)\n> \t\tLOG(VIMC, Warning) << \"no matching IPA found\";\n> \n> \tstd::unique_ptr data = utils::make_unique(this);\n> diff --git a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> index a10761e52b32..07380c16e2d5 100644\n> --- a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> +++ b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> @@ -72,9 +72,9 @@ int main(int argc, char **argv)\n> \t}\n> \tsocket.readyRead.connect(&readyRead);\n> \n> -\tstd::unique_ptr ipa = ipam->createInstance();\n> -\tif (!ipa) {\n> -\t\tLOG(IPAProxyLinuxWorker, Error) << \"Failed to create IPA interface\";\n> +\tstruct ipa_context *ipac = ipam->createContext();\n> +\tif (!ipac) {\n> +\t\tLOG(IPAProxyLinuxWorker, Error) << \"Failed to create IPA context\";\n> \t\treturn EXIT_FAILURE;\n> \t}\n> \n> @@ -85,5 +85,7 @@ int main(int argc, char **argv)\n> \twhile (1)\n> \t\tdispatcher->processEvents();\n> \n> +\tipac->ops->destroy(ipac);\n> +\n> \treturn 0;\n> }\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EE09D60BB0\n\tfor ;\n\tTue, 17 Sep 2019 11:36:06 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id d5so2725661lja.10\n\tfor ;\n\tTue, 17 Sep 2019 02:36:06 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tt8sm326748lfc.80.2019.09.17.02.36.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 17 Sep 2019 02:36:05 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=RYa53ayxzxac1xjFLouLBxZy+BNxKFqutPjFklyNUuI=;\n\tb=gY49Jw2X/NVIdu0GgVWBvt+YDarP33n8K/SFZp9LdDCw841t7x/yw4EvNHE2GpcfM9\n\tYr53rZFC8I1d8ddTAAAWSaZei0bpS+pU6TPeiiEx7jraOIZTx0oXpmRCnW4JFRCHwnGk\n\tahD+rkfkG0bY3HUCQNws/PwqNRCKZ24ioFaqTDFR+7Yyy5Ub1r97aiAz3LBLO87ogmu5\n\t7EpmXYt+/4seFakrXPaR1oBgzcwvFa6/jQ22y15R5uA9t+lJcuPOw3C3cXFzyOj2ArU8\n\t7roNOptbka5gHYyYhvAzs06VS+XTIh0ZaVEg4y/mkxegwdh1xcb0U5iCWkHXirSvD3EI\n\trVcA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=RYa53ayxzxac1xjFLouLBxZy+BNxKFqutPjFklyNUuI=;\n\tb=gfNQVjMbirjWTK8I5ABNMuBd8kPfnXaYBTB7syzBiTnE4IZz8Okxzi5wzutFLrSpaH\n\tnQsQvw8Qq8XVOkfwjksFZEdiOt5Z0NaW8Km5kXXT9KRdu/IS/03usrsv/3CqKEE7TOlY\n\t0YQnDZxI3EfYMew9UFarVGePM02BonUpit0k2955c4XC9V7y8KM40WMTQXj55Un1s977\n\t+U/0I/Jrvk9XyKYZ0VjHVk1H8+2kpTvRc4feYWqHDlT4kxNPfBxFDPO6Bw2dS4IkFOmA\n\te3uuhEiEh+Va4C+LqtEbT8BVOe3OUgXKjm7/49/JeJZcCkx6iXG5LwQg8rVOhfhYeixT\n\tuo3w==","X-Gm-Message-State":"APjAAAUKs4TeQVGg7jc3C1GbSY13nYa9VHaQgZmTWbWh0buwprq5TdxS\n\tYwPbHIJNT53ly3PhMA8lIjiYCg==","X-Google-Smtp-Source":"APXvYqze7yHkXGsWXNnsBp8EUGlLShBlGGX5IwA08RP06QJAIJu7aaEMA47RkkONbaWWcnLitJmPEg==","X-Received":"by 2002:a2e:86c5:: with SMTP id n5mr1267970ljj.153.1568712966092;\n\tTue, 17 Sep 2019 02:36:06 -0700 (PDT)","Date":"Tue, 17 Sep 2019 11:36:03 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190917093603.GC15610@bigcity.dyn.berto.se>","References":"<20190915190408.2507-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190915190408.2507-1-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH] ipa: Convert the IPA API to plain C","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 17 Sep 2019 09:36:07 -0000"}},{"id":2673,"web_url":"https://patchwork.libcamera.org/comment/2673/","msgid":"<20190918180552.GD4734@pendragon.ideasonboard.com>","date":"2019-09-18T18:05:52","subject":"Re: [libcamera-devel] [PATCH] ipa: Convert the IPA API to plain C","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Tue, Sep 17, 2019 at 11:36:03AM +0200, Niklas Söderlund wrote:\n> On 2019-09-15 22:04:08 +0300, Laurent Pinchart wrote:\n> > From: Jacopo Mondi \n> > \n> > The C++ objects that are expected to convey data through the IPA API\n> > will have associated methods that would require IPAs to link to\n> > libcamera. Even though the libcamera license allows this, suppliers of\n> > closed-source IPAs may have a different interpretation. To ease their\n> > mind and clearly separate vendor code and libcamera code, turn the IPA\n> > API to plain C. The corresponding C objects will be stored in plain C\n> > structures or have their binary format documented, removing the need for\n> > linking to libcamera code on the IPA side.\n> > \n> > This is implemented by adding two new C structures, ipa_context and\n> > ipa_operations. The ipa_operations contains function pointers for all\n> > the IPA API operations. The ipa_context represents a context of\n> > operation for the IPA, and is passed to the IPA oparations. The\n> > IPAInterface class is retained as it is easier to use than a plain C API\n> > for pipeline handlers, with a new IPAWrapper class that wraps the\n> > ipa_context and ipa_operations into and IPAInterface.\n> > \n> > On the IPA module side usage of IPAInterface may be desired for IPAs\n> > implemented in C++ that want to link to libcamera. For those IPAs, a new\n> > IPAWrapperContext helper class is introduce to wrap the IPAInterface\n> > implemented internally by the IPA module into an ipa_context and\n> > ipa_operations.\n> > \n> > Signed-off-by: Jacopo Mondi \n> > Signed-off-by: Laurent Pinchart \n> \n> The fix in vimc.cpp should really be it's own patch as it's not related \n> to the C API ;-) Other than that,\n\nAgreed, it could have been split. It was the result of a previous API\nversion, and when undoing that I've left the != nullptr to ! change.\nWould you like me to send a new version, or can I leave it as-is this\ntime ?\n\n> Reviewed-by: Niklas Söderlund \n> \n> > ---\n> > Documentation/Doxyfile.in | 1 +\n> > Documentation/meson.build | 2 +\n> > include/ipa/ipa_interface.h | 16 ++++\n> > src/ipa/ipa_dummy.cpp | 6 +-\n> > src/ipa/libipa/ipa_interface_wrapper.cpp | 74 ++++++++++++++++++\n> > src/ipa/libipa/ipa_interface_wrapper.h | 29 +++++++\n> > src/ipa/libipa/meson.build | 10 +++\n> > src/ipa/meson.build | 3 +\n> > src/libcamera/include/ipa_context_wrapper.h | 26 +++++++\n> > src/libcamera/include/ipa_module.h | 5 +-\n> > src/libcamera/include/meson.build | 1 +\n> > src/libcamera/ipa_context_wrapper.cpp | 52 +++++++++++++\n> > src/libcamera/ipa_interface.cpp | 77 ++++++++++++++++++-\n> > src/libcamera/ipa_manager.cpp | 67 +++++++++++++++-\n> > src/libcamera/ipa_module.cpp | 23 +++---\n> > src/libcamera/meson.build | 1 +\n> > src/libcamera/pipeline/vimc.cpp | 2 +-\n> > .../proxy/worker/ipa_proxy_linux_worker.cpp | 8 +-\n> > 18 files changed, 381 insertions(+), 22 deletions(-)\n> > create mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp\n> > create mode 100644 src/ipa/libipa/ipa_interface_wrapper.h\n> > create mode 100644 src/ipa/libipa/meson.build\n> > create mode 100644 src/libcamera/include/ipa_context_wrapper.h\n> > create mode 100644 src/libcamera/ipa_context_wrapper.cpp\n> > \n> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > index ecc058eec683..2ab93687755f 100644\n> > --- a/Documentation/Doxyfile.in\n> > +++ b/Documentation/Doxyfile.in\n> > @@ -793,6 +793,7 @@ WARN_LOGFILE =\n> > \n> > INPUT = \"@TOP_SRCDIR@/include/ipa\" \\\n> > \t\t\t \"@TOP_SRCDIR@/include/libcamera\" \\\n> > +\t\t\t \"@TOP_SRCDIR@/src/ipa/libipa\" \\\n> > \t\t\t \"@TOP_SRCDIR@/src/libcamera\"\n> > \n> > # This tag can be used to specify the character encoding of the source files\n> > diff --git a/Documentation/meson.build b/Documentation/meson.build\n> > index 4ff3fbeb0674..9136506f5d9c 100644\n> > --- a/Documentation/meson.build\n> > +++ b/Documentation/meson.build\n> > @@ -24,6 +24,8 @@ if doxygen.found()\n> > libcamera_ipa_api,\n> > libcamera_headers,\n> > libcamera_sources,\n> > + libipa_headers,\n> > + libipa_sources,\n> > ],\n> > output : 'api-html',\n> > command : [doxygen, doxyfile],\n> > diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\n> > index 9bbc4cf58ec6..f1ebac20f151 100644\n> > --- a/include/ipa/ipa_interface.h\n> > +++ b/include/ipa/ipa_interface.h\n> > @@ -7,6 +7,21 @@\n> > #ifndef __LIBCAMERA_IPA_INTERFACE_H__\n> > #define __LIBCAMERA_IPA_INTERFACE_H__\n> > \n> > +#ifdef __cplusplus\n> > +extern \"C\" {\n> > +#endif\n> > +\n> > +struct ipa_context {\n> > +\tconst struct ipa_operations *ops;\n> > +};\n> > +\n> > +struct ipa_operations {\n> > +\tvoid (*destroy)(struct ipa_context *ctx);\n> > +};\n> > +\n> > +#ifdef __cplusplus\n> > +}\n> > +\n> > namespace libcamera {\n> > \n> > class IPAInterface\n> > @@ -16,5 +31,6 @@ public:\n> > };\n> > \n> > } /* namespace libcamera */\n> > +#endif\n> > \n> > #endif /* __LIBCAMERA_IPA_INTERFACE_H__ */\n> > diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp\n> > index c833e5fb0b2d..6dc9448a3f56 100644\n> > --- a/src/ipa/ipa_dummy.cpp\n> > +++ b/src/ipa/ipa_dummy.cpp\n> > @@ -8,6 +8,8 @@\n> > #include \n> > #include \n> > \n> > +#include \"libipa/ipa_interface_wrapper.h\"\n> > +\n> > namespace libcamera {\n> > \n> > class IPADummy : public IPAInterface\n> > @@ -27,9 +29,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> > \tLICENSE,\n> > };\n> > \n> > -IPAInterface *ipaCreate()\n> > +struct ipa_context *ipaCreate()\n> > {\n> > -\treturn new IPADummy();\n> > +\treturn new IPAInterfaceWrapper(new IPADummy());\n> > }\n> > };\n> > \n> > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > new file mode 100644\n> > index 000000000000..aacd189851c3\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > @@ -0,0 +1,74 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * ipa_interface_wrapper.cpp - Image Processing Algorithm interface wrapper\n> > + */\n> > +\n> > +#include \"ipa_interface_wrapper.h\"\n> > +\n> > +#include \n> > +\n> > +/**\n> > + * \\file ipa_interface_wrapper.h\n> > + * \\brief Image Processing Algorithm interface wrapper\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +/**\n> > + * \\class IPAInterfaceWrapper\n> > + * \\brief Wrap an IPAInterface and expose it as an ipa_context\n> > + *\n> > + * This class implements the ipa_context API based on a provided IPAInterface.\n> > + * It helps IPAs that implement the IPAInterface API to provide the external\n> > + * ipa_context API.\n> > + *\n> > + * To use the wrapper, an IPA module simple creates a new instance of its\n> > + * IPAInterface implementation, and passes it to the constructor of the\n> > + * IPAInterfaceWrapper. As IPAInterfaceWrapper inherits from ipa_context, the\n> > + * constructed wrapper can then be directly returned from the IPA module's\n> > + * ipaCreate() function.\n> > + *\n> > + * \\code{.cpp}\n> > + * class MyIPA : public IPAInterface\n> > + * {\n> > + * \t...\n> > + * };\n> > + *\n> > + * struct ipa_context *ipaCreate()\n> > + * {\n> > + * \treturn new IPAInterfaceWrapper(new MyIPA());\n> > + * }\n> > + * \\endcode\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct an IPAInterfaceWrapper wrapping \\a interface\n> > + * \\param[in] interface The interface to wrap\n> > + */\n> > +IPAInterfaceWrapper::IPAInterfaceWrapper(IPAInterface *interface)\n> > +\t: ipa(interface)\n> > +{\n> > +\tops = &operations;\n> > +}\n> > +\n> > +void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx)\n> > +{\n> > +\tIPAInterfaceWrapper *ctx = static_cast(_ctx);\n> > +\n> > +\tdelete ctx->ipa;\n> > +\tdelete ctx;\n> > +}\n> > +\n> > +#ifndef __DOXYGEN__\n> > +/*\n> > + * This construct confuses Doygen and makes it believe that all members of the\n> > + * operations is a member of IPAInterfaceWrapper. It must thus be hidden.\n> > + */\n> > +const struct ipa_operations IPAInterfaceWrapper::operations = {\n> > +\t.destroy = &IPAInterfaceWrapper::destroy,\n> > +};\n> > +#endif\n> > +\n> > +}; /* namespace libcamera */\n> > diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h\n> > new file mode 100644\n> > index 000000000000..d2ab46f50d3c\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/ipa_interface_wrapper.h\n> > @@ -0,0 +1,29 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * ipa_interface_wrapper.h - Image Processing Algorithm interface wrapper\n> > + */\n> > +#ifndef __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__\n> > +#define __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__\n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class IPAInterfaceWrapper : public ipa_context\n> > +{\n> > +public:\n> > +\tIPAInterfaceWrapper(IPAInterface *interface);\n> > +\n> > +private:\n> > +\tstatic void destroy(struct ipa_context *ctx);\n> > +\n> > +\tstatic const struct ipa_operations operations;\n> > +\n> > +\tIPAInterface *ipa;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ */\n> > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > new file mode 100644\n> > index 000000000000..ab3515ed2021\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/meson.build\n> > @@ -0,0 +1,10 @@\n> > +libipa_headers = files([\n> > + 'ipa_interface_wrapper.h',\n> > +])\n> > +\n> > +libipa_sources = files([\n> > + 'ipa_interface_wrapper.cpp',\n> > +])\n> > +\n> > +libipa = static_library('libipa', libipa_sources,\n> > + dependencies : libcamera_dep)\n> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> > index f09915bc1388..6483ea66a478 100644\n> > --- a/src/ipa/meson.build\n> > +++ b/src/ipa/meson.build\n> > @@ -1,3 +1,5 @@\n> > +subdir('libipa')\n> > +\n> > ipa_dummy_sources = [\n> > ['ipa_dummy', 'LGPL-2.1-or-later'],\n> > ['ipa_dummy_isolate', 'Proprietary'],\n> > @@ -9,6 +11,7 @@ foreach t : ipa_dummy_sources\n> > ipa = shared_module(t[0], 'ipa_dummy.cpp',\n> > name_prefix : '',\n> > include_directories : libcamera_includes,\n> > + link_with : libipa,\n> > install : true,\n> > install_dir : ipa_install_dir,\n> > cpp_args : '-DLICENSE=\"' + t[1] + '\"')\n> > diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h\n> > new file mode 100644\n> > index 000000000000..12894ac6885e\n> > --- /dev/null\n> > +++ b/src/libcamera/include/ipa_context_wrapper.h\n> > @@ -0,0 +1,26 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * ipa_context_wrapper.h - Image Processing Algorithm context wrapper\n> > + */\n> > +#ifndef __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__\n> > +#define __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__\n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class IPAContextWrapper final : public IPAInterface\n> > +{\n> > +public:\n> > +\tIPAContextWrapper(struct ipa_context *context);\n> > +\t~IPAContextWrapper();\n> > +\n> > +private:\n> > +\tstruct ipa_context *ctx_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__ */\n> > diff --git a/src/libcamera/include/ipa_module.h b/src/libcamera/include/ipa_module.h\n> > index 97737587ab3a..2028b76a1913 100644\n> > --- a/src/libcamera/include/ipa_module.h\n> > +++ b/src/libcamera/include/ipa_module.h\n> > @@ -7,7 +7,6 @@\n> > #ifndef __LIBCAMERA_IPA_MODULE_H__\n> > #define __LIBCAMERA_IPA_MODULE_H__\n> > \n> > -#include \n> > #include \n> > \n> > #include \n> > @@ -30,7 +29,7 @@ public:\n> > \n> > \tbool load();\n> > \n> > -\tstd::unique_ptr createInstance();\n> > +\tstruct ipa_context *createContext();\n> > \n> > \tbool match(PipelineHandler *pipe,\n> > \t\t uint32_t minVersion, uint32_t maxVersion) const;\n> > @@ -45,7 +44,7 @@ private:\n> > \tbool loaded_;\n> > \n> > \tvoid *dlHandle_;\n> > -\ttypedef IPAInterface *(*IPAIntfFactory)(void);\n> > +\ttypedef struct ipa_context *(*IPAIntfFactory)(void);\n> > \tIPAIntfFactory ipaCreate_;\n> > \n> > \tint loadIPAModuleInfo();\n> > diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build\n> > index 933be8543a8d..0abb978a389a 100644\n> > --- a/src/libcamera/include/meson.build\n> > +++ b/src/libcamera/include/meson.build\n> > @@ -5,6 +5,7 @@ libcamera_headers = files([\n> > 'device_enumerator_udev.h',\n> > 'event_dispatcher_poll.h',\n> > 'formats.h',\n> > + 'ipa_context_wrapper.h',\n> > 'ipa_manager.h',\n> > 'ipa_module.h',\n> > 'ipa_proxy.h',\n> > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> > new file mode 100644\n> > index 000000000000..87ff98d45c99\n> > --- /dev/null\n> > +++ b/src/libcamera/ipa_context_wrapper.cpp\n> > @@ -0,0 +1,52 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * ipa_context_wrapper.cpp - Image Processing Algorithm context wrapper\n> > + */\n> > +\n> > +#include \"ipa_context_wrapper.h\"\n> > +\n> > +#include \n> > +\n> > +/**\n> > + * \\file ipa_context_wrapper.h\n> > + * \\brief Image Processing Algorithm context wrapper\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +/**\n> > + * \\class IPAContextWrapper\n> > + * \\brief Wrap an ipa_context and expose it as an IPAInterface\n> > + *\n> > + * The IPAContextWrapper class wraps an ipa_context, provided by an IPA module, and\n> > + * exposes an IPAInterface. This mechanism is used for IPAs that are not\n> > + * isolated in a separate process to allow direct calls from pipeline handler\n> > + * using the IPAInterface API instead of the lower-level ipa_context API.\n> > + *\n> > + * The IPAInterface methods are converted to the ipa_context API by serialising\n> > + * all C++ arguments into plain C structures or byte arrays that contain no\n> > + * pointer, as required by the ipa_context API.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct an IPAContextWrapper instance that wraps the \\a context\n> > + * \\param[in] context The IPA module context\n> > + *\n> > + * Ownership of the \\a context is passed to the IPAContextWrapper. The context remains\n> > + * valid for the whole lifetime of the wrapper and is destroyed automatically\n> > + * with it.\n> > + */\n> > +IPAContextWrapper::IPAContextWrapper(struct ipa_context *context)\n> > +\t: ctx_(context)\n> > +{\n> > +}\n> > +\n> > +IPAContextWrapper::~IPAContextWrapper()\n> > +{\n> > +\tif (ctx_)\n> > +\t\tctx_->ops->destroy(ctx_);\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> > index f70d91ded1ab..6e83aab0fb73 100644\n> > --- a/src/libcamera/ipa_interface.cpp\n> > +++ b/src/libcamera/ipa_interface.cpp\n> > @@ -10,13 +10,88 @@\n> > /**\n> > * \\file ipa_interface.h\n> > * \\brief Image Processing Algorithm interface\n> > + *\n> > + * libcamera interfaces with IPA modules through a plain C interface. IPA\n> > + * modules shall expose a public function name ipaCreate() with the following\n> > + * prototype.\n> > + *\n> > + * \\code{.c}\n> > + * struct ipa_context *ipaCreate();\n> > + * \\endcode\n> > + *\n> > + * The ipaCreate() function creates an instance of an IPA context, which models\n> > + * a context of execution for the IPA. IPA modules shall support creating one\n> > + * or multiple contexts, as required by their associated pipeline handler.\n> > + *\n> > + * The IPA module operations are defined in the struct ipa_operations. An IPA\n> > + * module stores a pointer to the operations corresponding to its context in\n> > + * the ipa_context::ops field. That pointer is immutable for the lifetime of\n> > + * the context, and may be different for multiple contexts created by the same\n> > + * IPA module.\n> > + *\n> > + * All argument to ipa_operations members are Plain Old Data and are documented\n> > + * either in the form of C data types, or as a textual description for byte\n> > + * arrays that can't be expressed using C data types (such as variable-length\n> > + * arrays). IPA modules can thus use the C API without calling into libcamera\n> > + * to access the data passed to the IPA operations.\n> > + *\n> > + * The IPAInterface class is a C++ representation of the ipa_operations, using\n> > + * C++ data classes provided by libcamera. This is the API exposed to pipeline\n> > + * handlers to communicate with IPA modules. IPA modules may use the\n> > + * IPAInterface API internally if they want to benefit from the data and helper\n> > + * classes offered by libcamera.\n> > + */\n> > +\n> > +/**\n> > + * \\struct ipa_context\n> > + * \\brief IPA module context of execution\n> > + *\n> > + * This structure models a context of execution for an IPA module. It is\n> > + * instantiated by the IPA module ipaCreate() function. IPA modules allocate\n> > + * context instances in an implementation-defined way, contexts shall thus be\n> > + * destroyed using the ipa_operation::destroy operation only.\n> > + *\n> > + * The ipa_context structure provides a pointer to the IPA operations. It shall\n> > + * otherwise be treated as a constant black-box cookie and passed unmodified to\n> > + * the operations defined in struct ipa_operations.\n> > + *\n> > + * IPA modules are expected to extend struct ipa_context by inheriting from it,\n> > + * either through structure embedding to model inheritance in plain C, or\n> > + * through C++ class inheritance. A simple example of the latter is available\n> > + * in the IPAContextWrapper class implementation.\n> > + *\n> > + * \\var ipa_context::ops\n> > + * \\brief The IPA context operations\n> > + */\n> > +\n> > +/**\n> > + * \\struct ipa_operations\n> > + * \\brief IPA context operations as a set of function pointers\n> > + *\n> > + * To allow for isolation of IPA modules in separate processes, the functions\n> > + * defined in the ipa_operations structure return only data related to the\n> > + * libcamera side of the operations. In particular, error related to the\n> > + * libcamera side of the IPC may be returned. Data returned by the IPA,\n> > + * including status information, shall be provided through callbacks from the\n> > + * IPA to libcamera.\n> > + *\n> > + * \\var ipa_operations::destroy\n> > + * \\brief Destroy the ipa_context created by the module's ipaCreate() function\n> > */\n> > \n> > namespace libcamera {\n> > \n> > /**\n> > * \\class IPAInterface\n> > - * \\brief Interface for IPA implementation\n> > + * \\brief IPA module interface\n> > + *\n> > + * This pure virtual class defines a C++ API corresponding to the ipa_context\n> > + * and ipa_operations API. It is used by pipeline handlers to interact with IPA\n> > + * modules, and may be used internally in IPA modules if desired to benefit\n> > + * from the data and helper classes provided by libcamera.\n> > + *\n> > + * As for the operations defined in struct ipa_operations, the methods defined\n> > + * by this class shall not return data from the IPA.\n> > */\n> > \n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> > index 4276d995bdd5..0a908eae6261 100644\n> > --- a/src/libcamera/ipa_manager.cpp\n> > +++ b/src/libcamera/ipa_manager.cpp\n> > @@ -11,6 +11,7 @@\n> > #include \n> > #include \n> > \n> > +#include \"ipa_context_wrapper.h\"\n> > #include \"ipa_module.h\"\n> > #include \"ipa_proxy.h\"\n> > #include \"log.h\"\n> > @@ -29,6 +30,66 @@ LOG_DEFINE_CATEGORY(IPAManager)\n> > /**\n> > * \\class IPAManager\n> > * \\brief Manager for IPA modules\n> > + *\n> > + * The IPA module manager discovers IPA modules from disk, queries and loads\n> > + * them, and creates IPA contexts. It supports isolation of the modules in a\n> > + * separate process with IPC communication and offers a unified IPAInterface\n> > + * view of the IPA contexts to pipeline handlers regardless of whether the\n> > + * modules are isolated or loaded in the same process.\n> > + *\n> > + * Module isolation is based on the module licence. Open-source modules are\n> > + * loaded without isolation, while closed-source module are forcefully isolated.\n> > + * The isolation mechanism ensures that no code from a closed-source module is\n> > + * ever run in the libcamera process.\n> > + *\n> > + * To create an IPA context, pipeline handlers call the IPAManager::ipaCreate()\n> > + * method. For a directly loaded module, the manager calls the module's\n> > + * ipaCreate() function directly and wraps the returned context in an\n> > + * IPAContextWrapper that exposes an IPAInterface.\n> > + *\n> > + * ~~~~\n> > + * +---------------+\n> > + * | Pipeline |\n> > + * | Handler |\n> > + * +---------------+\n> > + * |\n> > + * v\n> > + * +---------------+ +---------------+\n> > + * | IPA | | Open Source |\n> > + * | Interface | | IPA Module |\n> > + * | - - - - - - - | | - - - - - - - |\n> > + * | IPA Context | ipa_operations | ipa_context |\n> > + * | Wrapper | ----------------> | |\n> > + * +---------------+ +---------------+\n> > + * ~~~~\n> > + *\n> > + * For an isolated module, the manager instantiates an IPAProxy which spawns a\n> > + * new process for an IPA proxy worker. The worker loads the IPA module and\n> > + * creates the IPA context. The IPAProxy alse exposes an IPAInterface.\n> > + *\n> > + * ~~~~\n> > + * +---------------+ +---------------+\n> > + * | Pipeline | | Closed Source |\n> > + * | Handler | | IPA Module |\n> > + * +---------------+ | - - - - - - - |\n> > + * | | ipa_context |\n> > + * v | |\n> > + * +---------------+ +---------------+\n> > + * | IPA | ipa_operations ^\n> > + * | Interface | |\n> > + * | - - - - - - - | +---------------+\n> > + * | IPA Proxy | operations | IPA Proxy |\n> > + * | | ----------------> | Worker |\n> > + * +---------------+ over IPC +---------------+\n> > + * ~~~~\n> > + *\n> > + * The IPAInterface implemented by the IPAContextWrapper or IPAProxy is\n> > + * returned to the pipeline handler, and all interactions with the IPA context\n> > + * go the same interface regarless of process isolation.\n> > + *\n> > + * In all cases the data passed to the IPAInterface methods is serialised to\n> > + * Plain Old Data, either for the purpose of passing it to the IPA context\n> > + * plain C API, or to transmit the data to the isolated process through IPC.\n> > */\n> > \n> > IPAManager::IPAManager()\n> > @@ -177,7 +238,11 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe,\n> > \tif (!m->load())\n> > \t\treturn nullptr;\n> > \n> > -\treturn m->createInstance();\n> > +\tstruct ipa_context *ctx = m->createContext();\n> > +\tif (!ctx)\n> > +\t\treturn nullptr;\n> > +\n> > +\treturn utils::make_unique(ctx);\n> > }\n> > \n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> > index 99d308efd47b..9f5a01418f73 100644\n> > --- a/src/libcamera/ipa_module.cpp\n> > +++ b/src/libcamera/ipa_module.cpp\n> > @@ -385,13 +385,13 @@ const std::string &IPAModule::path() const\n> > /**\n> > * \\brief Load the IPA implementation factory from the shared object\n> > *\n> > - * The IPA module shared object implements an IPAInterface class to be used\n> > + * The IPA module shared object implements an ipa_context object to be used\n> > * by pipeline handlers. This method loads the factory function from the\n> > - * shared object. Later, createInstance() can be called to instantiate the\n> > - * IPAInterface.\n> > + * shared object. Later, createContext() can be called to instantiate the\n> > + * ipa_context.\n> > *\n> > * This method only needs to be called successfully once, after which\n> > - * createInstance() can be called as many times as IPAInterface instances are\n> > + * createContext() can be called as many times as ipa_context instances are\n> > * needed.\n> > *\n> > * Calling this function on an invalid module (as returned by isValid()) is\n> > @@ -433,24 +433,25 @@ bool IPAModule::load()\n> > }\n> > \n> > /**\n> > - * \\brief Instantiate an IPAInterface\n> > + * \\brief Instantiate an IPA context\n> > *\n> > - * After loading the IPA module with load(), this method creates an\n> > - * instance of the IPA module interface.\n> > + * After loading the IPA module with load(), this method creates an instance of\n> > + * the IPA module context. Ownership of the context is passed to the caller, and\n> > + * the context shall be destroyed by calling the \\ref ipa_operations::destroy\n> > + * \"ipa_context::ops::destroy()\" operation.\n> > *\n> > * Calling this function on a module that has not yet been loaded, or an\n> > * invalid module (as returned by load() and isValid(), respectively) is\n> > * an error.\n> > *\n> > - * \\return The IPA implementation as a new IPAInterface instance on success,\n> > - * or nullptr on error\n> > + * \\return The IPA context on success, or nullptr on error\n> > */\n> > -std::unique_ptr IPAModule::createInstance()\n> > +struct ipa_context *IPAModule::createContext()\n> > {\n> > \tif (!valid_ || !loaded_)\n> > \t\treturn nullptr;\n> > \n> > -\treturn std::unique_ptr(ipaCreate_());\n> > +\treturn ipaCreate_();\n> > }\n> > \n> > /**\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 755149c55c7b..d3ae305e5655 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -12,6 +12,7 @@ libcamera_sources = files([\n> > 'event_notifier.cpp',\n> > 'formats.cpp',\n> > 'geometry.cpp',\n> > + 'ipa_context_wrapper.cpp',\n> > 'ipa_interface.cpp',\n> > 'ipa_manager.cpp',\n> > 'ipa_module.cpp',\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index a401b1099f3c..612edcda69d6 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -362,7 +362,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> > \n> > \tipa_ = IPAManager::instance()->createIPA(this, 0, 0);\n> > -\tif (ipa_ == nullptr)\n> > +\tif (!ipa_)\n> > \t\tLOG(VIMC, Warning) << \"no matching IPA found\";\n> > \n> > \tstd::unique_ptr data = utils::make_unique(this);\n> > diff --git a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> > index a10761e52b32..07380c16e2d5 100644\n> > --- a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> > +++ b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> > @@ -72,9 +72,9 @@ int main(int argc, char **argv)\n> > \t}\n> > \tsocket.readyRead.connect(&readyRead);\n> > \n> > -\tstd::unique_ptr ipa = ipam->createInstance();\n> > -\tif (!ipa) {\n> > -\t\tLOG(IPAProxyLinuxWorker, Error) << \"Failed to create IPA interface\";\n> > +\tstruct ipa_context *ipac = ipam->createContext();\n> > +\tif (!ipac) {\n> > +\t\tLOG(IPAProxyLinuxWorker, Error) << \"Failed to create IPA context\";\n> > \t\treturn EXIT_FAILURE;\n> > \t}\n> > \n> > @@ -85,5 +85,7 @@ int main(int argc, char **argv)\n> > \twhile (1)\n> > \t\tdispatcher->processEvents();\n> > \n> > +\tipac->ops->destroy(ipac);\n> > +\n> > \treturn 0;\n> > }","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AB94360BB6\n\tfor ;\n\tWed, 18 Sep 2019 20:06:02 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [62.28.174.186])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCF7F325;\n\tWed, 18 Sep 2019 20:06:01 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1568829962;\n\tbh=n1BC92q9c9nMuqqerkTixZhNPmzHjFZ/7WkwS33fXtQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I0ihx83Ps3dN3/eqUinkv6F0Z7wVNh1WYDJD3Y9ziE8nhKP668pGwQxrUijOb/Xek\n\tKeTLX/AMnzAqoRYnDXaHsjJ10CJkjZ4QDecl97YlzMQB7lwlzpHuxeLlXtIJwibR4j\n\tqNfpECBnOuNqvQczqbrLXUEx6t1d+Mp8vGPjM/rM=","Date":"Wed, 18 Sep 2019 21:05:52 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190918180552.GD4734@pendragon.ideasonboard.com>","References":"<20190915190408.2507-1-laurent.pinchart@ideasonboard.com>\n\t<20190917093603.GC15610@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190917093603.GC15610@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] ipa: Convert the IPA API to plain C","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 18 Sep 2019 18:06:02 -0000"}},{"id":2674,"web_url":"https://patchwork.libcamera.org/comment/2674/","msgid":"<20190919064357.GA1727@bigcity.dyn.berto.se>","date":"2019-09-19T06:43:57","subject":"Re: [libcamera-devel] [PATCH] ipa: Convert the IPA API to plain C","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2019-09-18 21:05:52 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Tue, Sep 17, 2019 at 11:36:03AM +0200, Niklas Söderlund wrote:\n> > On 2019-09-15 22:04:08 +0300, Laurent Pinchart wrote:\n> > > From: Jacopo Mondi \n> > > \n> > > The C++ objects that are expected to convey data through the IPA API\n> > > will have associated methods that would require IPAs to link to\n> > > libcamera. Even though the libcamera license allows this, suppliers of\n> > > closed-source IPAs may have a different interpretation. To ease their\n> > > mind and clearly separate vendor code and libcamera code, turn the IPA\n> > > API to plain C. The corresponding C objects will be stored in plain C\n> > > structures or have their binary format documented, removing the need for\n> > > linking to libcamera code on the IPA side.\n> > > \n> > > This is implemented by adding two new C structures, ipa_context and\n> > > ipa_operations. The ipa_operations contains function pointers for all\n> > > the IPA API operations. The ipa_context represents a context of\n> > > operation for the IPA, and is passed to the IPA oparations. The\n> > > IPAInterface class is retained as it is easier to use than a plain C API\n> > > for pipeline handlers, with a new IPAWrapper class that wraps the\n> > > ipa_context and ipa_operations into and IPAInterface.\n> > > \n> > > On the IPA module side usage of IPAInterface may be desired for IPAs\n> > > implemented in C++ that want to link to libcamera. For those IPAs, a new\n> > > IPAWrapperContext helper class is introduce to wrap the IPAInterface\n> > > implemented internally by the IPA module into an ipa_context and\n> > > ipa_operations.\n> > > \n> > > Signed-off-by: Jacopo Mondi \n> > > Signed-off-by: Laurent Pinchart \n> > \n> > The fix in vimc.cpp should really be it's own patch as it's not related \n> > to the C API ;-) Other than that,\n> \n> Agreed, it could have been split. It was the result of a previous API\n> version, and when undoing that I've left the != nullptr to ! change.\n> Would you like me to send a new version, or can I leave it as-is this\n> time ?\n\nI'm fine either way ;-)\n\n> \n> > Reviewed-by: Niklas Söderlund \n> > \n> > > ---\n> > > Documentation/Doxyfile.in | 1 +\n> > > Documentation/meson.build | 2 +\n> > > include/ipa/ipa_interface.h | 16 ++++\n> > > src/ipa/ipa_dummy.cpp | 6 +-\n> > > src/ipa/libipa/ipa_interface_wrapper.cpp | 74 ++++++++++++++++++\n> > > src/ipa/libipa/ipa_interface_wrapper.h | 29 +++++++\n> > > src/ipa/libipa/meson.build | 10 +++\n> > > src/ipa/meson.build | 3 +\n> > > src/libcamera/include/ipa_context_wrapper.h | 26 +++++++\n> > > src/libcamera/include/ipa_module.h | 5 +-\n> > > src/libcamera/include/meson.build | 1 +\n> > > src/libcamera/ipa_context_wrapper.cpp | 52 +++++++++++++\n> > > src/libcamera/ipa_interface.cpp | 77 ++++++++++++++++++-\n> > > src/libcamera/ipa_manager.cpp | 67 +++++++++++++++-\n> > > src/libcamera/ipa_module.cpp | 23 +++---\n> > > src/libcamera/meson.build | 1 +\n> > > src/libcamera/pipeline/vimc.cpp | 2 +-\n> > > .../proxy/worker/ipa_proxy_linux_worker.cpp | 8 +-\n> > > 18 files changed, 381 insertions(+), 22 deletions(-)\n> > > create mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > create mode 100644 src/ipa/libipa/ipa_interface_wrapper.h\n> > > create mode 100644 src/ipa/libipa/meson.build\n> > > create mode 100644 src/libcamera/include/ipa_context_wrapper.h\n> > > create mode 100644 src/libcamera/ipa_context_wrapper.cpp\n> > > \n> > > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > > index ecc058eec683..2ab93687755f 100644\n> > > --- a/Documentation/Doxyfile.in\n> > > +++ b/Documentation/Doxyfile.in\n> > > @@ -793,6 +793,7 @@ WARN_LOGFILE =\n> > > \n> > > INPUT = \"@TOP_SRCDIR@/include/ipa\" \\\n> > > \t\t\t \"@TOP_SRCDIR@/include/libcamera\" \\\n> > > +\t\t\t \"@TOP_SRCDIR@/src/ipa/libipa\" \\\n> > > \t\t\t \"@TOP_SRCDIR@/src/libcamera\"\n> > > \n> > > # This tag can be used to specify the character encoding of the source files\n> > > diff --git a/Documentation/meson.build b/Documentation/meson.build\n> > > index 4ff3fbeb0674..9136506f5d9c 100644\n> > > --- a/Documentation/meson.build\n> > > +++ b/Documentation/meson.build\n> > > @@ -24,6 +24,8 @@ if doxygen.found()\n> > > libcamera_ipa_api,\n> > > libcamera_headers,\n> > > libcamera_sources,\n> > > + libipa_headers,\n> > > + libipa_sources,\n> > > ],\n> > > output : 'api-html',\n> > > command : [doxygen, doxyfile],\n> > > diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\n> > > index 9bbc4cf58ec6..f1ebac20f151 100644\n> > > --- a/include/ipa/ipa_interface.h\n> > > +++ b/include/ipa/ipa_interface.h\n> > > @@ -7,6 +7,21 @@\n> > > #ifndef __LIBCAMERA_IPA_INTERFACE_H__\n> > > #define __LIBCAMERA_IPA_INTERFACE_H__\n> > > \n> > > +#ifdef __cplusplus\n> > > +extern \"C\" {\n> > > +#endif\n> > > +\n> > > +struct ipa_context {\n> > > +\tconst struct ipa_operations *ops;\n> > > +};\n> > > +\n> > > +struct ipa_operations {\n> > > +\tvoid (*destroy)(struct ipa_context *ctx);\n> > > +};\n> > > +\n> > > +#ifdef __cplusplus\n> > > +}\n> > > +\n> > > namespace libcamera {\n> > > \n> > > class IPAInterface\n> > > @@ -16,5 +31,6 @@ public:\n> > > };\n> > > \n> > > } /* namespace libcamera */\n> > > +#endif\n> > > \n> > > #endif /* __LIBCAMERA_IPA_INTERFACE_H__ */\n> > > diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp\n> > > index c833e5fb0b2d..6dc9448a3f56 100644\n> > > --- a/src/ipa/ipa_dummy.cpp\n> > > +++ b/src/ipa/ipa_dummy.cpp\n> > > @@ -8,6 +8,8 @@\n> > > #include \n> > > #include \n> > > \n> > > +#include \"libipa/ipa_interface_wrapper.h\"\n> > > +\n> > > namespace libcamera {\n> > > \n> > > class IPADummy : public IPAInterface\n> > > @@ -27,9 +29,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> > > \tLICENSE,\n> > > };\n> > > \n> > > -IPAInterface *ipaCreate()\n> > > +struct ipa_context *ipaCreate()\n> > > {\n> > > -\treturn new IPADummy();\n> > > +\treturn new IPAInterfaceWrapper(new IPADummy());\n> > > }\n> > > };\n> > > \n> > > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > new file mode 100644\n> > > index 000000000000..aacd189851c3\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > @@ -0,0 +1,74 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * ipa_interface_wrapper.cpp - Image Processing Algorithm interface wrapper\n> > > + */\n> > > +\n> > > +#include \"ipa_interface_wrapper.h\"\n> > > +\n> > > +#include \n> > > +\n> > > +/**\n> > > + * \\file ipa_interface_wrapper.h\n> > > + * \\brief Image Processing Algorithm interface wrapper\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +/**\n> > > + * \\class IPAInterfaceWrapper\n> > > + * \\brief Wrap an IPAInterface and expose it as an ipa_context\n> > > + *\n> > > + * This class implements the ipa_context API based on a provided IPAInterface.\n> > > + * It helps IPAs that implement the IPAInterface API to provide the external\n> > > + * ipa_context API.\n> > > + *\n> > > + * To use the wrapper, an IPA module simple creates a new instance of its\n> > > + * IPAInterface implementation, and passes it to the constructor of the\n> > > + * IPAInterfaceWrapper. As IPAInterfaceWrapper inherits from ipa_context, the\n> > > + * constructed wrapper can then be directly returned from the IPA module's\n> > > + * ipaCreate() function.\n> > > + *\n> > > + * \\code{.cpp}\n> > > + * class MyIPA : public IPAInterface\n> > > + * {\n> > > + * \t...\n> > > + * };\n> > > + *\n> > > + * struct ipa_context *ipaCreate()\n> > > + * {\n> > > + * \treturn new IPAInterfaceWrapper(new MyIPA());\n> > > + * }\n> > > + * \\endcode\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Construct an IPAInterfaceWrapper wrapping \\a interface\n> > > + * \\param[in] interface The interface to wrap\n> > > + */\n> > > +IPAInterfaceWrapper::IPAInterfaceWrapper(IPAInterface *interface)\n> > > +\t: ipa(interface)\n> > > +{\n> > > +\tops = &operations;\n> > > +}\n> > > +\n> > > +void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx)\n> > > +{\n> > > +\tIPAInterfaceWrapper *ctx = static_cast(_ctx);\n> > > +\n> > > +\tdelete ctx->ipa;\n> > > +\tdelete ctx;\n> > > +}\n> > > +\n> > > +#ifndef __DOXYGEN__\n> > > +/*\n> > > + * This construct confuses Doygen and makes it believe that all members of the\n> > > + * operations is a member of IPAInterfaceWrapper. It must thus be hidden.\n> > > + */\n> > > +const struct ipa_operations IPAInterfaceWrapper::operations = {\n> > > +\t.destroy = &IPAInterfaceWrapper::destroy,\n> > > +};\n> > > +#endif\n> > > +\n> > > +}; /* namespace libcamera */\n> > > diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h\n> > > new file mode 100644\n> > > index 000000000000..d2ab46f50d3c\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/ipa_interface_wrapper.h\n> > > @@ -0,0 +1,29 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * ipa_interface_wrapper.h - Image Processing Algorithm interface wrapper\n> > > + */\n> > > +#ifndef __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__\n> > > +#define __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__\n> > > +\n> > > +#include \n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +class IPAInterfaceWrapper : public ipa_context\n> > > +{\n> > > +public:\n> > > +\tIPAInterfaceWrapper(IPAInterface *interface);\n> > > +\n> > > +private:\n> > > +\tstatic void destroy(struct ipa_context *ctx);\n> > > +\n> > > +\tstatic const struct ipa_operations operations;\n> > > +\n> > > +\tIPAInterface *ipa;\n> > > +};\n> > > +\n> > > +} /* namespace libcamera */\n> > > +\n> > > +#endif /* __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ */\n> > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > > new file mode 100644\n> > > index 000000000000..ab3515ed2021\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/meson.build\n> > > @@ -0,0 +1,10 @@\n> > > +libipa_headers = files([\n> > > + 'ipa_interface_wrapper.h',\n> > > +])\n> > > +\n> > > +libipa_sources = files([\n> > > + 'ipa_interface_wrapper.cpp',\n> > > +])\n> > > +\n> > > +libipa = static_library('libipa', libipa_sources,\n> > > + dependencies : libcamera_dep)\n> > > diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> > > index f09915bc1388..6483ea66a478 100644\n> > > --- a/src/ipa/meson.build\n> > > +++ b/src/ipa/meson.build\n> > > @@ -1,3 +1,5 @@\n> > > +subdir('libipa')\n> > > +\n> > > ipa_dummy_sources = [\n> > > ['ipa_dummy', 'LGPL-2.1-or-later'],\n> > > ['ipa_dummy_isolate', 'Proprietary'],\n> > > @@ -9,6 +11,7 @@ foreach t : ipa_dummy_sources\n> > > ipa = shared_module(t[0], 'ipa_dummy.cpp',\n> > > name_prefix : '',\n> > > include_directories : libcamera_includes,\n> > > + link_with : libipa,\n> > > install : true,\n> > > install_dir : ipa_install_dir,\n> > > cpp_args : '-DLICENSE=\"' + t[1] + '\"')\n> > > diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h\n> > > new file mode 100644\n> > > index 000000000000..12894ac6885e\n> > > --- /dev/null\n> > > +++ b/src/libcamera/include/ipa_context_wrapper.h\n> > > @@ -0,0 +1,26 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * ipa_context_wrapper.h - Image Processing Algorithm context wrapper\n> > > + */\n> > > +#ifndef __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__\n> > > +#define __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__\n> > > +\n> > > +#include \n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +class IPAContextWrapper final : public IPAInterface\n> > > +{\n> > > +public:\n> > > +\tIPAContextWrapper(struct ipa_context *context);\n> > > +\t~IPAContextWrapper();\n> > > +\n> > > +private:\n> > > +\tstruct ipa_context *ctx_;\n> > > +};\n> > > +\n> > > +} /* namespace libcamera */\n> > > +\n> > > +#endif /* __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__ */\n> > > diff --git a/src/libcamera/include/ipa_module.h b/src/libcamera/include/ipa_module.h\n> > > index 97737587ab3a..2028b76a1913 100644\n> > > --- a/src/libcamera/include/ipa_module.h\n> > > +++ b/src/libcamera/include/ipa_module.h\n> > > @@ -7,7 +7,6 @@\n> > > #ifndef __LIBCAMERA_IPA_MODULE_H__\n> > > #define __LIBCAMERA_IPA_MODULE_H__\n> > > \n> > > -#include \n> > > #include \n> > > \n> > > #include \n> > > @@ -30,7 +29,7 @@ public:\n> > > \n> > > \tbool load();\n> > > \n> > > -\tstd::unique_ptr createInstance();\n> > > +\tstruct ipa_context *createContext();\n> > > \n> > > \tbool match(PipelineHandler *pipe,\n> > > \t\t uint32_t minVersion, uint32_t maxVersion) const;\n> > > @@ -45,7 +44,7 @@ private:\n> > > \tbool loaded_;\n> > > \n> > > \tvoid *dlHandle_;\n> > > -\ttypedef IPAInterface *(*IPAIntfFactory)(void);\n> > > +\ttypedef struct ipa_context *(*IPAIntfFactory)(void);\n> > > \tIPAIntfFactory ipaCreate_;\n> > > \n> > > \tint loadIPAModuleInfo();\n> > > diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build\n> > > index 933be8543a8d..0abb978a389a 100644\n> > > --- a/src/libcamera/include/meson.build\n> > > +++ b/src/libcamera/include/meson.build\n> > > @@ -5,6 +5,7 @@ libcamera_headers = files([\n> > > 'device_enumerator_udev.h',\n> > > 'event_dispatcher_poll.h',\n> > > 'formats.h',\n> > > + 'ipa_context_wrapper.h',\n> > > 'ipa_manager.h',\n> > > 'ipa_module.h',\n> > > 'ipa_proxy.h',\n> > > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> > > new file mode 100644\n> > > index 000000000000..87ff98d45c99\n> > > --- /dev/null\n> > > +++ b/src/libcamera/ipa_context_wrapper.cpp\n> > > @@ -0,0 +1,52 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * ipa_context_wrapper.cpp - Image Processing Algorithm context wrapper\n> > > + */\n> > > +\n> > > +#include \"ipa_context_wrapper.h\"\n> > > +\n> > > +#include \n> > > +\n> > > +/**\n> > > + * \\file ipa_context_wrapper.h\n> > > + * \\brief Image Processing Algorithm context wrapper\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +/**\n> > > + * \\class IPAContextWrapper\n> > > + * \\brief Wrap an ipa_context and expose it as an IPAInterface\n> > > + *\n> > > + * The IPAContextWrapper class wraps an ipa_context, provided by an IPA module, and\n> > > + * exposes an IPAInterface. This mechanism is used for IPAs that are not\n> > > + * isolated in a separate process to allow direct calls from pipeline handler\n> > > + * using the IPAInterface API instead of the lower-level ipa_context API.\n> > > + *\n> > > + * The IPAInterface methods are converted to the ipa_context API by serialising\n> > > + * all C++ arguments into plain C structures or byte arrays that contain no\n> > > + * pointer, as required by the ipa_context API.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Construct an IPAContextWrapper instance that wraps the \\a context\n> > > + * \\param[in] context The IPA module context\n> > > + *\n> > > + * Ownership of the \\a context is passed to the IPAContextWrapper. The context remains\n> > > + * valid for the whole lifetime of the wrapper and is destroyed automatically\n> > > + * with it.\n> > > + */\n> > > +IPAContextWrapper::IPAContextWrapper(struct ipa_context *context)\n> > > +\t: ctx_(context)\n> > > +{\n> > > +}\n> > > +\n> > > +IPAContextWrapper::~IPAContextWrapper()\n> > > +{\n> > > +\tif (ctx_)\n> > > +\t\tctx_->ops->destroy(ctx_);\n> > > +}\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> > > index f70d91ded1ab..6e83aab0fb73 100644\n> > > --- a/src/libcamera/ipa_interface.cpp\n> > > +++ b/src/libcamera/ipa_interface.cpp\n> > > @@ -10,13 +10,88 @@\n> > > /**\n> > > * \\file ipa_interface.h\n> > > * \\brief Image Processing Algorithm interface\n> > > + *\n> > > + * libcamera interfaces with IPA modules through a plain C interface. IPA\n> > > + * modules shall expose a public function name ipaCreate() with the following\n> > > + * prototype.\n> > > + *\n> > > + * \\code{.c}\n> > > + * struct ipa_context *ipaCreate();\n> > > + * \\endcode\n> > > + *\n> > > + * The ipaCreate() function creates an instance of an IPA context, which models\n> > > + * a context of execution for the IPA. IPA modules shall support creating one\n> > > + * or multiple contexts, as required by their associated pipeline handler.\n> > > + *\n> > > + * The IPA module operations are defined in the struct ipa_operations. An IPA\n> > > + * module stores a pointer to the operations corresponding to its context in\n> > > + * the ipa_context::ops field. That pointer is immutable for the lifetime of\n> > > + * the context, and may be different for multiple contexts created by the same\n> > > + * IPA module.\n> > > + *\n> > > + * All argument to ipa_operations members are Plain Old Data and are documented\n> > > + * either in the form of C data types, or as a textual description for byte\n> > > + * arrays that can't be expressed using C data types (such as variable-length\n> > > + * arrays). IPA modules can thus use the C API without calling into libcamera\n> > > + * to access the data passed to the IPA operations.\n> > > + *\n> > > + * The IPAInterface class is a C++ representation of the ipa_operations, using\n> > > + * C++ data classes provided by libcamera. This is the API exposed to pipeline\n> > > + * handlers to communicate with IPA modules. IPA modules may use the\n> > > + * IPAInterface API internally if they want to benefit from the data and helper\n> > > + * classes offered by libcamera.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\struct ipa_context\n> > > + * \\brief IPA module context of execution\n> > > + *\n> > > + * This structure models a context of execution for an IPA module. It is\n> > > + * instantiated by the IPA module ipaCreate() function. IPA modules allocate\n> > > + * context instances in an implementation-defined way, contexts shall thus be\n> > > + * destroyed using the ipa_operation::destroy operation only.\n> > > + *\n> > > + * The ipa_context structure provides a pointer to the IPA operations. It shall\n> > > + * otherwise be treated as a constant black-box cookie and passed unmodified to\n> > > + * the operations defined in struct ipa_operations.\n> > > + *\n> > > + * IPA modules are expected to extend struct ipa_context by inheriting from it,\n> > > + * either through structure embedding to model inheritance in plain C, or\n> > > + * through C++ class inheritance. A simple example of the latter is available\n> > > + * in the IPAContextWrapper class implementation.\n> > > + *\n> > > + * \\var ipa_context::ops\n> > > + * \\brief The IPA context operations\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\struct ipa_operations\n> > > + * \\brief IPA context operations as a set of function pointers\n> > > + *\n> > > + * To allow for isolation of IPA modules in separate processes, the functions\n> > > + * defined in the ipa_operations structure return only data related to the\n> > > + * libcamera side of the operations. In particular, error related to the\n> > > + * libcamera side of the IPC may be returned. Data returned by the IPA,\n> > > + * including status information, shall be provided through callbacks from the\n> > > + * IPA to libcamera.\n> > > + *\n> > > + * \\var ipa_operations::destroy\n> > > + * \\brief Destroy the ipa_context created by the module's ipaCreate() function\n> > > */\n> > > \n> > > namespace libcamera {\n> > > \n> > > /**\n> > > * \\class IPAInterface\n> > > - * \\brief Interface for IPA implementation\n> > > + * \\brief IPA module interface\n> > > + *\n> > > + * This pure virtual class defines a C++ API corresponding to the ipa_context\n> > > + * and ipa_operations API. It is used by pipeline handlers to interact with IPA\n> > > + * modules, and may be used internally in IPA modules if desired to benefit\n> > > + * from the data and helper classes provided by libcamera.\n> > > + *\n> > > + * As for the operations defined in struct ipa_operations, the methods defined\n> > > + * by this class shall not return data from the IPA.\n> > > */\n> > > \n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> > > index 4276d995bdd5..0a908eae6261 100644\n> > > --- a/src/libcamera/ipa_manager.cpp\n> > > +++ b/src/libcamera/ipa_manager.cpp\n> > > @@ -11,6 +11,7 @@\n> > > #include \n> > > #include \n> > > \n> > > +#include \"ipa_context_wrapper.h\"\n> > > #include \"ipa_module.h\"\n> > > #include \"ipa_proxy.h\"\n> > > #include \"log.h\"\n> > > @@ -29,6 +30,66 @@ LOG_DEFINE_CATEGORY(IPAManager)\n> > > /**\n> > > * \\class IPAManager\n> > > * \\brief Manager for IPA modules\n> > > + *\n> > > + * The IPA module manager discovers IPA modules from disk, queries and loads\n> > > + * them, and creates IPA contexts. It supports isolation of the modules in a\n> > > + * separate process with IPC communication and offers a unified IPAInterface\n> > > + * view of the IPA contexts to pipeline handlers regardless of whether the\n> > > + * modules are isolated or loaded in the same process.\n> > > + *\n> > > + * Module isolation is based on the module licence. Open-source modules are\n> > > + * loaded without isolation, while closed-source module are forcefully isolated.\n> > > + * The isolation mechanism ensures that no code from a closed-source module is\n> > > + * ever run in the libcamera process.\n> > > + *\n> > > + * To create an IPA context, pipeline handlers call the IPAManager::ipaCreate()\n> > > + * method. For a directly loaded module, the manager calls the module's\n> > > + * ipaCreate() function directly and wraps the returned context in an\n> > > + * IPAContextWrapper that exposes an IPAInterface.\n> > > + *\n> > > + * ~~~~\n> > > + * +---------------+\n> > > + * | Pipeline |\n> > > + * | Handler |\n> > > + * +---------------+\n> > > + * |\n> > > + * v\n> > > + * +---------------+ +---------------+\n> > > + * | IPA | | Open Source |\n> > > + * | Interface | | IPA Module |\n> > > + * | - - - - - - - | | - - - - - - - |\n> > > + * | IPA Context | ipa_operations | ipa_context |\n> > > + * | Wrapper | ----------------> | |\n> > > + * +---------------+ +---------------+\n> > > + * ~~~~\n> > > + *\n> > > + * For an isolated module, the manager instantiates an IPAProxy which spawns a\n> > > + * new process for an IPA proxy worker. The worker loads the IPA module and\n> > > + * creates the IPA context. The IPAProxy alse exposes an IPAInterface.\n> > > + *\n> > > + * ~~~~\n> > > + * +---------------+ +---------------+\n> > > + * | Pipeline | | Closed Source |\n> > > + * | Handler | | IPA Module |\n> > > + * +---------------+ | - - - - - - - |\n> > > + * | | ipa_context |\n> > > + * v | |\n> > > + * +---------------+ +---------------+\n> > > + * | IPA | ipa_operations ^\n> > > + * | Interface | |\n> > > + * | - - - - - - - | +---------------+\n> > > + * | IPA Proxy | operations | IPA Proxy |\n> > > + * | | ----------------> | Worker |\n> > > + * +---------------+ over IPC +---------------+\n> > > + * ~~~~\n> > > + *\n> > > + * The IPAInterface implemented by the IPAContextWrapper or IPAProxy is\n> > > + * returned to the pipeline handler, and all interactions with the IPA context\n> > > + * go the same interface regarless of process isolation.\n> > > + *\n> > > + * In all cases the data passed to the IPAInterface methods is serialised to\n> > > + * Plain Old Data, either for the purpose of passing it to the IPA context\n> > > + * plain C API, or to transmit the data to the isolated process through IPC.\n> > > */\n> > > \n> > > IPAManager::IPAManager()\n> > > @@ -177,7 +238,11 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe,\n> > > \tif (!m->load())\n> > > \t\treturn nullptr;\n> > > \n> > > -\treturn m->createInstance();\n> > > +\tstruct ipa_context *ctx = m->createContext();\n> > > +\tif (!ctx)\n> > > +\t\treturn nullptr;\n> > > +\n> > > +\treturn utils::make_unique(ctx);\n> > > }\n> > > \n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> > > index 99d308efd47b..9f5a01418f73 100644\n> > > --- a/src/libcamera/ipa_module.cpp\n> > > +++ b/src/libcamera/ipa_module.cpp\n> > > @@ -385,13 +385,13 @@ const std::string &IPAModule::path() const\n> > > /**\n> > > * \\brief Load the IPA implementation factory from the shared object\n> > > *\n> > > - * The IPA module shared object implements an IPAInterface class to be used\n> > > + * The IPA module shared object implements an ipa_context object to be used\n> > > * by pipeline handlers. This method loads the factory function from the\n> > > - * shared object. Later, createInstance() can be called to instantiate the\n> > > - * IPAInterface.\n> > > + * shared object. Later, createContext() can be called to instantiate the\n> > > + * ipa_context.\n> > > *\n> > > * This method only needs to be called successfully once, after which\n> > > - * createInstance() can be called as many times as IPAInterface instances are\n> > > + * createContext() can be called as many times as ipa_context instances are\n> > > * needed.\n> > > *\n> > > * Calling this function on an invalid module (as returned by isValid()) is\n> > > @@ -433,24 +433,25 @@ bool IPAModule::load()\n> > > }\n> > > \n> > > /**\n> > > - * \\brief Instantiate an IPAInterface\n> > > + * \\brief Instantiate an IPA context\n> > > *\n> > > - * After loading the IPA module with load(), this method creates an\n> > > - * instance of the IPA module interface.\n> > > + * After loading the IPA module with load(), this method creates an instance of\n> > > + * the IPA module context. Ownership of the context is passed to the caller, and\n> > > + * the context shall be destroyed by calling the \\ref ipa_operations::destroy\n> > > + * \"ipa_context::ops::destroy()\" operation.\n> > > *\n> > > * Calling this function on a module that has not yet been loaded, or an\n> > > * invalid module (as returned by load() and isValid(), respectively) is\n> > > * an error.\n> > > *\n> > > - * \\return The IPA implementation as a new IPAInterface instance on success,\n> > > - * or nullptr on error\n> > > + * \\return The IPA context on success, or nullptr on error\n> > > */\n> > > -std::unique_ptr IPAModule::createInstance()\n> > > +struct ipa_context *IPAModule::createContext()\n> > > {\n> > > \tif (!valid_ || !loaded_)\n> > > \t\treturn nullptr;\n> > > \n> > > -\treturn std::unique_ptr(ipaCreate_());\n> > > +\treturn ipaCreate_();\n> > > }\n> > > \n> > > /**\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index 755149c55c7b..d3ae305e5655 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -12,6 +12,7 @@ libcamera_sources = files([\n> > > 'event_notifier.cpp',\n> > > 'formats.cpp',\n> > > 'geometry.cpp',\n> > > + 'ipa_context_wrapper.cpp',\n> > > 'ipa_interface.cpp',\n> > > 'ipa_manager.cpp',\n> > > 'ipa_module.cpp',\n> > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > > index a401b1099f3c..612edcda69d6 100644\n> > > --- a/src/libcamera/pipeline/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc.cpp\n> > > @@ -362,7 +362,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > > \t\treturn false;\n> > > \n> > > \tipa_ = IPAManager::instance()->createIPA(this, 0, 0);\n> > > -\tif (ipa_ == nullptr)\n> > > +\tif (!ipa_)\n> > > \t\tLOG(VIMC, Warning) << \"no matching IPA found\";\n> > > \n> > > \tstd::unique_ptr data = utils::make_unique(this);\n> > > diff --git a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> > > index a10761e52b32..07380c16e2d5 100644\n> > > --- a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> > > +++ b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp\n> > > @@ -72,9 +72,9 @@ int main(int argc, char **argv)\n> > > \t}\n> > > \tsocket.readyRead.connect(&readyRead);\n> > > \n> > > -\tstd::unique_ptr ipa = ipam->createInstance();\n> > > -\tif (!ipa) {\n> > > -\t\tLOG(IPAProxyLinuxWorker, Error) << \"Failed to create IPA interface\";\n> > > +\tstruct ipa_context *ipac = ipam->createContext();\n> > > +\tif (!ipac) {\n> > > +\t\tLOG(IPAProxyLinuxWorker, Error) << \"Failed to create IPA context\";\n> > > \t\treturn EXIT_FAILURE;\n> > > \t}\n> > > \n> > > @@ -85,5 +85,7 @@ int main(int argc, char **argv)\n> > > \twhile (1)\n> > > \t\tdispatcher->processEvents();\n> > > \n> > > +\tipac->ops->destroy(ipac);\n> > > +\n> > > \treturn 0;\n> > > }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9418160BED\n\tfor ;\n\tThu, 19 Sep 2019 08:43:59 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id f5so2367699ljg.8\n\tfor ;\n\tWed, 18 Sep 2019 23:43:59 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\t81sm1438715lje.70.2019.09.18.23.43.57\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 18 Sep 2019 23:43:57 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=o4TcSmPRBIwkiCyggzp7iFImTVsjGLo5cbZR+YtRFgE=;\n\tb=Sf8n6XxmkjYY2cPp/vcMkt8S95pp2JvwHZqvbqgyX2hiUniG7pxbeHb7TOHwSmdVH7\n\tDXSv7OQX3CD1CIgTfl+dUmFpsTFsrbB8q6J8PpEIui9A6DxR8zizDidplkxX8tiL2j4X\n\tBC1RYEBpfi1ckR4VmfaNATQEJ8gOFNjOjGTbK5zqQHef1vjolHb9QGdAsmJecaQAT+/X\n\t5jKyTrud5L5psWpwp6s7kdbDnnluyAvp8Pe5r4XyLbl1V16FlO4ZozBbXoxKWHSqK/cp\n\taHxfFMfDAzHRqBuQFtPBEvwBcuCohDhuiCzpZsmHxJy/xKQGtZSK4ukV9aqLpZbRaMNr\n\tJbWg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=o4TcSmPRBIwkiCyggzp7iFImTVsjGLo5cbZR+YtRFgE=;\n\tb=F9FOLnzKo0HI5knszteFeYPAtr3MYl+TLllqG5q5SSU3gpZcZVVsTneOECH08QKq9A\n\tpN/DSyXTBEOzgNPd+a6hoSKFYUu8XsA2ggGnvNPj2u0/O4/mOH6NtV6OBUFlICwyURej\n\t3rG3Os4Hb3If/OJGB6UJrOKKIRIydkkDUZyyBP0cNyMjBxEkw131kEzRNLQlQEllEA0L\n\tIRxHSd3SdupLxmhsPZwnxZGcvJEZOmYtLyp+qOjUunTQRi6oaAwZyU+yVjiADWpxWEWa\n\tzocdAmiAluBM7FX/r37fCCNCklOzlOuXnLdicNgkzy/72pLhti0/dkmO5+S+AoVMAw/Z\n\tlgsw==","X-Gm-Message-State":"APjAAAWJ9sRiYuYTjb5pEfiH3/UA9f7HxEwGV1eLU1huiM0g9Mx166Ce\n\tSxpXQOs+60XkwPRrs+kxaDOEmw==","X-Google-Smtp-Source":"APXvYqxDDEOUeEm8rQxgdoG5ltcgR4G5kJF4C/l2vg5LnDXExz5QQTjNKBQbJ4KoMQWXpG+vH4VC1Q==","X-Received":"by 2002:a2e:7c17:: with SMTP id\n\tx23mr4191925ljc.210.1568875438554; \n\tWed, 18 Sep 2019 23:43:58 -0700 (PDT)","Date":"Thu, 19 Sep 2019 08:43:57 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190919064357.GA1727@bigcity.dyn.berto.se>","References":"<20190915190408.2507-1-laurent.pinchart@ideasonboard.com>\n\t<20190917093603.GC15610@bigcity.dyn.berto.se>\n\t<20190918180552.GD4734@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190918180552.GD4734@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH] ipa: Convert the IPA API to plain C","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 19 Sep 2019 06:44:00 -0000"}}]