Patch Detail
Show a patch.
GET /api/patches/3102/?format=api
{ "id": 3102, "url": "https://patchwork.libcamera.org/api/patches/3102/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3102/", "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": "<20200314235728.15495-10-laurent.pinchart@ideasonboard.com>", "date": "2020-03-14T23:57:28", "name": "[libcamera-devel,9/9] libcamera: framebuffer_allocator: Lift camera restrictions on allocator", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "7d1db04b32aac0aec8658a776643d07c79f0f33f", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/3102/mbox/", "series": [ { "id": 719, "url": "https://patchwork.libcamera.org/api/series/719/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=719", "date": "2020-03-14T23:57:19", "name": "Simplify buffer management with V4L2 buffer orphaning", "version": 1, "mbox": "https://patchwork.libcamera.org/series/719/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3102/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3102/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B7F362958\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Mar 2020 00:57:43 +0100 (CET)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E8A4B1163\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Mar 2020 00:57:42 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584230263;\n\tbh=GC9I3JWIygZO07zgkhuf9lIP4Gq7GK8ea2U5ugQHlwI=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=OYxSoE1SmxBo0FW017Tzwo6jMI/0t0Cnt5RkeaKm+C9+nJgftmJGdyrCykgbveTm3\n\tD6aQprSz8gnE+3CBN+eU8pz1LxV/bdojXc2YzOM5Rm+3IC3dZZ5TVmttoogP7fFL1w\n\t7aoTqXCwUGv99CVMM9ch+zYueLuKsKsL1ArH8jM0=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sun, 15 Mar 2020 01:57:28 +0200", "Message-Id": "<20200314235728.15495-10-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200314235728.15495-1-laurent.pinchart@ideasonboard.com>", "References": "<20200314235728.15495-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 9/9] libcamera: framebuffer_allocator:\n\tLift camera restrictions on allocator", "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": "Sat, 14 Mar 2020 23:57:43 -0000" }, "content": "The Camera class currently requires the allocator to have no allocated\nbuffer before the camera is reconfigured, and the allocator to be\ndestroyed before the camera is released. There's no basis for these\nrestrictions anymore, remove them.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/camera.h | 2 --\n include/libcamera/framebuffer_allocator.h | 5 +----\n src/cam/capture.cpp | 2 +-\n src/gstreamer/gstlibcameraallocator.cpp | 2 +-\n src/libcamera/camera.cpp | 18 +---------------\n src/libcamera/framebuffer_allocator.cpp | 25 -----------------------\n src/qcam/main_window.cpp | 2 +-\n src/v4l2/v4l2_camera.cpp | 2 +-\n test/camera/capture.cpp | 2 +-\n test/camera/statemachine.cpp | 2 +-\n 10 files changed, 8 insertions(+), 54 deletions(-)", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex a5e2b49f0f25..9c0e58f7864b 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -113,8 +113,6 @@ private:\n \tfriend class FrameBufferAllocator;\n \tint exportFrameBuffers(Stream *stream,\n \t\t\t std::vector<std::unique_ptr<FrameBuffer>> *buffers);\n-\t/* \\todo Remove allocator_ from the exposed API */\n-\tFrameBufferAllocator *allocator_;\n };\n \n } /* namespace libcamera */\ndiff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h\nindex 42812253ada9..78f1353964eb 100644\n--- a/include/libcamera/framebuffer_allocator.h\n+++ b/include/libcamera/framebuffer_allocator.h\n@@ -20,8 +20,7 @@ class Stream;\n class FrameBufferAllocator\n {\n public:\n-\tstatic FrameBufferAllocator *create(std::shared_ptr<Camera> camera);\n-\n+\tFrameBufferAllocator(std::shared_ptr<Camera> camera);\n \tFrameBufferAllocator(const Camera &) = delete;\n \tFrameBufferAllocator &operator=(const Camera &) = delete;\n \n@@ -34,8 +33,6 @@ public:\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 };\ndiff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\nindex 7d970f991d3a..b62a9b24b216 100644\n--- a/src/cam/capture.cpp\n+++ b/src/cam/capture.cpp\n@@ -52,7 +52,7 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options)\n \t}\n \n \n-\tFrameBufferAllocator *allocator = FrameBufferAllocator::create(camera_);\n+\tFrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);\n \n \tret = capture(loop, allocator);\n \ndiff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp\nindex d0b90ecaa873..1d5959c076cb 100644\n--- a/src/gstreamer/gstlibcameraallocator.cpp\n+++ b/src/gstreamer/gstlibcameraallocator.cpp\n@@ -188,7 +188,7 @@ gst_libcamera_allocator_new(std::shared_ptr<Camera> camera)\n \tauto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,\n \t\t\t\t\t\t\t nullptr));\n \n-\tself->fb_allocator = FrameBufferAllocator::create(camera);\n+\tself->fb_allocator = new FrameBufferAllocator(camera);\n \tfor (Stream *stream : camera->streams()) {\n \t\tgint ret;\n \ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 5593c1b317a0..8c3bb2c2a01f 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -508,7 +508,7 @@ const std::string &Camera::name() const\n \n Camera::Camera(PipelineHandler *pipe, const std::string &name,\n \t const std::set<Stream *> &streams)\n-\t: p_(new Private(pipe, name, streams)), allocator_(nullptr)\n+\t: p_(new Private(pipe, name, streams))\n {\n }\n \n@@ -620,16 +620,6 @@ int Camera::release()\n \tif (ret < 0)\n \t\treturn ret == -EACCES ? -EBUSY : ret;\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 \tp_->pipe_->unlock();\n \n \tp_->setState(Private::CameraAvailable);\n@@ -763,12 +753,6 @@ int Camera::configure(CameraConfiguration *config)\n \tif (ret < 0)\n \t\treturn ret;\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\nindex 6f7a2e90b08a..a37b564c6701 100644\n--- a/src/libcamera/framebuffer_allocator.cpp\n+++ b/src/libcamera/framebuffer_allocator.cpp\n@@ -53,29 +53,6 @@ LOG_DEFINE_CATEGORY(Allocator)\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@@ -88,8 +65,6 @@ FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)\n FrameBufferAllocator::~FrameBufferAllocator()\n {\n \tbuffers_.clear();\n-\n-\tcamera_->allocator_ = nullptr;\n }\n \n /**\ndiff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex ae1760dfd647..47d37c3e62ce 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -240,7 +240,7 @@ int MainWindow::startCapture()\n \n \tadjustSize();\n \n-\tallocator_ = FrameBufferAllocator::create(camera_);\n+\tallocator_ = new FrameBufferAllocator(camera_);\n \tret = allocator_->allocate(stream);\n \tif (ret < 0) {\n \t\tstd::cerr << \"Failed to allocate capture buffers\" << std::endl;\ndiff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\nindex e7018b566475..f0b9f1804c94 100644\n--- a/src/v4l2/v4l2_camera.cpp\n+++ b/src/v4l2/v4l2_camera.cpp\n@@ -40,7 +40,7 @@ int V4L2Camera::open()\n \t\treturn -EINVAL;\n \t}\n \n-\tbufferAllocator_ = FrameBufferAllocator::create(camera_);\n+\tbufferAllocator_ = new FrameBufferAllocator(camera_);\n \n \treturn 0;\n }\ndiff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\nindex b304d59c1c2a..f6b2f348bda5 100644\n--- a/test/camera/capture.cpp\n+++ b/test/camera/capture.cpp\n@@ -63,7 +63,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tallocator_ = FrameBufferAllocator::create(camera_);\n+\t\tallocator_ = new FrameBufferAllocator(camera_);\n \n \t\treturn TestPass;\n \t}\ndiff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\nindex 20541b3e4752..325b4674bcc9 100644\n--- a/test/camera/statemachine.cpp\n+++ b/test/camera/statemachine.cpp\n@@ -117,7 +117,7 @@ protected:\n \t\t\treturn TestFail;\n \n \t\t/* Use internally allocated buffers. */\n-\t\tallocator_ = FrameBufferAllocator::create(camera_);\n+\t\tallocator_ = new FrameBufferAllocator(camera_);\n \t\tStream *stream = *camera_->streams().begin();\n \t\tif (allocator_->allocate(stream) < 0)\n \t\t\treturn TestFail;\n", "prefixes": [ "libcamera-devel", "9/9" ] }