From patchwork Thu Jan 24 10:16:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 359 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6132C60C78 for ; Thu, 24 Jan 2019 11:16:58 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E5E6E2F6; Thu, 24 Jan 2019 11:16:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1548325018; bh=IclIqyBCrpuH2EWaUHZdqmltq/6gEkpA7l356d5T3sc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XmnUou7/3oMQpueZpA/PSEi2tCdl0UxGgnS+2JarqBes/eBdJF3nll3saGIcnO1qo viZskZrCANHsB120GgBvEJwSu9bD+kpOpPDYhzr9ddOv4XJXZgpWcqgFVqlD/NpBe/ 2aydOMFetYaNZ7hXilC0aurCAhbXIIIFZguMddcc= From: Laurent Pinchart 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 Subject: [libcamera-devel] [PATCH 04/10] libcamera: device_enumerator: Reference-count MediaDevice instances X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Jan 2019 10:16:58 -0000 The MediaDevice class will be the entry point to hot-unplug, as it corresponds to the kernel devices that will report device removal events. The class will signal media device disconnection to pipeline handlers, which will clean up resources as a result. This can't be performed synchronously as references may exist to the related Camera objects in applications. The MediaDevice object thus needs to be reference-counted in order to support unplugging, as otherwise pipeline handlers would be required to drop all the references to the media device they have borrowed synchronously with the disconnection signal handler, which would be very error prone (if even possible at all in a sane way). Handle MedieDevice instances with std::shared_ptr<> to support this. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/device_enumerator.cpp | 23 ++++++++++---------- src/libcamera/include/device_enumerator.h | 4 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 11 ++++------ src/libcamera/pipeline/uvcvideo.cpp | 4 ++-- src/libcamera/pipeline/vimc.cpp | 4 ++-- test/media_device/media_device_link_test.cpp | 2 +- test/pipeline/ipu3/ipu3_pipeline_test.cpp | 2 +- 7 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index 8100972a8d04..149ffbf9aea6 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -166,12 +166,10 @@ std::unique_ptr DeviceEnumerator::create() DeviceEnumerator::~DeviceEnumerator() { - for (MediaDevice *dev : devices_) { - if (dev->busy()) + for (std::shared_ptr media : devices_) { + if (media->busy()) LOG(DeviceEnumerator, Error) << "Removing media device while still in use"; - - delete dev; } } @@ -211,7 +209,7 @@ DeviceEnumerator::~DeviceEnumerator() */ int DeviceEnumerator::addDevice(const std::string &deviceNode) { - MediaDevice *media = new MediaDevice(deviceNode); + std::shared_ptr media = std::make_shared(deviceNode); int ret = media->open(); if (ret < 0) @@ -243,9 +241,10 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) return ret; } - devices_.push_back(media); media->close(); + devices_.push_back(std::move(media)); + return 0; } @@ -260,17 +259,17 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) * * \return pointer to the matching MediaDevice, or nullptr if no match is found */ -MediaDevice *DeviceEnumerator::search(const DeviceMatch &dm) +std::shared_ptr DeviceEnumerator::search(const DeviceMatch &dm) { - for (MediaDevice *dev : devices_) { - if (dev->busy()) + for (std::shared_ptr media : devices_) { + if (media->busy()) continue; - if (dm.match(dev)) { + if (dm.match(media.get())) { LOG(DeviceEnumerator, Debug) << "Successful match for media device \"" - << dev->driver() << "\""; - return dev; + << media->driver() << "\""; + return std::move(media); } } diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h index 40c7750b49fc..3f87a6255303 100644 --- a/src/libcamera/include/device_enumerator.h +++ b/src/libcamera/include/device_enumerator.h @@ -42,13 +42,13 @@ public: virtual int init() = 0; virtual int enumerate() = 0; - MediaDevice *search(const DeviceMatch &dm); + std::shared_ptr search(const DeviceMatch &dm); protected: int addDevice(const std::string &deviceNode); private: - std::vector devices_; + std::vector> devices_; virtual std::string lookupDeviceNode(int major, int minor) = 0; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 13ff7da4c99e..9831f74fe53f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -29,8 +29,8 @@ public: bool match(DeviceEnumerator *enumerator); private: - MediaDevice *cio2_; - MediaDevice *imgu_; + std::shared_ptr cio2_; + std::shared_ptr imgu_; void registerCameras(); }; @@ -47,9 +47,6 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3() if (imgu_) imgu_->release(); - - cio2_ = nullptr; - imgu_ = nullptr; } bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) @@ -78,11 +75,11 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) imgu_dm.add("ipu3-imgu 1 viewfinder"); imgu_dm.add("ipu3-imgu 1 3a stat"); - cio2_ = enumerator->search(cio2_dm); + cio2_ = std::move(enumerator->search(cio2_dm)); if (!cio2_) return false; - imgu_ = enumerator->search(imgu_dm); + imgu_ = std::move(enumerator->search(imgu_dm)); if (!imgu_) return false; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 3ebc074093ab..73bad6714bb4 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -23,7 +23,7 @@ public: bool match(DeviceEnumerator *enumerator); private: - MediaDevice *dev_; + std::shared_ptr dev_; }; PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) @@ -41,7 +41,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) { DeviceMatch dm("uvcvideo"); - dev_ = enumerator->search(dm); + dev_ = std::move(enumerator->search(dm)); if (!dev_) return false; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 68bfe9b12ab6..521b20d3a120 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -23,7 +23,7 @@ public: bool match(DeviceEnumerator *enumerator); private: - MediaDevice *dev_; + std::shared_ptr dev_; }; PipeHandlerVimc::PipeHandlerVimc(CameraManager *manager) @@ -51,7 +51,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator) dm.add("RGB/YUV Input"); dm.add("Scaler"); - dev_ = enumerator->search(dm); + dev_ = std::move(enumerator->search(dm)); if (!dev_) return false; diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp index ac5b632f8ed1..58a55cdfee63 100644 --- a/test/media_device/media_device_link_test.cpp +++ b/test/media_device/media_device_link_test.cpp @@ -240,7 +240,7 @@ class MediaDeviceLinkTest : public Test private: unique_ptr enumerator; - MediaDevice *dev_; + shared_ptr dev_; }; TEST_REGISTER(MediaDeviceLinkTest); diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp index 482c12499a86..953f0233cde4 100644 --- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp +++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp @@ -65,7 +65,7 @@ int IPU3PipelineTest::init() return TestSkip; } - MediaDevice *cio2 = enumerator->search(cio2_dm); + std::shared_ptr cio2 = enumerator->search(cio2_dm); if (!cio2) { cerr << "Failed to find IPU3 CIO2: test skip" << endl; return TestSkip;