[libcamera-devel,v2,03/12] libcamera: buffer: Create a MappedBuffer

Message ID 20200803161816.107113-4-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • android: jpeg
Related show

Commit Message

Kieran Bingham Aug. 3, 2020, 4:18 p.m. UTC
Provide a MappedFrameBuffer helper class which will map
all of the Planes within a FrameBuffer and provide CPU addressable
pointers for those planes.

The MappedFrameBuffer implements the interface of the MappedBuffer
allowing other buffer types to be constructed of the same form, with a
common interface and cleanup.

This allows MappedBuffer instances to be created from Camera3Buffer types.

Mappings are removed upon destruction.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/internal/buffer.h |  46 ++++++++++
 src/libcamera/buffer.cpp            | 134 ++++++++++++++++++++++++++++
 2 files changed, 180 insertions(+)
 create mode 100644 include/libcamera/internal/buffer.h

Comments

Kieran Bingham Aug. 4, 2020, 11:44 a.m. UTC | #1
Hi all,

This patch as posted was missing doxygen documentation for maps(),
maps_, and the return type for error().

It was also missing context, and the title line for the class brief on
MappedBuffer.

These items are fixed with the following squash:


From b85827bb2c44dac0e042932a8949715583931bf8 Mon Sep 17 00:00:00 2001
From: Kieran Bingham <kieran.bingham@ideasonboard.com>
Date: Tue, 4 Aug 2020 12:38:17 +0100
Subject: [PATCH] fixup! libcamera: buffer: Create a MappedBuffer

---
 src/libcamera/buffer.cpp | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
index 0c289051bc07..6d335f1f7644 100644
--- a/src/libcamera/buffer.cpp
+++ b/src/libcamera/buffer.cpp
@@ -284,7 +284,7 @@ int FrameBuffer::copyFrom(const FrameBuffer *src)

 /**
  * \class MappedBuffer
- * \brief Provide
+ * \brief Provide an interface to support managing memory mapped buffers
  *
  * The MappedBuffer interface provides access to a set of MappedPlanes
which
  * are available for access by a CPU.
@@ -293,6 +293,10 @@ int FrameBuffer::copyFrom(const FrameBuffer *src)
  * defines the move operators and destructors for the derived
implementations,
  * which are able to construct according to their derived types and given
  * flags.
+ *
+ * This allows treating CPU accessible memory through a generic interface
+ * regardless of whether it originates from a libcamera FrameBuffer or
other
+ * source.
  */

 /**
@@ -359,6 +363,18 @@ MappedBuffer::~MappedBuffer()
  * This function retrieves the error status from the MappedBuffer.
  * The error status is a negative number as defined by errno.h. If
  * no error occurred, this function returns 0.
+ *
+ * \return 0 on success or a negative error code otherwise
+ */
+
+/**
+ * \fn MappedBuffer::maps()
+ * \brief Retrieve the mapped planes
+ *
+ * This function retrieves the successfully mapped planes stored as a
vector
+ * of Span<uint8_t> to provide access to the mapped memory.
+ *
+ * \return A vector of the mapped planes.
  */

 /**
@@ -377,6 +393,15 @@ MappedBuffer::~MappedBuffer()
  * by errno.h if an error occured during the mapping process
  */

+/**
+ * \var MappedBuffer::maps_
+ * \brief Stores the internal
+ *
+ * MappedBuffer implementations shall store the mappings they create in
this
+ * vector which is parsed during destruct to unmap any memory mappings
which
+ * completed successfully.
+ */
+
 /**
  * \class MappedFrameBuffer
  * \brief Maps a FrameBuffer using the MappedBuffer interface

Patch

diff --git a/include/libcamera/internal/buffer.h b/include/libcamera/internal/buffer.h
new file mode 100644
index 000000000000..c04c294fecda
--- /dev/null
+++ b/include/libcamera/internal/buffer.h
@@ -0,0 +1,46 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2020, Google Inc.
+ *
+ * buffer.h - Internal buffer handling
+ */
+#ifndef __LIBCAMERA_INTERNAL_BUFFER_H__
+#define __LIBCAMERA_INTERNAL_BUFFER_H__
+
+#include <vector>
+
+#include <libcamera/buffer.h>
+#include <libcamera/span.h>
+
+namespace libcamera {
+
+using MappedPlane = Span<uint8_t>;
+
+class MappedBuffer
+{
+public:
+	MappedBuffer();
+	~MappedBuffer();
+
+	MappedBuffer(MappedBuffer &&rhs);
+	MappedBuffer &operator=(MappedBuffer &&rhs);
+
+	bool isValid() const { return valid_; }
+	int error() const { return error_; }
+	const std::vector<MappedPlane> &maps() const { return maps_; }
+
+protected:
+	int error_;
+	bool valid_;
+	std::vector<MappedPlane> maps_;
+};
+
+class MappedFrameBuffer : public MappedBuffer
+{
+public:
+	MappedFrameBuffer(const FrameBuffer *buffer, int flags);
+};
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_INTERNAL_BUFFER_H__ */
diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
index 8278f8a92af4..2f667db42922 100644
--- a/src/libcamera/buffer.cpp
+++ b/src/libcamera/buffer.cpp
@@ -6,6 +6,7 @@ 
  */
 
 #include <libcamera/buffer.h>
+#include "libcamera/internal/buffer.h"
 
 #include <errno.h>
 #include <string.h>
@@ -290,4 +291,137 @@  int FrameBuffer::copyFrom(const FrameBuffer *src)
 	return 0;
 }
 
