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

2373 diff mbox series

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.

Signed-off-by: Niklas Söderlund <niklas.soderlund@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

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@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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Niklas,

Thank you for the patch.

On Mon, Dec 02, 2019 at 02:16:24PM +0100, Jacopo Mondi wrote:
> 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

s/outside/externally to/

> > make the API simpler and so that both internal and external buffers are
> > handled in the same way.

"This is meant to create a simpler API centered around usage of buffers,
regardless of where they come from."

> > 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.

"Linux however lacks a centralized allocator at the moment, and not all
users of libcamera are expected to use another device that could provide
suitable buffers for the camera. This patch thus adds a helper class to
allocate buffers internally in libcamera, in a way that matches the
needs of the FrameBuffer-based API."

> 
> 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 ?

Stream::allocateBuffers() is an internal API. The idea behind the buffer
allocator and the rest of the buffer rework is to create a single API
centered around buffer usage, but this isn't possible due to the lack of
a suitable centralized allocator at the moment. We thus need to provide
a way for applications to allocate buffers internally in the camera, and
we have decided to do so through a single helper class to avoid exposing
any other allocation-related API to applications. The helper could then
be dropped later when a centralized allocator will be available, or
converted to use the centralized allocator transparently.

> - 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 ?

The main point of the allocator is to handle as much of the accounting
needed for buffer allocation internally to avoid spreading it through
libcamera. We can't achieve 100% of this goal as we still need to
allocate the buffers on V4L2 video nodes.

>   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 ?
> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@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

buffer_allocator.h ? Or maybe even better framebuffer_allocator.h ? Same
for the .cpp file.

s/Buffer/FrameBuffer/

> > + */
> > +#ifndef __LIBCAMERA_ALLOCATOR_H__

And __LIBCAMERA_BUFFER_ALLOCATOR_H__ (or FRAMEBUFFER as appropriate) ?

> > +#define __LIBCAMERA_ALLOCATOR_H__
> > +
> > +#include <map>
> > +#include <memory>
> > +#include <vector>
> > +
> > +namespace libcamera {
> > +
> > +class Camera;
> > +class FrameBuffer;
> > +class Stream;
> > +struct StreamConfiguration;
> > +
> > +class BufferAllocator

FrameBufferAllocator ?

> > +{
> > +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

s/Buffer/FrameBuffer/ (and similar changes below where appropriate)

> > + */
> > +
> > +#include <libcamera/allocator.h>
> > +
> > +#include <errno.h>
> > +#include <string.h>
> > +#include <sys/mman.h>
> > +#include <unistd.h>

I think you only need errno.h.

> > +#include <libcamera/camera.h>
> > +#include <libcamera/stream.h>

You should also include <libcamera/buffer.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.

 * The libcamera API is designed to consume buffers provided by applications as
 * FrameBuffer instances. This makes libcamera a user of buffers exported by
 * other devices (such as displays or video encoders), or allocated from an
 * external allocator (such as ION on Android platforms). In some situations,
 * applications do not have any mean to allocate or get hold of suitable
 * buffers, for instance when no other device is involved, on Linux platforms
 * that lack a centralized allocator. The FrameBufferAllocator class provides a
 * buffer allocator that can be used in these situations.
 *
 * Applications create a framebuffer allocator for a Camera, and use it to
 * allocate buffers for streams of a CameraConfiguration with allocate(). They
 * control which streams to allocate buffers for, and can thus use external
 * buffers for a subset of the streams if desired.
 *
 * Buffers are deleted for a stream with release(), and destroying the allocator
 * automatically deletes all allocated buffers. Applications own the buffers
 * allocated by the FrameBufferAllocator and are responsible for ensuring the
 * buffers are not deleted while they are in use (part of a Request that has
 * been queued and hasn't completed yet).
 *
 * Usage of the FrameBufferAllocator is optional, if all buffers for a camera
 * are provided externally applications shall not use this class.

> > + */
> > +
> > +/**
> > + * \brief Create a BufferAllocator serving a camera
> > + * \param[in] camera Camera the allocator shall serve

s/Camera/The camera/

and maybe just

 * \param[in] camera The camera

> > + */
> > +
> > +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

 * \brief Allocate buffers for a stream with a configuration

> > + * \param[in] stream The stream to allocate buffers from

s/from/for/

> > + * \param[in] config The configuration described the buffers to be allocated

The configuration doesn't describe the buffers to be allocated. Unless
someone has a better proposal, I would just go for

 * \param[in] config The stream configuration

> > + *
> > + * 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.

How about

 * Allocate buffers suitable for capturing frames from the \a stream configured
 * as described by \a config. The configuration shall be valid for the stream as
 * checked CameraConfiguration::validate().
 *
 * Upon successful allocation, the allocated buffers can be retrieved with the
 * buffers() method.

I would drop the second sentence.

Doesn't the camera need to have been configured too ? I think the
documentation needs to be expanded to explain when allocating buffers is
allowed. I also wonder what happens when reconfiguring a camera for
which buffers have been allocated. Is this supported, and if so, in what
conditions ? If you explain how this works I can help expanding the
documentation.

> > + *
> > + * \return 0 on success or a negative error code otherwise

How about enumerating the error codes ? I think we should return -EINVAL
if the stream doesn't belong to the camera of the configuration isn't
valid, -ENOMEM if the buffers can't be allocated and -EBUSY if they are
already allocated. But this would require guaranteeing that
Stream::allocateBuffers() only return those codes. I think it's doable,
but should be documented.

> > + */
> > +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();

s/do/does/

> > +	return -EINVAL;

I think you should handle the errors as they are detected.

	auto iter = camera_->streams().find(stream);
	if (iter == camera_->streams().end()) {
		LOG(Allocator, Error)
			<< "Stream does not belong to " << camera_->name();
		return -EINVAL;
	}

	return stream->allocateBuffers(config, &buffers_[stream]);

Should you also protect against double allocation with

	if (buffers_.count(stream)) {
		LOG(Allocator, Error) << "Buffers already allocated for stream";
		return -EBUSY;
	}

> > +}
> > +
> > +/**
> > + * \brief Free allocated buffers

 * \brief Free buffers previously allocated for a \a stream

> > + * \param[in] stream The stream to free buffers for
> > + *
> > + * Free buffers allocated with allocate().

Add "This invalidates the buffers returned by buffers()."

> > + */
> > +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

 * \brief Retrieve the buffers allocated for a \a stream

> > + * \param[in] stream The stream to retrive buffers for

s/retrive/retrieve/

 *
 * This method shall only be called after successfully allocating buffers for
 * \a stream with allocate(). The returned buffers are valid until the earlier
 * of a call to release() for the same stream or the destruction of the
 * FrameBufferPool instance.
 *

> > + *
> > + * \return Array of buffers

 * \return The buffers allocated for the \a stream

> > + */
> > +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',