libcamera: Give MappedFrameBuffer its own implementation

The MappedFrameBuffer is a convenience feature which sits on top of the
FrameBuffer and facilitates mapping it to CPU accessible memory with
mmap.

This implementation is internal and currently sits in the same internal
files as the internal FrameBuffer, thus exposing those internals to
users of the MappedFramebuffer implementation.

Move the MappedFrameBuffer and MappedBuffer implementation to its own
implementation files, and fix the sources throughout to use that
accordingly.

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

include/libcamera/internal/framebuffer.h

@@ -7,46 +7,12 @@
 #ifndef __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__
 #define __LIBCAMERA_INTERNAL_FRAMEBUFFER_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);
-};
-
 class FrameBuffer::Private : public Extensible::Private
 {
 	LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)

include/libcamera/internal/mapped_framebuffer.h

@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021, Google Inc.
+ *
+ * mapped_framebuffer.h - Frame buffer memory mapping support
+ */
+#ifndef __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_H__
+#define __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_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_INTERNAL_MAPPED_FRAMEBUFFER_H__ */

include/libcamera/internal/meson.build

@@ -28,6 +28,7 @@ libcamera_internal_headers = files([
     'ipa_module.h',
     'ipa_proxy.h',
     'ipc_unixsocket.h',
+    'mapped_framebuffer.h',
     'media_device.h',
     'media_object.h',
     'pipeline_handler.h',

src/android/camera_device.h

@@ -24,8 +24,6 @@
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
-#include "libcamera/internal/framebuffer.h"
-
 #include "camera_capabilities.h"
 #include "camera_metadata.h"
 #include "camera_stream.h"

src/android/camera_stream.cpp

@@ -7,6 +7,8 @@
 
 #include "camera_stream.h"
 
+#include <sys/mman.h>
+
 #include "camera_buffer.h"
 #include "camera_device.h"
 #include "camera_metadata.h"

src/android/camera_stream.h

@@ -19,8 +19,6 @@
 #include <libcamera/geometry.h>
 #include <libcamera/pixel_format.h>
 
-#include "libcamera/internal/framebuffer.h"
-
 class CameraDevice;
 class CameraMetadata;
 class PostProcessor;

src/android/jpeg/encoder_libjpeg.cpp

@@ -23,6 +23,7 @@
 #include <libcamera/pixel_format.h>
 
 #include "libcamera/internal/formats.h"
+#include "libcamera/internal/mapped_framebuffer.h"
 
 using namespace libcamera;
 

src/android/jpeg/encoder_libjpeg.h

@@ -10,7 +10,6 @@
 #include "encoder.h"
 
 #include "libcamera/internal/formats.h"
-#include "libcamera/internal/framebuffer.h"
 
 #include <jpeglib.h>
 

src/android/jpeg/post_processor_jpeg.h

@@ -13,8 +13,6 @@
 
 #include <libcamera/geometry.h>
 
-#include "libcamera/internal/framebuffer.h"
-
 class CameraDevice;
 
 class PostProcessorJpeg : public PostProcessor

src/android/jpeg/thumbnailer.cpp

@@ -11,6 +11,8 @@
 
 #include <libcamera/formats.h>
 
+#include "libcamera/internal/mapped_framebuffer.h"
+
 using namespace libcamera;
 
 LOG_DEFINE_CATEGORY(Thumbnailer)

src/android/jpeg/thumbnailer.h

@@ -7,10 +7,10 @@
 #ifndef __ANDROID_JPEG_THUMBNAILER_H__
 #define __ANDROID_JPEG_THUMBNAILER_H__
 
+#include <libcamera/framebuffer.h>
 #include <libcamera/geometry.h>
 
 #include "libcamera/internal/formats.h"
-#include "libcamera/internal/framebuffer.h"
 
 class Thumbnailer
 {

src/android/mm/generic_camera_buffer.cpp

@@ -7,11 +7,12 @@
 
 #include "../camera_buffer.h"
 
+#include <sys/mman.h>
 #include <unistd.h>
 
 #include <libcamera/base/log.h>
 
-#include "libcamera/internal/framebuffer.h"
+#include "libcamera/internal/mapped_framebuffer.h"
 
 using namespace libcamera;
 

src/android/post_processor.h

@@ -10,8 +10,6 @@
 #include <libcamera/framebuffer.h>
 #include <libcamera/stream.h>
 
-#include "libcamera/internal/framebuffer.h"
-
 #include "camera_buffer.h"
 
 class CameraMetadata;

src/android/yuv/post_processor_yuv.cpp

@@ -16,6 +16,7 @@
 #include <libcamera/pixel_format.h>
 
 #include "libcamera/internal/formats.h"
+#include "libcamera/internal/mapped_framebuffer.h"
 
 using namespace libcamera;
 

src/ipa/ipu3/ipu3.cpp

@@ -20,7 +20,7 @@
 #include <libcamera/ipa/ipu3_ipa_interface.h>
 #include <libcamera/request.h>
 
-#include "libcamera/internal/framebuffer.h"
+#include "libcamera/internal/mapped_framebuffer.h"
 
 #include "ipu3_agc.h"
 #include "ipu3_awb.h"

src/ipa/raspberrypi/raspberrypi.cpp

@@ -28,7 +28,7 @@
 #include <libcamera/ipa/raspberrypi_ipa_interface.h>
 #include <libcamera/request.h>
 
-#include "libcamera/internal/framebuffer.h"
+#include "libcamera/internal/mapped_framebuffer.h"
 
 #include "agc_algorithm.hpp"
 #include "agc_status.h"

src/libcamera/framebuffer.cpp

@@ -8,11 +8,6 @@
 #include <libcamera/framebuffer.h>
 #include "libcamera/internal/framebuffer.h"
 
-#include <errno.h>
-#include <string.h>
-#include <sys/mman.h>
-#include <unistd.h>
-
 #include <libcamera/base/log.h>
 
 /**
@@ -251,149 +246,4 @@ Request *FrameBuffer::request() const
  * indicate that the metadata is invalid.
  */
 
-/**
- * \class MappedBuffer
- * \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 the CPU.
- *
- * This class is not meant to be constructed directly, but instead derived
- * classes should be used to implement the correct mapping of a source buffer.
- *
- * This allows treating CPU accessible memory through a generic interface
- * regardless of whether it originates from a libcamera FrameBuffer or other
- * source.
- */
-
-/**
- * \typedef MappedBuffer::Plane
- * \brief A mapped region of memory accessible to the CPU
- *
- * The MappedBuffer::Plane uses the Span interface to describe the mapped memory
- * region.
- */
-
-/**
- * \brief Construct an empty MappedBuffer
- */
-MappedBuffer::MappedBuffer()
-	: error_(0)
-{
-}
-
-/**
- * \brief Move constructor, construct the MappedBuffer with the contents of \a
- * other using move semantics
- * \param[in] other The other MappedBuffer
- *
- * Moving a MappedBuffer moves the mappings contained in the \a other to the new
- * MappedBuffer and invalidates the \a other.
- *
- * No mappings are unmapped or destroyed in this process.
- */
-MappedBuffer::MappedBuffer(MappedBuffer &&other)
-{
-	*this = std::move(other);
-}
-
-/**
- * \brief Move assignment operator, replace the mappings with those of \a other
-* \param[in] other The other MappedBuffer
- *
- * Moving a MappedBuffer moves the mappings contained in the \a other to the new
- * MappedBuffer and invalidates the \a other.
- *
- * No mappings are unmapped or destroyed in this process.
- */
-MappedBuffer &MappedBuffer::operator=(MappedBuffer &&other)
-{
-	error_ = other.error_;
-	maps_ = std::move(other.maps_);
-	other.error_ = -ENOENT;
-
-	return *this;
-}
-
-MappedBuffer::~MappedBuffer()
-{
-	for (Plane &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.
- *
- * \return The map error code
- */
-
-/**
- * \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
- */
-
-/**
- * \var MappedBuffer::error_
- * \brief Stores the error value if present
- *
- * MappedBuffer derived classes shall set this to a negative value as defined
- * by errno.h if an error occured during the mapping process.
- */
-
-/**
- * \var MappedBuffer::maps_
- * \brief Stores the internal mapped planes
- *
- * 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.
- */
-
-/**
- * \class MappedFrameBuffer
- * \brief Map a FrameBuffer using the MappedBuffer interface
- */
-
-/**
- * \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);
-	}
-}
-
 } /* namespace libcamera */

src/libcamera/mapped_framebuffer.cpp

@@ -0,0 +1,169 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021, Google Inc.
+ *
+ * mapped_framebuffer.cpp - Mapped Framebuffer support
+ */
+
+#include "libcamera/internal/mapped_framebuffer.h"
+
+#include <errno.h>
+#include <sys/mman.h>
+
+#include <libcamera/base/log.h>
+
+/**
+ * \file libcamera/internal/mapped_framebuffer.h
+ * \brief Frame buffer memory mapping support
+ */
+
+namespace libcamera {
+
+LOG_DECLARE_CATEGORY(Buffer)
+
+/**
+ * \class MappedBuffer
+ * \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 the CPU.
+ *
+ * This class is not meant to be constructed directly, but instead derived
+ * classes should be used to implement the correct mapping of a source buffer.
+ *
+ * This allows treating CPU accessible memory through a generic interface
+ * regardless of whether it originates from a libcamera FrameBuffer or other
+ * source.
+ */
+
+/**
+ * \typedef MappedBuffer::Plane
+ * \brief A mapped region of memory accessible to the CPU
+ *
+ * The MappedBuffer::Plane uses the Span interface to describe the mapped memory
+ * region.
+ */
+
+/**
+ * \brief Construct an empty MappedBuffer
+ */
+MappedBuffer::MappedBuffer()
+	: error_(0)
+{
+}
+
+/**
+ * \brief Move constructor, construct the MappedBuffer with the contents of \a
+ * other using move semantics
+ * \param[in] other The other MappedBuffer
+ *
+ * Moving a MappedBuffer moves the mappings contained in the \a other to the new
+ * MappedBuffer and invalidates the \a other.
+ *
+ * No mappings are unmapped or destroyed in this process.
+ */
+MappedBuffer::MappedBuffer(MappedBuffer &&other)
+{
+	*this = std::move(other);
+}
+
+/**
+ * \brief Move assignment operator, replace the mappings with those of \a other
+* \param[in] other The other MappedBuffer
+ *
+ * Moving a MappedBuffer moves the mappings contained in the \a other to the new
+ * MappedBuffer and invalidates the \a other.
+ *
+ * No mappings are unmapped or destroyed in this process.
+ */
+MappedBuffer &MappedBuffer::operator=(MappedBuffer &&other)
+{
+	error_ = other.error_;
+	maps_ = std::move(other.maps_);
+	other.error_ = -ENOENT;
+
+	return *this;
+}
+
+MappedBuffer::~MappedBuffer()
+{
+	for (Plane &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.
+ *
+ * \return The map error code
+ */
+
+/**
+ * \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
+ */
+
+/**
+ * \var MappedBuffer::error_
+ * \brief Stores the error value if present
+ *
+ * MappedBuffer derived classes shall set this to a negative value as defined
+ * by errno.h if an error occured during the mapping process.
+ */
+
+/**
+ * \var MappedBuffer::maps_
+ * \brief Stores the internal mapped planes
+ *
+ * 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.
+ */
+
+/**
+ * \class MappedFrameBuffer
+ * \brief Map a FrameBuffer using the MappedBuffer interface
+ */
+
+/**
+ * \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);
+	}
+}
+
+} /* namespace libcamera */

src/libcamera/meson.build

@@ -28,6 +28,7 @@ libcamera_sources = files([
     'ipc_pipe.cpp',
     'ipc_pipe_unixsocket.cpp',
     'ipc_unixsocket.cpp',
+    'mapped_framebuffer.cpp',
     'media_device.cpp',
     'media_object.cpp',
     'pipeline_handler.cpp',

test/mapped-buffer.cpp

@@ -9,7 +9,7 @@
 
 #include <libcamera/framebuffer_allocator.h>
 
-#include "libcamera/internal/framebuffer.h"
+#include "libcamera/internal/mapped_framebuffer.h"
 
 #include "camera_test.h"
 #include "test.h"