+/**
+ * \class MappedBuffer
+ * \brief Provide
+ *
+ * The MappedBuffer interface provides access to a set of MappedPlanes which
+ * are available for access by a CPU.
+ *
+ * The MappedBuffer interface does not implement any valid constructor but
+ * defines the move operators and destructors for the derived implementations,
+ * which are able to construct according to their derived types and given
+ * flags.
+ */
+
+/**
+ * \brief Construct an empty MappedBuffer
+ *
+ * A default constructor is required to allow subclassing the MappedBuffer
+ * class. Construct an initialised, but invalid MappedBuffer.
+ */
+MappedBuffer::MappedBuffer()
+	: error_(0), valid_(false)
+{
+}
+
+/**
+ * \brief Construct a MappedBuffer by taking the \a rhs mappings
+ * \param[in] rhs The other MappedBuffer
+ *
+ * Moving a MappedBuffer moves the mappings contained in the \a rhs to the new
+ * MappedBuffer and invalidates the \a rhs. No mappings are unmapped or
+ * destroyed in this process.
+ */
+MappedBuffer::MappedBuffer(MappedBuffer &&rhs)
+{
+	*this = std::move(rhs);
+}
+
+/**
+ * \brief Move assingment operator, move a MappedBuffer by taking the \a rhs mappings
+ * \param[in] rhs The other MappedBuffer
+ *
+ * Moving a MappedBuffer moves the mappings contained in the \a rhs to the new
+ * MappedBuffer and invalidates the \a rhs. No mappings are unmapped or
+ * destroyed in this process.
+ */
+MappedBuffer &MappedBuffer::operator=(MappedBuffer &&rhs)
+{
+	error_ = rhs.error_;
+	valid_ = rhs.valid_;
+	maps_ = std::move(rhs.maps_);
+	rhs.valid_ = false;
+	rhs.error_ = 0;
+
+	return *this;
+}
+
+MappedBuffer::~MappedBuffer()
+{
+	for (MappedPlane map : maps_)
+		munmap(map.data(), map.size());
+}
+
+/**
+ * \fn MappedBuffer::isValid()
+ * \brief Check if the MappedBuffer instance is valid
+ * \return True if the MappedBuffer has valid mappings, false otherwise
+ */
+
+/**
+ * \fn MappedBuffer::error()
+ * \brief Retrieve the map error status
+ *
+ * This function retrieves the error status from the MappedBuffer.
+ * The error status is a negative number as defined by errno.h. If
+ * no error occurred, this function returns 0.
+ */
+
+/**
+ * \var MappedBuffer::valid_
+ * \brief Stores the status of the mapping
+ *
+ * MappedBuffer implementations shall set this to represent if the mapping
+ * was successfully completed without errors.
+ */
+
+/**
+ * \var MappedBuffer::error_
+ * \brief Stores the error value if present
+ *
+ * MappedBuffer implementations shall set this to a negative value as defined
+ * by errno.h if an error occured during the mapping process
+ */
+
+/**
+ * \class MappedFrameBuffer
+ * \brief Maps a FrameBuffer using the MappedBuffer interface
+ *
+ * The MappedFrameBuffer interface maps a FrameBuffer instance
+ *
+ * The MappedBuffer interface does not implement any constructor but defines
+ * the move operators and destructors for the derived implementations, which
+ * are able to construct according to their derived types and given flags.
+ */
+
+/**
+ * \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.
+ */
+MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)
+{
+	maps_.reserve(buffer->planes().size());
+
+	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(Buffer, Error) << "Failed to mmap plane";
+			break;
+		}
+
+		maps_.emplace_back(static_cast<uint8_t *>(address), plane.length);
+	}
+
+	valid_ = buffer->planes().size() == maps_.size();
+}
+
 } /* namespace libcamera */