summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNiklas Söderlund <niklas.soderlund@ragnatech.se>2019-04-13 23:58:31 +0200
committerNiklas Söderlund <niklas.soderlund@ragnatech.se>2019-05-17 01:33:53 +0200
commit5868d73e77916812d60cb60ee3cade785f6492f8 (patch)
tree304a342be36bae9de07eb1506b6149a20e4b8e27
parent12053cf8e6a98104b6c765e1ac8a34b8f7a02eed (diff)
libcamera: media_device: Open and close media device inside populate()
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 <niklas.soderlund@ragnatech.se> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
-rw-r--r--src/libcamera/device_enumerator.cpp8
-rw-r--r--src/libcamera/media_device.cpp12
-rw-r--r--test/media_device/media_device_print_test.cpp6
-rw-r--r--test/pipeline/ipu3/ipu3_pipeline_test.cpp5
-rw-r--r--test/v4l2_subdevice/v4l2_subdevice_test.cpp10
5 files changed, 12 insertions, 29 deletions
diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
index 259eec41..6fcbae76 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<MediaDevice> media = std::make_shared<MediaDevice>(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 449571fb..c3977516 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 3eef9739..ceffd538 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 953f0233..1d4cc4d4 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 5810ef3c..562a638c 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;