[libcamera-devel] [RFC PATCH 1/5] libcamera: mapped_framebuffer: Return plane begin address by MappedBuffer::maps()

[Hirokazu Honda]

MappedBuffer::maps() returns std::vector<MappedBuffer::Plane>.
Plane has the address, but the address points the beginning of the
buffer containing the plane.
This makes the Plane point the beginning of the plane. So
MappedBuffer::maps()[i].data() returns the address of i-th plane.

Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
---
 .../libcamera/internal/mapped_framebuffer.h   |  4 +-
 src/libcamera/mapped_framebuffer.cpp          | 59 +++++++++++++++----
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h
index 3401a9fc..42479541 100644
--- a/include/libcamera/internal/mapped_framebuffer.h
+++ b/include/libcamera/internal/mapped_framebuffer.h
@@ -30,12 +30,14 @@ public:
 
 	bool isValid() const { return error_ == 0; }
 	int error() const { return error_; }
-	const std::vector<Plane> &maps() const { return maps_; }
+	/* \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:
diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp
index 2ebe9fdb..5d5b02f1 100644
--- a/src/libcamera/mapped_framebuffer.cpp
+++ b/src/libcamera/mapped_framebuffer.cpp
@@ -9,6 +9,7 @@
 
 #include <errno.h>
 #include <sys/mman.h>
+#include <unistd.h>
 
 #include <libcamera/base/log.h>
 
@@ -79,6 +80,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;
 
@@ -127,10 +129,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.
  */
@@ -167,7 +177,8 @@ MappedBuffer::~MappedBuffer()
  */
 MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags)
 {
-	maps_.reserve(buffer->planes().size());
+	ASSERT(!buffer->planes().empty());
+	planes_.reserve(buffer->planes().size());
 
 	int mmapFlags = 0;
 
@@ -177,18 +188,46 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags)
 	if (flags & MapFlag::Write)
 		mmapFlags |= PROT_WRITE;
 
+	size_t offset = 0;
+	int prevFd = -1;
 	for (const FrameBuffer::Plane &plane : buffer->planes()) {
-		void *address = mmap(nullptr, plane.length, mmapFlags,
-				     MAP_SHARED, plane.fd.fd(), 0);
-		if (address == MAP_FAILED) {
-			error_ = -errno;
-			LOG(Buffer, Error) << "Failed to mmap plane: "
-					   << strerror(-error_);
-			break;
+		const int fd = plane.fd.fd();
+		if (prevFd != fd) {
+			const size_t length = lseek(fd, 0, SEEK_END);
+			void *address = mmap(nullptr, length, mmapFlags,
+					     MAP_SHARED, fd, 0);
+			if (address == MAP_FAILED) {
+				error_ = -errno;
+				LOG(Buffer, Error) << "Failed to mmap plane: "
+						   << strerror(-error_);
+				break;
+			}
+			maps_.emplace_back(static_cast<uint8_t *>(address),
+					   length);
+			prevFd = fd;
 		}
 
-		maps_.emplace_back(static_cast<uint8_t *>(address), plane.length);
+		ASSERT(!maps_.empty());
+		uint8_t *buf = maps_.back().data();
+		const size_t length = maps_.back().size();
+
+		/*
+		 * This offset calculation assumes planes are consecutive.
+		 * \todo remove this assumption once offset is introduced to
+		 * FrameBuffer::Plane.
+		 */
+		planes_.emplace_back(buf + offset, plane.length);
+		offset += plane.length;
+
+		if (offset > length) {
+			LOG(Buffer, Fatal)
+				<< "plane length is too large: plane lengths="
+				<< offset << ", buffer length=" << length;
+			break;
+		}
 	}
+
+	ASSERT(maps_.size() == 1);
 }
 
 } /* namespace libcamera */
-- 
2.32.0.605.g8dce9f2422-goog