[libcamera-devel] [PATCH 25/30] libcamera: allocator: Add BufferAllocator to help applications allocate buffers

Jacopo Mondi jacopo at jmondi.org
Mon Dec 2 14:16:24 CET 2019


Hi Niklas,
On Wed, Nov 27, 2019 at 12:36:15AM +0100, Niklas Söderlund wrote:
> The FrameBuffer interface is based on the idea that all buffers are
> allocated outside libcamera and are only used by it. This is done to
> make the API simpler and so that both internal and external buffers are
> handled in the same way.
>
> As V4L2 do not provide a way to allocate buffers without the help of a
> video device add an application facing helper which hide this. This
> allow applications to allocate buffers and then use them with cameras.

I'm sorry but I really fail to see why we need this abstraction.

The BufferAllocator
- is Constructed with a camera
- allocate buffers from Streams with a StreamConfiguration
- maintain a list of allocated buffers, indexed by stream

The usage I see in cam is

allocator = new Allocator(Camera);
for (streamConfig : camera->config())
        allocator.allocate(streamConfig->stream(),
                           streamConfig);

for (nbufs : numBuffers)
        request = new Request()

        for (streamConfig : camera->config())
                buffer = allocator.buffer(streamConfig->stream())[nbufs];
                request->addBuffer(streamConfig->stream(), buffer);

