Patch Detail
Show a patch.
GET /api/1.1/patches/14157/?format=api
{ "id": 14157, "url": "https://patchwork.libcamera.org/api/1.1/patches/14157/?format=api", "web_url": "https://patchwork.libcamera.org/patch/14157/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20211015141656.242247-1-kieran.bingham@ideasonboard.com>", "date": "2021-10-15T14:16:56", "name": "[libcamera-devel,IPU3-IPA] libcamera-helpers: Integrate latest MappedFrameBuffer", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "f2baa95a669025ccea0004d7513c78bcb0c40f28", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/1.1/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/14157/mbox/", "series": [ { "id": 2632, "url": "https://patchwork.libcamera.org/api/1.1/series/2632/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2632", "date": "2021-10-15T14:16:56", "name": "[libcamera-devel,IPU3-IPA] libcamera-helpers: Integrate latest MappedFrameBuffer", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2632/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/14157/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/14157/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 75189C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Oct 2021 14:17:02 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED0C568F4A;\n\tFri, 15 Oct 2021 16:17:01 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C346568546\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Oct 2021 16:16:59 +0200 (CEST)", "from Monstersaurus.ksquared.org.uk.beta.tailscale.net\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 48DB22E3;\n\tFri, 15 Oct 2021 16:16:59 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uHMBe/sR\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634307419;\n\tbh=5ddUBykmljV5vQYmyN6RPcjAPfjW40C1ab9rHPzyWnw=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=uHMBe/sRcKl5YlAppws3NU+a+DuuzzHpLp0hxjSjOkIWsCmn1jT5ZYMN3yQ78MsjS\n\tOjOh3btePu1+MGOkY0VeVA5nI8YEs40aVvjxK/fdgm+LUQnpPmFMaR/KT0dtJi7Vnc\n\t4F7y5pOVD2YX564wZQ215q3eDbYgVpmXA2gnasmg=", "From": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Date": "Fri, 15 Oct 2021 15:16:56 +0100", "Message-Id": "<20211015141656.242247-1-kieran.bingham@ideasonboard.com>", "X-Mailer": "git-send-email 2.30.2", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [IPU3-IPA PATCH] libcamera-helpers: Integrate\n\tlatest MappedFrameBuffer", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The MappedFrameBuffer helper class has been updated in the libcamera\nsource code.\n\nThis makes use of the new enum MapFlags type, and corrects the mapping\nchanges that were made during 8708904fad6f (\"libcamera:\nmapped_framebuffer: Return plane begin address by MappedBuffer::maps()\")\n\nThis update also brings back isolated IPA functionality to this external\nIPA, which is otherwise broken due to the offset/plane changes.\n\nThe files are renamed to mapped_framebuffer to match the naming in\nlibcamera, but are kept within the 'libcamera-helper' hierarchy of the\nIPA.\n\nAlso, a minor todo is added to IPAIPU3::mapBuffers, to highlight that we\ncould potentially map Statistics buffers as read only rather than\nread/write if we could correctly identify them.\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\n include/libcamera-helpers/mapped_buffer.h | 53 ---------\n .../libcamera-helpers/mapped_framebuffer.h | 65 +++++++++++\n ipu3.cpp | 8 +-\n ...pped_buffer.cpp => mapped_framebuffer.cpp} | 110 +++++++++++++++---\n src/libcamera-helpers/meson.build | 2 +-\n 5 files changed, 163 insertions(+), 75 deletions(-)\n delete mode 100644 include/libcamera-helpers/mapped_buffer.h\n create mode 100644 include/libcamera-helpers/mapped_framebuffer.h\n rename src/libcamera-helpers/{mapped_buffer.cpp => mapped_framebuffer.cpp} (56%)", "diff": "diff --git a/include/libcamera-helpers/mapped_buffer.h b/include/libcamera-helpers/mapped_buffer.h\ndeleted file mode 100644\nindex 6cfc57217d75..000000000000\n--- a/include/libcamera-helpers/mapped_buffer.h\n+++ /dev/null\n@@ -1,53 +0,0 @@\n-/* SPDX-License-Identifier: LGPL-2.1-or-later */\n-/*\n- * Copyright (C) 2020, Google Inc.\n- *\n- * buffer.h - Internal buffer handling\n- */\n-#ifndef __LIBCAMERA_MAPPED_BUFFER_H__\n-#define __LIBCAMERA_MAPPED_BUFFER_H__\n-\n-#include <sys/mman.h>\n-#include <vector>\n-\n-#include <libcamera/base/class.h>\n-#include <libcamera/base/span.h>\n-\n-#include <libcamera/framebuffer.h>\n-\n-namespace libcamera {\n-\n-class MappedBuffer\n-{\n-public:\n- using Plane = Span<uint8_t>;\n-\n- ~MappedBuffer();\n-\n- MappedBuffer(MappedBuffer &&other);\n- MappedBuffer &operator=(MappedBuffer &&other);\n-\n- bool isValid() const { return error_ == 0; }\n- int error() const { return error_; }\n- const std::vector<Plane> &maps() const { return maps_; }\n-\n-protected:\n- MappedBuffer();\n-\n- int error_;\n- std::vector<Plane> maps_;\n-\n-private:\n- LIBCAMERA_DISABLE_COPY(MappedBuffer)\n-};\n-\n-class MappedFrameBuffer : public MappedBuffer\n-{\n-public:\n- MappedFrameBuffer(const FrameBuffer *buffer, int flags);\n-};\n-\n-} /* namespace libcamera */\n-\n-#endif /* __LIBCAMERA_MAPPED_BUFFER_H__ */\n-\ndiff --git a/include/libcamera-helpers/mapped_framebuffer.h b/include/libcamera-helpers/mapped_framebuffer.h\nnew file mode 100644\nindex 000000000000..42155279d44d\n--- /dev/null\n+++ b/include/libcamera-helpers/mapped_framebuffer.h\n@@ -0,0 +1,65 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2021, Google Inc.\n+ *\n+ * mapped_framebuffer.h - Frame buffer memory mapping support\n+ */\n+#ifndef __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__\n+#define __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__\n+\n+#include <stdint.h>\n+#include <vector>\n+\n+#include <libcamera/base/class.h>\n+#include <libcamera/base/flags.h>\n+#include <libcamera/base/span.h>\n+\n+#include <libcamera/framebuffer.h>\n+\n+namespace libcamera {\n+\n+class MappedBuffer\n+{\n+public:\n+\tusing Plane = Span<uint8_t>;\n+\n+\t~MappedBuffer();\n+\n+\tMappedBuffer(MappedBuffer &&other);\n+\tMappedBuffer &operator=(MappedBuffer &&other);\n+\n+\tbool isValid() const { return error_ == 0; }\n+\tint error() const { return error_; }\n+\t/* \\todo rename to planes(). */\n+\tconst std::vector<Plane> &maps() const { return planes_; }\n+\n+protected:\n+\tMappedBuffer();\n+\n+\tint error_;\n+\tstd::vector<Plane> planes_;\n+\tstd::vector<Plane> maps_;\n+\n+private:\n+\tLIBCAMERA_DISABLE_COPY(MappedBuffer)\n+};\n+\n+class MappedFrameBuffer : public MappedBuffer\n+{\n+public:\n+\tenum class MapFlag {\n+\t\tRead = 1 << 0,\n+\t\tWrite = 1 << 1,\n+\t\tReadWrite = Read | Write,\n+\t};\n+\n+\tusing MapFlags = Flags<MapFlag>;\n+\n+\tMappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags);\n+};\n+\n+LIBCAMERA_FLAGS_ENABLE_OPERATORS(MappedFrameBuffer::MapFlag)\n+\n+} /* namespace libcamera */\n+\n+#endif /* __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__ */\ndiff --git a/ipu3.cpp b/ipu3.cpp\nindex 3e89e6dd4e02..c1a254517845 100644\n--- a/ipu3.cpp\n+++ b/ipu3.cpp\n@@ -20,7 +20,7 @@\n \n #include <libcamera/base/log.h>\n \n-#include \"libcamera-helpers/mapped_buffer.h\"\n+#include \"libcamera-helpers/mapped_framebuffer.h\"\n \n /* IA AIQ Wrapper API */\n #include \"aic/aic.h\"\n@@ -258,10 +258,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n \n void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)\n {\n+\t/*\n+\t * todo: Statistics buffers could be mapped read-only if they\n+\t * could be easily identified.\n+\t */\n \tfor (const IPABuffer &buffer : buffers) {\n \t\tconst FrameBuffer fb(buffer.planes);\n \t\tbuffers_.emplace(buffer.id,\n-\t\t\t\t MappedFrameBuffer(&fb, PROT_READ | PROT_WRITE));\n+\t\t\t\t MappedFrameBuffer(&fb, MappedFrameBuffer::MapFlag::ReadWrite));\n \t}\n }\n \ndiff --git a/src/libcamera-helpers/mapped_buffer.cpp b/src/libcamera-helpers/mapped_framebuffer.cpp\nsimilarity index 56%\nrename from src/libcamera-helpers/mapped_buffer.cpp\nrename to src/libcamera-helpers/mapped_framebuffer.cpp\nindex 6f3248e1b31b..a65740831331 100644\n--- a/src/libcamera-helpers/mapped_buffer.cpp\n+++ b/src/libcamera-helpers/mapped_framebuffer.cpp\n@@ -2,26 +2,27 @@\n /*\n * Copyright (C) 2021, Google Inc.\n *\n- * mapped_buffer.cpp - Mapped Buffer handling\n+ * mapped_framebuffer.cpp - Mapped Framebuffer support\n */\n \n-#include \"libcamera-helpers/mapped_buffer.h\"\n+#include \"libcamera-helpers/mapped_framebuffer.h\"\n \n+#include <algorithm>\n #include <errno.h>\n-#include <string.h>\n+#include <map>\n #include <sys/mman.h>\n #include <unistd.h>\n \n #include <libcamera/base/log.h>\n \n /**\n- * \\file libcamera-helpers/mapped_buffer.h\n- * \\brief Mapped Buffer handling\n+ * \\file libcamera-helpers/mapped_framebuffer.h\n+ * \\brief Frame buffer memory mapping support\n */\n \n namespace libcamera {\n \n-LOG_DEFINE_CATEGORY(MappedBuffer)\n+LOG_DECLARE_CATEGORY(Buffer)\n \n /**\n * \\class MappedBuffer\n@@ -81,6 +82,7 @@ MappedBuffer::MappedBuffer(MappedBuffer &&other)\n MappedBuffer &MappedBuffer::operator=(MappedBuffer &&other)\n {\n \terror_ = other.error_;\n+\tplanes_ = std::move(other.planes_);\n \tmaps_ = std::move(other.maps_);\n \tother.error_ = -ENOENT;\n \n@@ -129,10 +131,18 @@ MappedBuffer::~MappedBuffer()\n */\n \n /**\n- * \\var MappedBuffer::maps_\n+ * \\var MappedBuffer::planes_\n * \\brief Stores the internal mapped planes\n *\n * MappedBuffer derived classes shall store the mappings they create in this\n+ * vector which points the beginning of mapped plane addresses.\n+ */\n+\n+/**\n+ * \\var MappedBuffer::maps_\n+ * \\brief Stores the mapped buffer\n+ *\n+ * MappedBuffer derived classes shall store the mappings they create in this\n * vector which is parsed during destruct to unmap any memory mappings which\n * completed successfully.\n */\n@@ -142,29 +152,91 @@ MappedBuffer::~MappedBuffer()\n * \\brief Map a FrameBuffer using the MappedBuffer interface\n */\n \n+/**\n+ * \\enum MappedFrameBuffer::MapFlag\n+ * \\brief Specify the mapping mode for the FrameBuffer\n+ * \\var MappedFrameBuffer::Read\n+ * \\brief Create a read-only mapping\n+ * \\var MappedFrameBuffer::Write\n+ * \\brief Create a write-only mapping\n+ * \\var MappedFrameBuffer::ReadWrite\n+ * \\brief Create a mapping that can be both read and written\n+ */\n+\n+/**\n+ * \\typedef MappedFrameBuffer::MapFlags\n+ * \\brief A bitwise combination of MappedFrameBuffer::MapFlag values\n+ */\n+\n /**\n * \\brief Map all planes of a FrameBuffer\n * \\param[in] buffer FrameBuffer to be mapped\n * \\param[in] flags Protection flags to apply to map\n *\n- * Construct an object to map a frame buffer for CPU access.\n- * The flags are passed directly to mmap and should be either PROT_READ,\n- * PROT_WRITE, or a bitwise-or combination of both.\n+ * Construct an object to map a frame buffer for CPU access. The mapping can be\n+ * made as Read only, Write only or support Read and Write operations by setting\n+ * the MapFlag flags accordingly.\n */\n-MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)\n+MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags)\n {\n-\tmaps_.reserve(buffer->planes().size());\n+\tASSERT(!buffer->planes().empty());\n+\tplanes_.reserve(buffer->planes().size());\n+\n+\tint mmapFlags = 0;\n+\n+\tif (flags & MapFlag::Read)\n+\t\tmmapFlags |= PROT_READ;\n+\n+\tif (flags & MapFlag::Write)\n+\t\tmmapFlags |= PROT_WRITE;\n+\n+\tstruct MappedBufferInfo {\n+\t\tuint8_t *address = nullptr;\n+\t\tsize_t mapLength = 0;\n+\t\tsize_t dmabufLength = 0;\n+\t};\n+\tstd::map<int, MappedBufferInfo> mappedBuffers;\n+\n+\tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n+\t\tconst int fd = plane.fd.fd();\n+\t\tif (mappedBuffers.find(fd) == mappedBuffers.end()) {\n+\t\t\tconst size_t length = lseek(fd, 0, SEEK_END);\n+\t\t\tmappedBuffers[fd] = MappedBufferInfo{ nullptr, 0, length };\n+\t\t}\n+\n+\t\tconst size_t length = mappedBuffers[fd].dmabufLength;\n+\n+\t\tif (plane.offset > length ||\n+\t\t plane.offset + plane.length > length) {\n+\t\t\tLOG(Buffer, Fatal) << \"plane is out of buffer: \"\n+\t\t\t\t\t << \"buffer length=\" << length\n+\t\t\t\t\t << \", plane offset=\" << plane.offset\n+\t\t\t\t\t << \", plane length=\" << plane.length;\n+\t\t\treturn;\n+\t\t}\n+\t\tsize_t &mapLength = mappedBuffers[fd].mapLength;\n+\t\tmapLength = std::max(mapLength,\n+\t\t\t\t static_cast<size_t>(plane.offset + plane.length));\n+\t}\n \n \tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n-\t\tvoid *address = mmap(nullptr, plane.length, flags,\n-\t\t\t\t MAP_SHARED, plane.fd.fd(), 0);\n-\t\tif (address == MAP_FAILED) {\n-\t\t\terror_ = -errno;\n-\t\t\tLOG(MappedBuffer, Error) << \"Failed to mmap plane\";\n-\t\t\tbreak;\n+\t\tconst int fd = plane.fd.fd();\n+\t\tauto &info = mappedBuffers[fd];\n+\t\tif (!info.address) {\n+\t\t\tvoid *address = mmap(nullptr, info.mapLength, mmapFlags,\n+\t\t\t\t\t MAP_SHARED, fd, 0);\n+\t\t\tif (address == MAP_FAILED) {\n+\t\t\t\terror_ = -errno;\n+\t\t\t\tLOG(Buffer, Error) << \"Failed to mmap plane: \"\n+\t\t\t\t\t\t << strerror(-error_);\n+\t\t\t\treturn;\n+\t\t\t}\n+\n+\t\t\tinfo.address = static_cast<uint8_t *>(address);\n+\t\t\tmaps_.emplace_back(info.address, info.mapLength);\n \t\t}\n \n-\t\tmaps_.emplace_back(static_cast<uint8_t *>(address), plane.length);\n+\t\tplanes_.emplace_back(info.address + plane.offset, plane.length);\n \t}\n }\n \ndiff --git a/src/libcamera-helpers/meson.build b/src/libcamera-helpers/meson.build\nindex 444f21204d5d..084bf65345ae 100644\n--- a/src/libcamera-helpers/meson.build\n+++ b/src/libcamera-helpers/meson.build\n@@ -2,5 +2,5 @@\n \n # Implementation of internal libcamera internals\n libcamera_helpers = files([\n- 'mapped_buffer.cpp',\n+ 'mapped_framebuffer.cpp',\n ])\n", "prefixes": [ "libcamera-devel", "IPU3-IPA" ] }