[libcamera-devel] [PATCH 3/5] mapped_framebuffer: Introduce MappedVectorBuffer

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Tue, Aug 31, 2021 at 03:34:36PM +0900, Hirokazu Honda wrote:
> MappedFrameBuffer is a main class of deriving MappedBuffer. It works
> with FrameBuffer, so the source planes must be given as file
> descriptors.
> MappedBuffer interface is a generic class of providing accessing
> planes in memory. This introduces a new class deriving MappedBuffer,
> MappedVectorBuffer. It is created with the plane data in heap,
> concretely, std::vector<uint8_t> and owns it.

The MappedBuffer interface is geared towards performing memory mappings
(hence the maps_ member in the base class), which isn't applicable here.
I see why you want this in this series, but I'm not sure it's the best
solution. It feels that what you need is a class that represents an
image accessible by the CPU, regardless of how the memory was obtained.
I fear that creating such a generic image class will quickly grow out of
control and become a fully fledged CPU image API.

I'll have to sleep over this.

> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  include/libcamera/internal/mapped_framebuffer.h |  9 +++++++++
>  src/libcamera/mapped_framebuffer.cpp            | 17 +++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/include/libcamera/internal/mapped_framebuffer.h b/include/libcamera/internal/mapped_framebuffer.h
> index 70ffbcbe..7deecded 100644
> --- a/include/libcamera/internal/mapped_framebuffer.h
> +++ b/include/libcamera/internal/mapped_framebuffer.h
> @@ -59,6 +59,15 @@ public:
>  
>  LIBCAMERA_FLAGS_ENABLE_OPERATORS(MappedFrameBuffer::MapFlag)
>  
> +class MappedVectorBuffer : public MappedBuffer
> +{
> +public:
> +	MappedVectorBuffer(std::vector<std::vector<uint8_t>> &&planes);
> +
> +private:
> +	std::vector<std::vector<uint8_t>> data_;
> +};
> +
>  } /* namespace libcamera */
>  
>  #endif /* __LIBCAMERA_INTERNAL_MAPPED_FRAMEBUFFER_H__ */
> diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp
> index 464d35fe..b09ad41a 100644
> --- a/src/libcamera/mapped_framebuffer.cpp
> +++ b/src/libcamera/mapped_framebuffer.cpp
> @@ -240,4 +240,21 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags)
>  	}
>  }
>  
> +/**
> + * \class MappedVectorBuffer
> + * \brief Owns planes data in heap and provides the MappedBuffer interface
> + */
> +
> +/**
> + * \brief Takes the ownership of the passed \a planes
> + * \param[in] planes the plane data that shall be owned by MappedVectorBuffer
> + */
> +MappedVectorBuffer::MappedVectorBuffer(std::vector<std::vector<uint8_t>> &&planes)
> +	: data_(std::move(planes))
> +{
> +	planes_.reserve(data_.size());
> +	for (auto &plane : data_)
> +		planes_.emplace_back(plane.data(), plane.size());
> +}
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart