[libcamera-devel] [IPU3-IPA PATCH] libcamera-helpers: Integrate latest MappedFrameBuffer
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Oct 15 16:16:56 CEST 2021
The MappedFrameBuffer helper class has been updated in the libcamera
source code.
This makes use of the new enum MapFlags type, and corrects the mapping
changes that were made during 8708904fad6f ("libcamera:
mapped_framebuffer: Return plane begin address by MappedBuffer::maps()")
This update also brings back isolated IPA functionality to this external
IPA, which is otherwise broken due to the offset/plane changes.
The files are renamed to mapped_framebuffer to match the naming in
libcamera, but are kept within the 'libcamera-helper' hierarchy of the
IPA.
Also, a minor todo is added to IPAIPU3::mapBuffers, to highlight that we
could potentially map Statistics buffers as read only rather than
read/write if we could correctly identify them.
Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
---
include/libcamera-helpers/mapped_buffer.h | 53 ---------
.../libcamera-helpers/mapped_framebuffer.h | 65 +++++++++++
ipu3.cpp | 8 +-
...pped_buffer.cpp => mapped_framebuffer.cpp} | 110 +++++++++++++++---
src/libcamera-helpers/meson.build | 2 +-
5 files changed, 163 insertions(+), 75 deletions(-)
delete mode 100644 include/libcamera-helpers/mapped_buffer.h
create mode 100644 include/libcamera-helpers/mapped_framebuffer.h
rename src/libcamera-helpers/{mapped_buffer.cpp => mapped_framebuffer.cpp} (56%)
diff --git a/include/libcamera-helpers/mapped_buffer.h b/include/libcamera-helpers/mapped_buffer.h
deleted file mode 100644
index 6cfc57217d75..000000000000
--- a/include/libcamera-helpers/mapped_buffer.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2020, Google Inc.
- *
- * buffer.h - Internal buffer handling
- */
-#ifndef __LIBCAMERA_MAPPED_BUFFER_H__
-#define __LIBCAMERA_MAPPED_BUFFER_H__
-
-#include <sys/mman.h>
-#include <vector>
-
-#include <libcamera/base/class.h>
-#include <libcamera/base/span.h>
-
-#include <libcamera/framebuffer.h>
-
-namespace libcamera {
-
-class MappedBuffer
-{
-public:
- using Plane = Span<uint8_t>;
-
- ~MappedBuffer();
-
- MappedBuffer(MappedBuffer &&other);
- MappedBuffer &operator=(MappedBuffer &&other);
-
- bool isValid() const { return error_ == 0; }
- int error() const { return error_; }
- const std::vector<Plane> &maps() const { return maps_; }
-
-protected:
- MappedBuffer();
-
- int error_;
- std::vector<Plane> maps_;
-
-private:
- LIBCAMERA_DISABLE_COPY(MappedBuffer)
-};
-
-class MappedFrameBuffer : public MappedBuffer
-{
-public:
- MappedFrameBuffer(const FrameBuffer *buffer, int flags);
-};
-
-} /* namespace libcamera */
-
-#endif /* __LIBCAMERA_MAPPED_BUFFER_H__ */
-
diff --git a/include/libcamera-helpers/mapped_framebuffer.h b/include/libcamera-helpers/mapped_framebuffer.h
new file mode 100644
index 000000000000..42155279d44d
--- /dev/null
+++ b/include/libcamera-helpers/mapped_framebuffer.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021, Google Inc.
+ *
+ * mapped_framebuffer.h - Frame buffer memory mapping support
+ */
+#ifndef __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__
+#define __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__
+
+#include <stdint.h>
+#include <vector>
+
+#include <libcamera/base/class.h>
+#include <libcamera/base/flags.h>
+#include <libcamera/base/span.h>
+
+#include <libcamera/framebuffer.h>
+
+namespace libcamera {
+
+class MappedBuffer
+{
+public:
+ using Plane = Span<uint8_t>;
+
+ ~MappedBuffer();
+
+ MappedBuffer(MappedBuffer &&other);
+ MappedBuffer &operator=(MappedBuffer &&other);
+
+ bool isValid() const { return error_ == 0; }
+ int error() const { return error_; }
+ /* \todo rename to planes(). */
+ const std::vector<Plane> &maps() const { return planes_; }
+
+protected:
+ MappedBuffer();
+
+ int error_;
+ std::vector<Plane> planes_;
+ std::vector<Plane> maps_;
+
+private:
+ LIBCAMERA_DISABLE_COPY(MappedBuffer)
+};
+
+class MappedFrameBuffer : public MappedBuffer
+{
+public:
+ enum class MapFlag {
+ Read = 1 << 0,
+ Write = 1 << 1,
+ ReadWrite = Read | Write,
+ };
+
+ using MapFlags = Flags<MapFlag>;
+
+ MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags);
+};
+
+LIBCAMERA_FLAGS_ENABLE_OPERATORS(MappedFrameBuffer::MapFlag)
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__ */
diff --git a/ipu3.cpp b/ipu3.cpp
index 3e89e6dd4e02..c1a254517845 100644
--- a/ipu3.cpp
+++ b/ipu3.cpp
@@ -20,7 +20,7 @@
#include <libcamera/base/log.h>
-#include "libcamera-helpers/mapped_buffer.h"
+#include "libcamera-helpers/mapped_framebuffer.h"
/* IA AIQ Wrapper API */
#include "aic/aic.h"
@@ -258,10 +258,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)
{
+ /*
+ * todo: Statistics buffers could be mapped read-only if they
+ * could be easily identified.
+ */
for (const IPABuffer &buffer : buffers) {
const FrameBuffer fb(buffer.planes);
buffers_.emplace(buffer.id,
- MappedFrameBuffer(&fb, PROT_READ | PROT_WRITE));
+ MappedFrameBuffer(&fb, MappedFrameBuffer::MapFlag::ReadWrite));
}
}
diff --git a/src/libcamera-helpers/mapped_buffer.cpp b/src/libcamera-helpers/mapped_framebuffer.cpp
similarity index 56%
rename from src/libcamera-helpers/mapped_buffer.cpp
rename to src/libcamera-helpers/mapped_framebuffer.cpp
index 6f3248e1b31b..a65740831331 100644
--- a/src/libcamera-helpers/mapped_buffer.cpp
+++ b/src/libcamera-helpers/mapped_framebuffer.cpp
@@ -2,26 +2,27 @@
/*
* Copyright (C) 2021, Google Inc.
*
- * mapped_buffer.cpp - Mapped Buffer handling
+ * mapped_framebuffer.cpp - Mapped Framebuffer support
*/
-#include "libcamera-helpers/mapped_buffer.h"
+#include "libcamera-helpers/mapped_framebuffer.h"
+#include <algorithm>
#include <errno.h>
-#include <string.h>
+#include <map>
#include <sys/mman.h>
#include <unistd.h>
#include <libcamera/base/log.h>
/**
- * \file libcamera-helpers/mapped_buffer.h
- * \brief Mapped Buffer handling
+ * \file libcamera-helpers/mapped_framebuffer.h
+ * \brief Frame buffer memory mapping support
*/
namespace libcamera {
-LOG_DEFINE_CATEGORY(MappedBuffer)
+LOG_DECLARE_CATEGORY(Buffer)
/**
* \class MappedBuffer
@@ -81,6 +82,7 @@ MappedBuffer::MappedBuffer(MappedBuffer &&other)
MappedBuffer &MappedBuffer::operator=(MappedBuffer &&other)
{
error_ = other.error_;
+ planes_ = std::move(other.planes_);
maps_ = std::move(other.maps_);
other.error_ = -ENOENT;
@@ -129,10 +131,18 @@ MappedBuffer::~MappedBuffer()
*/
/**
- * \var MappedBuffer::maps_
+ * \var MappedBuffer::planes_
* \brief Stores the internal mapped planes
*
* MappedBuffer derived classes shall store the mappings they create in this
+ * vector which points the beginning of mapped plane addresses.
+ */
+
+/**
+ * \var MappedBuffer::maps_
+ * \brief Stores the mapped buffer
+ *
+ * MappedBuffer derived classes shall store the mappings they create in this
* vector which is parsed during destruct to unmap any memory mappings which
* completed successfully.
*/
@@ -142,29 +152,91 @@ MappedBuffer::~MappedBuffer()
* \brief Map a FrameBuffer using the MappedBuffer interface
*/
+/**
+ * \enum MappedFrameBuffer::MapFlag
+ * \brief Specify the mapping mode for the FrameBuffer
+ * \var MappedFrameBuffer::Read
+ * \brief Create a read-only mapping
+ * \var MappedFrameBuffer::Write
+ * \brief Create a write-only mapping
+ * \var MappedFrameBuffer::ReadWrite
+ * \brief Create a mapping that can be both read and written
+ */
+
+/**
+ * \typedef MappedFrameBuffer::MapFlags
+ * \brief A bitwise combination of MappedFrameBuffer::MapFlag values
+ */
+
/**
* \brief Map all planes of a FrameBuffer
* \param[in] buffer FrameBuffer to be mapped
* \param[in] flags Protection flags to apply to map
*
- * Construct an object to map a frame buffer for CPU access.
- * The flags are passed directly to mmap and should be either PROT_READ,
- * PROT_WRITE, or a bitwise-or combination of both.
+ * Construct an object to map a frame buffer for CPU access. The mapping can be
+ * made as Read only, Write only or support Read and Write operations by setting
+ * the MapFlag flags accordingly.
*/
-MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)
+MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags)
{
- maps_.reserve(buffer->planes().size());
+ ASSERT(!buffer->planes().empty());
+ planes_.reserve(buffer->planes().size());
+
+ int mmapFlags = 0;
+
+ if (flags & MapFlag::Read)
+ mmapFlags |= PROT_READ;
+
+ if (flags & MapFlag::Write)
+ mmapFlags |= PROT_WRITE;
+
+ struct MappedBufferInfo {
+ uint8_t *address = nullptr;
+ size_t mapLength = 0;
+ size_t dmabufLength = 0;
+ };
+ std::map<int, MappedBufferInfo> mappedBuffers;
+
+ for (const FrameBuffer::Plane &plane : buffer->planes()) {
+ const int fd = plane.fd.fd();
+ if (mappedBuffers.find(fd) == mappedBuffers.end()) {
+ const size_t length = lseek(fd, 0, SEEK_END);
+ mappedBuffers[fd] = MappedBufferInfo{ nullptr, 0, length };
+ }
+
+ const size_t length = mappedBuffers[fd].dmabufLength;
+
+ if (plane.offset > length ||
+ plane.offset + plane.length > length) {
+ LOG(Buffer, Fatal) << "plane is out of buffer: "
+ << "buffer length=" << length
+ << ", plane offset=" << plane.offset
+ << ", plane length=" << plane.length;
+ return;
+ }
+ size_t &mapLength = mappedBuffers[fd].mapLength;
+ mapLength = std::max(mapLength,
+ static_cast<size_t>(plane.offset + plane.length));
+ }
for (const FrameBuffer::Plane &plane : buffer->planes()) {
- void *address = mmap(nullptr, plane.length, flags,
- MAP_SHARED, plane.fd.fd(), 0);
- if (address == MAP_FAILED) {
- error_ = -errno;
- LOG(MappedBuffer, Error) << "Failed to mmap plane";
- break;
+ const int fd = plane.fd.fd();
+ auto &info = mappedBuffers[fd];
+ if (!info.address) {
+ void *address = mmap(nullptr, info.mapLength, mmapFlags,
+ MAP_SHARED, fd, 0);
+ if (address == MAP_FAILED) {
+ error_ = -errno;
+ LOG(Buffer, Error) << "Failed to mmap plane: "
+ << strerror(-error_);
+ return;
+ }
+
+ info.address = static_cast<uint8_t *>(address);
+ maps_.emplace_back(info.address, info.mapLength);
}
- maps_.emplace_back(static_cast<uint8_t *>(address), plane.length);
+ planes_.emplace_back(info.address + plane.offset, plane.length);
}
}
diff --git a/src/libcamera-helpers/meson.build b/src/libcamera-helpers/meson.build
index 444f21204d5d..084bf65345ae 100644
--- a/src/libcamera-helpers/meson.build
+++ b/src/libcamera-helpers/meson.build
@@ -2,5 +2,5 @@
# Implementation of internal libcamera internals
libcamera_helpers = files([
- 'mapped_buffer.cpp',
+ 'mapped_framebuffer.cpp',
])
--
2.30.2
More information about the libcamera-devel
mailing list