My first question would be
- if allocator.allocate takes a Stream and a config (the stream could
  be accessed from the config, but that's a different issue) and just
  calls 'stream.allocateBuffers(config)' what prevents application from
  calling stream.allocateBuffers() themselves ?

- The whole point of allocator is to maintain a map to Stream to
  vectors of Buffers ? So can't that list live in the Stream itself ?
  Can't the map live in the camera if we want to ? They need to be put
  together when adding a buffer-stream pair to a request, and the
  whole dance of allocation and buffer access which all needs a Stream
  to work seems like quite a complex and API for applications.
  The above portion of code could be simplified to

for (stream : camera->streams()) {
        StreamConfiguration *config = camera->streamConfig(stream);
                                                ^- this is easy to implement
        stream->allocate(config);

for (nbufs : numBuffers)
        request = new Request()

        for (streamConfig : camera->config())
                buffer = streamConfig->stream().buffer(i);
                request->addBuffer(streamConfig->stream(), buffer);

Isn't this less convoluted ?

How do we expect the buffer importing to look like, as I am missing
how this series handles it ?

Thanks
   j
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/allocator.h |  39 ++++++++++
>  include/libcamera/meson.build |   1 +
>  include/libcamera/stream.h    |   1 +
>  src/libcamera/allocator.cpp   | 141 ++++++++++++++++++++++++++++++++++
>  src/libcamera/meson.build     |   1 +
>  5 files changed, 183 insertions(+)
>  create mode 100644 include/libcamera/allocator.h
>  create mode 100644 src/libcamera/allocator.cpp
>
> diff --git a/include/libcamera/allocator.h b/include/libcamera/allocator.h
> new file mode 100644
> index 0000000000000000..36fce38491b5f3ef
> --- /dev/null
> +++ b/include/libcamera/allocator.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * allocator.h - Buffer allocator
> + */
> +#ifndef __LIBCAMERA_ALLOCATOR_H__
> +#define __LIBCAMERA_ALLOCATOR_H__
> +
> +#include <map>
> +#include <memory>
> +#include <vector>
> +
> +namespace libcamera {
> +
> +class Camera;
> +class FrameBuffer;
> +class Stream;
> +struct StreamConfiguration;
> +
> +class BufferAllocator
> +{
> +public:
> +	BufferAllocator(std::shared_ptr<Camera> camera);
> +	~BufferAllocator();
> +
> +	int allocate(Stream *stream, const StreamConfiguration &config);
> +	void release(Stream *stream);
> +
> +	const std::vector<FrameBuffer *> &buffers(Stream *stream) const;
> +
> +private:
> +	std::shared_ptr<Camera> camera_;
> +	std::map<Stream *, std::vector<FrameBuffer *>> buffers_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_ALLOCATOR_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 99abf06099407c1f..0d0ba2248fd8a679 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -1,4 +1,5 @@
>  libcamera_api = files([
> +    'allocator.h',
>      'bound_method.h',
>      'buffer.h',
>      'camera.h',
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index a3c692c347340382..8e653408fd54cf74 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -85,6 +85,7 @@ public:
>  	MemoryType memoryType() const { return memoryType_; }
>
>  protected:
> +	friend class BufferAllocator; /* To allocate and release buffers. */
>  	friend class Camera;
>
>  	virtual int allocateBuffers(const StreamConfiguration &config,
> diff --git a/src/libcamera/allocator.cpp b/src/libcamera/allocator.cpp
> new file mode 100644
> index 0000000000000000..8b517c809c05cbcd
> --- /dev/null
> +++ b/src/libcamera/allocator.cpp
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * allocator.cpp - Buffer allocator
> + */
> +
> +#include <libcamera/allocator.h>
> +
> +#include <errno.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/stream.h>
> +
> +#include "log.h"
> +
> +/**
> + * \file allocator.h
> + * \brief Buffer allocator
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(Allocator)
> +
> +/**
> + * \class BufferAllocator
> + * \brief Buffer allocator for applications
> + *
> + * All buffers are to be treated as if they where allocated outside of the
> + * camera. In some situations however the only source applications can allocate
> + * buffers from is the camera. The BufferAllocator is the interface applications
> + * shall use in these situations.
> + *
> + * The buffer allocator creates buffers using resources from a camera and/or
> + * a stream. The buffers allocated this way are owned by the application and it
> + * is responsible for their lifetime management. But just as buffers allocated
> + * external from cameras and streams it's not valid to free a buffer while it's
> + * queue to a camera.
> + *
> + * All buffers allocator are automatically freed when the allocator object is
> + * deleted. It is the applications responsibility to make sure this do not
> + * happen while one or more of the buffers are queued to a camera.
> + *
> + * If buffers are allocated outside the scope of libcamera by the application it
> + * do not need to interact with the buffer allocator.
> + */
> +
> +/**
> + * \brief Create a BufferAllocator serving a camera
> + * \param[in] camera Camera the allocator shall serve
> + */
> +
> +BufferAllocator::BufferAllocator(std::shared_ptr<Camera> camera)
> +	: camera_(camera)
> +{
> +}
> +
> +BufferAllocator::~BufferAllocator()
> +{
> +	for (auto &value : buffers_) {
> +		Stream *stream = value.first;
> +		std::vector<FrameBuffer *> &buffers = value.second;
> +
> +		for (FrameBuffer *buffer : buffers)
> +			delete buffer;
> +
> +		buffers.clear();
> +
> +		stream->releaseBuffers();
> +	}
> +
> +	buffers_.clear();
> +}
> +
> +/**
> + * \brief Allocate buffers from a stream
> + * \param[in] stream The stream to allocate buffers from
> + * \param[in] config The configuration described the buffers to be allocated
> + *
> + * Allocate buffers matching exactly what is described in \a config. If buffers
> + * matching \a config can't be allocated an error is returned and no buffers are
> + * allocated.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int BufferAllocator::allocate(Stream *stream, const StreamConfiguration &config)
> +{
> +	auto iter = camera_->streams().find(stream);
> +	if (iter != camera_->streams().end())
> +		return stream->allocateBuffers(config, &buffers_[stream]);
> +
> +	LOG(Allocator, Error) << "Stream do not belong to " << camera_->name();
> +	return -EINVAL;
> +}
> +
> +/**
> + * \brief Free allocated buffers
> + * \param[in] stream The stream to free buffers for
> + *
> + * Free buffers allocated with allocate().
> + */
> +void BufferAllocator::release(Stream *stream)
> +{
> +	auto iter = buffers_.find(stream);
> +	if (iter == buffers_.end())
> +		return;
> +
> +	std::vector<FrameBuffer *> &buffers = iter->second;
> +
> +	for (FrameBuffer *buffer : buffers)
> +		delete buffer;
> +
> +	buffers.clear();
> +
> +	stream->releaseBuffers();
> +
> +	buffers_.erase(iter);
> +}
> +
> +/**
> + * \brief Retrive array of allocated buffers
> + * \param[in] stream The stream to retrive buffers for
> + *
> + * \return Array of buffers
> + */
> +const std::vector<FrameBuffer *> &BufferAllocator::buffers(Stream *stream) const
> +{
> +	static std::vector<FrameBuffer *> empty;
> +
> +	auto iter = buffers_.find(stream);
> +	if (iter == buffers_.end())
> +		return empty;
> +
> +	return iter->second;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index c4f965bd7413b37e..b7041e003fdb1d49 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -1,4 +1,5 @@
>  libcamera_sources = files([
> +    'allocator.cpp',
>      'bound_method.cpp',
>      'buffer.cpp',
>      'byte_stream_buffer.cpp',
> --
> 2.24.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191202/ee639afa/attachment.sig>


More information about the libcamera-devel mailing list