Patch Detail
Show a patch.
GET /api/patches/359/?format=api
{ "id": 359, "url": "https://patchwork.libcamera.org/api/patches/359/?format=api", "web_url": "https://patchwork.libcamera.org/patch/359/", "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-5-laurent.pinchart@ideasonboard.com>", "date": "2019-01-24T10:16:45", "name": "[libcamera-devel,04/10] libcamera: device_enumerator: Reference-count MediaDevice instances", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "0694ea02ab5e0979a36b836324ae5fc54a861084", "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/359/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/359/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/359/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 6132C60C78\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 E5E6E2F6;\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=1548325018;\n\tbh=IclIqyBCrpuH2EWaUHZdqmltq/6gEkpA7l356d5T3sc=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=XmnUou7/3oMQpueZpA/PSEi2tCdl0UxGgnS+2JarqBes/eBdJF3nll3saGIcnO1qo\n\tviZskZrCANHsB120GgBvEJwSu9bD+kpOpPDYhzr9ddOv4XJXZgpWcqgFVqlD/NpBe/\n\t2aydOMFetYaNZ7hXilC0aurCAhbXIIIFZguMddcc=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 24 Jan 2019 12:16:45 +0200", "Message-Id": "<20190124101651.9993-5-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-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 04/10] libcamera: device_enumerator:\n\tReference-count MediaDevice instances", "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": "The MediaDevice class will be the entry point to hot-unplug, as it\ncorresponds to the kernel devices that will report device removal\nevents. The class will signal media device disconnection to pipeline\nhandlers, which will clean up resources as a result.\n\nThis can't be performed synchronously as references may exist to the\nrelated Camera objects in applications. The MediaDevice object thus\nneeds to be reference-counted in order to support unplugging, as\notherwise pipeline handlers would be required to drop all the references\nto the media device they have borrowed synchronously with the\ndisconnection signal handler, which would be very error prone (if even\npossible at all in a sane way).\n\nHandle MedieDevice instances with std::shared_ptr<> to support this.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/device_enumerator.cpp | 23 ++++++++++----------\n src/libcamera/include/device_enumerator.h | 4 ++--\n src/libcamera/pipeline/ipu3/ipu3.cpp | 11 ++++------\n src/libcamera/pipeline/uvcvideo.cpp | 4 ++--\n src/libcamera/pipeline/vimc.cpp | 4 ++--\n test/media_device/media_device_link_test.cpp | 2 +-\n test/pipeline/ipu3/ipu3_pipeline_test.cpp | 2 +-\n 7 files changed, 23 insertions(+), 27 deletions(-)", "diff": "diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\nindex 8100972a8d04..149ffbf9aea6 100644\n--- a/src/libcamera/device_enumerator.cpp\n+++ b/src/libcamera/device_enumerator.cpp\n@@ -166,12 +166,10 @@ std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create()\n \n DeviceEnumerator::~DeviceEnumerator()\n {\n-\tfor (MediaDevice *dev : devices_) {\n-\t\tif (dev->busy())\n+\tfor (std::shared_ptr<MediaDevice> media : devices_) {\n+\t\tif (media->busy())\n \t\t\tLOG(DeviceEnumerator, Error)\n \t\t\t\t<< \"Removing media device while still in use\";\n-\n-\t\tdelete dev;\n \t}\n }\n \n@@ -211,7 +209,7 @@ DeviceEnumerator::~DeviceEnumerator()\n */\n int DeviceEnumerator::addDevice(const std::string &deviceNode)\n {\n-\tMediaDevice *media = new MediaDevice(deviceNode);\n+\tstd::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);\n \n \tint ret = media->open();\n \tif (ret < 0)\n@@ -243,9 +241,10 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode)\n \t\t\treturn ret;\n \t}\n \n-\tdevices_.push_back(media);\n \tmedia->close();\n \n+\tdevices_.push_back(std::move(media));\n+\n \treturn 0;\n }\n \n@@ -260,17 +259,17 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode)\n *\n * \\return pointer to the matching MediaDevice, or nullptr if no match is found\n */\n-MediaDevice *DeviceEnumerator::search(const DeviceMatch &dm)\n+std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)\n {\n-\tfor (MediaDevice *dev : devices_) {\n-\t\tif (dev->busy())\n+\tfor (std::shared_ptr<MediaDevice> media : devices_) {\n+\t\tif (media->busy())\n \t\t\tcontinue;\n \n-\t\tif (dm.match(dev)) {\n+\t\tif (dm.match(media.get())) {\n \t\t\tLOG(DeviceEnumerator, Debug)\n \t\t\t\t<< \"Successful match for media device \\\"\"\n-\t\t\t\t<< dev->driver() << \"\\\"\";\n-\t\t\treturn dev;\n+\t\t\t\t<< media->driver() << \"\\\"\";\n+\t\t\treturn std::move(media);\n \t\t}\n \t}\n \ndiff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h\nindex 40c7750b49fc..3f87a6255303 100644\n--- a/src/libcamera/include/device_enumerator.h\n+++ b/src/libcamera/include/device_enumerator.h\n@@ -42,13 +42,13 @@ public:\n \tvirtual int init() = 0;\n \tvirtual int enumerate() = 0;\n \n-\tMediaDevice *search(const DeviceMatch &dm);\n+\tstd::shared_ptr<MediaDevice> search(const DeviceMatch &dm);\n \n protected:\n \tint addDevice(const std::string &deviceNode);\n \n private:\n-\tstd::vector<MediaDevice *> devices_;\n+\tstd::vector<std::shared_ptr<MediaDevice>> devices_;\n \n \tvirtual std::string lookupDeviceNode(int major, int minor) = 0;\n };\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 13ff7da4c99e..9831f74fe53f 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -29,8 +29,8 @@ public:\n \tbool match(DeviceEnumerator *enumerator);\n \n private:\n-\tMediaDevice *cio2_;\n-\tMediaDevice *imgu_;\n+\tstd::shared_ptr<MediaDevice> cio2_;\n+\tstd::shared_ptr<MediaDevice> imgu_;\n \n \tvoid registerCameras();\n };\n@@ -47,9 +47,6 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n \n \tif (imgu_)\n \t\timgu_->release();\n-\n-\tcio2_ = nullptr;\n-\timgu_ = nullptr;\n }\n \n bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n@@ -78,11 +75,11 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n \timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n \timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n \n-\tcio2_ = enumerator->search(cio2_dm);\n+\tcio2_ = std::move(enumerator->search(cio2_dm));\n \tif (!cio2_)\n \t\treturn false;\n \n-\timgu_ = enumerator->search(imgu_dm);\n+\timgu_ = std::move(enumerator->search(imgu_dm));\n \tif (!imgu_)\n \t\treturn false;\n \ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 3ebc074093ab..73bad6714bb4 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -23,7 +23,7 @@ public:\n \tbool match(DeviceEnumerator *enumerator);\n \n private:\n-\tMediaDevice *dev_;\n+\tstd::shared_ptr<MediaDevice> dev_;\n };\n \n PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n@@ -41,7 +41,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n {\n \tDeviceMatch dm(\"uvcvideo\");\n \n-\tdev_ = enumerator->search(dm);\n+\tdev_ = std::move(enumerator->search(dm));\n \n \tif (!dev_)\n \t\treturn false;\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex 68bfe9b12ab6..521b20d3a120 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -23,7 +23,7 @@ public:\n \tbool match(DeviceEnumerator *enumerator);\n \n private:\n-\tMediaDevice *dev_;\n+\tstd::shared_ptr<MediaDevice> dev_;\n };\n \n PipeHandlerVimc::PipeHandlerVimc(CameraManager *manager)\n@@ -51,7 +51,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n \tdm.add(\"RGB/YUV Input\");\n \tdm.add(\"Scaler\");\n \n-\tdev_ = enumerator->search(dm);\n+\tdev_ = std::move(enumerator->search(dm));\n \tif (!dev_)\n \t\treturn false;\n \ndiff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp\nindex ac5b632f8ed1..58a55cdfee63 100644\n--- a/test/media_device/media_device_link_test.cpp\n+++ b/test/media_device/media_device_link_test.cpp\n@@ -240,7 +240,7 @@ class MediaDeviceLinkTest : public Test\n \n private:\n \tunique_ptr<DeviceEnumerator> enumerator;\n-\tMediaDevice *dev_;\n+\tshared_ptr<MediaDevice> dev_;\n };\n \n TEST_REGISTER(MediaDeviceLinkTest);\ndiff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp\nindex 482c12499a86..953f0233cde4 100644\n--- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp\n+++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp\n@@ -65,7 +65,7 @@ int IPU3PipelineTest::init()\n \t\treturn TestSkip;\n \t}\n \n-\tMediaDevice *cio2 = enumerator->search(cio2_dm);\n+\tstd::shared_ptr<MediaDevice> cio2 = enumerator->search(cio2_dm);\n \tif (!cio2) {\n \t\tcerr << \"Failed to find IPU3 CIO2: test skip\" << endl;\n \t\treturn TestSkip;\n", "prefixes": [ "libcamera-devel", "04/10" ] }