[libcamera-devel] [PATCH v2 03/12] libcamera: buffer: Create a MappedBuffer
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Aug 4 13:44:07 CEST 2020
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 at 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
--
2.25.1
On 03/08/2020 17:18, Kieran Bingham wrote:
> 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 at 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
>
> 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 */
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list