[libcamera-devel] [PATCH v3 3/3] libcamera: MappedFrameBuffer: Use typed Flags<MapFlags>

Kieran Bingham kieran.bingham at ideasonboard.com
Mon Aug 9 15:29:29 CEST 2021


Remove the need for callers to reference PROT_READ/PROT_WRITE directly
from <sys/mman.h> by instead exposing the Read/Write mapping options as
flags from the MappedFrameBuffer class itself.

While here, introduce the <stdint.h> header which is required for the
uint8_t as part of the Plane.

Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

---
v2
 - Use fully scoped enum class
 - Swap MapMode and MapModes
 - s/mmap_flags/mmapFlags/
 - Remove and fix documentation regarding the modes
 - add LIBCAMERA_FLAGS_ENABLE_OPERATORS()

v3:
 - Remove no longer necessary sys/mman.h inclusions throughout
 - fix documentation
 - rename MapMode as MapFlag and MapModes as MapFlags
---
 .../libcamera/internal/mapped_framebuffer.h   | 15 ++++++--
 src/android/jpeg/encoder_libjpeg.cpp          |  3 +-
 src/android/jpeg/thumbnailer.cpp              |  2 +-
 src/android/yuv/post_processor_yuv.cpp        |  2 +-
 src/ipa/ipu3/ipu3.cpp                         |  3 +-
 src/ipa/raspberrypi/raspberrypi.cpp           |  3 +-
 src/libcamera/framebuffer.cpp                 |  1 -
 src/libcamera/mapped_framebuffer.cpp          | 34 ++++++++++++++++---
 test/mapped-buffer.cpp                        |  6 ++--
 9 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h
index 41e587364260..68c8a43c35dc 100644
--- a/include/libcamera/internal/mapped_framebuffer.h
+++ b/include/libcamera/internal/mapped_framebuffer.h
@@ -7,10 +7,11 @@
 #ifndef __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_H__
 #define __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_H__
 
