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

[Jacopo Mondi]

Hi Harvey

On Tue, May 16, 2023 at 08:03:19AM +0000, 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            | 61 +++++++++++++++++++++
>  2 files changed, 70 insertions(+)
>
> diff --git a/include/libcamera/internal/heap_allocator.h b/include/libcamera/internal/heap_allocator.h
> index d061fdce..b6488fbc 100644
> --- a/include/libcamera/internal/heap_allocator.h
> +++ b/include/libcamera/internal/heap_allocator.h
> @@ -8,12 +8,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
>  {
> @@ -23,7 +27,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);
> +
>  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 682a7e01..ce3a815d 100644
> --- a/src/libcamera/heap_allocator.cpp
> +++ b/src/libcamera/heap_allocator.cpp
> @@ -21,6 +21,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 {
>
>  /*
> @@ -231,4 +237,59 @@ 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|.

no c++ style comments, please

> +	unsigned int numPage = (frameSize + pageSize - 1) / pageSize;
> +
> +	UniqueFD fd = alloc("FrameBuffer", numPage * pageSize);
> +	if (!fd.isValid())
> +		return nullptr;
> +

And I would add here a comment as reported in the review of v4

Can be fixed when applying
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

> +	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;
> +
> +		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 */
> --
> 2.40.1.606.ga4b1b128d6-goog
>