Patch Detail
Show a patch.
GET /api/patches/358/?format=api
{ "id": 358, "url": "https://patchwork.libcamera.org/api/patches/358/?format=api", "web_url": "https://patchwork.libcamera.org/patch/358/", "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": "<20190124101651.9993-4-laurent.pinchart@ideasonboard.com>", "date": "2019-01-24T10:16:44", "name": "[libcamera-devel,03/10] libcamera: camera: Associate cameras with their pipeline handler", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "17875290e7d3c38c6c5608c36df9875666334621", "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/358/mbox/", "series": [ { "id": 125, "url": "https://patchwork.libcamera.org/api/series/125/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=125", "date": "2019-01-24T10:16:41", "name": "Hotplug support and object lifetime management", "version": 1, "mbox": "https://patchwork.libcamera.org/series/125/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/358/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/358/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 035E760C78\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Jan 2019 11:16:58 +0100 (CET)", "from pendragon.bb.dnainternet.fi\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D94623A;\n\tThu, 24 Jan 2019 11:16:57 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548325017;\n\tbh=6gV9aidWQeYC32QKe+bYH03vMjAHzkuR/hfxsbBAwoA=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=g0blyj8aHBAmVs5BwTA8+IeCpDndUF0OztU7t0HFRO4+cRfEXJt75tmy4JjkrzfoU\n\t6bQbBlbwV4WTPGqH3AMvvpfNEEGyvIlkww+r0vESWTdE867IIUbcEcbhtDebdfS2Is\n\tIC3O4BBt2dHsZjgYX/Z65mT4aniZL5mTTjSzBNAc=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 24 Jan 2019 12:16:44 +0200", "Message-Id": "<20190124101651.9993-4-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.19.2", "In-Reply-To": "<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>", "References": "<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 03/10] libcamera: camera: Associate\n\tcameras with their pipeline handler", "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": "Thu, 24 Jan 2019 10:16:58 -0000" }, "content": "From: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nThe PipelineHandler which creates a Camera is responsible for serving\nany operation requested by the user. In order forward the public API\ncalls, the camera needs to store a reference to its pipeline handler.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nChanges since v1:\n\n- Create pipeline handlers is shared pointers, make them inherit from\n std::enable_shared_from_this<> and stored them in shared pointers.\n---\n include/libcamera/camera.h | 8 ++++++--\n include/libcamera/camera_manager.h | 2 +-\n src/libcamera/camera.cpp | 16 ++++++++++------\n src/libcamera/camera_manager.cpp | 17 +++++++++--------\n src/libcamera/include/pipeline_handler.h | 9 +++++----\n src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n src/libcamera/pipeline/uvcvideo.cpp | 2 +-\n src/libcamera/pipeline/vimc.cpp | 9 +--------\n src/libcamera/pipeline_handler.cpp | 10 ++++++++++\n 9 files changed, 44 insertions(+), 31 deletions(-)", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex 2ea1a6883311..efafb9e28c56 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -12,10 +12,13 @@\n \n namespace libcamera {\n \n+class PipelineHandler;\n+\n class Camera final\n {\n public:\n-\tstatic std::shared_ptr<Camera> create(const std::string &name);\n+\tstatic std::shared_ptr<Camera> create(PipelineHandler *pipe,\n+\t\t\t\t\t const std::string &name);\n \n \tCamera(const Camera &) = delete;\n \tvoid operator=(const Camera &) = delete;\n@@ -23,9 +26,10 @@ public:\n \tconst std::string &name() const;\n \n private:\n-\texplicit Camera(const std::string &name);\n+\tCamera(PipelineHandler *pipe, const std::string &name);\n \t~Camera();\n \n+\tstd::shared_ptr<PipelineHandler> pipe_;\n \tstd::string name_;\n };\n \ndiff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\nindex 9ade29d76692..45e72df0ef65 100644\n--- a/include/libcamera/camera_manager.h\n+++ b/include/libcamera/camera_manager.h\n@@ -41,7 +41,7 @@ private:\n \t~CameraManager();\n \n \tstd::unique_ptr<DeviceEnumerator> enumerator_;\n-\tstd::vector<PipelineHandler *> pipes_;\n+\tstd::vector<std::shared_ptr<PipelineHandler>> pipes_;\n \tstd::vector<std::shared_ptr<Camera>> cameras_;\n \n \tstd::unique_ptr<EventDispatcher> dispatcher_;\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex acf912bee95c..3a531c7e4d8f 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -8,6 +8,7 @@\n #include <libcamera/camera.h>\n \n #include \"log.h\"\n+#include \"pipeline_handler.h\"\n \n /**\n * \\file camera.h\n@@ -52,17 +53,20 @@ namespace libcamera {\n /**\n * \\brief Create a camera instance\n * \\param[in] name The name of the camera device\n+ * \\param[in] pipe The pipeline handler responsible for the camera device\n *\n * The caller is responsible for guaranteeing unicity of the camera name.\n *\n * \\return A shared pointer to the newly created camera object\n */\n-std::shared_ptr<Camera> Camera::create(const std::string &name)\n+std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,\n+\t\t\t\t const std::string &name)\n {\n \tstruct Allocator : std::allocator<Camera> {\n-\t\tvoid construct(void *p, const std::string &name)\n+\t\tvoid construct(void *p, PipelineHandler *pipe,\n+\t\t\t const std::string &name)\n \t\t{\n-\t\t\t::new(p) Camera(name);\n+\t\t\t::new(p) Camera(pipe, name);\n \t\t}\n \t\tvoid destroy(Camera *p)\n \t\t{\n@@ -70,7 +74,7 @@ std::shared_ptr<Camera> Camera::create(const std::string &name)\n \t\t}\n \t};\n \n-\treturn std::allocate_shared<Camera>(Allocator(), name);\n+\treturn std::allocate_shared<Camera>(Allocator(), pipe, name);\n }\n \n /**\n@@ -83,8 +87,8 @@ const std::string &Camera::name() const\n \treturn name_;\n }\n \n-Camera::Camera(const std::string &name)\n-\t: name_(name)\n+Camera::Camera(PipelineHandler *pipe, const std::string &name)\n+\t: pipe_(pipe->shared_from_this()), name_(name)\n {\n }\n \ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 14410d4dcda7..3eccf20c4ce9 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -98,16 +98,14 @@ int CameraManager::start()\n \t\t * all pipelines it can provide.\n \t\t */\n \t\twhile (1) {\n-\t\t\tPipelineHandler *pipe = factory->create(this);\n-\t\t\tif (!pipe->match(enumerator_.get())) {\n-\t\t\t\tdelete pipe;\n+\t\t\tstd::shared_ptr<PipelineHandler> pipe = factory->create(this);\n+\t\t\tif (!pipe->match(enumerator_.get()))\n \t\t\t\tbreak;\n-\t\t\t}\n \n \t\t\tLOG(Camera, Debug)\n \t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n \t\t\t\t<< \"\\\" matched\";\n-\t\t\tpipes_.push_back(pipe);\n+\t\t\tpipes_.push_back(std::move(pipe));\n \t\t}\n \t}\n \n@@ -130,10 +128,13 @@ void CameraManager::stop()\n {\n \t/* TODO: unregister hot-plug callback here */\n \n-\tfor (PipelineHandler *pipe : pipes_)\n-\t\tdelete pipe;\n-\n+\t/*\n+\t * Release all references to cameras and pipeline handlers to ensure\n+\t * they all get destroyed before the device enumerator deletes the\n+\t * media devices.\n+\t */\n \tpipes_.clear();\n+\tcameras_.clear();\n \n \tenumerator_.reset(nullptr);\n }\ndiff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\nindex 1da6dc758ca6..e1d6369eb0c4 100644\n--- a/src/libcamera/include/pipeline_handler.h\n+++ b/src/libcamera/include/pipeline_handler.h\n@@ -8,6 +8,7 @@\n #define __LIBCAMERA_PIPELINE_HANDLER_H__\n \n #include <map>\n+#include <memory>\n #include <string>\n #include <vector>\n \n@@ -16,7 +17,7 @@ namespace libcamera {\n class CameraManager;\n class DeviceEnumerator;\n \n-class PipelineHandler\n+class PipelineHandler : public std::enable_shared_from_this<PipelineHandler>\n {\n public:\n \tPipelineHandler(CameraManager *manager);\n@@ -34,7 +35,7 @@ public:\n \tPipelineHandlerFactory(const char *name);\n \tvirtual ~PipelineHandlerFactory() { };\n \n-\tvirtual PipelineHandler *create(CameraManager *manager) = 0;\n+\tvirtual std::shared_ptr<PipelineHandler> create(CameraManager *manager) = 0;\n \n \tconst std::string &name() const { return name_; }\n \n@@ -50,9 +51,9 @@ class handler##Factory final : public PipelineHandlerFactory\t\t\\\n {\t\t\t\t\t\t\t\t\t\\\n public:\t\t\t\t\t\t\t\t\t\\\n \thandler##Factory() : PipelineHandlerFactory(#handler) {}\t\\\n-\tPipelineHandler *create(CameraManager *manager) \t\t\\\n+\tstd::shared_ptr<PipelineHandler> create(CameraManager *manager)\t\\\n \t{\t\t\t\t\t\t\t\t\\\n-\t\treturn new handler(manager);\t\t\t\t\\\n+\t\treturn std::make_shared<handler>(manager);\t\t\\\n \t}\t\t\t\t\t\t\t\t\\\n };\t\t\t\t\t\t\t\t\t\\\n static handler##Factory global_##handler##Factory;\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 589b3078f301..13ff7da4c99e 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -171,7 +171,7 @@ void PipelineHandlerIPU3::registerCameras()\n \t\t\tcontinue;\n \n \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n-\t\tstd::shared_ptr<Camera> camera = Camera::create(cameraName);\n+\t\tstd::shared_ptr<Camera> camera = Camera::create(this, cameraName);\n \t\tmanager_->addCamera(std::move(camera));\n \n \t\tLOG(IPU3, Info)\ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 92b23da73901..3ebc074093ab 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n \n \tdev_->acquire();\n \n-\tstd::shared_ptr<Camera> camera = Camera::create(dev_->model());\n+\tstd::shared_ptr<Camera> camera = Camera::create(this, dev_->model());\n \tmanager_->addCamera(std::move(camera));\n \n \treturn true;\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex f12d007cd956..68bfe9b12ab6 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -57,14 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n \n \tdev_->acquire();\n \n-\t/*\n-\t * NOTE: A more complete Camera implementation could\n-\t * be passed the MediaDevice(s) it controls here or\n-\t * a reference to the PipelineHandler. Which method\n-\t * will be chosen depends on how the Camera\n-\t * object is modeled.\n-\t */\n-\tstd::shared_ptr<Camera> camera = Camera::create(\"Dummy VIMC Camera\");\n+\tstd::shared_ptr<Camera> camera = Camera::create(this, \"Dummy VIMC Camera\");\n \tmanager_->addCamera(std::move(camera));\n \n \treturn true;\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 45788487b5c0..3850ea8fadb5 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -32,11 +32,21 @@ LOG_DEFINE_CATEGORY(Pipeline)\n *\n * The PipelineHandler matches the media devices provided by a DeviceEnumerator\n * with the pipelines it supports and creates corresponding Camera devices.\n+ *\n+ * Pipeline handler instances are reference-counted through std::shared_ptr<>.\n+ * They implement std::enable_shared_from_this<> in order to create new\n+ * std::shared_ptr<> in code paths originating from member functions of the\n+ * PipelineHandler class where only the 'this' pointer is available.\n */\n \n /**\n * \\brief Construct a PipelineHandler instance\n * \\param[in] manager The camera manager\n+ *\n+ * In order to honour the std::enable_shared_from_this<> contract,\n+ * PipelineHandler instances shall never be constructed manually, but always\n+ * through the PipelineHandlerFactory::create() method implemented by the\n+ * respective factories.\n */\n PipelineHandler::PipelineHandler(CameraManager *manager)\n \t: manager_(manager)\n", "prefixes": [ "libcamera-devel", "03/10" ] }