[{"id":29219,"web_url":"https://patchwork.libcamera.org/comment/29219/","msgid":"<20240414214755.GH12561@pendragon.ideasonboard.com>","date":"2024-04-14T21:47:55","subject":"Re: [PATCH v7 06/18] libcamera: shared_mem_object: reorganize the\n\tcode and document the SharedMemObject class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan and Andrei,\n\nThank you for the patch.\n\nOn Thu, Apr 04, 2024 at 10:46:43AM +0200, Milan Zamazal wrote:\n> From: Andrei Konovalov \n> \n> The SharedMemObject class template contains a fair amount of inline code that\n\nThe standard line length for commit messages is 72. Out of curiosity,\nare you using an editor that has a different setting by default ?\n\n> does not depend on the template types T. To avoid duplicating it in every\n> template specialization, split that code to a separate base SharedMem class.\n> \n> We don't define copy semantics for the classes (we don't need one at the moment)\n> and we make them non-copyable since the default copy constructor would lead to\n> use-after-unmap.\n> \n> Doxygen documentation by Dennis Bonke and Andrei Konovalov.\n> \n> Reviewed-by: Pavel Machek \n> Co-developed-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Signed-off-by: Andrei Konovalov \n> Signed-off-by: Hans de Goede \n> Reviewed-by: Milan Zamazal \n> ---\n> .../libcamera/internal/shared_mem_object.h | 108 ++++----\n> src/libcamera/meson.build | 1 +\n> src/libcamera/shared_mem_object.cpp | 244 ++++++++++++++++++\n> 3 files changed, 303 insertions(+), 50 deletions(-)\n> create mode 100644 src/libcamera/shared_mem_object.cpp\n> \n> diff --git a/include/libcamera/internal/shared_mem_object.h b/include/libcamera/internal/shared_mem_object.h\n> index 8f2e7120..b2ab9ad1 100644\n> --- a/include/libcamera/internal/shared_mem_object.h\n> +++ b/include/libcamera/internal/shared_mem_object.h\n> @@ -1,85 +1,103 @@\n> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n> /*\n> - * Copyright (C) 2023, Raspberry Pi Ltd\n> + * Copyright (C) 2023 Raspberry Pi Ltd\n> + * Copyright (C) 2024 Andrei Konovalov\n> + * Copyright (C) 2024 Dennis Bonke\n> *\n> - * shared_mem_object.h - Helper class for shared memory allocations\n> + * shared_mem_object.h - Helpers for shared memory allocations\n> */\n> #pragma once\n> \n> -#include \n> #include \n> +#include \n> #include \n> #include \n> -#include \n> -#include \n> +#include \n> #include \n> \n> #include \n> #include \n> +#include \n> \n> namespace libcamera {\n> \n> -template\n> -class SharedMemObject\n> +class SharedMem\n> {\n> public:\n> -\tstatic constexpr std::size_t size = sizeof(T);\n> +\tusing span = Span;\n\nThe type would need to be named Span, but I think it should be dropped,\nit doesn't seem to increase readability to me.\n\n> \n> -\tSharedMemObject()\n> -\t\t: obj_(nullptr)\n> +\tSharedMem()\n> +\t\t: mem_(span())\n\nThere's no need for an explicit initializer for mem_. You're\ninitializing it to a default-constructed value, which is exactly what\nthe default constructor would do. You can write\n\n\tSharedMem();\n\nhere, and\n\nSharedMem::SharedMem() = default;\n\nin the .cpp file. This is one less user of the span type. The other two\nin this file will need to spell out Span explicitly.\n\n> \t{\n> \t}\n> \n> -\ttemplate\n> -\tSharedMemObject(const std::string &name, Args &&...args)\n> -\t\t: name_(name), obj_(nullptr)\n> +\tSharedMem(const std::string &name, std::size_t size);\n> +\tSharedMem(SharedMem &&rhs);\n> +\n> +\tvirtual ~SharedMem();\n> +\n> +\tSharedMem &operator=(SharedMem &&rhs);\n> +\n> +\tconst SharedFD &fd() const\n> \t{\n> -\t\tvoid *mem;\n> -\t\tint ret;\n> +\t\treturn fd_;\n> +\t}\n> \n> -\t\tret = memfd_create(name_.c_str(), MFD_CLOEXEC);\n> -\t\tif (ret < 0)\n> -\t\t\treturn;\n> +\tspan mem() const\n> +\t{\n> +\t\treturn mem_;\n> +\t}\n> \n> -\t\tfd_ = SharedFD(std::move(ret));\n> -\t\tif (!fd_.isValid())\n> -\t\t\treturn;\n> +\texplicit operator bool() const\n> +\t{\n> +\t\treturn !mem_.empty();\n> +\t}\n> \n> -\t\tret = ftruncate(fd_.get(), size);\n> -\t\tif (ret < 0)\n> -\t\t\treturn;\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY(SharedMem)\n> +\n> +\tSharedFD fd_;\n> +\n> +\tspan mem_;\n> +};\n> +\n> +template::value>>\n> +class SharedMemObject : public SharedMem\n> +{\n> +public:\n> +\tstatic constexpr std::size_t size = sizeof(T);\n\ns/size/kSize/\n\n> +\n> +\tSharedMemObject()\n> +\t\t: SharedMem(), obj_(nullptr)\n> +\t{\n> +\t}\n> \n> -\t\tmem = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED,\n> -\t\t\t fd_.get(), 0);\n> -\t\tif (mem == MAP_FAILED)\n> +\ttemplate\n> +\tSharedMemObject(const std::string &name, Args &&...args)\n> +\t\t: SharedMem(name, size), obj_(nullptr)\n> +\t{\n> +\t\tif (mem().empty())\n> \t\t\treturn;\n> \n> -\t\tobj_ = new (mem) T(std::forward(args)...);\n> +\t\tobj_ = new (mem().data()) T(std::forward(args)...);\n> \t}\n> \n> \tSharedMemObject(SharedMemObject &&rhs)\n> +\t\t: SharedMem(std::move(rhs))\n> \t{\n> -\t\tthis->name_ = std::move(rhs.name_);\n> -\t\tthis->fd_ = std::move(rhs.fd_);\n> \t\tthis->obj_ = rhs.obj_;\n> \t\trhs.obj_ = nullptr;\n> \t}\n> \n> \t~SharedMemObject()\n> \t{\n> -\t\tif (obj_) {\n> +\t\tif (obj_)\n> \t\t\tobj_->~T();\n> -\t\t\tmunmap(obj_, size);\n> -\t\t}\n> \t}\n> \n> -\t/* Make SharedMemObject non-copyable for now. */\n> -\tLIBCAMERA_DISABLE_COPY(SharedMemObject)\n> -\n> \tSharedMemObject &operator=(SharedMemObject &&rhs)\n> \t{\n> -\t\tthis->name_ = std::move(rhs.name_);\n> -\t\tthis->fd_ = std::move(rhs.fd_);\n> +\t\tSharedMem::operator=(std::move(rhs));\n> \t\tthis->obj_ = rhs.obj_;\n> \t\trhs.obj_ = nullptr;\n> \t\treturn *this;\n> @@ -105,19 +123,9 @@ public:\n> \t\treturn *obj_;\n> \t}\n> \n> -\tconst SharedFD &fd() const\n> -\t{\n> -\t\treturn fd_;\n> -\t}\n> -\n> -\texplicit operator bool() const\n> -\t{\n> -\t\treturn !!obj_;\n> -\t}\n> -\n> private:\n> -\tstd::string name_;\n> -\tSharedFD fd_;\n> +\tLIBCAMERA_DISABLE_COPY(SharedMemObject)\n> +\n> \tT *obj_;\n> };\n> \n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index dd8107fa..ce31180b 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -39,6 +39,7 @@ libcamera_sources = files([\n> 'process.cpp',\n> 'pub_key.cpp',\n> 'request.cpp',\n> + 'shared_mem_object.cpp',\n> 'source_paths.cpp',\n> 'stream.cpp',\n> 'sysfs.cpp',\n> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp\n> new file mode 100644\n> index 00000000..210a7a37\n> --- /dev/null\n> +++ b/src/libcamera/shared_mem_object.cpp\n> @@ -0,0 +1,244 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023 Raspberry Pi Ltd\n> + * Copyright (C) 2024 Andrei Konovalov\n> + * Copyright (C) 2024 Dennis Bonke\n> + * Copyright (C) 2024 Ideas on Board Oy\n> + *\n> + * shared_mem_object.cpp - Helpers for shared memory allocations\n> + */\n> +\n> +#include \"libcamera/internal/shared_mem_object.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n\nNo need to include shared_fd.h, the header file does so.\n\n> +\n> +/**\n> + * \\file shared_mem_object.cpp\n> + * \\brief Helpers for shared memory allocations\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class SharedMem\n> + * \\brief Helper class to allocate and manage memory shareable between processes\n> + *\n> + * SharedMem manages memory suitable for sharing between processes. When an\n> + * instance is constructed, it allocates a memory buffer of the requested size\n> + * backed by an anonymous file, using the memfd API.\n> + *\n> + * The allocated memory is exposed by the mem() function. If memory allocation\n> + * fails, the function returns an empty Span. This can be also checked using the\n> + * bool() operator.\n> + *\n> + * The file descriptor for the backing file is exposed as a SharedFD by the fd()\n> + * function. It can be shared with other processes across IPC boundaries, which\n> + * can then map the memory with mmap().\n> + *\n> + * A single memfd is created for every SharedMem. If there is a need to allocate\n> + * a large number of objects in shared memory, these objects should be grouped\n> + * together and use the shared memory allocated by a single SharedMem object if\n> + * possible. This will help to minimize the number of created memfd's.\n> + */\n> +\n> +/**\n> + * \\typedef SharedMem::span\n> + * \\brief Type of the Span wrapping the memory pointer\n> + */\n> +\n> +/**\n> + * \\brief Construct a SharedMem with memory of the given \\a size\n> + * \\param[in] name Name of the SharedMem\n> + * \\param[in] size Size of the shared memory to allocate and map\n> + *\n> + * The \\a name is used for debugging purpose only. Multiple SharedMem instances\n> + * can have the same name.\n> + */\n> +SharedMem::SharedMem(const std::string &name, std::size_t size)\n> +\t: mem_(span())\n\nYou can drop the explicit initialization here too.\n\n> +{\n> +\tint fd = memfd_create(name.c_str(), MFD_CLOEXEC);\n> +\tif (fd < 0)\n> +\t\treturn;\n> +\n> +\tfd_ = SharedFD(std::move(fd));\n> +\tif (!fd_.isValid())\n> +\t\treturn;\n> +\n> +\tif (ftruncate(fd_.get(), size) < 0) {\n> +\t\tfd_ = SharedFD();\n> +\t\treturn;\n> +\t}\n> +\n> +\tmem_ = span{\n> +\t\tstatic_cast(\n> +\t\t\tmmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED,\n> +\t\t\t fd_.get(), 0)),\n> +\t\tsize\n> +\t};\n\nThat's hard to read.\n\n\tvoid *mem = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED,\n\t\t\t fd_.get(), 0);\n\tif (mem == MAP_FAILED) {\n\t\tfd_ = SharedFD();\n\t\treturn;\n\t}\n\n\tmem_ = { static_cast(mem), size };\n\nOne less user of the span type :-)\n\n> +\tif (mem_.data() == MAP_FAILED) {\n> +\t\tmem_ = span();\n> +\t\tfd_ = SharedFD();\n> +\t}\n> +}\n> +\n> +/**\n> + * \\brief Move constructor for SharedMem\n> + * \\param[in] rhs The object to move\n> + */\n> +SharedMem::SharedMem(SharedMem &&rhs)\n> +{\n> +\tthis->fd_ = std::move(rhs.fd_);\n> +\tthis->mem_ = rhs.mem_;\n> +\trhs.mem_ = span();\n\n\trhs.mem_ = {};\n\nand similarly for all the span type users below.\n\nReviewed-by: Laurent Pinchart \n\n> +}\n> +\n> +/**\n> + * \\brief Destroy the SharedMem instance\n> + *\n> + * Destroying an instance invalidates the memory mapping exposed with mem().\n> + * Other mappings of the backing file, created in this or other processes with\n> + * mmap(), remain valid.\n> + *\n> + * Similarly, other references to the backing file descriptor created by copying\n> + * the SharedFD returned by fd() remain valid. The underlying memory will be\n> + * freed only when all file descriptors that reference the anonymous file get\n> + * closed.\n> + */\n> +SharedMem::~SharedMem()\n> +{\n> +\tif (!mem_.empty())\n> +\t\tmunmap(mem_.data(), mem_.size_bytes());\n> +}\n> +\n> +/**\n> + * \\brief Move assignment operator for SharedMem\n> + * \\param[in] rhs The object to move\n> + */\n> +SharedMem &SharedMem::operator=(SharedMem &&rhs)\n> +{\n> +\tthis->fd_ = std::move(rhs.fd_);\n> +\tthis->mem_ = rhs.mem_;\n> +\trhs.mem_ = span();\n> +\treturn *this;\n> +}\n> +\n> +/**\n> + * \\fn const SharedFD &SharedMem::fd() const\n> + * \\brief Retrieve the file descriptor for the underlying shared memory\n> + * \\return The file descriptor, or an invalid SharedFD if allocation failed\n> + */\n> +\n> +/**\n> + * \\fn span SharedMem::mem() const\n> + * \\brief Retrieve the underlying shared memory\n> + * \\return The memory buffer, or an empty span if allocation failed\n> + */\n> +\n> +/**\n> + * \\fn SharedMem::operator bool()\n> + * \\brief Check if the shared memory allocation succeeded\n> + * \\return True if allocation of the shared memory succeeded, false otherwise\n> + */\n> +\n> +/**\n> + * \\class SharedMemObject\n> + * \\brief Helper class to allocate an object in shareable memory\n> + * \\tparam The object type\n> + *\n> + * The SharedMemObject class is a specialization of the SharedMem class that\n> + * wraps an object of type \\a T and constructs it in shareable memory. It uses\n> + * the same underlying memory allocation and sharing mechanism as the SharedMem\n> + * class.\n> + *\n> + * The wrapped object is constructed at the same time as the SharedMemObject\n> + * instance, by forwarding the arguments passed to the SharedMemObject\n> + * constructor. The underlying memory allocation is sized to the object \\a T\n> + * size. The bool() operator should be used to check the allocation was\n> + * successful. The object can be accessed using the dereference operators\n> + * operator*() and operator->().\n> + *\n> + * While no restriction on the type \\a T is enforced, not all types are suitable\n> + * for sharing between multiple processes. Most notably, any object type that\n> + * contains pointer or reference members will likely cause issues. Even if those\n> + * members refer to other members of the same object, the shared memory will be\n> + * mapped at different addresses in different processes, and the pointers will\n> + * not be valid.\n> + *\n> + * A new anonymous file is created for every SharedMemObject instance. If there\n> + * is a need to share a large number of small objects, these objects should be\n> + * grouped into a single larger object to limit the number of file descriptors.\n> + *\n> + * To share the object with other processes, see the SharedMem documentation.\n> + */\n> +\n> +/**\n> + * \\var SharedMemObject::size\n> + * \\brief The size of the object stored in shared memory\n> + */\n> +\n> +/**\n> + * \\fn SharedMemObject::SharedMemObject(const std::string &name, Args &&...args)\n> + * \\brief Construct a SharedMemObject\n> + * \\param[in] name Name of the SharedMemObject\n> + * \\param[in] args Arguments to pass to the constructor of the object T\n> + *\n> + * The \\a name is used for debugging purpose only. Multiple SharedMem instances\n> + * can have the same name.\n> + */\n> +\n> +/**\n> + * \\fn SharedMemObject::SharedMemObject(SharedMemObject &&rhs)\n> + * \\brief Move constructor for SharedMemObject\n> + * \\param[in] rhs The object to move\n> + */\n> +\n> +/**\n> + * \\fn SharedMemObject::~SharedMemObject()\n> + * \\brief Destroy the SharedMemObject instance\n> + *\n> + * Destroying a SharedMemObject calls the wrapped T object's destructor. While\n> + * the underlying memory may not be freed immediately if other mappings have\n> + * been created manually (see SharedMem::~SharedMem() for more information), the\n> + * stored object may be modified. Depending on the ~T() destructor, accessing\n> + * the object after destruction of the SharedMemObject causes undefined\n> + * behaviour. It is the responsibility of the user of this class to synchronize\n> + * with other users who have access to the shared object.\n> + */\n> +\n> +/**\n> + * \\fn SharedMemObject::operator=(SharedMemObject &&rhs)\n> + * \\brief Move assignment operator for SharedMemObject\n> + * \\param[in] rhs The SharedMemObject object to take the data from\n> + *\n> + * Moving a SharedMemObject does not affect the stored object.\n> + */\n> +\n> +/**\n> + * \\fn SharedMemObject::operator->()\n> + * \\brief Dereference the stored object\n> + * \\return Pointer to the stored object\n> + */\n> +\n> +/**\n> + * \\fn const T *SharedMemObject::operator->() const\n> + * \\copydoc SharedMemObject::operator->\n> + */\n> +\n> +/**\n> + * \\fn SharedMemObject::operator*()\n> + * \\brief Dereference the stored object\n> + * \\return Reference to the stored object\n> + */\n> +\n> +/**\n> + * \\fn const T &SharedMemObject::operator*() const\n> + * \\copydoc SharedMemObject::operator*\n> + */\n> +\n> +} /* namespace libcamera */","headers":{"Return-Path":"","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 8DC36C3213\n\tfor ;\n\tSun, 14 Apr 2024 21:48:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AED2B63352;\n\tSun, 14 Apr 2024 23:48:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE6176333B\n\tfor ;\n\tSun, 14 Apr 2024 23:48:04 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 20954844;\n\tSun, 14 Apr 2024 23:47:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WoBPsxnN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713131239;\n\tbh=dBud3uQnZjoGiAvT38pDKBSxLUPyF2zUGM4COFwR43s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WoBPsxnNfjbl1gO+AYjX02wjGceXKTDAob5NnjXbLJSaGzM9dBZH4DHTfR83GWULw\n\ttNtfD+Clk6f+AmyZc2a4nxYHMFQT2YNe9FWjmyoVnI/RWB+ZlYFqfNP/PohrEj0LQE\n\tCs5VDPCGgerKUmx/Py3bWsTbw9ngMNeOm8cEiG04=","Date":"Mon, 15 Apr 2024 00:47:55 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tAndrei Konovalov ,\n\tBryan O'Donoghue ,\n\tMaxime Ripard , Pavel Machek ,\n\tHans de Goede ,\n\tKieran Bingham ,\n\tDennis Bonke ","Subject":"Re: [PATCH v7 06/18] libcamera: shared_mem_object: reorganize the\n\tcode and document the SharedMemObject class","Message-ID":"<20240414214755.GH12561@pendragon.ideasonboard.com>","References":"<20240404084657.353464-1-mzamazal@redhat.com>\n\t<20240404084657.353464-7-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240404084657.353464-7-mzamazal@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29225,"web_url":"https://patchwork.libcamera.org/comment/29225/","msgid":"<871q76hklq.fsf@redhat.com>","date":"2024-04-15T12:32:49","subject":"Re: [PATCH v7 06/18] libcamera: shared_mem_object: reorganize the\n\tcode and document the SharedMemObject class","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nLaurent Pinchart writes:\n\n> Hi Milan and Andrei,\n>\n> Thank you for the patch.\n>\n> On Thu, Apr 04, 2024 at 10:46:43AM +0200, Milan Zamazal wrote:\n>> From: Andrei Konovalov \n>> \n>> The SharedMemObject class template contains a fair amount of inline code that\n>\n> The standard line length for commit messages is 72. Out of curiosity,\n> are you using an editor that has a different setting by default ?\n\nYes. Magit used to set it to 72 but due to complaints from users who have\ndifferent project policies, it stopped doing so and leaves it at whatever value\nis set. I'll change it in my environment to 72 for libcamera commit messages.\n\n>> does not depend on the template types T. To avoid duplicating it in every\n>> template specialization, split that code to a separate base SharedMem class.\n>> \n>> We don't define copy semantics for the classes (we don't need one at the moment)\n>> and we make them non-copyable since the default copy constructor would lead to\n>> use-after-unmap.\n>> \n>> Doxygen documentation by Dennis Bonke and Andrei Konovalov.\n>> \n>> Reviewed-by: Pavel Machek \n>> Co-developed-by: Dennis Bonke \n>> Signed-off-by: Dennis Bonke \n>> Signed-off-by: Andrei Konovalov \n>> Signed-off-by: Hans de Goede \n>> Reviewed-by: Milan Zamazal \n>> ---\n>> .../libcamera/internal/shared_mem_object.h | 108 ++++----\n>> src/libcamera/meson.build | 1 +\n>> src/libcamera/shared_mem_object.cpp | 244 ++++++++++++++++++\n>> 3 files changed, 303 insertions(+), 50 deletions(-)\n>> create mode 100644 src/libcamera/shared_mem_object.cpp\n>> \n>> diff --git a/include/libcamera/internal/shared_mem_object.h b/include/libcamera/internal/shared_mem_object.h\n>> index 8f2e7120..b2ab9ad1 100644\n>> --- a/include/libcamera/internal/shared_mem_object.h\n>> +++ b/include/libcamera/internal/shared_mem_object.h\n>> @@ -1,85 +1,103 @@\n>> /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> /*\n>> - * Copyright (C) 2023, Raspberry Pi Ltd\n>> + * Copyright (C) 2023 Raspberry Pi Ltd\n>> + * Copyright (C) 2024 Andrei Konovalov\n>> + * Copyright (C) 2024 Dennis Bonke\n>> *\n>> - * shared_mem_object.h - Helper class for shared memory allocations\n>> + * shared_mem_object.h - Helpers for shared memory allocations\n>> */\n>> #pragma once\n>> \n>> -#include \n>> #include \n>> +#include \n>> #include \n>> #include \n>> -#include \n>> -#include \n>> +#include \n>> #include \n>> \n>> #include \n>> #include \n>> +#include \n>> \n>> namespace libcamera {\n>> \n>> -template\n>> -class SharedMemObject\n>> +class SharedMem\n>> {\n>> public:\n>> -\tstatic constexpr std::size_t size = sizeof(T);\n>> +\tusing span = Span;\n>\n> The type would need to be named Span, but I think it should be dropped,\n> it doesn't seem to increase readability to me.\n\nI haven't done this for readability but to have the type specification in a\nsingle place. But yes, in this case the type is given and if we decide to\nchange it to something else (e.g. C++20 std::span) then it'll be handled by a\nmass update. So I can drop the definition.","headers":{"Return-Path":"","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 73E74C3213\n\tfor ;\n\tMon, 15 Apr 2024 12:33:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A2B963352;\n\tMon, 15 Apr 2024 14:32:59 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1EBA063339\n\tfor ;\n\tMon, 15 Apr 2024 14:32:57 +0200 (CEST)","from mail-wr1-f69.google.com (mail-wr1-f69.google.com\n\t[209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-14-Ne0Tj_G3O1ClgZJZ-fDg_w-1; Mon, 15 Apr 2024 08:32:52 -0400","by mail-wr1-f69.google.com with SMTP id\n\tffacd0b85a97d-343e74dcf0bso2012571f8f.0\n\tfor ;\n\tMon, 15 Apr 2024 05:32:52 -0700 (PDT)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tq9-20020a05600000c900b00346c0062b0dsm11596918wrx.11.2024.04.15.05.32.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 15 Apr 2024 05:32:50 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"AIARQYuJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1713184376;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=4iIRWDzK48JjbM1xslFOg+7vljuBTJB623Rtebp+mWM=;\n\tb=AIARQYuJjLxKaeE5oHbYL4G6zOqgJCGkUH4DvBj9UBXnUui7qA7nVrgAX05CRfHxECLIA1\n\tSw/7dB3JmhqddnRlYgYZmRSl9khxosFAAB8fHQTxXKyQqskv7fq6KaSDOooUW0DYfQP0CT\n\tO8ztMZX8Z0KKILRVArs+x6CGr5Cl/CM=","X-MC-Unique":"Ne0Tj_G3O1ClgZJZ-fDg_w-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1713184371; x=1713789171;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=4iIRWDzK48JjbM1xslFOg+7vljuBTJB623Rtebp+mWM=;\n\tb=heUFcpMqtIjH1CFQ0VCdysL1ryI3fJZ+h6a/Iz9UJzpw6cT1mb8fE8eHUMFfYL6uGX\n\txWRocb3bRxIJsNNb7K9LbLh3fqQB+1p5iuHaYxyOiky3W+XNz6TtXysnA0ZvuW1GA8Ep\n\t9SYmF66uzn6+U9l4CA3pNOSeHgWd133Qk8lRE7TMHvUmoD+PnFQJX8+jeqD4QUmGbElD\n\toMjWPUWgEsPsv+3AQ2JqvbmkGpENVVgZVJe0Vcftemot1B0bH3WtojWeiq1zapSwH60j\n\tpELIawRU5//E2RgjmUk7qLA2dcEvce0Qz+F85kEAUMQzw0vTKY2F+oy4Xg3ievbShGO/\n\tNnSw==","X-Gm-Message-State":"AOJu0YwuQ4y5uJNTl50oxGpWXdr89qguyXe/6WF80kC5YxlAixPoy7QD\n\tEDp0WNhKOmlewrK7skuCcLmR/icd6JmfUFVwwMw+aMtZES/K571YN4BlVzXpw0jAc1jHJvW4u/O\n\tqPFh44dxub7g6sZplnWeQEcc9yUacMvu6f2eguX/EZ4xdEtcid/UEFPz0YjCgM1auwXU35jk=","X-Received":["by 2002:a5d:6a0c:0:b0:343:44c4:5fd6 with SMTP id\n\tm12-20020a5d6a0c000000b0034344c45fd6mr5754830wru.30.1713184371364; \n\tMon, 15 Apr 2024 05:32:51 -0700 (PDT)","by 2002:a5d:6a0c:0:b0:343:44c4:5fd6 with SMTP id\n\tm12-20020a5d6a0c000000b0034344c45fd6mr5754815wru.30.1713184371046; \n\tMon, 15 Apr 2024 05:32:51 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IF3Az1Z2xllYXKEiI6jVk57lvSa9rY4XST8fmZ7DOUsW2SWwtYSXPkJjcdmX82kkIscwmF5iw==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Andrei Konovalov\n\t, Bryan O'Donoghue\n\t, Maxime Ripard , Pavel\n\tMachek , Hans de Goede , Kieran\n\tBingham , Dennis Bonke\n\t","Subject":"Re: [PATCH v7 06/18] libcamera: shared_mem_object: reorganize the\n\tcode and document the SharedMemObject class","In-Reply-To":"<20240414214755.GH12561@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 15 Apr 2024 00:47:55 +0300\")","References":"<20240404084657.353464-1-mzamazal@redhat.com>\n\t<20240404084657.353464-7-mzamazal@redhat.com>\n\t<20240414214755.GH12561@pendragon.ideasonboard.com>","Date":"Mon, 15 Apr 2024 14:32:49 +0200","Message-ID":"<871q76hklq.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]