Patch Detail
Show a patch.
GET /api/1.1/patches/1687/?format=api
{ "id": 1687, "url": "https://patchwork.libcamera.org/api/1.1/patches/1687/?format=api", "web_url": "https://patchwork.libcamera.org/patch/1687/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20190713172351.25452-7-laurent.pinchart@ideasonboard.com>", "date": "2019-07-13T17:23:41", "name": "[libcamera-devel,v2,06/16] libcamera: buffer: Split memory information to BufferMemory", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "7625050e29d3f199222925bd612eda290e6f1594", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/1687/mbox/", "series": [ { "id": 430, "url": "https://patchwork.libcamera.org/api/1.1/series/430/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=430", "date": "2019-07-13T17:23:35", "name": "Add support for external buffers", "version": 2, "mbox": "https://patchwork.libcamera.org/series/430/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/1687/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/1687/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 09A886175D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 13 Jul 2019 19:24:35 +0200 (CEST)", "from pendragon.ideasonboard.com (softbank126209254147.bbtec.net\n\t[126.209.254.147])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 86B142B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 13 Jul 2019 19:24:33 +0200 (CEST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1563038674;\n\tbh=SVZM/RZQRsI+9BbTvQyNfG3b6MrlDwmv6HLWgJSiKoA=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=ftktXEvAq1oOzU/BPz4sqHSLYP6Mfh2VGHl4JHkxJ0Cmbsl1tl/U7SBK0wncFry1e\n\tBTjWwRQq9NvDRudz4UWPyAaazMAIkqCgtlgU+hhFcumn69hqWlMMrVi1zeTK9bRIiN\n\tqk1Ot7RY8IxBnwZRn4hfBN6Buza5Tc+7uf06BgFE=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 13 Jul 2019 20:23:41 +0300", "Message-Id": "<20190713172351.25452-7-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.21.0", "In-Reply-To": "<20190713172351.25452-1-laurent.pinchart@ideasonboard.com>", "References": "<20190713172351.25452-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 06/16] libcamera: buffer: Split memory\n\tinformation to BufferMemory", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.23", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Sat, 13 Jul 2019 17:24:35 -0000" }, "content": "The Buffer class is a large beast the stores information about the\nbuffer memory, dynamic metadata related to the frame stored in the\nbuffer, and buffer reference data (in the index). In order to implement\nbuffer import we will need to extend this with dmabuf file descriptors,\nmaking usage of the class even more complex.\n\nRefactor the Buffer class by splitting the buffer memory information to\na BufferMemory class, and repurposing the Buffer class to reference a\nbuffer and to store dynamic metadata. The BufferMemory class becomes a\nlong term storage, valid and stable from the time buffer memory is\nallocated to the time it is freed. The Buffer class, on the other hand,\nbecomes transient, is created on demand when an application requires a\nbuffer, is given to a request, and is deleted when the request\ncompletes.\n\nBuffer and BufferMemory don't need to be copied, so their copy\nconstructor and assignment operators are deleted.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/buffer.h | 55 +++++---\n include/libcamera/request.h | 4 +-\n include/libcamera/stream.h | 4 +\n src/cam/buffer_writer.cpp | 4 +-\n src/cam/buffer_writer.h | 3 +-\n src/cam/capture.cpp | 35 +++--\n src/libcamera/buffer.cpp | 164 ++++++++++++++---------\n src/libcamera/include/v4l2_videodevice.h | 8 +-\n src/libcamera/request.cpp | 35 +++--\n src/libcamera/stream.cpp | 24 ++++\n src/libcamera/v4l2_videodevice.cpp | 40 +++---\n src/qcam/main_window.cpp | 40 ++++--\n src/qcam/main_window.h | 2 +-\n test/camera/capture.cpp | 19 ++-\n test/camera/statemachine.cpp | 6 +-\n 15 files changed, 298 insertions(+), 145 deletions(-)", "diff": "diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\nindex 260a62e9e77e..f5ba6207bcef 100644\n--- a/include/libcamera/buffer.h\n+++ b/include/libcamera/buffer.h\n@@ -14,6 +14,7 @@ namespace libcamera {\n \n class BufferPool;\n class Request;\n+class Stream;\n \n class Plane final\n {\n@@ -36,6 +37,30 @@ private:\n \tvoid *mem_;\n };\n \n+class BufferMemory final\n+{\n+public:\n+\tstd::vector<Plane> &planes() { return planes_; }\n+\n+private:\n+\tstd::vector<Plane> planes_;\n+};\n+\n+class BufferPool final\n+{\n+public:\n+\t~BufferPool();\n+\n+\tvoid createBuffers(unsigned int count);\n+\tvoid destroyBuffers();\n+\n+\tunsigned int count() const { return buffers_.size(); }\n+\tstd::vector<BufferMemory> &buffers() { return buffers_; }\n+\n+private:\n+\tstd::vector<BufferMemory> buffers_;\n+};\n+\n class Buffer final\n {\n public:\n@@ -45,20 +70,24 @@ public:\n \t\tBufferCancelled,\n \t};\n \n-\tBuffer();\n+\tBuffer(unsigned int index = -1, const Buffer *metadata = nullptr);\n+\tBuffer(const Buffer &) = delete;\n+\tBuffer &operator=(const Buffer &) = delete;\n \n \tunsigned int index() const { return index_; }\n+\n \tunsigned int bytesused() const { return bytesused_; }\n \tuint64_t timestamp() const { return timestamp_; }\n \tunsigned int sequence() const { return sequence_; }\n+\n \tStatus status() const { return status_; }\n-\tstd::vector<Plane> &planes() { return planes_; }\n \tRequest *request() const { return request_; }\n+\tStream *stream() const { return stream_; }\n \n private:\n-\tfriend class BufferPool;\n \tfriend class PipelineHandler;\n \tfriend class Request;\n+\tfriend class Stream;\n \tfriend class V4L2VideoDevice;\n \n \tvoid cancel();\n@@ -66,28 +95,14 @@ private:\n \tvoid setRequest(Request *request) { request_ = request; }\n \n \tunsigned int index_;\n+\n \tunsigned int bytesused_;\n \tuint64_t timestamp_;\n \tunsigned int sequence_;\n+\n \tStatus status_;\n-\n-\tstd::vector<Plane> planes_;\n \tRequest *request_;\n-};\n-\n-class BufferPool final\n-{\n-public:\n-\t~BufferPool();\n-\n-\tvoid createBuffers(unsigned int count);\n-\tvoid destroyBuffers();\n-\n-\tunsigned int count() const { return buffers_.size(); }\n-\tstd::vector<Buffer> &buffers() { return buffers_; }\n-\n-private:\n-\tstd::vector<Buffer> buffers_;\n+\tStream *stream_;\n };\n \n } /* namespace libcamera */\ndiff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex dd165bc21c03..7a60be617645 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -8,6 +8,7 @@\n #define __LIBCAMERA_REQUEST_H__\n \n #include <map>\n+#include <memory>\n #include <stdint.h>\n #include <unordered_set>\n \n@@ -33,10 +34,11 @@ public:\n \tRequest(Camera *camera, uint64_t cookie = 0);\n \tRequest(const Request &) = delete;\n \tRequest &operator=(const Request &) = delete;\n+\t~Request();\n \n \tControlList &controls() { return controls_; }\n \tconst std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }\n-\tint setBuffers(const std::map<Stream *, Buffer *> &streamMap);\n+\tint addBuffer(std::unique_ptr<Buffer> buffer);\n \tBuffer *findBuffer(Stream *stream) const;\n \n \tuint64_t cookie() const { return cookie_; }\ndiff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\nindex 5b4fea324ce4..f595a630eb4a 100644\n--- a/include/libcamera/stream.h\n+++ b/include/libcamera/stream.h\n@@ -8,6 +8,7 @@\n #define __LIBCAMERA_STREAM_H__\n \n #include <map>\n+#include <memory>\n #include <string>\n #include <vector>\n \n@@ -66,6 +67,9 @@ class Stream\n {\n public:\n \tStream();\n+\n+\tstd::unique_ptr<Buffer> createBuffer(unsigned int index);\n+\n \tBufferPool &bufferPool() { return bufferPool_; }\n \tconst StreamConfiguration &configuration() const { return configuration_; }\n \ndiff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp\nindex e0374ffcb319..b7f2ed4f2d2d 100644\n--- a/src/cam/buffer_writer.cpp\n+++ b/src/cam/buffer_writer.cpp\n@@ -19,7 +19,7 @@ BufferWriter::BufferWriter(const std::string &pattern)\n {\n }\n \n-int BufferWriter::write(libcamera::Buffer *buffer,\n+int BufferWriter::write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem,\n \t\t\tconst std::string &streamName)\n {\n \tstd::string filename;\n@@ -41,7 +41,7 @@ int BufferWriter::write(libcamera::Buffer *buffer,\n \tif (fd == -1)\n \t\treturn -errno;\n \n-\tfor (libcamera::Plane &plane : buffer->planes()) {\n+\tfor (libcamera::Plane &plane : mem->planes()) {\n \t\tvoid *data = plane.mem();\n \t\tunsigned int length = plane.length();\n \ndiff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h\nindex 7bf785d1e832..9bea205fac1d 100644\n--- a/src/cam/buffer_writer.h\n+++ b/src/cam/buffer_writer.h\n@@ -16,7 +16,8 @@ class BufferWriter\n public:\n \tBufferWriter(const std::string &pattern = \"frame-#.bin\");\n \n-\tint write(libcamera::Buffer *buffer, const std::string &streamName);\n+\tint write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem,\n+\t\t const std::string &streamName);\n \n private:\n \tstd::string pattern_;\ndiff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\nindex 6b842d73390d..1cf59063c93b 100644\n--- a/src/cam/capture.cpp\n+++ b/src/cam/capture.cpp\n@@ -95,13 +95,14 @@ int Capture::capture(EventLoop *loop)\n \t\tstd::map<Stream *, Buffer *> map;\n \t\tfor (StreamConfiguration &cfg : *config_) {\n \t\t\tStream *stream = cfg.stream();\n-\t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n-\t\t}\n+\t\t\tstd::unique_ptr<Buffer> buffer = stream->createBuffer(i);\n \n-\t\tret = request->setBuffers(map);\n-\t\tif (ret < 0) {\n-\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n-\t\t\treturn ret;\n+\t\t\tret = request->addBuffer(std::move(buffer));\n+\t\t\tif (ret < 0) {\n+\t\t\t\tstd::cerr << \"Can't set buffer for request\"\n+\t\t\t\t\t << std::endl;\n+\t\t\t\treturn ret;\n+\t\t\t}\n \t\t}\n \n \t\trequests.push_back(request);\n@@ -155,6 +156,7 @@ void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer\n \tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n \t\tStream *stream = it->first;\n \t\tBuffer *buffer = it->second;\n+\t\tBufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()];\n \t\tconst std::string &name = streamName_[stream];\n \n \t\tinfo << \" \" << name\n@@ -163,17 +165,34 @@ void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer\n \t\t << \" bytesused: \" << buffer->bytesused();\n \n \t\tif (writer_)\n-\t\t\twriter_->write(buffer, name);\n+\t\t\twriter_->write(buffer, mem, name);\n \t}\n \n \tstd::cout << info.str() << std::endl;\n \n+\t/*\n+\t * Create a new request and populate it with one buffer for each\n+\t * stream.\n+\t */\n \trequest = camera_->createRequest();\n \tif (!request) {\n \t\tstd::cerr << \"Can't create request\" << std::endl;\n \t\treturn;\n \t}\n \n-\trequest->setBuffers(buffers);\n+\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n+\t\tStream *stream = it->first;\n+\t\tBuffer *buffer = it->second;\n+\t\tunsigned int index = buffer->index();\n+\n+\t\tstd::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);\n+\t\tif (!newBuffer) {\n+\t\t\tstd::cerr << \"Can't create buffer \" << index << std::endl;\n+\t\t\treturn;\n+\t\t}\n+\n+\t\trequest->addBuffer(std::move(newBuffer));\n+\t}\n+\n \tcamera_->queueRequest(request);\n }\ndiff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp\nindex c0a020942f5c..ecbf25246a55 100644\n--- a/src/libcamera/buffer.cpp\n+++ b/src/libcamera/buffer.cpp\n@@ -173,12 +173,78 @@ void *Plane::mem()\n */\n \n /**\n- * \\class Buffer\n+ * \\class BufferMemory\n * \\brief A memory buffer to store an image\n *\n- * The Buffer class represents the memory buffers used to store a\n- * full frame image, which may contain multiple separate memory Plane\n- * objects if the image format is multi-planar.\n+ * The BufferMemory class represents the memory buffers used to store full frame\n+ * images, which may contain multiple separate memory Plane objects if the\n+ * image format is multi-planar.\n+ */\n+\n+/**\n+ * \\fn BufferMemory::planes()\n+ * \\brief Retrieve the planes within the buffer\n+ * \\return A reference to a vector holding all Planes within the buffer\n+ */\n+\n+/**\n+ * \\class BufferPool\n+ * \\brief A pool of buffers\n+ *\n+ * The BufferPool class groups together a collection of Buffers to store frames.\n+ * The buffers must be exported by a device before they can be imported into\n+ * another device for further use.\n+ */\n+\n+BufferPool::~BufferPool()\n+{\n+\tdestroyBuffers();\n+}\n+\n+/**\n+ * \\brief Create buffers in the Pool\n+ * \\param[in] count The number of buffers to create\n+ */\n+void BufferPool::createBuffers(unsigned int count)\n+{\n+\tbuffers_.resize(count);\n+}\n+\n+/**\n+ * \\brief Release all buffers from pool\n+ *\n+ * If no buffers have been created or if buffers have already been released no\n+ * operation is performed.\n+ */\n+void BufferPool::destroyBuffers()\n+{\n+\tbuffers_.resize(0);\n+}\n+\n+/**\n+ * \\fn BufferPool::count()\n+ * \\brief Retrieve the number of buffers contained within the pool\n+ * \\return The number of buffers contained in the pool\n+ */\n+\n+/**\n+ * \\fn BufferPool::buffers()\n+ * \\brief Retrieve all the buffers in the pool\n+ * \\return A vector containing all the buffers in the pool.\n+ */\n+\n+/**\n+ * \\class Buffer\n+ * \\brief A buffer handle and dynamic metadata\n+ *\n+ * The Buffer class references a buffer memory and associates dynamic metadata\n+ * related to the frame contained in the buffer. It allows referencing buffer\n+ * memory through a single interface regardless of whether the memory is\n+ * allocated internally in libcamera or provided externally through dmabuf.\n+ *\n+ * Buffer instances are allocated dynamically for a stream through\n+ * Stream::createBuffer(), added to a request with Request::addBuffer() and\n+ * deleted automatically after the request complete handler returns.\n */\n \n /**\n@@ -195,9 +261,26 @@ void *Plane::mem()\n * invalid and shall not be used.\n */\n \n-Buffer::Buffer()\n-\t: index_(-1), request_(nullptr)\n+/**\n+ * \\brief Construct a buffer not associated with any stream\n+ *\n+ * This method constructs an orphaned buffer not associated with any stream. It\n+ * is not meant to be called by applications, they should instead create buffers\n+ * for a stream with Stream::createBuffer().\n+ */\n+Buffer::Buffer(unsigned int index, const Buffer *metadata)\n+\t: index_(index), status_(Buffer::BufferSuccess), request_(nullptr),\n+\t stream_(nullptr)\n {\n+\tif (metadata) {\n+\t\tbytesused_ = metadata->bytesused_;\n+\t\tsequence_ = metadata->sequence_;\n+\t\ttimestamp_ = metadata->timestamp_;\n+\t} else {\n+\t\tbytesused_ = 0;\n+\t\tsequence_ = 0;\n+\t\ttimestamp_ = 0;\n+\t}\n }\n \n /**\n@@ -206,12 +289,6 @@ Buffer::Buffer()\n * \\return The buffer index\n */\n \n-/**\n- * \\fn Buffer::planes()\n- * \\brief Retrieve the planes within the buffer\n- * \\return A reference to a vector holding all Planes within the buffer\n- */\n-\n /**\n * \\fn Buffer::bytesused()\n * \\brief Retrieve the number of bytes occupied by the data in the buffer\n@@ -264,6 +341,19 @@ Buffer::Buffer()\n * \\sa Buffer::setRequest()\n */\n \n+/**\n+ * \\fn Buffer::stream()\n+ * \\brief Retrieve the stream this buffer is associated with\n+ *\n+ * A Buffer is associated to the stream that created it with\n+ * Stream::createBuffer() and the association is valid until the buffer is\n+ * destroyed. Buffer instances that are created directly are not associated\n+ * with any stream.\n+ *\n+ * \\return The Stream the Buffer is associated with, or nullptr if the buffer\n+ * is not associated with a stream\n+ */\n+\n /**\n * \\brief Mark a buffer as cancel by setting its status to BufferCancelled\n */\n@@ -282,54 +372,4 @@ void Buffer::cancel()\n * The intended callers are Request::prepare() and Request::completeBuffer().\n */\n \n-/**\n- * \\class BufferPool\n- * \\brief A pool of buffers\n- *\n- * The BufferPool class groups together a collection of Buffers to store frames.\n- * The buffers must be exported by a device before they can be imported into\n- * another device for further use.\n- */\n-\n-BufferPool::~BufferPool()\n-{\n-\tdestroyBuffers();\n-}\n-\n-/**\n- * \\brief Create buffers in the Pool\n- * \\param[in] count The number of buffers to create\n- */\n-void BufferPool::createBuffers(unsigned int count)\n-{\n-\tunsigned int index = 0;\n-\n-\tbuffers_.resize(count);\n-\tfor (Buffer &buffer : buffers_)\n-\t\tbuffer.index_ = index++;\n-}\n-\n-/**\n- * \\brief Release all buffers from pool\n- *\n- * If no buffers have been created or if buffers have already been released no\n- * operation is performed.\n- */\n-void BufferPool::destroyBuffers()\n-{\n-\tbuffers_.resize(0);\n-}\n-\n-/**\n- * \\fn BufferPool::count()\n- * \\brief Retrieve the number of buffers contained within the pool\n- * \\return The number of buffers contained in the pool\n- */\n-\n-/**\n- * \\fn BufferPool::buffers()\n- * \\brief Retrieve all the buffers in the pool\n- * \\return A vector containing all the buffers in the pool.\n- */\n-\n } /* namespace libcamera */\ndiff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h\nindex f948a41fb8c1..8238a39b0da4 100644\n--- a/src/libcamera/include/v4l2_videodevice.h\n+++ b/src/libcamera/include/v4l2_videodevice.h\n@@ -7,7 +7,6 @@\n #ifndef __LIBCAMERA_V4L2_VIDEODEVICE_H__\n #define __LIBCAMERA_V4L2_VIDEODEVICE_H__\n \n-#include <atomic>\n #include <string>\n #include <vector>\n \n@@ -23,6 +22,7 @@\n namespace libcamera {\n \n class Buffer;\n+class BufferMemory;\n class BufferPool;\n class EventNotifier;\n class MediaDevice;\n@@ -165,8 +165,8 @@ private:\n \tstd::vector<SizeRange> enumSizes(unsigned int pixelFormat);\n \n \tint requestBuffers(unsigned int count);\n-\tint createPlane(Buffer *buffer, unsigned int plane,\n-\t\t\tunsigned int length);\n+\tint createPlane(BufferMemory *buffer, unsigned int index,\n+\t\t\tunsigned int plane, unsigned int length);\n \n \tBuffer *dequeueBuffer();\n \tvoid bufferAvailable(EventNotifier *notifier);\n@@ -177,7 +177,7 @@ private:\n \tenum v4l2_memory memoryType_;\n \n \tBufferPool *bufferPool_;\n-\tstd::atomic<unsigned int> queuedBuffersCount_;\n+\tstd::map<unsigned int, Buffer *> queuedBuffers_;\n \n \tEventNotifier *fdEvent_;\n };\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex 8cf41a43a80e..45e7133febb0 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -60,6 +60,14 @@ Request::Request(Camera *camera, uint64_t cookie)\n {\n }\n \n+Request::~Request()\n+{\n+\tfor (auto it : bufferMap_) {\n+\t\tBuffer *buffer = it.second;\n+\t\tdelete buffer;\n+\t}\n+}\n+\n /**\n * \\fn Request::controls()\n * \\brief Retrieve the request's ControlList\n@@ -87,19 +95,30 @@ Request::Request(Camera *camera, uint64_t cookie)\n */\n \n /**\n- * \\brief Set the streams to capture with associated buffers\n- * \\param[in] streamMap The map of streams to buffers\n+ * \\brief Store a Buffer with its associated Stream in the Request\n+ * \\param[in] buffer The Buffer to store in the request\n+ *\n+ * Ownership of the buffer is passed to the request. It will be deleted when\n+ * the request is destroyed after completing.\n+ *\n+ * A request can only contain one buffer per stream. If a buffer has already\n+ * been added to the request for the same stream, this method returns -EEXIST.\n+ *\n * \\return 0 on success or a negative error code otherwise\n- * \\retval -EBUSY Buffers have already been set\n+ * \\retval -EEXIST The request already contains a buffer for the stream\n */\n-int Request::setBuffers(const std::map<Stream *, Buffer *> &streamMap)\n+int Request::addBuffer(std::unique_ptr<Buffer> buffer)\n {\n-\tif (!bufferMap_.empty()) {\n-\t\tLOG(Request, Error) << \"Buffers already set\";\n-\t\treturn -EBUSY;\n+\tStream *stream = buffer->stream();\n+\n+\tauto it = bufferMap_.find(stream);\n+\tif (it != bufferMap_.end()) {\n+\t\tLOG(Request, Error) << \"Buffer already set for stream\";\n+\t\treturn -EEXIST;\n \t}\n \n-\tbufferMap_ = streamMap;\n+\tbufferMap_[stream] = buffer.release();\n+\n \treturn 0;\n }\n \ndiff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\nindex d8e87c62281c..6d80646b55cd 100644\n--- a/src/libcamera/stream.cpp\n+++ b/src/libcamera/stream.cpp\n@@ -407,6 +407,30 @@ Stream::Stream()\n {\n }\n \n+/**\n+ * \\brief Create a Buffer instance\n+ * \\param[in] index The desired buffer index\n+ *\n+ * This method creates a Buffer instance that references a BufferMemory from\n+ * the stream's buffers pool by its \\a index. The index shall be lower than the\n+ * number of buffers in the pool.\n+ *\n+ * \\return A newly created Buffer on success or nullptr otherwise\n+ */\n+std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index)\n+{\n+\tif (index >= bufferPool_.count()) {\n+\t\tLOG(Stream, Error) << \"Invalid buffer index \" << index;\n+\t\treturn nullptr;\n+\t}\n+\n+\tBuffer *buffer = new Buffer();\n+\tbuffer->index_ = index;\n+\tbuffer->stream_ = this;\n+\n+\treturn std::unique_ptr<Buffer>(buffer);\n+}\n+\n /**\n * \\fn Stream::bufferPool()\n * \\brief Retrieve the buffer pool for the stream\ndiff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\nindex af5ba803de45..65b4098abc05 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -268,8 +268,7 @@ const std::string V4L2DeviceFormat::toString() const\n * \\param[in] deviceNode The file-system path to the video device node\n */\n V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)\n-\t: V4L2Device(deviceNode), bufferPool_(nullptr),\n-\t queuedBuffersCount_(0), fdEvent_(nullptr)\n+\t: V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr)\n {\n \t/*\n \t * We default to an MMAP based CAPTURE video device, however this will\n@@ -764,7 +763,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)\n \tfor (i = 0; i < pool->count(); ++i) {\n \t\tstruct v4l2_plane planes[VIDEO_MAX_PLANES] = {};\n \t\tstruct v4l2_buffer buf = {};\n-\t\tBuffer &buffer = pool->buffers()[i];\n+\t\tBufferMemory &buffer = pool->buffers()[i];\n \n \t\tbuf.index = i;\n \t\tbuf.type = bufferType_;\n@@ -782,13 +781,13 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)\n \n \t\tif (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) {\n \t\t\tfor (unsigned int p = 0; p < buf.length; ++p) {\n-\t\t\t\tret = createPlane(&buffer, p,\n+\t\t\t\tret = createPlane(&buffer, i, p,\n \t\t\t\t\t\t buf.m.planes[p].length);\n \t\t\t\tif (ret)\n \t\t\t\t\tbreak;\n \t\t\t}\n \t\t} else {\n-\t\t\tret = createPlane(&buffer, 0, buf.length);\n+\t\t\tret = createPlane(&buffer, i, 0, buf.length);\n \t\t}\n \n \t\tif (ret) {\n@@ -808,19 +807,19 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)\n \treturn 0;\n }\n \n-int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,\n-\t\t\t\t unsigned int length)\n+int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,\n+\t\t\t\t unsigned int planeIndex, unsigned int length)\n {\n \tstruct v4l2_exportbuffer expbuf = {};\n \tint ret;\n \n \tLOG(V4L2, Debug)\n-\t\t<< \"Buffer \" << buffer->index()\n+\t\t<< \"Buffer \" << index\n \t\t<< \" plane \" << planeIndex\n \t\t<< \": length=\" << length;\n \n \texpbuf.type = bufferType_;\n-\texpbuf.index = buffer->index();\n+\texpbuf.index = index;\n \texpbuf.plane = planeIndex;\n \texpbuf.flags = O_RDWR;\n \n@@ -904,7 +903,8 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)\n \tbuf.field = V4L2_FIELD_NONE;\n \n \tbool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);\n-\tconst std::vector<Plane> &planes = buffer->planes();\n+\tBufferMemory *mem = &bufferPool_->buffers()[buf.index];\n+\tconst std::vector<Plane> &planes = mem->planes();\n \n \tif (buf.memory == V4L2_MEMORY_DMABUF) {\n \t\tif (multiPlanar) {\n@@ -937,9 +937,11 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)\n \t\treturn ret;\n \t}\n \n-\tif (queuedBuffersCount_++ == 0)\n+\tif (queuedBuffers_.empty())\n \t\tfdEvent_->setEnabled(true);\n \n+\tqueuedBuffers_[buf.index] = buffer;\n+\n \treturn 0;\n }\n \n@@ -971,7 +973,7 @@ int V4L2VideoDevice::queueAllBuffers(std::vector<std::unique_ptr<Buffer>> *buffe\n {\n \tint ret;\n \n-\tif (queuedBuffersCount_)\n+\tif (!queuedBuffers_.empty())\n \t\treturn -EINVAL;\n \n \tif (V4L2_TYPE_IS_OUTPUT(bufferType_))\n@@ -980,8 +982,7 @@ int V4L2VideoDevice::queueAllBuffers(std::vector<std::unique_ptr<Buffer>> *buffe\n \tbuffers->clear();\n \n \tfor (unsigned int i = 0; i < bufferPool_->count(); ++i) {\n-\t\tBuffer *buffer = new Buffer();\n-\t\tbuffer->index_ = i;\n+\t\tBuffer *buffer = new Buffer(i);\n \t\tbuffers->emplace_back(buffer);\n \t\tret = queueBuffer(buffer);\n \t\tif (ret)\n@@ -1022,11 +1023,14 @@ Buffer *V4L2VideoDevice::dequeueBuffer()\n \n \tASSERT(buf.index < bufferPool_->count());\n \n-\tif (--queuedBuffersCount_ == 0)\n+\tauto it = queuedBuffers_.find(buf.index);\n+\tBuffer *buffer = it->second;\n+\tqueuedBuffers_.erase(it);\n+\n+\tif (queuedBuffers_.empty())\n \t\tfdEvent_->setEnabled(false);\n \n-\tBuffer *buffer = &bufferPool_->buffers()[buf.index];\n-\n+\tbuffer->index_ = buf.index;\n \tbuffer->bytesused_ = buf.bytesused;\n \tbuffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL\n \t\t\t + buf.timestamp.tv_usec * 1000ULL;\n@@ -1101,7 +1105,7 @@ int V4L2VideoDevice::streamOff()\n \t\treturn ret;\n \t}\n \n-\tqueuedBuffersCount_ = 0;\n+\tqueuedBuffers_.clear();\n \tfdEvent_->setEnabled(false);\n \n \treturn 0;\ndiff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex 907d2423ed15..6ecf30e33bcf 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -142,8 +142,7 @@ int MainWindow::startCapture()\n \n \tBufferPool &pool = stream->bufferPool();\n \tstd::vector<Request *> requests;\n-\n-\tfor (Buffer &buffer : pool.buffers()) {\n+\tfor (unsigned int i = 0; i < pool.count(); ++i) {\n \t\tRequest *request = camera_->createRequest();\n \t\tif (!request) {\n \t\t\tstd::cerr << \"Can't create request\" << std::endl;\n@@ -151,11 +150,15 @@ int MainWindow::startCapture()\n \t\t\tgoto error;\n \t\t}\n \n-\t\tstd::map<Stream *, Buffer *> map;\n-\t\tmap[stream] = &buffer;\n-\t\tret = request->setBuffers(map);\n+\t\tstd::unique_ptr<Buffer> buffer = stream->createBuffer(i);\n+\t\tif (!buffer) {\n+\t\t\tstd::cerr << \"Can't create buffer \" << i << std::endl;\n+\t\t\tgoto error;\n+\t\t}\n+\n+\t\tret = request->addBuffer(std::move(buffer));\n \t\tif (ret < 0) {\n-\t\t\tstd::cerr << \"Can't set buffers for request\" << std::endl;\n+\t\t\tstd::cerr << \"Can't set buffer for request\" << std::endl;\n \t\t\tgoto error;\n \t\t}\n \n@@ -219,6 +222,7 @@ void MainWindow::requestComplete(Request *request,\n \n \tframesCaptured_++;\n \n+\tStream *stream = buffers.begin()->first;\n \tBuffer *buffer = buffers.begin()->second;\n \n \tdouble fps = buffer->timestamp() - lastBufferTime_;\n@@ -232,7 +236,8 @@ void MainWindow::requestComplete(Request *request,\n \t\t << \" fps: \" << std::fixed << std::setprecision(2) << fps\n \t\t << std::endl;\n \n-\tdisplay(buffer);\n+\tBufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()];\n+\tdisplay(buffer, mem);\n \n \trequest = camera_->createRequest();\n \tif (!request) {\n@@ -240,16 +245,29 @@ void MainWindow::requestComplete(Request *request,\n \t\treturn;\n \t}\n \n-\trequest->setBuffers(buffers);\n+\tfor (auto it = buffers.begin(); it != buffers.end(); ++it) {\n+\t\tStream *stream = it->first;\n+\t\tBuffer *buffer = it->second;\n+\t\tunsigned int index = buffer->index();\n+\n+\t\tstd::unique_ptr<Buffer> newBuffer = stream->createBuffer(index);\n+\t\tif (!newBuffer) {\n+\t\t\tstd::cerr << \"Can't create buffer \" << index << std::endl;\n+\t\t\treturn;\n+\t\t}\n+\n+\t\trequest->addBuffer(std::move(newBuffer));\n+\t}\n+\n \tcamera_->queueRequest(request);\n }\n \n-int MainWindow::display(Buffer *buffer)\n+int MainWindow::display(Buffer *buffer, BufferMemory *mem)\n {\n-\tif (buffer->planes().size() != 1)\n+\tif (mem->planes().size() != 1)\n \t\treturn -EINVAL;\n \n-\tPlane &plane = buffer->planes().front();\n+\tPlane &plane = mem->planes().front();\n \tunsigned char *raw = static_cast<unsigned char *>(plane.mem());\n \tviewfinder_->display(raw, buffer->bytesused());\n \ndiff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\nindex f58cb6a65b2b..b4f6f747e16e 100644\n--- a/src/qcam/main_window.h\n+++ b/src/qcam/main_window.h\n@@ -48,7 +48,7 @@ private:\n \n \tvoid requestComplete(Request *request,\n \t\t\t const std::map<Stream *, Buffer *> &buffers);\n-\tint display(Buffer *buffer);\n+\tint display(Buffer *buffer, BufferMemory *mem);\n \n \tQString title_;\n \tQTimer titleTimer_;\ndiff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\nindex 7ce247cc482d..ff1cbd6cbac0 100644\n--- a/test/camera/capture.cpp\n+++ b/test/camera/capture.cpp\n@@ -34,9 +34,13 @@ protected:\n \n \t\tcompleteRequestsCount_++;\n \n-\t\t/* Reuse the buffers for a new request. */\n+\t\t/* Create a new request. */\n+\t\tStream *stream = buffers.begin()->first;\n+\t\tBuffer *buffer = buffers.begin()->second;\n+\t\tstd::unique_ptr<Buffer> newBuffer = stream->createBuffer(buffer->index());\n+\n \t\trequest = camera_->createRequest();\n-\t\trequest->setBuffers(buffers);\n+\t\trequest->addBuffer(std::move(newBuffer));\n \t\tcamera_->queueRequest(request);\n \t}\n \n@@ -78,15 +82,20 @@ protected:\n \t\tStream *stream = cfg.stream();\n \t\tBufferPool &pool = stream->bufferPool();\n \t\tstd::vector<Request *> requests;\n-\t\tfor (Buffer &buffer : pool.buffers()) {\n+\t\tfor (unsigned int i = 0; i < pool.count(); ++i) {\n \t\t\tRequest *request = camera_->createRequest();\n \t\t\tif (!request) {\n \t\t\t\tcout << \"Failed to create request\" << endl;\n \t\t\t\treturn TestFail;\n \t\t\t}\n \n-\t\t\tstd::map<Stream *, Buffer *> map = { { stream, &buffer } };\n-\t\t\tif (request->setBuffers(map)) {\n+\t\t\tstd::unique_ptr<Buffer> buffer = stream->createBuffer(i);\n+\t\t\tif (!buffer) {\n+\t\t\t\tcout << \"Failed to create buffer \" << i << endl;\n+\t\t\t\treturn TestFail;\n+\t\t\t}\n+\n+\t\t\tif (request->addBuffer(std::move(buffer))) {\n \t\t\t\tcout << \"Failed to associating buffer with request\" << endl;\n \t\t\t\treturn TestFail;\n \t\t\t}\ndiff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\nindex 84d2a6fab5f0..12d5e0e1d534 100644\n--- a/test/camera/statemachine.cpp\n+++ b/test/camera/statemachine.cpp\n@@ -211,10 +211,8 @@ protected:\n \t\t\treturn TestFail;\n \n \t\tStream *stream = *camera_->streams().begin();\n-\t\tBufferPool &pool = stream->bufferPool();\n-\t\tBuffer &buffer = pool.buffers().front();\n-\t\tstd::map<Stream *, Buffer *> map = { { stream, &buffer } };\n-\t\tif (request->setBuffers(map))\n+\t\tstd::unique_ptr<Buffer> buffer = stream->createBuffer(0);\n+\t\tif (request->addBuffer(std::move(buffer)))\n \t\t\treturn TestFail;\n \n \t\tif (camera_->queueRequest(request))\n", "prefixes": [ "libcamera-devel", "v2", "06/16" ] }