From f3629363c4d25f32c76449740e918445180906db Mon Sep 17 00:00:00 2001 From: Kieran Bingham Date: Fri, 6 Aug 2021 10:18:30 +0100 Subject: 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 Reviewed-by: Laurent Pinchart Signed-off-by: Kieran Bingham --- include/libcamera/internal/framebuffer.h | 34 ----- include/libcamera/internal/mapped_framebuffer.h | 52 ++++++++ include/libcamera/internal/meson.build | 1 + src/android/camera_device.h | 2 - src/android/camera_stream.cpp | 2 + src/android/camera_stream.h | 2 - src/android/jpeg/encoder_libjpeg.cpp | 1 + src/android/jpeg/encoder_libjpeg.h | 1 - src/android/jpeg/post_processor_jpeg.h | 2 - src/android/jpeg/thumbnailer.cpp | 2 + src/android/jpeg/thumbnailer.h | 2 +- src/android/mm/generic_camera_buffer.cpp | 3 +- src/android/post_processor.h | 2 - src/android/yuv/post_processor_yuv.cpp | 1 + src/ipa/ipu3/ipu3.cpp | 2 +- src/ipa/raspberrypi/raspberrypi.cpp | 2 +- src/libcamera/framebuffer.cpp | 150 --------------------- src/libcamera/mapped_framebuffer.cpp | 169 ++++++++++++++++++++++++ src/libcamera/meson.build | 1 + test/mapped-buffer.cpp | 2 +- 20 files changed, 235 insertions(+), 198 deletions(-) create mode 100644 include/libcamera/internal/mapped_framebuffer.h create mode 100644 src/libcamera/mapped_framebuffer.cpp diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h index 8c187adf..606aed2b 100644 --- a/include/libcamera/internal/framebuffer.h +++ b/include/libcamera/internal/framebuffer.h @@ -7,46 +7,12 @@ #ifndef __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__ #define __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__ -#include -#include - #include -#include #include namespace libcamera { -class MappedBuffer -{ -public: - using Plane = Span; - - ~MappedBuffer(); - - MappedBuffer(MappedBuffer &&other); - MappedBuffer &operator=(MappedBuffer &&other); - - bool isValid() const { return error_ == 0; } - int error() const { return error_; } - const std::vector &maps() const { return maps_; } - -protected: - MappedBuffer(); - - int error_; - std::vector 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) diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h new file mode 100644 index 00000000..41e58736 --- /dev/null +++ b/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 +#include + +#include +#include + +#include + +namespace libcamera { + +class MappedBuffer +{ +public: + using Plane = Span; + + ~MappedBuffer(); + + MappedBuffer(MappedBuffer &&other); + MappedBuffer &operator=(MappedBuffer &&other); + + bool isValid() const { return error_ == 0; } + int error() const { return error_; } + const std::vector &maps() const { return maps_; } + +protected: + MappedBuffer(); + + int error_; + std::vector 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__ */ diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index dac1a2d3..665fd6de 100644 --- a/include/libcamera/internal/meson.build +++ b/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', diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 089a6204..dd9aebba 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -24,8 +24,6 @@ #include #include -#include "libcamera/internal/framebuffer.h" - #include "camera_capabilities.h" #include "camera_metadata.h" #include "camera_stream.h" diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index bf4a7b41..61b44183 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -7,6 +7,8 @@ #include "camera_stream.h" +#include + #include "camera_buffer.h" #include "camera_device.h" #include "camera_metadata.h" diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index 629d9e00..2dab6c3a 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -19,8 +19,6 @@ #include #include -#include "libcamera/internal/framebuffer.h" - class CameraDevice; class CameraMetadata; class PostProcessor; diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp index e6358ca9..372018d2 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/encoder_libjpeg.cpp @@ -23,6 +23,7 @@ #include #include "libcamera/internal/formats.h" +#include "libcamera/internal/mapped_framebuffer.h" using namespace libcamera; diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h index 14bf8922..61fbd1a6 100644 --- a/src/android/jpeg/encoder_libjpeg.h +++ b/src/android/jpeg/encoder_libjpeg.h @@ -10,7 +10,6 @@ #include "encoder.h" #include "libcamera/internal/formats.h" -#include "libcamera/internal/framebuffer.h" #include diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index 5c399be9..6fd31022 100644 --- a/src/android/jpeg/post_processor_jpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -13,8 +13,6 @@ #include -#include "libcamera/internal/framebuffer.h" - class CameraDevice; class PostProcessorJpeg : public PostProcessor diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp index 5cb00744..535e2cec 100644 --- a/src/android/jpeg/thumbnailer.cpp +++ b/src/android/jpeg/thumbnailer.cpp @@ -11,6 +11,8 @@ #include +#include "libcamera/internal/mapped_framebuffer.h" + using namespace libcamera; LOG_DEFINE_CATEGORY(Thumbnailer) diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h index 68cbf743..4d086c49 100644 --- a/src/android/jpeg/thumbnailer.h +++ b/src/android/jpeg/thumbnailer.h @@ -7,10 +7,10 @@ #ifndef __ANDROID_JPEG_THUMBNAILER_H__ #define __ANDROID_JPEG_THUMBNAILER_H__ +#include #include #include "libcamera/internal/formats.h" -#include "libcamera/internal/framebuffer.h" class Thumbnailer { diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp index 2a4b77ea..b3af194c 100644 --- a/src/android/mm/generic_camera_buffer.cpp +++ b/src/android/mm/generic_camera_buffer.cpp @@ -7,11 +7,12 @@ #include "../camera_buffer.h" +#include #include #include -#include "libcamera/internal/framebuffer.h" +#include "libcamera/internal/mapped_framebuffer.h" using namespace libcamera; diff --git a/src/android/post_processor.h b/src/android/post_processor.h index 689f85d9..ab2b2c60 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -10,8 +10,6 @@ #include #include -#include "libcamera/internal/framebuffer.h" - #include "camera_buffer.h" class CameraMetadata; diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 772e805b..509d4244 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -16,6 +16,7 @@ #include #include "libcamera/internal/formats.h" +#include "libcamera/internal/mapped_framebuffer.h" using namespace libcamera; diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 9f6694ed..38992a98 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -20,7 +20,7 @@ #include #include -#include "libcamera/internal/framebuffer.h" +#include "libcamera/internal/mapped_framebuffer.h" #include "ipu3_agc.h" #include "ipu3_awb.h" diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 69d36b7d..e74bd8c7 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -28,7 +28,7 @@ #include #include -#include "libcamera/internal/framebuffer.h" +#include "libcamera/internal/mapped_framebuffer.h" #include "agc_algorithm.hpp" #include "agc_status.h" diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index 41e6e68f..42c73134 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -8,11 +8,6 @@ #include #include "libcamera/internal/framebuffer.h" -#include -#include -#include -#include - #include /** @@ -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 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(address), plane.length); - } -} - } /* namespace libcamera */ diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp new file mode 100644 index 00000000..4ace400c --- /dev/null +++ b/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 +#include + +#include + +/** + * \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 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(address), plane.length); + } +} + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 4f085801..e9230b98 100644 --- a/src/libcamera/meson.build +++ b/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', diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp index c9479194..a3d1511b 100644 --- a/test/mapped-buffer.cpp +++ b/test/mapped-buffer.cpp @@ -9,7 +9,7 @@ #include -#include "libcamera/internal/framebuffer.h" +#include "libcamera/internal/mapped_framebuffer.h" #include "camera_test.h" #include "test.h" -- cgit v1.2.1