[libcamera-devel] [PATCH v2 06/16] libcamera: buffer: Split memory information to BufferMemory
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Jul 14 12:42:42 CEST 2019
Hi Laurent,
On 2019-07-13 20:23:41 +0300, Laurent Pinchart wrote:
> The Buffer class is a large beast the stores information about the
> buffer memory, dynamic metadata related to the frame stored in the
> buffer, and buffer reference data (in the index). In order to implement
> buffer import we will need to extend this with dmabuf file descriptors,
> making usage of the class even more complex.
>
> Refactor the Buffer class by splitting the buffer memory information to
> a BufferMemory class, and repurposing the Buffer class to reference a
> buffer and to store dynamic metadata. The BufferMemory class becomes a
> long term storage, valid and stable from the time buffer memory is
> allocated to the time it is freed. The Buffer class, on the other hand,
> becomes transient, is created on demand when an application requires a
> buffer, is given to a request, and is deleted when the request
> completes.
>
> Buffer and BufferMemory don't need to be copied, so their copy
> constructor and assignment operators are deleted.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I really like the split, nice work.
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/buffer.h | 55 +++++---
> include/libcamera/request.h | 4 +-
> include/libcamera/stream.h | 4 +
> src/cam/buffer_writer.cpp | 4 +-
> src/cam/buffer_writer.h | 3 +-
> src/cam/capture.cpp | 35 +++--
> src/libcamera/buffer.cpp | 164 ++++++++++++++---------
> src/libcamera/include/v4l2_videodevice.h | 8 +-
> src/libcamera/request.cpp | 35 +++--
> src/libcamera/stream.cpp | 24 ++++
> src/libcamera/v4l2_videodevice.cpp | 40 +++---
> src/qcam/main_window.cpp | 40 ++++--
> src/qcam/main_window.h | 2 +-
> test/camera/capture.cpp | 19 ++-
> test/camera/statemachine.cpp | 6 +-
> 15 files changed, 298 insertions(+), 145 deletions(-)
>
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 260a62e9e77e..f5ba6207bcef 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -14,6 +14,7 @@ namespace libcamera {
>
> class BufferPool;
> class Request;
> +class Stream;
>
> class Plane final
> {
> @@ -36,6 +37,30 @@ private:
> void *mem_;
> };
>
> +class BufferMemory final
> +{
> +public:
> + std::vector<Plane> &planes() { return planes_; }
> +
> +private:
> + std::vector<Plane> planes_;
> +};
> +
> +class BufferPool final
> +{
> +public:
> + ~BufferPool();
> +
> + void createBuffers(unsigned int count);
> + void destroyBuffers();
> +
> + unsigned int count() const { return buffers_.size(); }
> + std::vector<BufferMemory> &buffers() { return buffers_; }
> +
> +private:
> + std::vector<BufferMemory> buffers_;
> +};
> +
> class Buffer final
> {
> public:
> @@ -45,20 +70,24 @@ public:
> BufferCancelled,
> };
>
> - Buffer();
> + Buffer(unsigned int index = -1, const Buffer *metadata = nullptr);
> + Buffer(const Buffer &) = delete;
> + Buffer &operator=(const Buffer &) = delete;
>
> unsigned int index() const { return index_; }
> +
> unsigned int bytesused() const { return bytesused_; }
> uint64_t timestamp() const { return timestamp_; }
> unsigned int sequence() const { return sequence_; }
> +
> Status status() const { return status_; }
> - std::vector<Plane> &planes() { return planes_; }
> Request *request() const { return request_; }
> + Stream *stream() const { return stream_; }
>
> private:
> - friend class BufferPool;
> friend class PipelineHandler;
> friend class Request;
> + friend class Stream;
> friend class V4L2VideoDevice;
>
> void cancel();
> @@ -66,28 +95,14 @@ private:
> void setRequest(Request *request) { request_ = request; }
>
> unsigned int index_;
> +
> unsigned int bytesused_;
> uint64_t timestamp_;
> unsigned int sequence_;
> +
> Status status_;
> -
> - std::vector<Plane> planes_;
> Request *request_;
> -};
> -
> -class BufferPool final
> -{
> -public:
> - ~BufferPool();
> -
> - void createBuffers(unsigned int count);
> - void destroyBuffers();
> -
> - unsigned int count() const { return buffers_.size(); }
> - std::vector<Buffer> &buffers() { return buffers_; }
> -
> -private:
> - std::vector<Buffer> buffers_;
> + Stream *stream_;
> };
>
> } /* namespace libcamera */
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index dd165bc21c03..7a60be617645 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -8,6 +8,7 @@
> #define __LIBCAMERA_REQUEST_H__
>
> #include <map>
> +#include <memory>
> #include <stdint.h>
> #include <unordered_set>
>
> @@ -33,10 +34,11 @@ public:
> Request(Camera *camera, uint64_t cookie = 0);
> Request(const Request &) = delete;
> Request &operator=(const Request &) = delete;
> + ~Request();
>
> ControlList &controls() { return controls_; }
> const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
> - int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
> + int addBuffer(std::unique_ptr<Buffer> buffer);
> Buffer *findBuffer(Stream *stream) const;
>
> uint64_t cookie() const { return cookie_; }
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index 5b4fea324ce4..f595a630eb4a 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -8,6 +8,7 @@
> #define __LIBCAMERA_STREAM_H__
>
> #include <map>
> +#include <memory>
> #include <string>
> #include <vector>
>
> @@ -66,6 +67,9 @@ class Stream
> {
> public:
> Stream();
> +
> + std::unique_ptr<Buffer> createBuffer(unsigned int index);
> +
> BufferPool &bufferPool() { return bufferPool_; }
> const StreamConfiguration &configuration() const { return configuration_; }
>
> diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp
> index e0374ffcb319..b7f2ed4f2d2d 100644
> --- a/src/cam/buffer_writer.cpp
> +++ b/src/cam/buffer_writer.cpp
> @@ -19,7 +19,7 @@ BufferWriter::BufferWriter(const std::string &pattern)
> {
> }
>
> -int BufferWriter::write(libcamera::Buffer *buffer,
> +int BufferWriter::write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem,
> const std::string &streamName)
> {
> std::string filename;
> @@ -41,7 +41,7 @@ int BufferWriter::write(libcamera::Buffer *buffer,
> if (fd == -1)
> return -errno;
>
> - for (libcamera::Plane &plane : buffer->planes()) {
> + for (libcamera::Plane &plane : mem->planes()) {
> void *data = plane.mem();
> unsigned int length = plane.length();
>
> diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h
> index 7bf785d1e832..9bea205fac1d 100644
> --- a/src/cam/buffer_writer.h
> +++ b/src/cam/buffer_writer.h
> @@ -16,7 +16,8 @@ class BufferWriter
> public:
> BufferWriter(const std::string &pattern = "frame-#.bin");
>
> - int write(libcamera::Buffer *buffer, const std::string &streamName);
> + int write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem,
> + const std::string &streamName);
>
> private:
> std::string pattern_;
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 6b842d73390d..1cf59063c93b 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -95,13 +95,14 @@ int Capture::capture(EventLoop *loop)
> std::map<Stream *, Buffer *> map;
> for (StreamConfiguration &cfg : *config_) {
> Stream *stream = cfg.stream();
> - map[stream] = &stream->bufferPool().buffers()[i];
> - }
> + std::unique_ptr<Buffer> buffer = stream->createBuffer(i);
>
> - ret = request->setBuffers(map);
> - if (ret < 0) {
> - std::cerr << "Can't set buffers for request" << std::endl;
> - return ret;
> + ret = request->addBuffer(std::move(buffer));
> + if (ret < 0) {
> + std::cerr << "Can't set buffer for request"
> + << std::endl;
> + return ret;
> + }
> }
>
> requests.push_back(request);
> @@ -155,6 +156,7 @@ void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer
> for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> Stream *stream = it->first;
> Buffer *buffer = it->second;
> + BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()];
> const std::string &name = streamName_[stream];
>
> info << " " << name
> @@ -163,17 +165,34 @@ void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer
> << " bytesused: " << buffer->bytesused();
>
> if (writer_)
> - writer_->write(buffer, name);
> + writer_->write(buffer, mem, name);
> }
>
> std::cout << info.str() << std::endl;
>
> + /*
> + * Create a new request and populate it with one buffer for each
> + * stream.
> + */
> request = camera_->createRequest();
> if (!request) {
> std::cerr << "Can't create request" << std::endl;
> return;
> }
>
> - request->setBuffers(buffers);
> + for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> + Stream *stream = it->first;
> + Buffer *buffer = it->second;
> + unsigned int index = buffer->index();
> +
> + std::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);
> + if (!newBuffer) {
> + std::cerr << "Can't create buffer " << index << std::endl;
> + return;
> + }
> +
> + request->addBuffer(std::move(newBuffer));
> + }
> +
> camera_->queueRequest(request);
> }
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index c0a020942f5c..ecbf25246a55 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -173,12 +173,78 @@ void *Plane::mem()
> */
>
> /**
> - * \class Buffer
> + * \class BufferMemory
> * \brief A memory buffer to store an image
> *
> - * The Buffer class represents the memory buffers used to store a
> - * full frame image, which may contain multiple separate memory Plane
> - * objects if the image format is multi-planar.
> + * The BufferMemory class represents the memory buffers used to store full frame
> + * images, which may contain multiple separate memory Plane objects if the
> + * image format is multi-planar.
> + */
> +
> +/**
> + * \fn BufferMemory::planes()
> + * \brief Retrieve the planes within the buffer
> + * \return A reference to a vector holding all Planes within the buffer
> + */
> +
> +/**
> + * \class BufferPool
> + * \brief A pool of buffers
> + *
> + * The BufferPool class groups together a collection of Buffers to store frames.
> + * The buffers must be exported by a device before they can be imported into
> + * another device for further use.
> + */
> +
> +BufferPool::~BufferPool()
> +{
> + destroyBuffers();
> +}
> +
> +/**
> + * \brief Create buffers in the Pool
> + * \param[in] count The number of buffers to create
> + */
> +void BufferPool::createBuffers(unsigned int count)
> +{
> + buffers_.resize(count);
> +}
> +
> +/**
> + * \brief Release all buffers from pool
> + *
> + * If no buffers have been created or if buffers have already been released no
> + * operation is performed.
> + */
> +void BufferPool::destroyBuffers()
> +{
> + buffers_.resize(0);
> +}
> +
> +/**
> + * \fn BufferPool::count()
> + * \brief Retrieve the number of buffers contained within the pool
> + * \return The number of buffers contained in the pool
> + */
> +
> +/**
> + * \fn BufferPool::buffers()
> + * \brief Retrieve all the buffers in the pool
> + * \return A vector containing all the buffers in the pool.
> + */
> +
> +/**
> + * \class Buffer
> + * \brief A buffer handle and dynamic metadata
> + *
> + * The Buffer class references a buffer memory and associates dynamic metadata
> + * related to the frame contained in the buffer. It allows referencing buffer
> + * memory through a single interface regardless of whether the memory is
> + * allocated internally in libcamera or provided externally through dmabuf.
> + *
> + * Buffer instances are allocated dynamically for a stream through
> + * Stream::createBuffer(), added to a request with Request::addBuffer() and
> + * deleted automatically after the request complete handler returns.
> */
>
> /**
> @@ -195,9 +261,26 @@ void *Plane::mem()
> * invalid and shall not be used.
> */
>
> -Buffer::Buffer()
> - : index_(-1), request_(nullptr)
> +/**
> + * \brief Construct a buffer not associated with any stream
> + *
> + * This method constructs an orphaned buffer not associated with any stream. It
> + * is not meant to be called by applications, they should instead create buffers
> + * for a stream with Stream::createBuffer().
> + */
> +Buffer::Buffer(unsigned int index, const Buffer *metadata)
> + : index_(index), status_(Buffer::BufferSuccess), request_(nullptr),
> + stream_(nullptr)
> {
> + if (metadata) {
> + bytesused_ = metadata->bytesused_;
> + sequence_ = metadata->sequence_;
> + timestamp_ = metadata->timestamp_;
> + } else {
> + bytesused_ = 0;
> + sequence_ = 0;
> + timestamp_ = 0;
> + }
> }
>
> /**
> @@ -206,12 +289,6 @@ Buffer::Buffer()
> * \return The buffer index
> */
>
> -/**
> - * \fn Buffer::planes()
> - * \brief Retrieve the planes within the buffer
> - * \return A reference to a vector holding all Planes within the buffer
> - */
> -
> /**
> * \fn Buffer::bytesused()
> * \brief Retrieve the number of bytes occupied by the data in the buffer
> @@ -264,6 +341,19 @@ Buffer::Buffer()
> * \sa Buffer::setRequest()
> */
>
> +/**
> + * \fn Buffer::stream()
> + * \brief Retrieve the stream this buffer is associated with
> + *
> + * A Buffer is associated to the stream that created it with
> + * Stream::createBuffer() and the association is valid until the buffer is
> + * destroyed. Buffer instances that are created directly are not associated
> + * with any stream.
> + *
> + * \return The Stream the Buffer is associated with, or nullptr if the buffer
> + * is not associated with a stream
> + */
> +
> /**
> * \brief Mark a buffer as cancel by setting its status to BufferCancelled
> */
> @@ -282,54 +372,4 @@ void Buffer::cancel()
> * The intended callers are Request::prepare() and Request::completeBuffer().
> */
>
> -/**
> - * \class BufferPool
> - * \brief A pool of buffers
> - *
> - * The BufferPool class groups together a collection of Buffers to store frames.
> - * The buffers must be exported by a device before they can be imported into
> - * another device for further use.
> - */
> -
> -BufferPool::~BufferPool()
> -{
> - destroyBuffers();
> -}
> -
> -/**
> - * \brief Create buffers in the Pool
> - * \param[in] count The number of buffers to create
> - */
> -void BufferPool::createBuffers(unsigned int count)
> -{
> - unsigned int index = 0;
> -
> - buffers_.resize(count);
> - for (Buffer &buffer : buffers_)
> - buffer.index_ = index++;
> -}
> -
> -/**
> - * \brief Release all buffers from pool
> - *
> - * If no buffers have been created or if buffers have already been released no
> - * operation is performed.
> - */
> -void BufferPool::destroyBuffers()
> -{
> - buffers_.resize(0);
> -}
> -
> -/**
> - * \fn BufferPool::count()
> - * \brief Retrieve the number of buffers contained within the pool
> - * \return The number of buffers contained in the pool
> - */
> -
> -/**
> - * \fn BufferPool::buffers()
> - * \brief Retrieve all the buffers in the pool
> - * \return A vector containing all the buffers in the pool.
> - */
> -
> } /* namespace libcamera */
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index f948a41fb8c1..8238a39b0da4 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -7,7 +7,6 @@
> #ifndef __LIBCAMERA_V4L2_VIDEODEVICE_H__
> #define __LIBCAMERA_V4L2_VIDEODEVICE_H__
>
> -#include <atomic>
> #include <string>
> #include <vector>
>
> @@ -23,6 +22,7 @@
> namespace libcamera {
>
> class Buffer;
> +class BufferMemory;
> class BufferPool;
> class EventNotifier;
> class MediaDevice;
> @@ -165,8 +165,8 @@ private:
> std::vector<SizeRange> enumSizes(unsigned int pixelFormat);
>
> int requestBuffers(unsigned int count);
> - int createPlane(Buffer *buffer, unsigned int plane,
> - unsigned int length);
> + int createPlane(BufferMemory *buffer, unsigned int index,
> + unsigned int plane, unsigned int length);
>
> Buffer *dequeueBuffer();
> void bufferAvailable(EventNotifier *notifier);
> @@ -177,7 +177,7 @@ private:
> enum v4l2_memory memoryType_;
>
> BufferPool *bufferPool_;
> - std::atomic<unsigned int> queuedBuffersCount_;
> + std::map<unsigned int, Buffer *> queuedBuffers_;
>
> EventNotifier *fdEvent_;
> };
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 8cf41a43a80e..45e7133febb0 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -60,6 +60,14 @@ Request::Request(Camera *camera, uint64_t cookie)
> {
> }
>
> +Request::~Request()
> +{
> + for (auto it : bufferMap_) {
> + Buffer *buffer = it.second;
> + delete buffer;
> + }
> +}
> +
> /**
> * \fn Request::controls()
> * \brief Retrieve the request's ControlList
> @@ -87,19 +95,30 @@ Request::Request(Camera *camera, uint64_t cookie)
> */
>
> /**
> - * \brief Set the streams to capture with associated buffers
> - * \param[in] streamMap The map of streams to buffers
> + * \brief Store a Buffer with its associated Stream in the Request
> + * \param[in] buffer The Buffer to store in the request
> + *
> + * Ownership of the buffer is passed to the request. It will be deleted when
> + * the request is destroyed after completing.
> + *
> + * A request can only contain one buffer per stream. If a buffer has already
> + * been added to the request for the same stream, this method returns -EEXIST.
> + *
> * \return 0 on success or a negative error code otherwise
> - * \retval -EBUSY Buffers have already been set
> + * \retval -EEXIST The request already contains a buffer for the stream
> */
> -int Request::setBuffers(const std::map<Stream *, Buffer *> &streamMap)
> +int Request::addBuffer(std::unique_ptr<Buffer> buffer)
> {
> - if (!bufferMap_.empty()) {
> - LOG(Request, Error) << "Buffers already set";
> - return -EBUSY;
> + Stream *stream = buffer->stream();
> +
> + auto it = bufferMap_.find(stream);
> + if (it != bufferMap_.end()) {
> + LOG(Request, Error) << "Buffer already set for stream";
> + return -EEXIST;
> }
>
> - bufferMap_ = streamMap;
> + bufferMap_[stream] = buffer.release();
> +
> return 0;
> }
>
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index d8e87c62281c..6d80646b55cd 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -407,6 +407,30 @@ Stream::Stream()
> {
> }
>
> +/**
> + * \brief Create a Buffer instance
> + * \param[in] index The desired buffer index
> + *
> + * This method creates a Buffer instance that references a BufferMemory from
> + * the stream's buffers pool by its \a index. The index shall be lower than the
> + * number of buffers in the pool.
> + *
> + * \return A newly created Buffer on success or nullptr otherwise
> + */
> +std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index)
> +{
> + if (index >= bufferPool_.count()) {
> + LOG(Stream, Error) << "Invalid buffer index " << index;
> + return nullptr;
> + }
> +
> + Buffer *buffer = new Buffer();
> + buffer->index_ = index;
> + buffer->stream_ = this;
> +
> + return std::unique_ptr<Buffer>(buffer);
> +}
> +
> /**
> * \fn Stream::bufferPool()
> * \brief Retrieve the buffer pool for the stream
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index af5ba803de45..65b4098abc05 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -268,8 +268,7 @@ const std::string V4L2DeviceFormat::toString() const
> * \param[in] deviceNode The file-system path to the video device node
> */
> V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)
> - : V4L2Device(deviceNode), bufferPool_(nullptr),
> - queuedBuffersCount_(0), fdEvent_(nullptr)
> + : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr)
> {
> /*
> * We default to an MMAP based CAPTURE video device, however this will
> @@ -764,7 +763,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
> for (i = 0; i < pool->count(); ++i) {
> struct v4l2_plane planes[VIDEO_MAX_PLANES] = {};
> struct v4l2_buffer buf = {};
> - Buffer &buffer = pool->buffers()[i];
> + BufferMemory &buffer = pool->buffers()[i];
>
> buf.index = i;
> buf.type = bufferType_;
> @@ -782,13 +781,13 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
>
> if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) {
> for (unsigned int p = 0; p < buf.length; ++p) {
> - ret = createPlane(&buffer, p,
> + ret = createPlane(&buffer, i, p,
> buf.m.planes[p].length);
> if (ret)
> break;
> }
> } else {
> - ret = createPlane(&buffer, 0, buf.length);
> + ret = createPlane(&buffer, i, 0, buf.length);
> }
>
> if (ret) {
> @@ -808,19 +807,19 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
> return 0;
> }
>
> -int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,
> - unsigned int length)
> +int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,
> + unsigned int planeIndex, unsigned int length)
> {
> struct v4l2_exportbuffer expbuf = {};
> int ret;
>
> LOG(V4L2, Debug)
> - << "Buffer " << buffer->index()
> + << "Buffer " << index
> << " plane " << planeIndex
> << ": length=" << length;
>
> expbuf.type = bufferType_;
> - expbuf.index = buffer->index();
> + expbuf.index = index;
> expbuf.plane = planeIndex;
> expbuf.flags = O_RDWR;
>
> @@ -904,7 +903,8 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)
> buf.field = V4L2_FIELD_NONE;
>
> bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);
> - const std::vector<Plane> &planes = buffer->planes();
> + BufferMemory *mem = &bufferPool_->buffers()[buf.index];
> + const std::vector<Plane> &planes = mem->planes();
>
> if (buf.memory == V4L2_MEMORY_DMABUF) {
> if (multiPlanar) {
> @@ -937,9 +937,11 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)
> return ret;
> }
>
> - if (queuedBuffersCount_++ == 0)
> + if (queuedBuffers_.empty())
> fdEvent_->setEnabled(true);
>
> + queuedBuffers_[buf.index] = buffer;
> +
> return 0;
> }
>
> @@ -971,7 +973,7 @@ int V4L2VideoDevice::queueAllBuffers(std::vector<std::unique_ptr<Buffer>> *buffe
> {
> int ret;
>
> - if (queuedBuffersCount_)
> + if (!queuedBuffers_.empty())
> return -EINVAL;
>
> if (V4L2_TYPE_IS_OUTPUT(bufferType_))
> @@ -980,8 +982,7 @@ int V4L2VideoDevice::queueAllBuffers(std::vector<std::unique_ptr<Buffer>> *buffe
> buffers->clear();
>
> for (unsigned int i = 0; i < bufferPool_->count(); ++i) {
> - Buffer *buffer = new Buffer();
> - buffer->index_ = i;
> + Buffer *buffer = new Buffer(i);
> buffers->emplace_back(buffer);
> ret = queueBuffer(buffer);
> if (ret)
> @@ -1022,11 +1023,14 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
>
> ASSERT(buf.index < bufferPool_->count());
>
> - if (--queuedBuffersCount_ == 0)
> + auto it = queuedBuffers_.find(buf.index);
> + Buffer *buffer = it->second;
> + queuedBuffers_.erase(it);
> +
> + if (queuedBuffers_.empty())
> fdEvent_->setEnabled(false);
>
> - Buffer *buffer = &bufferPool_->buffers()[buf.index];
> -
> + buffer->index_ = buf.index;
> buffer->bytesused_ = buf.bytesused;
> buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL
> + buf.timestamp.tv_usec * 1000ULL;
> @@ -1101,7 +1105,7 @@ int V4L2VideoDevice::streamOff()
> return ret;
> }
>
> - queuedBuffersCount_ = 0;
> + queuedBuffers_.clear();
> fdEvent_->setEnabled(false);
>
> return 0;
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 907d2423ed15..6ecf30e33bcf 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -142,8 +142,7 @@ int MainWindow::startCapture()
>
> BufferPool &pool = stream->bufferPool();
> std::vector<Request *> requests;
> -
> - for (Buffer &buffer : pool.buffers()) {
> + for (unsigned int i = 0; i < pool.count(); ++i) {
> Request *request = camera_->createRequest();
> if (!request) {
> std::cerr << "Can't create request" << std::endl;
> @@ -151,11 +150,15 @@ int MainWindow::startCapture()
> goto error;
> }
>
> - std::map<Stream *, Buffer *> map;
> - map[stream] = &buffer;
> - ret = request->setBuffers(map);
> + std::unique_ptr<Buffer> buffer = stream->createBuffer(i);
> + if (!buffer) {
> + std::cerr << "Can't create buffer " << i << std::endl;
> + goto error;
> + }
> +
> + ret = request->addBuffer(std::move(buffer));
> if (ret < 0) {
> - std::cerr << "Can't set buffers for request" << std::endl;
> + std::cerr << "Can't set buffer for request" << std::endl;
> goto error;
> }
>
> @@ -219,6 +222,7 @@ void MainWindow::requestComplete(Request *request,
>
> framesCaptured_++;
>
> + Stream *stream = buffers.begin()->first;
> Buffer *buffer = buffers.begin()->second;
>
> double fps = buffer->timestamp() - lastBufferTime_;
> @@ -232,7 +236,8 @@ void MainWindow::requestComplete(Request *request,
> << " fps: " << std::fixed << std::setprecision(2) << fps
> << std::endl;
>
> - display(buffer);
> + BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()];
> + display(buffer, mem);
>
> request = camera_->createRequest();
> if (!request) {
> @@ -240,16 +245,29 @@ void MainWindow::requestComplete(Request *request,
> return;
> }
>
> - request->setBuffers(buffers);
> + for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> + Stream *stream = it->first;
> + Buffer *buffer = it->second;
> + unsigned int index = buffer->index();
> +
> + std::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);
> + if (!newBuffer) {
> + std::cerr << "Can't create buffer " << index << std::endl;
> + return;
> + }
> +
> + request->addBuffer(std::move(newBuffer));
> + }
> +
> camera_->queueRequest(request);
> }
>
> -int MainWindow::display(Buffer *buffer)
> +int MainWindow::display(Buffer *buffer, BufferMemory *mem)
> {
> - if (buffer->planes().size() != 1)
> + if (mem->planes().size() != 1)
> return -EINVAL;
>
> - Plane &plane = buffer->planes().front();
> + Plane &plane = mem->planes().front();
> unsigned char *raw = static_cast<unsigned char *>(plane.mem());
> viewfinder_->display(raw, buffer->bytesused());
>
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index f58cb6a65b2b..b4f6f747e16e 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -48,7 +48,7 @@ private:
>
> void requestComplete(Request *request,
> const std::map<Stream *, Buffer *> &buffers);
> - int display(Buffer *buffer);
> + int display(Buffer *buffer, BufferMemory *mem);
>
> QString title_;
> QTimer titleTimer_;
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 7ce247cc482d..ff1cbd6cbac0 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -34,9 +34,13 @@ protected:
>
> completeRequestsCount_++;
>
> - /* Reuse the buffers for a new request. */
> + /* Create a new request. */
> + Stream *stream = buffers.begin()->first;
> + Buffer *buffer = buffers.begin()->second;
> + std::unique_ptr<Buffer> newBuffer = stream->createBuffer(buffer->index());
> +
> request = camera_->createRequest();
> - request->setBuffers(buffers);
> + request->addBuffer(std::move(newBuffer));
> camera_->queueRequest(request);
> }
>
> @@ -78,15 +82,20 @@ protected:
> Stream *stream = cfg.stream();
> BufferPool &pool = stream->bufferPool();
> std::vector<Request *> requests;
> - for (Buffer &buffer : pool.buffers()) {
> + for (unsigned int i = 0; i < pool.count(); ++i) {
> Request *request = camera_->createRequest();
> if (!request) {
> cout << "Failed to create request" << endl;
> return TestFail;
> }
>
> - std::map<Stream *, Buffer *> map = { { stream, &buffer } };
> - if (request->setBuffers(map)) {
> + std::unique_ptr<Buffer> buffer = stream->createBuffer(i);
> + if (!buffer) {
> + cout << "Failed to create buffer " << i << endl;
> + return TestFail;
> + }
> +
> + if (request->addBuffer(std::move(buffer))) {
> cout << "Failed to associating buffer with request" << endl;
> return TestFail;
> }
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index 84d2a6fab5f0..12d5e0e1d534 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -211,10 +211,8 @@ protected:
> return TestFail;
>
> Stream *stream = *camera_->streams().begin();
> - BufferPool &pool = stream->bufferPool();
> - Buffer &buffer = pool.buffers().front();
> - std::map<Stream *, Buffer *> map = { { stream, &buffer } };
> - if (request->setBuffers(map))
> + std::unique_ptr<Buffer> buffer = stream->createBuffer(0);
> + if (request->addBuffer(std::move(buffer)))
> return TestFail;
>
> if (camera_->queueRequest(request))
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list