From 5c85e7024027c90b1b054782e510691b8b9c7419 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Sun, 28 Nov 2021 05:45:34 +0200 Subject: libcamera: base: Rename FileDescriptor to SharedFD Now that we have a UniqueFD class, the name FileDescriptor is ambiguous. Rename it to SharedFD. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- include/libcamera/base/file_descriptor.h | 49 ---- include/libcamera/base/meson.build | 2 +- include/libcamera/base/shared_fd.h | 49 ++++ include/libcamera/framebuffer.h | 4 +- include/libcamera/internal/ipa_data_serializer.h | 40 ++-- include/libcamera/internal/ipc_pipe.h | 8 +- include/libcamera/internal/v4l2_videodevice.h | 2 +- include/libcamera/ipa/core.mojom | 6 +- include/libcamera/ipa/raspberrypi.mojom | 2 +- src/android/camera_device.cpp | 2 +- src/ipa/raspberrypi/raspberrypi.cpp | 4 +- src/libcamera/base/file.cpp | 1 + src/libcamera/base/file_descriptor.cpp | 266 --------------------- src/libcamera/base/meson.build | 2 +- src/libcamera/base/shared_fd.cpp | 262 ++++++++++++++++++++ src/libcamera/framebuffer.cpp | 10 +- src/libcamera/ipa_data_serializer.cpp | 100 ++++---- src/libcamera/ipc_pipe.cpp | 4 +- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 6 +- src/libcamera/v4l2_videodevice.cpp | 12 +- src/v4l2/v4l2_camera.h | 2 +- test/file-descriptor.cpp | 243 ------------------- test/meson.build | 2 +- test/serialization/ipa_data_serializer_test.cpp | 14 +- test/shared-fd.cpp | 243 +++++++++++++++++++ .../libcamera_templates/module_ipa_proxy.cpp.tmpl | 2 +- .../libcamera_templates/module_ipa_proxy.h.tmpl | 2 +- .../libcamera_templates/proxy_functions.tmpl | 2 +- .../generators/libcamera_templates/serializer.tmpl | 22 +- utils/ipc/generators/mojom_libcamera_generator.py | 6 +- 30 files changed, 683 insertions(+), 686 deletions(-) delete mode 100644 include/libcamera/base/file_descriptor.h create mode 100644 include/libcamera/base/shared_fd.h delete mode 100644 src/libcamera/base/file_descriptor.cpp create mode 100644 src/libcamera/base/shared_fd.cpp delete mode 100644 test/file-descriptor.cpp create mode 100644 test/shared-fd.cpp diff --git a/include/libcamera/base/file_descriptor.h b/include/libcamera/base/file_descriptor.h deleted file mode 100644 index 12a43f95..00000000 --- a/include/libcamera/base/file_descriptor.h +++ /dev/null @@ -1,49 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * file_descriptor.h - File descriptor wrapper - */ - -#pragma once - -#include - -namespace libcamera { - -class UniqueFD; - -class FileDescriptor final -{ -public: - explicit FileDescriptor(const int &fd = -1); - explicit FileDescriptor(int &&fd); - explicit FileDescriptor(UniqueFD fd); - FileDescriptor(const FileDescriptor &other); - FileDescriptor(FileDescriptor &&other); - ~FileDescriptor(); - - FileDescriptor &operator=(const FileDescriptor &other); - FileDescriptor &operator=(FileDescriptor &&other); - - bool isValid() const { return fd_ != nullptr; } - int fd() const { return fd_ ? fd_->fd() : -1; } - UniqueFD dup() const; - -private: - class Descriptor - { - public: - Descriptor(int fd, bool duplicate); - ~Descriptor(); - - int fd() const { return fd_; } - - private: - int fd_; - }; - - std::shared_ptr fd_; -}; - -} /* namespace libcamera */ diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build index fce6eaa4..4410aba8 100644 --- a/include/libcamera/base/meson.build +++ b/include/libcamera/base/meson.build @@ -11,7 +11,6 @@ libcamera_base_headers = files([ 'event_dispatcher_poll.h', 'event_notifier.h', 'file.h', - 'file_descriptor.h', 'flags.h', 'log.h', 'message.h', @@ -19,6 +18,7 @@ libcamera_base_headers = files([ 'object.h', 'private.h', 'semaphore.h', + 'shared_fd.h', 'signal.h', 'span.h', 'thread.h', diff --git a/include/libcamera/base/shared_fd.h b/include/libcamera/base/shared_fd.h new file mode 100644 index 00000000..a786885c --- /dev/null +++ b/include/libcamera/base/shared_fd.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * shared_fd.h - File descriptor wrapper with shared ownership + */ + +#pragma once + +#include + +namespace libcamera { + +class UniqueFD; + +class SharedFD final +{ +public: + explicit SharedFD(const int &fd = -1); + explicit SharedFD(int &&fd); + explicit SharedFD(UniqueFD fd); + SharedFD(const SharedFD &other); + SharedFD(SharedFD &&other); + ~SharedFD(); + + SharedFD &operator=(const SharedFD &other); + SharedFD &operator=(SharedFD &&other); + + bool isValid() const { return fd_ != nullptr; } + int fd() const { return fd_ ? fd_->fd() : -1; } + UniqueFD dup() const; + +private: + class Descriptor + { + public: + Descriptor(int fd, bool duplicate); + ~Descriptor(); + + int fd() const { return fd_; } + + private: + int fd_; + }; + + std::shared_ptr fd_; +}; + +} /* namespace libcamera */ diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h index 2fbea9c5..357bbe18 100644 --- a/include/libcamera/framebuffer.h +++ b/include/libcamera/framebuffer.h @@ -13,7 +13,7 @@ #include #include -#include +#include #include namespace libcamera { @@ -51,7 +51,7 @@ class FrameBuffer final : public Extensible public: struct Plane { static constexpr unsigned int kInvalidOffset = std::numeric_limits::max(); - FileDescriptor fd; + SharedFD fd; unsigned int offset = kInvalidOffset; unsigned int length; }; diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h index c2f602d5..a87449c9 100644 --- a/include/libcamera/internal/ipa_data_serializer.h +++ b/include/libcamera/internal/ipa_data_serializer.h @@ -66,7 +66,7 @@ template class IPADataSerializer { public: - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const T &data, ControlSerializer *cs = nullptr); static T deserialize(const std::vector &data, @@ -76,12 +76,12 @@ public: ControlSerializer *cs = nullptr); static T deserialize(const std::vector &data, - const std::vector &fds, + const std::vector &fds, ControlSerializer *cs = nullptr); static T deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr); }; @@ -104,11 +104,11 @@ template class IPADataSerializer> { public: - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const std::vector &data, ControlSerializer *cs = nullptr) { std::vector dataVec; - std::vector fdsVec; + std::vector fdsVec; /* Serialize the length. */ uint32_t vecLen = data.size(); @@ -117,7 +117,7 @@ public: /* Serialize the members. */ for (auto const &it : data) { std::vector dvec; - std::vector fvec; + std::vector fvec; std::tie(dvec, fvec) = IPADataSerializer::serialize(it, cs); @@ -141,11 +141,11 @@ public: std::vector::const_iterator dataEnd, ControlSerializer *cs = nullptr) { - std::vector fds; + std::vector fds; return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); } - static std::vector deserialize(std::vector &data, std::vector &fds, + static std::vector deserialize(std::vector &data, std::vector &fds, ControlSerializer *cs = nullptr) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); @@ -153,15 +153,15 @@ public: static std::vector deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { uint32_t vecLen = readPOD(dataBegin, 0, dataEnd); std::vector ret(vecLen); std::vector::const_iterator dataIter = dataBegin + 4; - std::vector::const_iterator fdIter = fdsBegin; + std::vector::const_iterator fdIter = fdsBegin; for (uint32_t i = 0; i < vecLen; i++) { uint32_t sizeofData = readPOD(dataIter, 0, dataEnd); uint32_t sizeofFds = readPOD(dataIter, 4, dataEnd); @@ -201,11 +201,11 @@ template class IPADataSerializer> { public: - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const std::map &data, ControlSerializer *cs = nullptr) { std::vector dataVec; - std::vector fdsVec; + std::vector fdsVec; /* Serialize the length. */ uint32_t mapLen = data.size(); @@ -214,7 +214,7 @@ public: /* Serialize the members. */ for (auto const &it : data) { std::vector dvec; - std::vector fvec; + std::vector fvec; std::tie(dvec, fvec) = IPADataSerializer::serialize(it.first, cs); @@ -247,11 +247,11 @@ public: std::vector::const_iterator dataEnd, ControlSerializer *cs = nullptr) { - std::vector fds; + std::vector fds; return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); } - static std::map deserialize(std::vector &data, std::vector &fds, + static std::map deserialize(std::vector &data, std::vector &fds, ControlSerializer *cs = nullptr) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); @@ -259,8 +259,8 @@ public: static std::map deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { std::map ret; @@ -268,7 +268,7 @@ public: uint32_t mapLen = readPOD(dataBegin, 0, dataEnd); std::vector::const_iterator dataIter = dataBegin + 4; - std::vector::const_iterator fdIter = fdsBegin; + std::vector::const_iterator fdIter = fdsBegin; for (uint32_t i = 0; i < mapLen; i++) { uint32_t sizeofData = readPOD(dataIter, 0, dataEnd); uint32_t sizeofFds = readPOD(dataIter, 4, dataEnd); diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h index 986f8d88..ab5dd67c 100644 --- a/include/libcamera/internal/ipc_pipe.h +++ b/include/libcamera/internal/ipc_pipe.h @@ -9,7 +9,7 @@ #include -#include +#include #include #include "libcamera/internal/ipc_unixsocket.h" @@ -33,17 +33,17 @@ public: Header &header() { return header_; } std::vector &data() { return data_; } - std::vector &fds() { return fds_; } + std::vector &fds() { return fds_; } const Header &header() const { return header_; } const std::vector &data() const { return data_; } - const std::vector &fds() const { return fds_; } + const std::vector &fds() const { return fds_; } private: Header header_; std::vector data_; - std::vector fds_; + std::vector fds_; }; class IPCPipe diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 9f556f99..5ba2b546 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -184,7 +184,7 @@ public: ~V4L2VideoDevice(); int open(); - int open(FileDescriptor handle, enum v4l2_buf_type type); + int open(SharedFD handle, enum v4l2_buf_type type); void close(); const char *driverName() const { return caps_.driver(); } diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom index f7eff0c7..74f3339e 100644 --- a/include/libcamera/ipa/core.mojom +++ b/include/libcamera/ipa/core.mojom @@ -32,7 +32,7 @@ module libcamera; * - This attribute instructs the build system that a (de)serializer is * available for the type and there's no need to generate one * - hasFd - struct fields or empty structs only - * - Designate that this field or empty struct contains a FileDescriptor + * - Designate that this field or empty struct contains a SharedFD * * Rules: * - If the type is defined in a libcamera C++ header *and* a (de)serializer is @@ -60,7 +60,7 @@ module libcamera; * - In mojom, reference the type as FrameBuffer.Plane and only as map/array * member * - [skipHeader] and [skipSerdes] only work here in core.mojom - * - If a field in a struct has a FileDescriptor, but is not explicitly + * - If a field in a struct has a SharedFD, but is not explicitly * defined so in mojom, then the field must be marked with the [hasFd] * attribute * @@ -71,7 +71,7 @@ module libcamera; */ [skipSerdes, skipHeader] struct ControlInfoMap {}; [skipSerdes, skipHeader] struct ControlList {}; -[skipSerdes, skipHeader] struct FileDescriptor {}; +[skipSerdes, skipHeader] struct SharedFD {}; [skipHeader] struct Point { int32 x; diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom index e453d46c..acd3cafe 100644 --- a/include/libcamera/ipa/raspberrypi.mojom +++ b/include/libcamera/ipa/raspberrypi.mojom @@ -35,7 +35,7 @@ struct ISPConfig { struct IPAConfig { uint32 transform; - libcamera.FileDescriptor lsTableHandle; + libcamera.SharedFD lsTableHandle; }; struct StartConfig { diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index db400e74..9f772b58 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -743,7 +743,7 @@ CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer, std::vector planes(buf.numPlanes()); for (size_t i = 0; i < buf.numPlanes(); ++i) { - FileDescriptor fd{ camera3buffer->data[i] }; + SharedFD fd{ camera3buffer->data[i] }; if (!fd.isValid()) { LOG(HAL, Fatal) << "No valid fd"; return nullptr; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index c6aec090..aaf629ee 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -15,8 +15,8 @@ #include -#include #include +#include #include #include @@ -164,7 +164,7 @@ private: bool processPending_; /* LS table allocation passed in from the pipeline handler. */ - FileDescriptor lsTableHandle_; + SharedFD lsTableHandle_; void *lsTable_; /* Distinguish the first camera start from others. */ diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp index 923e03fa..fb3e276d 100644 --- a/src/libcamera/base/file.cpp +++ b/src/libcamera/base/file.cpp @@ -15,6 +15,7 @@ #include #include +#include /** * \file base/file.h diff --git a/src/libcamera/base/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp deleted file mode 100644 index 8e181abc..00000000 --- a/src/libcamera/base/file_descriptor.cpp +++ /dev/null @@ -1,266 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * file_descriptor.cpp - File descriptor wrapper - */ - -#include - -#include -#include -#include -#include - -#include -#include - -/** - * \file base/file_descriptor.h - * \brief File descriptor wrapper - */ - -namespace libcamera { - -LOG_DEFINE_CATEGORY(FileDescriptor) - -/** - * \class FileDescriptor - * \brief RAII-style wrapper for file descriptors - * - * The FileDescriptor class provides RAII-style lifetime management of file - * descriptors with an efficient mechanism for ownership sharing. At its core, - * an internal Descriptor object wraps a file descriptor (expressed as a signed - * integer) with an RAII-style interface. The Descriptor is then implicitly - * shared with all FileDescriptor instances constructed as copies. - * - * When constructed from a numerical file descriptor, the FileDescriptor - * instance either duplicates or takes over the file descriptor: - * - * - The FileDescriptor(const int &) constructor duplicates the numerical file - * descriptor and wraps the duplicate in a Descriptor. The caller is - * responsible for closing the original file descriptor, and the value - * returned by fd() will be different from the value passed to the - * constructor. - * - * - The FileDescriptor(int &&) constructor takes over the numerical file - * descriptor and wraps it in a Descriptor. The caller shall not touch the - * original file descriptor once the function returns, and the value returned - * by fd() will be identical to the value passed to the constructor. - * - * The copy constructor and assignment operator create copies that share the - * Descriptor, while the move versions of those functions additionally make the - * other FileDescriptor invalid. When the last FileDescriptor that references a - * Descriptor is destroyed, the file descriptor is closed. - * - * The numerical file descriptor is available through the fd() function. All - * FileDescriptor instances created as copies of a FileDescriptor will report - * the same fd() value. Callers can perform operations on the fd(), but shall - * never close it manually. - */ - -/** - * \brief Create a FileDescriptor copying a given \a fd - * \param[in] fd File descriptor - * - * Construct a FileDescriptor from a numerical file descriptor by duplicating - * the \a fd, and take ownership of the copy. The original \a fd is left - * untouched, and the caller is responsible for closing it when appropriate. - * The duplicated file descriptor will be closed automatically when all - * FileDescriptor instances that reference it are destroyed. - * - * If the \a fd is negative, the FileDescriptor is constructed as invalid and - * the fd() function will return -1. - */ -FileDescriptor::FileDescriptor(const int &fd) -{ - if (fd < 0) - return; - - fd_ = std::make_shared(fd, true); - if (fd_->fd() < 0) - fd_.reset(); -} - -/** - * \brief Create a FileDescriptor taking ownership of a given \a fd - * \param[in] fd File descriptor - * - * Construct a FileDescriptor from a numerical file descriptor by taking - * ownership of the \a fd. The original \a fd is set to -1 and shall not be - * touched by the caller anymore. In particular, the caller shall not close the - * original \a fd manually. The duplicated file descriptor will be closed - * automatically when all FileDescriptor instances that reference it are - * destroyed. - * - * If the \a fd is negative, the FileDescriptor is constructed as invalid and - * the fd() function will return -1. - */ -FileDescriptor::FileDescriptor(int &&fd) -{ - if (fd < 0) - return; - - fd_ = std::make_shared(fd, false); - /* - * The Descriptor constructor can't have failed here, as it took over - * the fd without duplicating it. Just set the original fd to -1 to - * implement move semantics. - */ - fd = -1; -} - -/** - * \brief Create a FileDescriptor taking ownership of a given UniqueFD \a fd - * \param[in] fd UniqueFD - * - * Construct a FileDescriptor from UniqueFD by taking ownership of the \a fd. - * The original \a fd becomes invalid. - */ -FileDescriptor::FileDescriptor(UniqueFD fd) - : FileDescriptor(fd.release()) -{ -} - -/** - * \brief Copy constructor, create a FileDescriptor from a copy of \a other - * \param[in] other The other FileDescriptor - * - * Copying a FileDescriptor implicitly shares ownership of the wrapped file - * descriptor. The original FileDescriptor is left untouched, and the caller is - * responsible for destroying it when appropriate. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - */ -FileDescriptor::FileDescriptor(const FileDescriptor &other) - : fd_(other.fd_) -{ -} - -/** - * \brief Move constructor, create a FileDescriptor by taking over \a other - * \param[in] other The other FileDescriptor - * - * Moving a FileDescriptor moves the reference to the wrapped descriptor owned - * by \a other to the new FileDescriptor. The \a other FileDescriptor is - * invalidated and its fd() function will return -1. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - */ -FileDescriptor::FileDescriptor(FileDescriptor &&other) - : fd_(std::move(other.fd_)) -{ -} - -/** - * \brief Destroy the FileDescriptor instance - * - * Destroying a FileDescriptor instance releases its reference to the wrapped - * descriptor, if any. When the last instance that references a wrapped - * descriptor is destroyed, the file descriptor is automatically closed. - */ -FileDescriptor::~FileDescriptor() -{ -} - -/** - * \brief Copy assignment operator, replace the wrapped file descriptor with a - * copy of \a other - * \param[in] other The other FileDescriptor - * - * Copying a FileDescriptor creates a new reference to the wrapped file - * descriptor owner by \a other. If \a other is invalid, *this will also be - * invalid. The original FileDescriptor is left untouched, and the caller is - * responsible for destroying it when appropriate. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - * - * \return A reference to this FileDescriptor - */ -FileDescriptor &FileDescriptor::operator=(const FileDescriptor &other) -{ - fd_ = other.fd_; - - return *this; -} - -/** - * \brief Move assignment operator, replace the wrapped file descriptor by - * taking over \a other - * \param[in] other The other FileDescriptor - * - * Moving a FileDescriptor moves the reference to the wrapped descriptor owned - * by \a other to the new FileDescriptor. If \a other is invalid, *this will - * also be invalid. The \a other FileDescriptor is invalidated and its fd() - * function will return -1. The wrapped file descriptor will be closed - * automatically when all FileDescriptor instances that reference it are - * destroyed. - * - * \return A reference to this FileDescriptor - */ -FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) -{ - fd_ = std::move(other.fd_); - - return *this; -} - -/** - * \fn FileDescriptor::isValid() - * \brief Check if the FileDescriptor instance is valid - * \return True if the FileDescriptor is valid, false otherwise - */ - -/** - * \fn FileDescriptor::fd() - * \brief Retrieve the numerical file descriptor - * \return The numerical file descriptor, which may be -1 if the FileDescriptor - * instance is invalid - */ - -/** - * \brief Duplicate a FileDescriptor - * - * Duplicating a FileDescriptor creates a duplicate of the wrapped file - * descriptor and returns a UniqueFD that owns the duplicate. The fd() function - * of the original and the get() function of the duplicate will return different - * values. The duplicate instance will not be affected by destruction of the - * original instance or its copies. - * - * \return A UniqueFD owning a duplicate of the original file descriptor - */ -UniqueFD FileDescriptor::dup() const -{ - UniqueFD dupFd(::dup(fd())); - if (!dupFd.isValid()) { - int ret = -errno; - LOG(FileDescriptor, Error) - << "Failed to dup() fd: " << strerror(-ret); - } - - return dupFd; -} - -FileDescriptor::Descriptor::Descriptor(int fd, bool duplicate) -{ - if (!duplicate) { - fd_ = fd; - return; - } - - /* Failing to dup() a fd should not happen and is fatal. */ - fd_ = ::dup(fd); - if (fd_ == -1) { - int ret = -errno; - LOG(FileDescriptor, Fatal) - << "Failed to dup() fd: " << strerror(-ret); - } -} - -FileDescriptor::Descriptor::~Descriptor() -{ - if (fd_ != -1) - close(fd_); -} - -} /* namespace libcamera */ diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index 352ec6f0..0ae3b0aa 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -8,13 +8,13 @@ libcamera_base_sources = files([ 'event_dispatcher_poll.cpp', 'event_notifier.cpp', 'file.cpp', - 'file_descriptor.cpp', 'flags.cpp', 'log.cpp', 'message.cpp', 'mutex.cpp', 'object.cpp', 'semaphore.cpp', + 'shared_fd.cpp', 'signal.cpp', 'thread.cpp', 'timer.cpp', diff --git a/src/libcamera/base/shared_fd.cpp b/src/libcamera/base/shared_fd.cpp new file mode 100644 index 00000000..346b4a85 --- /dev/null +++ b/src/libcamera/base/shared_fd.cpp @@ -0,0 +1,262 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * shared_fd.cpp - File descriptor wrapper with shared ownership + */ + +#include + +#include +#include +#include +#include + +#include +#include + +/** + * \file base/shared_fd.h + * \brief File descriptor wrapper + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(SharedFD) + +/** + * \class SharedFD + * \brief RAII-style wrapper for file descriptors + * + * The SharedFD class provides RAII-style lifetime management of file + * descriptors with an efficient mechanism for ownership sharing. At its core, + * an internal Descriptor object wraps a file descriptor (expressed as a signed + * integer) with an RAII-style interface. The Descriptor is then implicitly + * shared with all SharedFD instances constructed as copies. + * + * When constructed from a numerical file descriptor, the SharedFD instance + * either duplicates or takes over the file descriptor: + * + * - The SharedFD(const int &) constructor duplicates the numerical file + * descriptor and wraps the duplicate in a Descriptor. The caller is + * responsible for closing the original file descriptor, and the value + * returned by fd() will be different from the value passed to the + * constructor. + * + * - The SharedFD(int &&) constructor takes over the numerical file descriptor + * and wraps it in a Descriptor. The caller shall not touch the original file + * descriptor once the function returns, and the value returned by fd() will + * be identical to the value passed to the constructor. + * + * The copy constructor and assignment operator create copies that share the + * Descriptor, while the move versions of those functions additionally make the + * other SharedFD invalid. When the last SharedFD that references a Descriptor + * is destroyed, the file descriptor is closed. + * + * The numerical file descriptor is available through the fd() function. All + * SharedFD instances created as copies of a SharedFD will report the same fd() + * value. Callers can perform operations on the fd(), but shall never close it + * manually. + */ + +/** + * \brief Create a SharedFD copying a given \a fd + * \param[in] fd File descriptor + * + * Construct a SharedFD from a numerical file descriptor by duplicating the + * \a fd, and take ownership of the copy. The original \a fd is left untouched, + * and the caller is responsible for closing it when appropriate. The duplicated + * file descriptor will be closed automatically when all SharedFD instances that + * reference it are destroyed. + * + * If the \a fd is negative, the SharedFD is constructed as invalid and the fd() + * function will return -1. + */ +SharedFD::SharedFD(const int &fd) +{ + if (fd < 0) + return; + + fd_ = std::make_shared(fd, true); + if (fd_->fd() < 0) + fd_.reset(); +} + +/** + * \brief Create a SharedFD taking ownership of a given \a fd + * \param[in] fd File descriptor + * + * Construct a SharedFD from a numerical file descriptor by taking ownership of + * the \a fd. The original \a fd is set to -1 and shall not be touched by the + * caller anymore. In particular, the caller shall not close the original \a fd + * manually. The duplicated file descriptor will be closed automatically when + * all SharedFD instances that reference it are destroyed. + * + * If the \a fd is negative, the SharedFD is constructed as invalid and the fd() + * function will return -1. + */ +SharedFD::SharedFD(int &&fd) +{ + if (fd < 0) + return; + + fd_ = std::make_shared(fd, false); + /* + * The Descriptor constructor can't have failed here, as it took over + * the fd without duplicating it. Just set the original fd to -1 to + * implement move semantics. + */ + fd = -1; +} + +/** + * \brief Create a SharedFD taking ownership of a given UniqueFD \a fd + * \param[in] fd UniqueFD + * + * Construct a SharedFD from UniqueFD by taking ownership of the \a fd. The + * original \a fd becomes invalid. + */ +SharedFD::SharedFD(UniqueFD fd) + : SharedFD(fd.release()) +{ +} + +/** + * \brief Copy constructor, create a SharedFD from a copy of \a other + * \param[in] other The other SharedFD + * + * Copying a SharedFD implicitly shares ownership of the wrapped file + * descriptor. The original SharedFD is left untouched, and the caller is + * responsible for destroying it when appropriate. The wrapped file descriptor + * will be closed automatically when all SharedFD instances that reference it + * are destroyed. + */ +SharedFD::SharedFD(const SharedFD &other) + : fd_(other.fd_) +{ +} + +/** + * \brief Move constructor, create a SharedFD by taking over \a other + * \param[in] other The other SharedFD + * + * Moving a SharedFD moves the reference to the wrapped descriptor owned by + * \a other to the new SharedFD. The \a other SharedFD is invalidated and its + * fd() function will return -1. The wrapped file descriptor will be closed + * automatically when all SharedFD instances that reference it are destroyed. + */ +SharedFD::SharedFD(SharedFD &&other) + : fd_(std::move(other.fd_)) +{ +} + +/** + * \brief Destroy the SharedFD instance + * + * Destroying a SharedFD instance releases its reference to the wrapped + * descriptor, if any. When the last instance that references a wrapped + * descriptor is destroyed, the file descriptor is automatically closed. + */ +SharedFD::~SharedFD() +{ +} + +/** + * \brief Copy assignment operator, replace the wrapped file descriptor with a + * copy of \a other + * \param[in] other The other SharedFD + * + * Copying a SharedFD creates a new reference to the wrapped file descriptor + * owner by \a other. If \a other is invalid, *this will also be invalid. The + * original SharedFD is left untouched, and the caller is responsible for + * destroying it when appropriate. The wrapped file descriptor will be closed + * automatically when all SharedFD instances that reference it are destroyed. + * + * \return A reference to this SharedFD + */ +SharedFD &SharedFD::operator=(const SharedFD &other) +{ + fd_ = other.fd_; + + return *this; +} + +/** + * \brief Move assignment operator, replace the wrapped file descriptor by + * taking over \a other + * \param[in] other The other SharedFD + * + * Moving a SharedFD moves the reference to the wrapped descriptor owned by + * \a other to the new SharedFD. If \a other is invalid, *this will also be + * invalid. The \a other SharedFD is invalidated and its fd() function will + * return -1. The wrapped file descriptor will be closed automatically when + * all SharedFD instances that reference it are destroyed. + * + * \return A reference to this SharedFD + */ +SharedFD &SharedFD::operator=(SharedFD &&other) +{ + fd_ = std::move(other.fd_); + + return *this; +} + +/** + * \fn SharedFD::isValid() + * \brief Check if the SharedFD instance is valid + * \return True if the SharedFD is valid, false otherwise + */ + +/** + * \fn SharedFD::fd() + * \brief Retrieve the numerical file descriptor + * \return The numerical file descriptor, which may be -1 if the SharedFD + * instance is invalid + */ + +/** + * \brief Duplicate a SharedFD + * + * Duplicating a SharedFD creates a duplicate of the wrapped file descriptor and + * returns a UniqueFD that owns the duplicate. The fd() function of the original + * and the get() function of the duplicate will return different values. The + * duplicate instance will not be affected by destruction of the original + * instance or its copies. + * + * \return A UniqueFD owning a duplicate of the original file descriptor + */ +UniqueFD SharedFD::dup() const +{ + UniqueFD dupFd(::dup(fd())); + if (!dupFd.isValid()) { + int ret = -errno; + LOG(SharedFD, Error) + << "Failed to dup() fd: " << strerror(-ret); + } + + return dupFd; +} + +SharedFD::Descriptor::Descriptor(int fd, bool duplicate) +{ + if (!duplicate) { + fd_ = fd; + return; + } + + /* Failing to dup() a fd should not happen and is fatal. */ + fd_ = ::dup(fd); + if (fd_ == -1) { + int ret = -errno; + LOG(SharedFD, Fatal) + << "Failed to dup() fd: " << strerror(-ret); + } +} + +SharedFD::Descriptor::~Descriptor() +{ + if (fd_ != -1) + close(fd_); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index fcb475c2..fbf8f1c2 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -10,8 +10,8 @@ #include -#include #include +#include /** * \file libcamera/framebuffer.h @@ -182,9 +182,9 @@ FrameBuffer::Private::Private() * offset and length. * * To support DMA access, planes are associated with dmabuf objects represented - * by FileDescriptor handles. The Plane class doesn't handle mapping of the - * memory to the CPU, but applications and IPAs may use the dmabuf file - * descriptors to map the plane memory with mmap() and access its contents. + * by SharedFD handles. The Plane class doesn't handle mapping of the memory to + * the CPU, but applications and IPAs may use the dmabuf file descriptors to map + * the plane memory with mmap() and access its contents. * * \todo Specify how an application shall decide whether to use a single or * multiple dmabufs, based on the camera requirements. @@ -212,7 +212,7 @@ FrameBuffer::Private::Private() namespace { -ino_t fileDescriptorInode(const FileDescriptor &fd) +ino_t fileDescriptorInode(const SharedFD &fd) { if (!fd.isValid()) return 0; diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp index 82ec9b20..0a259305 100644 --- a/src/libcamera/ipa_data_serializer.cpp +++ b/src/libcamera/ipa_data_serializer.cpp @@ -31,7 +31,7 @@ LOG_DEFINE_CATEGORY(IPADataSerializer) * * \todo Harden the vector and map deserializer * - * \todo For FileDescriptors, instead of storing a validity flag, store an + * \todo For SharedFDs, instead of storing a validity flag, store an * index into the fd array. This will allow us to use views instead of copying. */ @@ -112,7 +112,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() can be used if the object type \a T and its - * members don't have any FileDescriptor. + * members don't have any SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -132,7 +132,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() can be used if the object type \a T and its - * members don't have any FileDescriptor. + * members don't have any SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -143,7 +143,7 @@ namespace { /** * \fn template IPADataSerializer::deserialize( * const std::vector &data, - * const std::vector &fds, + * const std::vector &fds, * ControlSerializer *cs = nullptr) * \brief Deserialize byte vector and fd vector into an object * \tparam T Type of object to deserialize to @@ -152,7 +152,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() (or the iterator version) must be used if - * the object type \a T or its members contain FileDescriptor. + * the object type \a T or its members contain SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -164,8 +164,8 @@ namespace { * \fn template IPADataSerializer::deserialize( * std::vector::const_iterator dataBegin, * std::vector::const_iterator dataEnd, - * std::vector::const_iterator fdsBegin, - * std::vector::const_iterator fdsEnd, + * std::vector::const_iterator fdsBegin, + * std::vector::const_iterator fdsEnd, * ControlSerializer *cs = nullptr) * \brief Deserialize byte vector and fd vector into an object * \tparam T Type of object to deserialize to @@ -176,7 +176,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() (or the vector version) must be used if - * the object type \a T or its members contain FileDescriptor. + * the object type \a T or its members contain SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -189,7 +189,7 @@ namespace { #define DEFINE_POD_SERIALIZER(type) \ \ template<> \ -std::tuple, std::vector> \ +std::tuple, std::vector> \ IPADataSerializer::serialize(const type &data, \ [[maybe_unused]] ControlSerializer *cs) \ { \ @@ -217,7 +217,7 @@ type IPADataSerializer::deserialize(const std::vector &data, \ \ template<> \ type IPADataSerializer::deserialize(const std::vector &data, \ - [[maybe_unused]] const std::vector &fds, \ + [[maybe_unused]] const std::vector &fds, \ ControlSerializer *cs) \ { \ return deserialize(data.cbegin(), data.end(), cs); \ @@ -226,8 +226,8 @@ type IPADataSerializer::deserialize(const std::vector &data, \ template<> \ type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \ std::vector::const_iterator dataEnd, \ - [[maybe_unused]] std::vector::const_iterator fdsBegin, \ - [[maybe_unused]] std::vector::const_iterator fdsEnd, \ + [[maybe_unused]] std::vector::const_iterator fdsBegin, \ + [[maybe_unused]] std::vector::const_iterator fdsEnd, \ ControlSerializer *cs) \ { \ return deserialize(dataBegin, dataEnd, cs); \ @@ -251,7 +251,7 @@ DEFINE_POD_SERIALIZER(double) * function parameter serdes). */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const std::string &data, [[maybe_unused]] ControlSerializer *cs) { @@ -278,7 +278,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator template<> std::string IPADataSerializer::deserialize(const std::vector &data, - [[maybe_unused]] const std::vector &fds, + [[maybe_unused]] const std::vector &fds, [[maybe_unused]] ControlSerializer *cs) { return { data.cbegin(), data.cend() }; @@ -288,8 +288,8 @@ template<> std::string IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, [[maybe_unused]] ControlSerializer *cs) { return { dataBegin, dataEnd }; @@ -307,7 +307,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator * be used. The serialized ControlInfoMap will have zero length. */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const ControlList &data, ControlSerializer *cs) { if (!cs) @@ -407,7 +407,7 @@ IPADataSerializer::deserialize(const std::vector &data, template<> ControlList IPADataSerializer::deserialize(const std::vector &data, - [[maybe_unused]] const std::vector &fds, + [[maybe_unused]] const std::vector &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), cs); @@ -417,8 +417,8 @@ template<> ControlList IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs) { return deserialize(dataBegin, dataEnd, cs); @@ -431,7 +431,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator * X bytes - Serialized ControlInfoMap (using ControlSerializer) */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const ControlInfoMap &map, ControlSerializer *cs) { @@ -493,7 +493,7 @@ IPADataSerializer::deserialize(const std::vector &data, template<> ControlInfoMap IPADataSerializer::deserialize(const std::vector &data, - [[maybe_unused]] const std::vector &fds, + [[maybe_unused]] const std::vector &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), cs); @@ -503,30 +503,30 @@ template<> ControlInfoMap IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs) { return deserialize(dataBegin, dataEnd, cs); } /* - * FileDescriptors are serialized into four bytes that tells if the - * FileDescriptor is valid or not. If it is valid, then for serialization - * the fd will be written to the fd vector, or for deserialization the - * fd vector const_iterator will be valid. + * SharedFD instances are serialized into four bytes that tells if the SharedFD + * is valid or not. If it is valid, then for serialization the fd will be + * written to the fd vector, or for deserialization the fd vector const_iterator + * will be valid. * * This validity is necessary so that we don't send -1 fd over sendmsg(). It * also allows us to simply send the entire fd vector into the deserializer * and it will be recursively consumed as necessary. */ template<> -std::tuple, std::vector> -IPADataSerializer::serialize(const FileDescriptor &data, - [[maybe_unused]] ControlSerializer *cs) +std::tuple, std::vector> +IPADataSerializer::serialize(const SharedFD &data, + [[maybe_unused]] ControlSerializer *cs) { std::vector dataVec; - std::vector fdVec; + std::vector fdVec; /* * Store as uint32_t to prepare for conversion from validity flag @@ -542,11 +542,11 @@ IPADataSerializer::serialize(const FileDescriptor &data, } template<> -FileDescriptor IPADataSerializer::deserialize([[maybe_unused]] std::vector::const_iterator dataBegin, - [[maybe_unused]] std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - std::vector::const_iterator fdsEnd, - [[maybe_unused]] ControlSerializer *cs) +SharedFD IPADataSerializer::deserialize([[maybe_unused]] std::vector::const_iterator dataBegin, + [[maybe_unused]] std::vector::const_iterator dataEnd, + std::vector::const_iterator fdsBegin, + std::vector::const_iterator fdsEnd, + [[maybe_unused]] ControlSerializer *cs) { ASSERT(std::distance(dataBegin, dataEnd) >= 4); @@ -554,13 +554,13 @@ FileDescriptor IPADataSerializer::deserialize([[maybe_unused]] s ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1)); - return valid ? *fdsBegin : FileDescriptor(); + return valid ? *fdsBegin : SharedFD(); } template<> -FileDescriptor IPADataSerializer::deserialize(const std::vector &data, - const std::vector &fds, - [[maybe_unused]] ControlSerializer *cs) +SharedFD IPADataSerializer::deserialize(const std::vector &data, + const std::vector &fds, + [[maybe_unused]] ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end()); } @@ -568,22 +568,22 @@ FileDescriptor IPADataSerializer::deserialize(const std::vector< /* * FrameBuffer::Plane is serialized as: * - * 4 byte - FileDescriptor + * 4 byte - SharedFD * 4 bytes - uint32_t Offset * 4 bytes - uint32_t Length */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const FrameBuffer::Plane &data, [[maybe_unused]] ControlSerializer *cs) { std::vector dataVec; - std::vector fdsVec; + std::vector fdsVec; std::vector fdBuf; - std::vector fdFds; + std::vector fdFds; std::tie(fdBuf, fdFds) = - IPADataSerializer::serialize(data.fd); + IPADataSerializer::serialize(data.fd); dataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end()); fdsVec.insert(fdsVec.end(), fdFds.begin(), fdFds.end()); @@ -597,13 +597,13 @@ template<> FrameBuffer::Plane IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, [[maybe_unused]] ControlSerializer *cs) { FrameBuffer::Plane ret; - ret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4, + ret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4, fdsBegin, fdsBegin + 1); ret.offset = readPOD(dataBegin, 4, dataEnd); ret.length = readPOD(dataBegin, 8, dataEnd); @@ -614,7 +614,7 @@ IPADataSerializer::deserialize(std::vector::const_i template<> FrameBuffer::Plane IPADataSerializer::deserialize(const std::vector &data, - const std::vector &fds, + const std::vector &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp index ad870fd4..3b47032d 100644 --- a/src/libcamera/ipc_pipe.cpp +++ b/src/libcamera/ipc_pipe.cpp @@ -87,7 +87,7 @@ IPCMessage::IPCMessage(IPCUnixSocket::Payload &payload) data_ = std::vector(payload.data.begin() + sizeof(header_), payload.data.end()); for (int32_t &fd : payload.fds) - fds_.push_back(FileDescriptor(std::move(fd))); + fds_.push_back(SharedFD(std::move(fd))); } /** @@ -112,7 +112,7 @@ IPCUnixSocket::Payload IPCMessage::payload() const data_.data(), data_.size()); } - for (const FileDescriptor &fd : fds_) + for (const SharedFD &fd : fds_) payload.fds.push_back(fd.fd()); return payload; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 6debea63..4885a939 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -13,7 +13,7 @@ #include #include -#include +#include #include #include @@ -224,7 +224,7 @@ public: /* DMAHEAP allocation helper. */ RPi::DmaHeap dmaHeap_; - FileDescriptor lsTable_; + SharedFD lsTable_; std::unique_ptr delayedCtrls_; bool sensorMetadata_; @@ -1393,7 +1393,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) /* Allocate the lens shading table via dmaHeap and pass to the IPA. */ if (!lsTable_.isValid()) { - lsTable_ = FileDescriptor(dmaHeap_.alloc("ls_grid", ipa::RPi::MaxLsGridSize)); + lsTable_ = SharedFD(dmaHeap_.alloc("ls_grid", ipa::RPi::MaxLsGridSize)); if (!lsTable_.isValid()) return -ENOMEM; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 5f1cc6e2..0d214d9e 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -22,8 +22,8 @@ #include #include -#include #include +#include #include #include @@ -617,7 +617,7 @@ int V4L2VideoDevice::open() * * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::open(FileDescriptor handle, enum v4l2_buf_type type) +int V4L2VideoDevice::open(SharedFD handle, enum v4l2_buf_type type) { UniqueFD newFd = handle.dup(); if (!newFd.isValid()) { @@ -1323,7 +1323,7 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) return nullptr; FrameBuffer::Plane plane; - plane.fd = FileDescriptor(std::move(fd)); + plane.fd = SharedFD(std::move(fd)); /* * V4L2 API doesn't provide dmabuf offset information of plane. * Set 0 as a placeholder offset. @@ -1352,7 +1352,7 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) ASSERT(numPlanes == 1u); planes.resize(formatInfo_->numPlanes()); - const FileDescriptor &fd = planes[0].fd; + const SharedFD &fd = planes[0].fd; size_t offset = 0; for (auto [i, plane] : utils::enumerate(planes)) { @@ -1900,8 +1900,8 @@ int V4L2M2MDevice::open() * The output and capture V4L2VideoDevice instances use the same file * handle for the same device node. */ - FileDescriptor fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), - O_RDWR | O_NONBLOCK)); + SharedFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), + O_RDWR | O_NONBLOCK)); if (!fd.isValid()) { ret = -errno; LOG(V4L2, Error) << "Failed to open V4L2 M2M device: " diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index 658c9297..03e74118 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -10,9 +10,9 @@ #include #include -#include #include #include +#include #include #include diff --git a/test/file-descriptor.cpp b/test/file-descriptor.cpp deleted file mode 100644 index 76badc4c..00000000 --- a/test/file-descriptor.cpp +++ /dev/null @@ -1,243 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * file_descriptor.cpp - FileDescriptor test - */ - -#include -#include -#include -#include -#include - -#include -#include - -#include "test.h" - -using namespace libcamera; -using namespace std; - -class FileDescriptorTest : public Test -{ -protected: - int init() - { - desc1_ = nullptr; - desc2_ = nullptr; - - fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); - if (fd_ < 0) - return TestFail; - - /* Cache inode number of temp file. */ - struct stat s; - if (fstat(fd_, &s)) - return TestFail; - - inodeNr_ = s.st_ino; - - return 0; - } - - int run() - { - /* Test creating empty FileDescriptor. */ - desc1_ = new FileDescriptor(); - - if (desc1_->fd() != -1) { - std::cout << "Failed fd numerical check (default constructor)" - << std::endl; - return TestFail; - } - - delete desc1_; - desc1_ = nullptr; - - /* - * Test creating FileDescriptor by copying numerical file - * descriptor. - */ - desc1_ = new FileDescriptor(fd_); - if (desc1_->fd() == fd_) { - std::cout << "Failed fd numerical check (lvalue ref constructor)" - << std::endl; - return TestFail; - } - - if (!isValidFd(fd_) || !isValidFd(desc1_->fd())) { - std::cout << "Failed fd validity after construction (lvalue ref constructor)" - << std::endl; - return TestFail; - } - - int fd = desc1_->fd(); - - delete desc1_; - desc1_ = nullptr; - - if (!isValidFd(fd_) || isValidFd(fd)) { - std::cout << "Failed fd validity after destruction (lvalue ref constructor)" - << std::endl; - return TestFail; - } - - /* - * Test creating FileDescriptor by taking ownership of - * numerical file descriptor. - */ - int dupFd = dup(fd_); - int dupFdCopy = dupFd; - - desc1_ = new FileDescriptor(std::move(dupFd)); - if (desc1_->fd() != dupFdCopy) { - std::cout << "Failed fd numerical check (rvalue ref constructor)" - << std::endl; - return TestFail; - } - - if (dupFd != -1 || !isValidFd(fd_) || !isValidFd(desc1_->fd())) { - std::cout << "Failed fd validity after construction (rvalue ref constructor)" - << std::endl; - return TestFail; - } - - fd = desc1_->fd(); - - delete desc1_; - desc1_ = nullptr; - - if (!isValidFd(fd_) || isValidFd(fd)) { - std::cout << "Failed fd validity after destruction (rvalue ref constructor)" - << std::endl; - return TestFail; - } - - /* Test creating FileDescriptor from other FileDescriptor. */ - desc1_ = new FileDescriptor(fd_); - desc2_ = new FileDescriptor(*desc1_); - - if (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() != desc2_->fd()) { - std::cout << "Failed fd numerical check (copy constructor)" - << std::endl; - return TestFail; - } - - if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd())) { - std::cout << "Failed fd validity after construction (copy constructor)" - << std::endl; - return TestFail; - } - - delete desc1_; - desc1_ = nullptr; - - if (!isValidFd(desc2_->fd())) { - std::cout << "Failed fd validity after destruction (copy constructor)" - << std::endl; - return TestFail; - } - - delete desc2_; - desc2_ = nullptr; - - /* Test creating FileDescriptor by taking over other FileDescriptor. */ - desc1_ = new FileDescriptor(fd_); - fd = desc1_->fd(); - desc2_ = new FileDescriptor(std::move(*desc1_)); - - if (desc1_->fd() != -1 || desc2_->fd() != fd) { - std::cout << "Failed fd numerical check (move constructor)" - << std::endl; - return TestFail; - } - - if (!isValidFd(desc2_->fd())) { - std::cout << "Failed fd validity after construction (move constructor)" - << std::endl; - return TestFail; - } - - delete desc1_; - desc1_ = nullptr; - delete desc2_; - desc2_ = nullptr; - - /* Test creating FileDescriptor by copy assignment. */ - desc1_ = new FileDescriptor(); - desc2_ = new FileDescriptor(fd_); - - fd = desc2_->fd(); - *desc1_ = *desc2_; - - if (desc1_->fd() != fd || desc2_->fd() != fd) { - std::cout << "Failed fd numerical check (copy assignment)" - << std::endl; - return TestFail; - } - - if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd())) { - std::cout << "Failed fd validity after construction (copy assignment)" - << std::endl; - return TestFail; - } - - delete desc1_; - desc1_ = nullptr; - delete desc2_; - desc2_ = nullptr; - - /* Test creating FileDescriptor by move assignment. */ - desc1_ = new FileDescriptor(); - desc2_ = new FileDescriptor(fd_); - - fd = desc2_->fd(); - *desc1_ = std::move(*desc2_); - - if (desc1_->fd() != fd || desc2_->fd() != -1) { - std::cout << "Failed fd numerical check (move assignment)" - << std::endl; - return TestFail; - } - - if (!isValidFd(desc1_->fd())) { - std::cout << "Failed fd validity after construction (move assignment)" - << std::endl; - return TestFail; - } - - delete desc1_; - desc1_ = nullptr; - delete desc2_; - desc2_ = nullptr; - - return TestPass; - } - - void cleanup() - { - delete desc2_; - delete desc1_; - - if (fd_ > 0) - close(fd_); - } - -private: - bool isValidFd(int fd) - { - struct stat s; - if (fstat(fd, &s)) - return false; - - /* Check that inode number matches cached temp file. */ - return s.st_ino == inodeNr_; - } - - int fd_; - ino_t inodeNr_; - FileDescriptor *desc1_, *desc2_; -}; - -TEST_REGISTER(FileDescriptorTest) diff --git a/test/meson.build b/test/meson.build index 42dfbc1f..daaa3862 100644 --- a/test/meson.build +++ b/test/meson.build @@ -40,7 +40,6 @@ internal_tests = [ ['event-dispatcher', 'event-dispatcher.cpp'], ['event-thread', 'event-thread.cpp'], ['file', 'file.cpp'], - ['file-descriptor', 'file-descriptor.cpp'], ['flags', 'flags.cpp'], ['hotplug-cameras', 'hotplug-cameras.cpp'], ['mapped-buffer', 'mapped-buffer.cpp'], @@ -49,6 +48,7 @@ internal_tests = [ ['object-delete', 'object-delete.cpp'], ['object-invoke', 'object-invoke.cpp'], ['pixel-format', 'pixel-format.cpp'], + ['shared-fd', 'shared-fd.cpp'], ['signal-threads', 'signal-threads.cpp'], ['threads', 'threads.cpp'], ['timer', 'timer.cpp'], diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp index 5fcdcb8e..d2050a86 100644 --- a/test/serialization/ipa_data_serializer_test.cpp +++ b/test/serialization/ipa_data_serializer_test.cpp @@ -53,7 +53,7 @@ template int testPodSerdes(T in) { std::vector buf; - std::vector fds; + std::vector fds; std::tie(buf, fds) = IPADataSerializer::serialize(in); T out = IPADataSerializer::deserialize(buf, fds); @@ -72,7 +72,7 @@ int testVectorSerdes(const std::vector &in, ControlSerializer *cs = nullptr) { std::vector buf; - std::vector fds; + std::vector fds; std::tie(buf, fds) = IPADataSerializer>::serialize(in, cs); std::vector out = IPADataSerializer>::deserialize(buf, fds, cs); @@ -92,7 +92,7 @@ int testMapSerdes(const std::map &in, ControlSerializer *cs = nullptr) { std::vector buf; - std::vector fds; + std::vector fds; std::tie(buf, fds) = IPADataSerializer>::serialize(in, cs); std::map out = IPADataSerializer>::deserialize(buf, fds, cs); @@ -198,7 +198,7 @@ private: ControlSerializer cs(ControlSerializer::Role::Proxy); /* - * We don't test FileDescriptor serdes because it dup()s, so we + * We don't test SharedFD serdes because it dup()s, so we * can't check for equality. */ std::vector vecUint8 = { 1, 2, 3, 4, 5, 6 }; @@ -219,7 +219,7 @@ private: }; std::vector buf; - std::vector fds; + std::vector fds; if (testVectorSerdes(vecUint8) != TestPass) return TestFail; @@ -291,7 +291,7 @@ private: { { "a", { 1, 2, 3 } }, { "b", { 4, 5, 6 } }, { "c", { 7, 8, 9 } } }; std::vector buf; - std::vector fds; + std::vector fds; if (testMapSerdes(mapUintStr) != TestPass) return TestFail; @@ -359,7 +359,7 @@ private: std::string strEmpty = ""; std::vector buf; - std::vector fds; + std::vector fds; if (testPodSerdes(u32min) != TestPass) return TestFail; diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp new file mode 100644 index 00000000..60e5d0aa --- /dev/null +++ b/test/shared-fd.cpp @@ -0,0 +1,243 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * shared_fd.cpp - SharedFD test + */ + +#include +#include +#include +#include +#include + +#include +#include + +#include "test.h" + +using namespace libcamera; +using namespace std; + +class SharedFDTest : public Test +{ +protected: + int init() + { + desc1_ = nullptr; + desc2_ = nullptr; + + fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); + if (fd_ < 0) + return TestFail; + + /* Cache inode number of temp file. */ + struct stat s; + if (fstat(fd_, &s)) + return TestFail; + + inodeNr_ = s.st_ino; + + return 0; + } + + int run() + { + /* Test creating empty SharedFD. */ + desc1_ = new SharedFD(); + + if (desc1_->fd() != -1) { + std::cout << "Failed fd numerical check (default constructor)" + << std::endl; + return TestFail; + } + + delete desc1_; + desc1_ = nullptr; + + /* + * Test creating SharedFD by copying numerical file + * descriptor. + */ + desc1_ = new SharedFD(fd_); + if (desc1_->fd() == fd_) { + std::cout << "Failed fd numerical check (lvalue ref constructor)" + << std::endl; + return TestFail; + } + + if (!isValidFd(fd_) || !isValidFd(desc1_->fd())) { + std::cout << "Failed fd validity after construction (lvalue ref constructor)" + << std::endl; + return TestFail; + } + + int fd = desc1_->fd(); + + delete desc1_; + desc1_ = nullptr; + + if (!isValidFd(fd_) || isValidFd(fd)) { + std::cout << "Failed fd validity after destruction (lvalue ref constructor)" + << std::endl; + return TestFail; + } + + /* + * Test creating SharedFD by taking ownership of + * numerical file descriptor. + */ + int dupFd = dup(fd_); + int dupFdCopy = dupFd; + + desc1_ = new SharedFD(std::move(dupFd)); + if (desc1_->fd() != dupFdCopy) { + std::cout << "Failed fd numerical check (rvalue ref constructor)" + << std::endl; + return TestFail; + } + + if (dupFd != -1 || !isValidFd(fd_) || !isValidFd(desc1_->fd())) { + std::cout << "Failed fd validity after construction (rvalue ref constructor)" + << std::endl; + return TestFail; + } + + fd = desc1_->fd(); + + delete desc1_; + desc1_ = nullptr; + + if (!isValidFd(fd_) || isValidFd(fd)) { + std::cout << "Failed fd validity after destruction (rvalue ref constructor)" + << std::endl; + return TestFail; + } + + /* Test creating SharedFD from other SharedFD. */ + desc1_ = new SharedFD(fd_); + desc2_ = new SharedFD(*desc1_); + + if (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() != desc2_->fd()) { + std::cout << "Failed fd numerical check (copy constructor)" + << std::endl; + return TestFail; + } + + if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd())) { + std::cout << "Failed fd validity after construction (copy constructor)" + << std::endl; + return TestFail; + } + + delete desc1_; + desc1_ = nullptr; + + if (!isValidFd(desc2_->fd())) { + std::cout << "Failed fd validity after destruction (copy constructor)" + << std::endl; + return TestFail; + } + + delete desc2_; + desc2_ = nullptr; + + /* Test creating SharedFD by taking over other SharedFD. */ + desc1_ = new SharedFD(fd_); + fd = desc1_->fd(); + desc2_ = new SharedFD(std::move(*desc1_)); + + if (desc1_->fd() != -1 || desc2_->fd() != fd) { + std::cout << "Failed fd numerical check (move constructor)" + << std::endl; + return TestFail; + } + + if (!isValidFd(desc2_->fd())) { + std::cout << "Failed fd validity after construction (move constructor)" + << std::endl; + return TestFail; + } + + delete desc1_; + desc1_ = nullptr; + delete desc2_; + desc2_ = nullptr; + + /* Test creating SharedFD by copy assignment. */ + desc1_ = new SharedFD(); + desc2_ = new SharedFD(fd_); + + fd = desc2_->fd(); + *desc1_ = *desc2_; + + if (desc1_->fd() != fd || desc2_->fd() != fd) { + std::cout << "Failed fd numerical check (copy assignment)" + << std::endl; + return TestFail; + } + + if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd())) { + std::cout << "Failed fd validity after construction (copy assignment)" + << std::endl; + return TestFail; + } + + delete desc1_; + desc1_ = nullptr; + delete desc2_; + desc2_ = nullptr; + + /* Test creating SharedFD by move assignment. */ + desc1_ = new SharedFD(); + desc2_ = new SharedFD(fd_); + + fd = desc2_->fd(); + *desc1_ = std::move(*desc2_); + + if (desc1_->fd() != fd || desc2_->fd() != -1) { + std::cout << "Failed fd numerical check (move assignment)" + << std::endl; + return TestFail; + } + + if (!isValidFd(desc1_->fd())) { + std::cout << "Failed fd validity after construction (move assignment)" + << std::endl; + return TestFail; + } + + delete desc1_; + desc1_ = nullptr; + delete desc2_; + desc2_ = nullptr; + + return TestPass; + } + + void cleanup() + { + delete desc2_; + delete desc1_; + + if (fd_ > 0) + close(fd_); + } + +private: + bool isValidFd(int fd) + { + struct stat s; + if (fstat(fd, &s)) + return false; + + /* Check that inode number matches cached temp file. */ + return s.st_ino == inodeNr_; + } + + int fd_; + ino_t inodeNr_; + SharedFD *desc1_, *desc2_; +}; + +TEST_REGISTER(SharedFDTest) diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index d856339a..c37c4941 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -237,7 +237,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) void {{proxy_name}}::{{method.mojom_name}}IPC( std::vector::const_iterator data, size_t dataSize, - [[maybe_unused]] const std::vector &fds) + [[maybe_unused]] const std::vector &fds) { {%- for param in method.parameters %} {{param|name}} {{param.mojom_name}}; diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index ce396c18..c308dd10 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -64,7 +64,7 @@ private: void {{method.mojom_name}}IPC( std::vector::const_iterator data, size_t dataSize, - const std::vector &fds); + const std::vector &fds); {% endfor %} /* Helper class to invoke async functions in another thread. */ diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl index ebcd2aaa..bac826a7 100644 --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl @@ -54,7 +54,7 @@ {%- for param in params %} std::vector {{param.mojom_name}}Buf; {%- if param|has_fd %} - std::vector {{param.mojom_name}}Fds; + std::vector {{param.mojom_name}}Fds; std::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) = {%- else %} std::tie({{param.mojom_name}}Buf, std::ignore) = diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl index b8ef8e7b..77bae36f 100644 --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl @@ -40,7 +40,7 @@ retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); {%- elif field|is_fd %} std::vector {{field.mojom_name}}; - std::vector {{field.mojom_name}}Fds; + std::vector {{field.mojom_name}}Fds; std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}); retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); @@ -58,7 +58,7 @@ {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %} std::vector {{field.mojom_name}}; {%- if field|has_fd %} - std::vector {{field.mojom_name}}Fds; + std::vector {{field.mojom_name}}Fds; std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = {%- else %} std::tie({{field.mojom_name}}, std::ignore) = @@ -177,7 +177,7 @@ # \a struct. #} {%- macro serializer(struct, namespace) %} - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const {{struct|name_full}} &data, {%- if struct|needs_control_serializer %} ControlSerializer *cs) @@ -187,7 +187,7 @@ { std::vector retData; {%- if struct|has_fd %} - std::vector retFds; + std::vector retFds; {%- endif %} {%- for field in struct.fields %} {{serializer_field(field, namespace, loop)}} @@ -210,7 +210,7 @@ {%- macro deserializer_fd(struct, namespace) %} static {{struct|name_full}} deserialize(std::vector &data, - std::vector &fds, + std::vector &fds, {%- if struct|needs_control_serializer %} ControlSerializer *cs) {%- else %} @@ -224,8 +224,8 @@ static {{struct|name_full}} deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + std::vector::const_iterator fdsEnd, {%- if struct|needs_control_serializer %} ControlSerializer *cs) {%- else %} @@ -234,7 +234,7 @@ { {{struct|name_full}} ret; std::vector::const_iterator m = dataBegin; - std::vector::const_iterator n = fdsBegin; + std::vector::const_iterator n = fdsBegin; size_t dataSize = std::distance(dataBegin, dataEnd); [[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd); @@ -255,7 +255,7 @@ {%- macro deserializer_fd_simple(struct, namespace) %} static {{struct|name_full}} deserialize(std::vector &data, - [[maybe_unused]] std::vector &fds, + [[maybe_unused]] std::vector &fds, ControlSerializer *cs = nullptr) { return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); @@ -264,8 +264,8 @@ static {{struct|name_full}} deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs); diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py index c609f4e5..753bfc73 100644 --- a/utils/ipc/generators/mojom_libcamera_generator.py +++ b/utils/ipc/generators/mojom_libcamera_generator.py @@ -77,7 +77,7 @@ def GetDefaultValue(element): if mojom.IsEnumKind(element.kind): return f'static_cast<{element.kind.mojom_name}>(0)' if isinstance(element.kind, mojom.Struct) and \ - element.kind.mojom_name == 'FileDescriptor': + element.kind.mojom_name == 'SharedFD': return '-1' return '' @@ -140,7 +140,7 @@ def HasFd(element): types = GetAllTypes(element) else: types = GetAllTypes(element.kind) - return "FileDescriptor" in types or (attrs is not None and "hasFd" in attrs) + return "SharedFD" in types or (attrs is not None and "hasFd" in attrs) def WithDefaultValues(element): return [x for x in element if HasDefaultValue(x)] @@ -221,7 +221,7 @@ def IsEnum(element): return mojom.IsEnumKind(element.kind) def IsFd(element): - return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "FileDescriptor" + return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "SharedFD" def IsMap(element): return mojom.IsMapKind(element.kind) -- cgit v1.2.1