From patchwork Fri May 17 00:54:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 1207 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B451960103 for ; Fri, 17 May 2019 02:55:09 +0200 (CEST) X-Halon-ID: 6a1cdf6d-783e-11e9-8d05-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 6a1cdf6d-783e-11e9-8d05-005056917f90; Fri, 17 May 2019 02:55:08 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Fri, 17 May 2019 02:54:38 +0200 Message-Id: <20190517005447.27171-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> References: <20190517005447.27171-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 02/11] libcamera: media_device: Open and close media device inside populate() 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: Fri, 17 May 2019 00:55:10 -0000 Remove the need for the caller to open and close the media device when populating the MediaDevice. This is done as an effort to make the usage of the MediaDevice less error prone and the interface stricter. The rework also revealed and fixes a potential memory leak in MediaDevice::populate() where resources would not be deleted if the second MEDIA_IOC_G_TOPOLOGY would fail. Signed-off-by: Niklas Söderlund Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/device_enumerator.cpp | 8 +------- src/libcamera/media_device.cpp | 12 ++++++++++-- test/media_device/media_device_print_test.cpp | 6 ------ test/pipeline/ipu3/ipu3_pipeline_test.cpp | 5 ----- test/v4l2_subdevice/v4l2_subdevice_test.cpp | 10 +--------- 5 files changed, 12 insertions(+), 29 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index 259eec41776e2ec4..6fcbae76b64e3774 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -207,11 +207,7 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) { std::shared_ptr media = std::make_shared(deviceNode); - int ret = media->open(); - if (ret < 0) - return ret; - - ret = media->populate(); + int ret = media->populate(); if (ret < 0) { LOG(DeviceEnumerator, Info) << "Unable to populate media device " << deviceNode @@ -238,8 +234,6 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) return ret; } - media->close(); - LOG(DeviceEnumerator, Debug) << "Added device " << deviceNode << ": " << media->driver(); diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 449571fb4b78fb94..c39775164fab7487 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -221,6 +221,10 @@ int MediaDevice::populate() clear(); + ret = open(); + if (ret) + return ret; + /* * Keep calling G_TOPOLOGY until the version number stays stable. */ @@ -237,7 +241,7 @@ int MediaDevice::populate() LOG(MediaDevice, Error) << "Failed to enumerate topology: " << strerror(-ret); - return ret; + goto done; } if (version == topology.topology_version) @@ -262,6 +266,10 @@ int MediaDevice::populate() populateLinks(topology)) valid_ = true; + ret = 0; +done: + close(); + delete[] ents; delete[] interfaces; delete[] pads; @@ -272,7 +280,7 @@ int MediaDevice::populate() return -EINVAL; } - return 0; + return ret; } /** diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp index 3eef973939201b27..ceffd538e13fca73 100644 --- a/test/media_device/media_device_print_test.cpp +++ b/test/media_device/media_device_print_test.cpp @@ -124,10 +124,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode) dev.close(); - ret = dev.open(); - if (ret) - return ret; - ret = dev.populate(); if (ret) return ret; @@ -135,8 +131,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode) /* Print out the media graph. */ printMediaGraph(dev, cerr); - dev.close(); - return 0; } diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp index 953f0233cde485cb..1d4cc4d4950b8391 100644 --- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp +++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp @@ -71,11 +71,6 @@ int IPU3PipelineTest::init() return TestSkip; } - if (cio2->open()) { - cerr << "Failed to open media device " << cio2->deviceNode() << endl; - return TestFail; - } - /* * Camera sensor are connected to the CIO2 unit. * Count how many sensors are connected in the system diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp index 5810ef3c962fab8e..562a638cb28ebfb2 100644 --- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp +++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp @@ -45,13 +45,6 @@ int V4L2SubdeviceTest::init() return TestSkip; } - int ret = media_->open(); - if (ret) { - cerr << "Unable to open media device: " << media_->deviceNode() - << ": " << strerror(ret) << endl; - return TestSkip; - } - MediaEntity *videoEntity = media_->getEntityByName("Scaler"); if (!videoEntity) { cerr << "Unable to find media entity 'Scaler'" << endl; @@ -59,8 +52,7 @@ int V4L2SubdeviceTest::init() } scaler_ = new V4L2Subdevice(videoEntity); - ret = scaler_->open(); - if (ret) { + if (scaler_->open()) { cerr << "Unable to open video subdevice " << scaler_->entity()->deviceNode() << endl; return TestSkip;