[{"id":12721,"web_url":"https://patchwork.libcamera.org/comment/12721/","msgid":"<20200924103244.u7dej3dxkobkkwp4@uno.localdomain>","date":"2020-09-24T10:32:44","subject":"Re: [libcamera-devel] [PATCH 14/38] libcamera: Add IPAIPC","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Tue, Sep 22, 2020 at 10:35:13PM +0900, Paul Elder wrote:\n> Create a virtual IPAIPC class that models an IPC/RPC system. IPA proxies\n> and proxy workers will call into the IPAIPC, rather than implementing\n> the IPC themselves.\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n>\n> ---\n> Changes in v2:\n> - add documentation\n> ---\n>  include/libcamera/internal/ipa_ipc.h |  41 ++++++++++\n>  src/libcamera/ipa_ipc.cpp            | 110 +++++++++++++++++++++++++++\n>  src/libcamera/meson.build            |   1 +\n>  3 files changed, 152 insertions(+)\n>  create mode 100644 include/libcamera/internal/ipa_ipc.h\n>  create mode 100644 src/libcamera/ipa_ipc.cpp\n>\n> diff --git a/include/libcamera/internal/ipa_ipc.h b/include/libcamera/internal/ipa_ipc.h\n> new file mode 100644\n> index 00000000..34ca6eda\n> --- /dev/null\n> +++ b/include/libcamera/internal/ipa_ipc.h\n> @@ -0,0 +1,41 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipa_ipc.h - Image Processing Algorithm IPC module for IPA proxies\n> + */\n> +#ifndef __LIBCAMERA_INTERNAL_IPA_IPC_H__\n> +#define __LIBCAMERA_INTERNAL_IPA_IPC_H__\n> +\n> +#include <vector>\n> +\n> +namespace libcamera {\n> +\n> +class IPAIPC\n> +{\n> +public:\n> +\tIPAIPC([[maybe_unused]] const char *ipa_module_path,\n> +\t       [[maybe_unused]] const char *ipa_proxy_worker_path);\n> +\tvirtual ~IPAIPC();\n> +\n> +\tbool isValid() const { return valid_; }\n> +\n> +\tvirtual int sendSync(uint32_t cmd,\n> +\t\t\t     const std::vector<uint8_t> &data_in,\n> +\t\t\t     const std::vector<int32_t> &fds_in,\n> +\t\t\t     std::vector<uint8_t> *data_out = nullptr,\n> +\t\t\t     std::vector<int32_t> *fds_out = nullptr) = 0;\n> +\n> +\tvirtual int sendAsync(uint32_t cmd,\n> +\t\t\t      const std::vector<uint8_t> &data_in,\n> +\t\t\t      const std::vector<int32_t> &fds_in) = 0;\n> +\n> +\tSignal<std::vector<uint8_t> &, std::vector<int32_t> &> recvIPC;\n\nProxyes and workers need to connect a slot to this Signal and IPAIPC\nderived classses have to emit it, right ?\n\n> +\n> +protected:\n> +\tbool valid_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_INTERNAL_IPA_IPC_H__ */\n> diff --git a/src/libcamera/ipa_ipc.cpp b/src/libcamera/ipa_ipc.cpp\n> new file mode 100644\n> index 00000000..88af648e\n> --- /dev/null\n> +++ b/src/libcamera/ipa_ipc.cpp\n> @@ -0,0 +1,110 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipa_ipc.cpp - Image Processing Algorithm IPC module for IPA proxies\n> + */\n> +\n> +#include <vector>\n> +\n> +#include \"libcamera/internal/ipc_unixsocket.h\"\n> +#include \"libcamera/internal/log.h\"\n> +#include \"libcamera/internal/process.h\"\n> +#include \"libcamera/internal/thread.h\"\n> +\n> +#include <libcamera/event_dispatcher.h>\n> +#include <libcamera/timer.h>\n> +\n> +#include \"libcamera/internal/ipa_ipc.h\"\n> +\n> +/**\n> + * \\file ipa_ipc.h\n> + * \\brief IPC mechanism for IPA isolation\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPAIPC)\n> +\n> +/**\n> + * \\class IPAIPC\n> + * \\brief IPC mechanism for IPA isolation\n> + *\n> + * Virtual class to model an IPC mechanism for use by IPA proxies for IPA\n> + * isolation. sendSync(), sendAsync(), and the emitter for the recvIPC signal\n> + * must be implemented.\n\nSo I would\n\n        sendSync(), sendAsync() must be implemented and the recvIPC\n        signal emitted whenever new data are available.\n\n\n> + */\n> +\n> +/**\n> + * \\brief Construct an IPAIPC instance\n> + * \\param[in] ipa_module_path Path to the IPA module shared object\n> + * \\param[in] ipa_proxy_worker_path Path to the IPA proxy worker shared object\n> + */\n> +IPAIPC::IPAIPC([[maybe_unused]] const char *ipa_module_path,\n> +\t       [[maybe_unused]] const char *ipa_proxy_worker_path)\n> +\t: valid_(false)\n> +{\n> +}\n> +\n> +IPAIPC::~IPAIPC()\n> +{\n> +}\n> +\n> +/**\n> + * \\fn IPAIPC::isValid()\n> + * \\brief Check if the IPAIPC instance is valid\n> + *\n> + * An IPAIPC instance is valid if the IPA interface is successfully created in\n> + * isolation, and IPC is successfully set up.\n> + *\n> + * \\return True if the IPAIPC is valid, false otherwise\n> + */\n> +\n> +/**\n> + * \\fn IPAIPC::sendSync()\n> + * \\brief Send a message over IPC synchronously\n> + * \\param[in] cmd Command ID\n> + * \\param[in] data_in Data to send, as a byte vector\n> + * \\param[in] fds_in Fds to send, in a vector\n> + * \\param[in] data_out Byte vector in which to receive data, if applicable\n> + * \\param[in] fds_out Fds vector in which to receive fds, if applicable\n> + *\n> + * This function will not return until a response is received. The event loop\n> + * will still continue to execute, however.\n> + *\n> + * \\return Zero on success, negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\fn IPAIPC::sendAsync()\n> + * \\brief Send a message over IPC asynchronously\n> + * \\param[in] cmd Command ID\n> + * \\param[in] data_in Data to send, as a byte vector\n> + * \\param[in] fds_in Fds to send, in a vector\n> + *\n> + * This function will return immediately after sending the message.\n> + *\n> + * \\return Zero on success, negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\var IPAIPC::recvIPC\n> + * \\brief Signal to be emitted when data is received over IPC\n> + *\n> + * When data is received over IPC, this signal shall be emitted.\n\nAnd specify users have to connect to this in order to receive\nnew data\n\nI wonder if we can document the signal parameters\n\n> + */\n> +\n> +/**\n> + * \\var IPAIPC::valid_\n> + * \\brief Flag to indicate if the IPAIPC instance is valid\n> + *\n> + * An IPAIPC instance is valid if the IPA interface is successfully created in\n> + * isolation, and IPC is successfully set up.\n> + *\n> + * This flag can be read via IPAIPC::isValid().\n> + *\n> + * Implementations of the IPAIPC class should set this flag upon successful\n> + * construction.\n> + */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 61aad08e..e3eade57 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -24,6 +24,7 @@ libcamera_sources = files([\n>      'ipa_context_wrapper.cpp',\n>      'ipa_controls.cpp',\n>      'ipa_data_serializer.cpp',\n> +    'ipa_ipc.cpp',\n>      'ipa_interface.cpp',\n>      'ipa_manager.cpp',\n>      'ipa_module.cpp',\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4B3EBC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Sep 2020 10:28:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2369A62FDE;\n\tThu, 24 Sep 2020 12:28:53 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E216560362\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Sep 2020 12:28:51 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 56D171BF214;\n\tThu, 24 Sep 2020 10:28:50 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 24 Sep 2020 12:32:44 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Paul Elder <paul.elder@ideasonboard.com>","Message-ID":"<20200924103244.u7dej3dxkobkkwp4@uno.localdomain>","References":"<20200922133537.258098-1-paul.elder@ideasonboard.com>\n\t<20200922133537.258098-15-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200922133537.258098-15-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 14/38] libcamera: Add IPAIPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]