[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