Patch Detail
Show a patch.
GET /api/patches/2607/?format=api
{ "id": 2607, "url": "https://patchwork.libcamera.org/api/patches/2607/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2607/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20200112010212.2609025-26-niklas.soderlund@ragnatech.se>", "date": "2020-01-12T01:02:05", "name": "[libcamera-devel,v4,25/32] libcamera: allocator: Add FrameBufferAllocator to help applications allocate buffers", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "6ae19eca44b032522792457d3023ad73c7d439db", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2607/mbox/", "series": [ { "id": 617, "url": "https://patchwork.libcamera.org/api/series/617/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=617", "date": "2020-01-12T01:01:40", "name": "libcamera: Rework buffer API", "version": 4, "mbox": "https://patchwork.libcamera.org/series/617/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2607/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2607/checks/", "tags": {}, "headers": { "Return-Path": "<niklas.soderlund@ragnatech.se>", "Received": [ "from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net\n\t[195.74.38.228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A3CA606DB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 12 Jan 2020 02:03:22 +0100 (CET)", "from bismarck.berto.se (p54ac5d7b.dip0.t-ipconnect.de\n\t[84.172.93.123]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid 5162b0d8-34d7-11ea-b6d8-005056917f90;\n\tSun, 12 Jan 2020 02:03:17 +0100 (CET)" ], "X-Halon-ID": "5162b0d8-34d7-11ea-b6d8-005056917f90", "Authorized-sender": "niklas@soderlund.pp.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sun, 12 Jan 2020 02:02:05 +0100", "Message-Id": "<20200112010212.2609025-26-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200112010212.2609025-1-niklas.soderlund@ragnatech.se>", "References": "<20200112010212.2609025-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v4 25/32] libcamera: allocator: Add\n\tFrameBufferAllocator to help applications allocate buffers", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "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": "Sun, 12 Jan 2020 01:03:22 -0000" }, "content": "The FrameBuffer interface is based on the idea that all buffers are\nallocated externally to libcamera and are only used by it. This is meant\nto create a simpler API centered around usage of buffers, regardless of\nwhere they come from.\n\nLinux however lacks a centralized allocator at the moment, and not all\nusers of libcamera are expected to use another device that could provide\nsuitable buffers for the camera. This patch thus adds a helper class to\nallocate buffers internally in libcamera, in a way that matches the\nneeds of the FrameBuffer-based API.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n* Changes since v3\n- Code style update\n- Dropped leftover \\brief\n\n* Changes since v2\n- Replace FrameBufferAllocator::streams() with FrameBufferAllocator::allocated()\n- Add todo to see if we can find a better API for Camera::relase() with a active FrameBufferAllocator\n- Touch up documentation\n- Make sure stream to be allocated is part of the cameras active configuration not just the camera\n\n* Changes since v1\n- Update commit message\n- Rename class to FrameBufferAllocator\n- Rename source files framebuffer_allocator.{cpp,h}\n- Update include headers\n- Update documentation\n- Check for double allocation for the same stream in allocate()\n- Add interactions with Camera::allocator_ and enforce buffers may only\n be allocated when the camera is in the configured state.\n---\n include/libcamera/camera.h | 5 +\n include/libcamera/framebuffer_allocator.h | 45 +++++\n include/libcamera/meson.build | 1 +\n src/libcamera/camera.cpp | 19 +-\n src/libcamera/framebuffer_allocator.cpp | 216 ++++++++++++++++++++++\n src/libcamera/meson.build | 1 +\n 6 files changed, 286 insertions(+), 1 deletion(-)\n create mode 100644 include/libcamera/framebuffer_allocator.h\n create mode 100644 src/libcamera/framebuffer_allocator.cpp", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex ef6a37bb142c83a6..2fd5870b15c69087 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -20,6 +20,7 @@\n namespace libcamera {\n \n class Buffer;\n+class FrameBufferAllocator;\n class PipelineHandler;\n class Request;\n \n@@ -126,6 +127,10 @@ private:\n \n \tbool disconnected_;\n \tState state_;\n+\n+\t/* Needed to update allocator_ and to read state_ and activeStreams_. */\n+\tfriend class FrameBufferAllocator;\n+\tFrameBufferAllocator *allocator_;\n };\n \n } /* namespace libcamera */\ndiff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h\nnew file mode 100644\nindex 0000000000000000..42812253ada94d94\n--- /dev/null\n+++ b/include/libcamera/framebuffer_allocator.h\n@@ -0,0 +1,45 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2019, Google Inc.\n+ *\n+ * framebuffer_allocator.h - FrameBuffer allocator\n+ */\n+#ifndef __LIBCAMERA_FRAMEBUFFER_ALLOCATOR_H__\n+#define __LIBCAMERA_FRAMEBUFFER_ALLOCATOR_H__\n+\n+#include <map>\n+#include <memory>\n+#include <vector>\n+\n+namespace libcamera {\n+\n+class Camera;\n+class FrameBuffer;\n+class Stream;\n+\n+class FrameBufferAllocator\n+{\n+public:\n+\tstatic FrameBufferAllocator *create(std::shared_ptr<Camera> camera);\n+\n+\tFrameBufferAllocator(const Camera &) = delete;\n+\tFrameBufferAllocator &operator=(const Camera &) = delete;\n+\n+\t~FrameBufferAllocator();\n+\n+\tint allocate(Stream *stream);\n+\tint free(Stream *stream);\n+\n+\tbool allocated() const { return !buffers_.empty(); }\n+\tconst std::vector<std::unique_ptr<FrameBuffer>> &buffers(Stream *stream) const;\n+\n+private:\n+\tFrameBufferAllocator(std::shared_ptr<Camera> camera);\n+\n+\tstd::shared_ptr<Camera> camera_;\n+\tstd::map<Stream *, std::vector<std::unique_ptr<FrameBuffer>>> buffers_;\n+};\n+\n+} /* namespace libcamera */\n+\n+#endif /* __LIBCAMERA_FRAMEBUFFER_ALLOCATOR_H__ */\ndiff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\nindex 543e6773cc5158a0..8db217bb782c1443 100644\n--- a/include/libcamera/meson.build\n+++ b/include/libcamera/meson.build\n@@ -7,6 +7,7 @@ libcamera_api = files([\n 'event_dispatcher.h',\n 'event_notifier.h',\n 'file_descriptor.h',\n+ 'framebuffer_allocator.h',\n 'geometry.h',\n 'logging.h',\n 'object.h',\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 5d294b10647db8da..20340167a140f33a 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -9,6 +9,7 @@\n \n #include <iomanip>\n \n+#include <libcamera/framebuffer_allocator.h>\n #include <libcamera/request.h>\n #include <libcamera/stream.h>\n \n@@ -405,7 +406,7 @@ const std::string &Camera::name() const\n \n Camera::Camera(PipelineHandler *pipe, const std::string &name)\n \t: pipe_(pipe->shared_from_this()), name_(name), disconnected_(false),\n-\t state_(CameraAvailable)\n+\t state_(CameraAvailable), allocator_(nullptr)\n {\n }\n \n@@ -541,6 +542,16 @@ int Camera::release()\n \tif (!stateBetween(CameraAvailable, CameraConfigured))\n \t\treturn -EBUSY;\n \n+\tif (allocator_) {\n+\t\t/*\n+\t\t * \\todo Try to find a better API that would make this error\n+\t\t * impossible.\n+\t\t */\n+\t\tLOG(Camera, Error)\n+\t\t\t<< \"Buffers must be freed before the camera can be reconfigured\";\n+\t\treturn -EBUSY;\n+\t}\n+\n \tpipe_->unlock();\n \n \tstate_ = CameraAvailable;\n@@ -649,6 +660,12 @@ int Camera::configure(CameraConfiguration *config)\n \tif (!stateBetween(CameraAcquired, CameraConfigured))\n \t\treturn -EACCES;\n \n+\tif (allocator_ && allocator_->allocated()) {\n+\t\tLOG(Camera, Error)\n+\t\t\t<< \"Allocator must be deleted before camera can be reconfigured\";\n+\t\treturn -EBUSY;\n+\t}\n+\n \tif (config->validate() != CameraConfiguration::Valid) {\n \t\tLOG(Camera, Error)\n \t\t\t<< \"Can't configure camera with invalid configuration\";\ndiff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\nnew file mode 100644\nindex 0000000000000000..57789b24e9615fa6\n--- /dev/null\n+++ b/src/libcamera/framebuffer_allocator.cpp\n@@ -0,0 +1,216 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2019, Google Inc.\n+ *\n+ * framebuffer_allocator.cpp - FrameBuffer allocator\n+ */\n+\n+#include <libcamera/framebuffer_allocator.h>\n+\n+#include <errno.h>\n+\n+#include <libcamera/buffer.h>\n+#include <libcamera/camera.h>\n+#include <libcamera/stream.h>\n+\n+#include \"log.h\"\n+#include \"pipeline_handler.h\"\n+\n+/**\n+ * \\file framebuffer_allocator.h\n+ * \\brief FrameBuffer allocator\n+ */\n+\n+namespace libcamera {\n+\n+LOG_DEFINE_CATEGORY(Allocator)\n+\n+/**\n+ * \\class FrameBufferAllocator\n+ * \\brief FrameBuffer allocator for applications\n+ *\n+ * The libcamera API is designed to consume buffers provided by applications as\n+ * FrameBuffer instances. This makes libcamera a user of buffers exported by\n+ * other devices (such as displays or video encoders), or allocated from an\n+ * external allocator (such as ION on Android platforms). In some situations,\n+ * applications do not have any mean to allocate or get hold of suitable\n+ * buffers, for instance when no other device is involved, on Linux platforms\n+ * that lack a centralized allocator. The FrameBufferAllocator class provides a\n+ * buffer allocator that can be used in these situations.\n+ *\n+ * Applications create a framebuffer allocator for a Camera, and use it to\n+ * allocate buffers for streams of a CameraConfiguration with allocate(). They\n+ * control which streams to allocate buffers for, and can thus use external\n+ * buffers for a subset of the streams if desired.\n+ *\n+ * Buffers are deleted for a stream with free(), and destroying the allocator\n+ * automatically deletes all allocated buffers. Applications own the buffers\n+ * allocated by the FrameBufferAllocator and are responsible for ensuring the\n+ * buffers are not deleted while they are in use (part of a Request that has\n+ * been queued and hasn't completed yet).\n+ *\n+ * Usage of the FrameBufferAllocator is optional, if all buffers for a camera\n+ * are provided externally applications shall not use this class.\n+ */\n+\n+/**\n+ * \\brief Create a FrameBuffer allocator\n+ * \\param[in] camera The camera the allocator serves\n+ *\n+ * A single allocator may be created for a Camera instance.\n+ *\n+ * The caller is responsible for deleting the allocator before the camera is\n+ * released.\n+ *\n+ * \\return A pointer to the newly created allocator object or nullptr on error\n+ */\n+FrameBufferAllocator *\n+FrameBufferAllocator::create(std::shared_ptr<Camera> camera)\n+{\n+\tif (camera->allocator_) {\n+\t\tLOG(Allocator, Error) << \"Camera already has an allocator\";\n+\t\treturn nullptr;\n+\t}\n+\n+\tcamera->allocator_ = new FrameBufferAllocator(camera);\n+\treturn camera->allocator_;\n+}\n+\n+/**\n+ * \\brief Construct a FrameBufferAllocator serving a camera\n+ * \\param[in] camera The camera\n+ */\n+FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)\n+\t: camera_(camera)\n+{\n+}\n+\n+FrameBufferAllocator::~FrameBufferAllocator()\n+{\n+\tfor (auto &value : buffers_) {\n+\t\tStream *stream = value.first;\n+\t\tcamera_->pipe_->freeFrameBuffers(camera_.get(), stream);\n+\t}\n+\n+\tbuffers_.clear();\n+\n+\tcamera_->allocator_ = nullptr;\n+}\n+\n+/**\n+ * \\brief Allocate buffers for a configured stream\n+ * \\param[in] stream The stream to allocate buffers for\n+ *\n+ * Allocate buffers suitable for capturing frames from the \\a stream. The Camera\n+ * shall have been previously configured with Camera::configure() and shall be\n+ * stopped, and the stream shall be part of the active camera configuration.\n+ *\n+ * Upon successful allocation, the allocated buffers can be retrieved with the\n+ * buffers() method.\n+ *\n+ * \\return 0 on success or a negative error code otherwise\n+ * \\retval -EACCES The camera is not in a state where buffers can be allocated\n+ * \\retval -EINVAL The \\a stream does not belong to the camera or the stream is\n+ * not part of the active camera configuration\n+ * \\retval -EBUSY Buffers are already allocated for the \\a stream\n+ */\n+int FrameBufferAllocator::allocate(Stream *stream)\n+{\n+\tif (camera_->state_ != Camera::CameraConfigured &&\n+\t camera_->state_ != Camera::CameraPrepared) {\n+\t\tLOG(Allocator, Error)\n+\t\t\t<< \"Camera must be in the configured state to allocate buffers\";\n+\t\treturn -EACCES;\n+\t}\n+\n+\tif (camera_->streams().find(stream) == camera_->streams().end()) {\n+\t\tLOG(Allocator, Error)\n+\t\t\t<< \"Stream does not belong to \" << camera_->name();\n+\t\treturn -EINVAL;\n+\t}\n+\n+\tif (camera_->activeStreams_.find(stream) == camera_->activeStreams_.end()) {\n+\t\tLOG(Allocator, Error)\n+\t\t\t<< \"Stream is not part of \" << camera_->name()\n+\t\t\t<< \" active configuration\";\n+\t\treturn -EINVAL;\n+\t}\n+\n+\tif (buffers_.count(stream)) {\n+\t\tLOG(Allocator, Error) << \"Buffers already allocated for stream\";\n+\t\treturn -EBUSY;\n+\t}\n+\n+\tint ret = camera_->pipe_->exportFrameBuffers(camera_.get(), stream,\n+\t\t\t\t\t\t &buffers_[stream]);\n+\tif (ret)\n+\t\treturn ret;\n+\n+\treturn 0;\n+}\n+\n+/**\n+ * \\brief Free buffers previously allocated for a \\a stream\n+ * \\param[in] stream The stream\n+ *\n+ * Free buffers allocated with allocate().\n+ *\n+ * This invalidates the buffers returned by buffers().\n+ *\n+ * \\return 0 on success or a negative error code otherwise\n+ * \\retval -EACCES The camera is not in a state where buffers can be freed\n+ * \\retval -EINVAL The allocator do not handle the \\a stream\n+ */\n+int FrameBufferAllocator::free(Stream *stream)\n+{\n+\tif (camera_->state_ != Camera::CameraConfigured && camera_->state_ != Camera::CameraPrepared) {\n+\t\tLOG(Allocator, Error)\n+\t\t\t<< \"Camera must be in the configured state to free buffers\";\n+\t\treturn -EACCES;\n+\t}\n+\n+\tauto iter = buffers_.find(stream);\n+\tif (iter == buffers_.end())\n+\t\treturn -EINVAL;\n+\n+\tstd::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second;\n+\n+\tbuffers.clear();\n+\n+\tcamera_->pipe_->freeFrameBuffers(camera_.get(), stream);\n+\n+\tbuffers_.erase(iter);\n+\n+\treturn 0;\n+}\n+\n+/**\n+ * \\fn FrameBufferAllocator::allocated()\n+ * \\brief Check if the allocator has allocated buffers for any stream\n+ * \\return True if the allocator has allocated buffers for one or more\n+ * streams, false otherwise\n+ */\n+\n+/**\n+ * \\brief Retrieve the buffers allocated for a \\a stream\n+ * \\param[in] stream The stream to retrive buffers for\n+ *\n+ * This method shall only be called after successfully allocating buffers for\n+ * \\a stream with allocate(). The returned buffers are valid until free() is\n+ * called for the same stream or the FrameBufferAllocator instance is destroyed.\n+ *\n+ * \\return The buffers allocated for the \\a stream\n+ */\n+const std::vector<std::unique_ptr<FrameBuffer>> &\n+FrameBufferAllocator::buffers(Stream *stream) const\n+{\n+\tstatic std::vector<std::unique_ptr<FrameBuffer>> empty;\n+\n+\tauto iter = buffers_.find(stream);\n+\tif (iter == buffers_.end())\n+\t\treturn empty;\n+\n+\treturn iter->second;\n+}\n+\n+} /* namespace libcamera */\ndiff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\nindex 722c5bc15afe52ef..68d89559b290befd 100644\n--- a/src/libcamera/meson.build\n+++ b/src/libcamera/meson.build\n@@ -16,6 +16,7 @@ libcamera_sources = files([\n 'event_notifier.cpp',\n 'file_descriptor.cpp',\n 'formats.cpp',\n+ 'framebuffer_allocator.cpp',\n 'geometry.cpp',\n 'ipa_context_wrapper.cpp',\n 'ipa_controls.cpp',\n", "prefixes": [ "libcamera-devel", "v4", "25/32" ] }