[libcamera-devel] [PATCH v6 3/3] libcamera: Add exportFrameBuffers in HeapAllocator

[Umang Jain]

Hi Harvey,

Thank you for the patch.

On 5/22/23 2:05 PM, Harvey Yang via libcamera-devel wrote:
> From: Cheng-Hao Yang <chenghaoyang at chromium.org>
>
> Add a helper function exportFrameBuffers in HeapAllocator to make it
> easier to use.
>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>   include/libcamera/internal/heap_allocator.h |  9 +++
>   src/libcamera/heap_allocator.cpp            | 62 +++++++++++++++++++++
>   2 files changed, 71 insertions(+)
>
> diff --git a/include/libcamera/internal/heap_allocator.h b/include/libcamera/internal/heap_allocator.h
> index 92d4488a..92beaaa4 100644
> --- a/include/libcamera/internal/heap_allocator.h
> +++ b/include/libcamera/internal/heap_allocator.h
> @@ -9,12 +9,16 @@
>   #pragma once
>   
>   #include <stddef.h>
> +#include <vector>
>   
>   #include <libcamera/base/unique_fd.h>
>   
>   namespace libcamera {
>   
> +class Camera;
> +class FrameBuffer;
>   class Heap;
> +class Stream;
>   
>   class HeapAllocator
>   {
> @@ -24,7 +28,12 @@ public:
>   	bool isValid() const;
>   	UniqueFD alloc(const char *name, std::size_t size);
>   
> +	int exportFrameBuffers(Camera *camera, Stream *stream,
> +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> +

Can this be made to accept references instead of pointers? I believe 
every argument is non-nullable here...
>   private:
> +	std::unique_ptr<FrameBuffer> createBuffer(Stream *stream);
> +
>   	std::unique_ptr<Heap> heap_;
>   };
>   
> diff --git a/src/libcamera/heap_allocator.cpp b/src/libcamera/heap_allocator.cpp
> index 8f99f590..e99cdbe5 100644
> --- a/src/libcamera/heap_allocator.cpp
> +++ b/src/libcamera/heap_allocator.cpp
> @@ -22,6 +22,12 @@
>   
>   #include <libcamera/base/log.h>
>   
> +#include <libcamera/camera.h>
> +#include <libcamera/framebuffer.h>
> +#include <libcamera/stream.h>
> +
> +#include "libcamera/internal/formats.h"
> +
>   namespace libcamera {
>   
>   /*
> @@ -227,4 +233,60 @@ UniqueFD HeapAllocator::alloc(const char *name, std::size_t size)
>   	return heap_->alloc(name, size);
>   }
>   
> +int HeapAllocator::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> +				      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	unsigned int count = stream->configuration().bufferCount;
> +
> +	/** \todo Support multiplanar allocations */
> +
> +	for (unsigned i = 0; i < count; ++i) {
> +		std::unique_ptr<FrameBuffer> buffer = createBuffer(stream);
> +		if (!buffer) {
> +			LOG(HeapAllocator, Error) << "Unable to create buffer";
> +
> +			buffers->clear();
> +			return -EINVAL;
> +		}
> +
> +		buffers->push_back(std::move(buffer));
> +	}
> +
> +	return count;
> +}
> +
> +std::unique_ptr<FrameBuffer> HeapAllocator::createBuffer(Stream *stream)
> +{
> +	std::vector<FrameBuffer::Plane> planes;
> +
> +	unsigned int frameSize = stream->configuration().frameSize;
> +	int pageSize = getpagesize();
> +	/** Allocation size need to be a direct multiple of |pageSize|. */

	/* Allocation size need to be a direct multiple of pageSize. */

> +	unsigned int numPage = (frameSize + pageSize - 1) / pageSize;
> +
> +	UniqueFD fd = alloc("FrameBuffer", numPage * pageSize);
> +	if (!fd.isValid())
> +		return nullptr;
> +
> +	auto info = PixelFormatInfo::info(stream->configuration().pixelFormat);
> +	SharedFD shared_fd(std::move(fd));
> +	unsigned int offset = 0;
> +	for (size_t i = 0; i < info.planes.size(); ++i) {
> +		unsigned int planeSize = info.planeSize(stream->configuration().size, i);
> +		if (planeSize == 0)
> +			continue;
> +
> +		/** \todo We don't support allocating buffers with a dedicated fd per plane */

		/* \todo We don't support allocating buffers with a dedicated fd per plane */

> +		FrameBuffer::Plane plane;
> +		plane.fd = shared_fd;
> +		plane.offset = offset;
> +		plane.length = planeSize;
> +		planes.push_back(std::move(plane));
> +
> +		offset += planeSize;
> +	}
> +
> +	return std::make_unique<FrameBuffer>(planes);
> +}
> +
>   } /* namespace libcamera */