[libcamera-devel] [PATCH v4 10/37] libcamera: Add IPAIPC implementation based on unix socket
Jacopo Mondi
jacopo at jmondi.org
Tue Nov 17 16:42:28 CET 2020
Hi Paul,
On Fri, Nov 06, 2020 at 07:36:40PM +0900, Paul Elder wrote:
> Add an implementation of IPAIPC using unix socket.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v4:
> - change snake_case to camelCase
> - change proc_ and socket_ to unique pointers
> - move inclusion of corresponding header to first in the include list
> - reserve message data and fds size (for sending)
>
> Changes in v3:
> - remove unused writeUInt32() and readUInt32()
> - remove redundant definition of IPAIPCUnixSocket::isValid()
> - remove & 0xff in writeHeader()
> - make readHeader, writeHeader, and eraseHeader static class functions
> of IPAIPCUnixSocket instead of globals
>
> Changes in v2:
> - specify in doxygen to skip generating documentation for
> IPAIPCUnixSocket
> ---
> Documentation/Doxyfile.in | 2 +
> .../libcamera/internal/ipa_ipc_unixsocket.h | 62 +++++
> src/libcamera/ipa_ipc_unixsocket.cpp | 213 ++++++++++++++++++
> src/libcamera/meson.build | 1 +
> 4 files changed, 278 insertions(+)
> create mode 100644 include/libcamera/internal/ipa_ipc_unixsocket.h
> create mode 100644 src/libcamera/ipa_ipc_unixsocket.cpp
>
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index a6754a47..20fa1349 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -837,8 +837,10 @@ RECURSIVE = YES
> EXCLUDE = @TOP_SRCDIR@/include/libcamera/span.h \
> @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_sysfs.h \
> @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_udev.h \
> + @TOP_SRCDIR@/include/libcamera/internal/ipa_ipc_unixsocket.h \
> @TOP_SRCDIR@/src/libcamera/device_enumerator_sysfs.cpp \
> @TOP_SRCDIR@/src/libcamera/device_enumerator_udev.cpp \
> + @TOP_SRCDIR@/src/libcamera/ipa_ipc_unixsocket.cpp \
> @TOP_SRCDIR@/src/libcamera/pipeline/ \
> @TOP_SRCDIR@/src/libcamera/proxy/ \
> @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
> diff --git a/include/libcamera/internal/ipa_ipc_unixsocket.h b/include/libcamera/internal/ipa_ipc_unixsocket.h
> new file mode 100644
> index 00000000..f7248ca0
> --- /dev/null
> +++ b/include/libcamera/internal/ipa_ipc_unixsocket.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ipa_ipc_unixsocket.h - Image Processing Algorithm IPC module using unix socket
> + */
> +#ifndef __LIBCAMERA_INTERNAL_IPA_IPC_UNIXSOCKET_H__
> +#define __LIBCAMERA_INTERNAL_IPA_IPC_UNIXSOCKET_H__
> +
> +#include <vector>
> +
> +#include <libcamera/span.h>
> +
> +#include "libcamera/internal/ipa_ipc.h"
> +#include "libcamera/internal/ipa_module.h"
> +#include "libcamera/internal/ipc_unixsocket.h"
> +
> +namespace libcamera {
> +
> +class Process;
> +
> +class IPAIPCUnixSocket : public IPAIPC
> +{
> +public:
> + IPAIPCUnixSocket(const char *ipaModulePath, const char *ipaProxyWorkerPath);
> + ~IPAIPCUnixSocket();
> +
> + int sendSync(uint32_t cmd,
> + const std::vector<uint8_t> &dataIn,
> + const std::vector<int32_t> &fdsIn,
> + std::vector<uint8_t> *dataOut = nullptr,
> + std::vector<int32_t> *fdsOut = nullptr) override;
> +
> + int sendAsync(uint32_t cmd,
> + const std::vector<uint8_t> &dataIn,
> + const std::vector<int32_t> &fdsIn) override;
> +
> + static void writeHeader(IPCUnixSocket::Payload &payload, uint32_t cmd, uint32_t seq);
> + static std::tuple<uint32_t, uint32_t> readHeader(IPCUnixSocket::Payload &payload);
> + static void eraseHeader(IPCUnixSocket::Payload &payload);
There's one thing I don't fully get yet.
sendSycn/Async are methods of the interface, whatever IPC mechanism is
used, the generated classes that call those methods are guaranteed to
be independent from it.
writeHeader and readHeader are specific to the IPCUnixSocket
implementation, and I see the generated worker using them and using
explicit calls to, for example, socket_.send() which is not IPC
mechanism agnostic.
Is this by-design ? The workers are meant to know which IPC mechamism
is in use ?
> +
> +private:
> + struct CallData {
> + IPCUnixSocket::Payload *response;
> + bool done;
> + };
> +
> + void readyRead(IPCUnixSocket *socket);
> + int call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response, uint32_t seq);
> +
> + uint32_t seq_;
> +
> + std::unique_ptr<Process> proc_;
> +
> + std::unique_ptr<IPCUnixSocket> socket_;
> +
> + std::map<uint32_t, struct CallData> callData_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_IPA_IPC_UNIXSOCKET_H__ */
> diff --git a/src/libcamera/ipa_ipc_unixsocket.cpp b/src/libcamera/ipa_ipc_unixsocket.cpp
> new file mode 100644
> index 00000000..eebb39fd
> --- /dev/null
> +++ b/src/libcamera/ipa_ipc_unixsocket.cpp
> @@ -0,0 +1,213 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ipa_ipc_unixsocket.cpp - Image Processing Algorithm IPC module using unix socket
> + */
> +
> +#include "libcamera/internal/ipa_ipc_unixsocket.h"
> +
> +#include <vector>
> +
> +#include "libcamera/internal/ipa_ipc.h"
> +#include "libcamera/internal/ipc_unixsocket.h"
> +#include "libcamera/internal/log.h"
> +#include "libcamera/internal/process.h"
> +#include "libcamera/internal/thread.h"
> +
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/timer.h>
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(IPAIPC)
> +
> +IPAIPCUnixSocket::IPAIPCUnixSocket(const char *ipaModulePath,
> + const char *ipaProxyWorkerPath)
> + : IPAIPC(), seq_(0),
> + proc_(nullptr), socket_(nullptr)
> +{
> + std::vector<int> fds;
> + std::vector<std::string> args;
> + args.push_back(ipaModulePath);
> +
> + socket_ = std::make_unique<IPCUnixSocket>();
> + int fd = socket_->create();
> + if (fd < 0) {
> + LOG(IPAIPC, Error) << "Failed to create socket";
> + return;
> + }
> + socket_->readyRead.connect(this, &IPAIPCUnixSocket::readyRead);
> + args.push_back(std::to_string(fd));
> + fds.push_back(fd);
> +
> + proc_ = std::make_unique<Process>();
> + int ret = proc_->start(ipaProxyWorkerPath, args, fds);
> + if (ret) {
> + LOG(IPAIPC, Error)
> + << "Failed to start proxy worker process";
> + return;
> + }
> +
> + valid_ = true;
> +}
> +
> +IPAIPCUnixSocket::~IPAIPCUnixSocket()
> +{
> +}
> +
> +int IPAIPCUnixSocket::sendSync(uint32_t cmd,
> + const std::vector<uint8_t> &dataIn,
> + const std::vector<int32_t> &fdsIn,
> + std::vector<uint8_t> *dataOut,
> + std::vector<int32_t> *fdsOut)
> +{
> + IPCUnixSocket::Payload message, response;
> + int ret;
> +
> + message.data.reserve(8 + dataIn.size());
> + message.fds.reserve(fdsIn.size());
> +
> + /* It's fine if seq_ overflows; that'll just be the new epoch. */
> + seq_++;
> + writeHeader(message, cmd, seq_);
> + message.data.insert(message.data.end(), dataIn.begin(), dataIn.end());
> +
> + message.fds = const_cast<std::vector<int32_t> &>(fdsIn);
> +
> + ret = call(message, &response, seq_);
> + if (ret) {
> + LOG(IPAIPC, Error) << "Failed to call sync";
> + callData_.erase(seq_);
> + return ret;
> + }
> +
> + if (dataOut)
> + dataOut->insert(dataOut->end(), response.data.begin(), response.data.end());
> +
> + if (fdsOut)
> + fdsOut->insert(fdsOut->end(), response.fds.begin(), response.fds.end());
> +
> + return 0;
> +}
> +
> +int IPAIPCUnixSocket::sendAsync(uint32_t cmd,
> + const std::vector<uint8_t> &dataIn,
> + const std::vector<int32_t> &fdsIn)
> +{
> + IPCUnixSocket::Payload message;
> + int ret;
> +
> + message.data.reserve(8 + dataIn.size());
> + message.fds.reserve(fdsIn.size());
> +
> + writeHeader(message, cmd, 0);
> + message.data.insert(message.data.end(), dataIn.begin(), dataIn.end());
> +
> + message.fds = const_cast<std::vector<int32_t> &>(fdsIn);
> +
> + ret = socket_->send(message);
> + if (ret) {
> + LOG(IPAIPC, Error) << "Failed to call async";
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +void IPAIPCUnixSocket::writeHeader(IPCUnixSocket::Payload &payload,
> + uint32_t cmd, uint32_t seq)
> +{
> + uint8_t cmd_arr[] = {static_cast<uint8_t>(cmd),
> + static_cast<uint8_t>((cmd >> 8)),
> + static_cast<uint8_t>((cmd >> 16)),
> + static_cast<uint8_t>((cmd >> 24))};
> + uint8_t seq_arr[] = {static_cast<uint8_t>(seq),
> + static_cast<uint8_t>((seq >> 8)),
> + static_cast<uint8_t>((seq >> 16)),
> + static_cast<uint8_t>((seq >> 24))};
> + payload.data.insert(payload.data.begin(), cmd_arr, cmd_arr+4);
> + payload.data.insert(payload.data.begin() + 4, seq_arr, seq_arr+4);
> +}
> +
> +std::tuple<uint32_t, uint32_t>
> +IPAIPCUnixSocket::readHeader(IPCUnixSocket::Payload &payload)
> +{
> + uint32_t cmd = payload.data[0] |
> + (payload.data[1] << 8) |
> + (payload.data[2] << 16) |
> + (payload.data[3] << 24);
> + uint32_t seq = payload.data[4] |
> + (payload.data[5] << 8) |
> + (payload.data[6] << 16) |
> + (payload.data[7] << 24);
> +
> + return {cmd, seq};
> +}
> +
> +void IPAIPCUnixSocket::eraseHeader(IPCUnixSocket::Payload &payload)
> +{
> + payload.data.erase(payload.data.begin(), payload.data.begin() + 8);
> +}
> +
> +void IPAIPCUnixSocket::readyRead(IPCUnixSocket *socket)
> +{
> + IPCUnixSocket::Payload message;
> + int ret = socket->receive(&message);
> + if (ret) {
> + LOG(IPAIPC, Error) << "Receive message failed" << ret;
> + return;
> + }
> +
> + uint32_t cmd, seq;
> + std::tie(cmd, seq) = readHeader(message);
> +
> + auto callData = callData_.find(seq);
> + if (callData != callData_.end()) {
> + eraseHeader(message);
> + /* Is there any way to avoid this copy? */
> + *callData->second.response = message;
> + callData->second.done = true;
> + return;
> + }
> +
> + /*
> + * Received unexpected data, this means it's a call from the IPA.
> + * We can't return anything to the IPA (gotta keep them under *our*
> + * control, plus returning would require blocking the caller, and we
> + * can't afford to do that). Let the proxy do switch-case on cmd.
> + */
> + recvIPC.emit(message.data, message.fds);
> +
> + return;
> +}
> +
> +int IPAIPCUnixSocket::call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response, uint32_t seq)
> +{
> + Timer timeout;
> + int ret;
> +
> + callData_[seq].response = response;
> + callData_[seq].done = false;
> +
> + ret = socket_->send(message);
> + if (ret)
> + return ret;
> +
> + timeout.start(200);
> + while (!callData_[seq].done) {
> + if (!timeout.isRunning()) {
> + LOG(IPAIPC, Error) << "Call timeout!";
> + callData_.erase(seq);
> + return -ETIMEDOUT;
> + }
> +
> + Thread::current()->eventDispatcher()->processEvents();
> + }
> +
> + callData_.erase(seq);
> +
> + return 0;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 85f3a202..d6bd9a05 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -26,6 +26,7 @@ libcamera_sources = files([
> 'ipa_controls.cpp',
> 'ipa_data_serializer.cpp',
> 'ipa_ipc.cpp',
> + 'ipa_ipc_unixsocket.cpp',
> 'ipa_interface.cpp',
> 'ipa_manager.cpp',
> 'ipa_module.cpp',
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list