-#include <sys/mman.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>
@@ -44,9 +45,19 @@ private:
 class MappedFrameBuffer : public MappedBuffer
 {
 public:
-	MappedFrameBuffer(const FrameBuffer *buffer, int flags);
+	enum class MapFlag {
+		Read = 0 << 1,
+		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_INTERNAL_MAPPED_FRAMEBUFFER_H__ */
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
index 372018d2207f..a7a636011045 100644
--- a/src/android/jpeg/encoder_libjpeg.cpp
+++ b/src/android/jpeg/encoder_libjpeg.cpp
@@ -12,7 +12,6 @@
 #include <iostream>
 #include <sstream>
 #include <string.h>
-#include <sys/mman.h>
 #include <unistd.h>
 #include <vector>
 
@@ -182,7 +181,7 @@ void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
 int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,
 			   Span<const uint8_t> exifData, unsigned int quality)
 {
-	MappedFrameBuffer frame(&source, PROT_READ);
+	MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
 	if (!frame.isValid()) {
 		LOG(JPEG, Error) << "Failed to map FrameBuffer : "
 				 << strerror(frame.error());
diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
index 535e2cece914..79d83926af5c 100644
--- a/src/android/jpeg/thumbnailer.cpp
+++ b/src/android/jpeg/thumbnailer.cpp
@@ -41,7 +41,7 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source,
 				  const Size &targetSize,
 				  std::vector<unsigned char> *destination)
 {
-	MappedFrameBuffer frame(&source, PROT_READ);
+	MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
 	if (!frame.isValid()) {
 		LOG(Thumbnailer, Error)
 			<< "Failed to map FrameBuffer : "
diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
index 509d4244d614..3e793a580377 100644
--- a/src/android/yuv/post_processor_yuv.cpp
+++ b/src/android/yuv/post_processor_yuv.cpp
@@ -57,7 +57,7 @@ int PostProcessorYuv::process(const FrameBuffer &source,
 	if (!isValidBuffers(source, *destination))
 		return -EINVAL;
 
-	const MappedFrameBuffer sourceMapped(&source, PROT_READ);
+	const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read);
 	if (!sourceMapped.isValid()) {
 		LOG(YUV, Error) << "Failed to mmap camera frame buffer";
 		return -EINVAL;
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 2647bf2f3b96..10093acdad30 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -6,7 +6,6 @@
  */
 
 #include <stdint.h>
-#include <sys/mman.h>
 
 #include <linux/intel-ipu3.h>
 #include <linux/v4l2-controls.h>
@@ -211,7 +210,7 @@ void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)
 	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/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 76f67dd4567a..592e2fe41617 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -411,7 +411,8 @@ void IPARPi::mapBuffers(const std::vector<IPABuffer> &buffers)
 {
 	for (const IPABuffer &buffer : buffers) {
 		const FrameBuffer fb(buffer.planes);
-		buffers_.emplace(buffer.id, MappedFrameBuffer(&fb, PROT_READ | PROT_WRITE));
+		buffers_.emplace(buffer.id,
+				 MappedFrameBuffer(&fb, MappedFrameBuffer::MapFlag::ReadWrite));
 	}
 }
 
diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
index 3d98affb20f9..2223e24321ca 100644
--- a/src/libcamera/framebuffer.cpp
+++ b/src/libcamera/framebuffer.cpp
@@ -10,7 +10,6 @@
 
 #include <errno.h>
 #include <string.h>
-#include <sys/mman.h>
 #include <unistd.h>
 
 #include <libcamera/base/log.h>
diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp
index 0e30fc542154..5a70c18147f2 100644
--- a/src/libcamera/mapped_framebuffer.cpp
+++ b/src/libcamera/mapped_framebuffer.cpp
@@ -142,21 +142,45 @@ 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());
 
+	int mmapFlags = 0;
+
+	if (flags & MapFlag::Read)
+		mmapFlags |= PROT_READ;
+
+	if (flags & MapFlag::Write)
+		mmapFlags |= PROT_WRITE;
+
 	for (const FrameBuffer::Plane &plane : buffer->planes()) {
-		void *address = mmap(nullptr, plane.length, flags,
+		void *address = mmap(nullptr, plane.length, mmapFlags,
 				     MAP_SHARED, plane.fd.fd(), 0);
 		if (address == MAP_FAILED) {
 			error_ = -errno;
diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp
index a3d1511b74ce..97571945cbca 100644
--- a/test/mapped-buffer.cpp
+++ b/test/mapped-buffer.cpp
@@ -71,7 +71,7 @@ protected:
 		const std::unique_ptr<FrameBuffer> &buffer = allocator_->buffers(stream_).front();
 		std::vector<MappedBuffer> maps;
 
-		MappedFrameBuffer map(buffer.get(), PROT_READ);
+		MappedFrameBuffer map(buffer.get(), MappedFrameBuffer::MapFlag::Read);
 		if (!map.isValid()) {
 			cout << "Failed to successfully map buffer" << endl;
 			return TestFail;
@@ -90,13 +90,13 @@ protected:
 		}
 
 		/* Test for multiple successful maps on the same buffer. */
-		MappedFrameBuffer write_map(buffer.get(), PROT_WRITE);
+		MappedFrameBuffer write_map(buffer.get(), MappedFrameBuffer::MapFlag::Write);
 		if (!write_map.isValid()) {
 			cout << "Failed to map write buffer" << endl;
 			return TestFail;
 		}
 
-		MappedFrameBuffer rw_map(buffer.get(), PROT_READ | PROT_WRITE);
+		MappedFrameBuffer rw_map(buffer.get(), MappedFrameBuffer::MapFlag::ReadWrite);
 		if (!rw_map.isValid()) {
 			cout << "Failed to map RW buffer" << endl;
 			return TestFail;
-- 
2.30.2



More information about the libcamera-devel mailing list