[libcamera-devel] [RFC 04/11] libcamera: media_device: Handle media device fd in acquire() and release()
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Apr 14 03:34:59 CEST 2019
To gain better control of when a file descriptor is open to the media
device and reduce the work needed in pipeline handler implementations,
handle the file descriptor in acquire() and release().
This changes the current behavior where a file descriptor is only open
when requested by the pipeline handler to that one is always open as
long a media device is acquired. This new behavior is desired to allow
implementing exclusive access to a pipeline handler between processes.
Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
---
src/libcamera/include/media_device.h | 2 +-
src/libcamera/media_device.cpp | 33 ++++++++++----------
src/libcamera/pipeline/ipu3/ipu3.cpp | 25 +--------------
test/media_device/media_device_link_test.cpp | 7 -----
test/v4l2_subdevice/v4l2_subdevice_test.cpp | 10 +-----
5 files changed, 19 insertions(+), 58 deletions(-)
diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
index 9f038093b2b22fc7..883985055eb419dd 100644
--- a/src/libcamera/include/media_device.h
+++ b/src/libcamera/include/media_device.h
@@ -27,7 +27,7 @@ public:
~MediaDevice();
bool acquire();
- void release() { acquired_ = false; }
+ void release();
bool busy() const { return acquired_; }
int open();
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
index 54706bb73a7591d5..644c6143fb15e1fa 100644
--- a/src/libcamera/media_device.cpp
+++ b/src/libcamera/media_device.cpp
@@ -39,10 +39,9 @@ LOG_DEFINE_CATEGORY(MediaDevice)
* instance.
*
* The instance is created with an empty media graph. Before performing any
- * other operation, it must be opened with the open() function and the media
- * graph populated by calling populate(). Instances of MediaEntity, MediaPad and
- * MediaLink are created to model the media graph, and stored in a map indexed
- * by object id.
+ * other operation, it must be populate by calling populate(). Instances of
+ * MediaEntity, MediaPad and MediaLink are created to model the media graph,
+ * and stored in a map indexed by object id.
*
* The graph is valid once successfully populated, as reported by the valid()
* function. It can be queried to list all entities(), or entities can be
@@ -50,12 +49,6 @@ LOG_DEFINE_CATEGORY(MediaDevice)
* entity to entity through pads and links as exposed by the corresponding
* classes.
*
- * An open media device will keep an open file handle for the underlying media
- * controller device node. It can be closed at any time with a call to close().
- * This will not invalidate the media graph and all cached media objects remain
- * valid and can be accessed normally. The device can then be later reopened if
- * needed to perform other operations that interact with the device node.
- *
* Media device can be claimed for exclusive use with acquire(), released with
* release() and tested with busy(). This mechanism is aimed at pipeline
* managers to claim media devices they support during enumeration.
@@ -65,8 +58,8 @@ LOG_DEFINE_CATEGORY(MediaDevice)
* \brief Construct a MediaDevice
* \param deviceNode The media device node path
*
- * Once constructed the media device is invalid, and must be opened and
- * populated with open() and populate() before the media graph can be queried.
+ * Once constructed the media device is invalid, and must be populated with
+ * populate() before the media graph can be queried.
*/
MediaDevice::MediaDevice(const std::string &deviceNode)
: deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false)
@@ -106,15 +99,22 @@ bool MediaDevice::acquire()
if (acquired_)
return false;
+ if (open())
+ return false;
+
acquired_ = true;
return true;
}
/**
- * \fn MediaDevice::release()
* \brief Release a device previously claimed for exclusive use
* \sa acquire(), busy()
*/
+void MediaDevice::release()
+{
+ close();
+ acquired_ = false;
+}
/**
* \fn MediaDevice::busy()
@@ -127,10 +127,6 @@ bool MediaDevice::acquire()
/**
* \brief Open a media device and retrieve device information
*
- * Before populating the media graph or performing any operation that interact
- * with the device node associated with the media device, the device node must
- * be opened.
- *
* If the device is already open the function returns -EBUSY.
*
* \return 0 on success or a negative error code otherwise
@@ -201,6 +197,9 @@ int MediaDevice::populate()
__u64 version = -1;
int ret;
+ if (fd_ != -1)
+ return -EBUSY;
+
clear();
ret = open();
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index d8de6b3c36314fc0..a87eff8dfec6d143 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -468,23 +468,10 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
/*
* Disable all links that are enabled by default on CIO2, as camera
* creation enables all valid links it finds.
- *
- * Close the CIO2 media device after, as links are enabled and should
- * not need to be changed after.
*/
- if (cio2MediaDev_->open())
+ if (cio2MediaDev_->disableLinks())
return false;
- if (cio2MediaDev_->disableLinks()) {
- cio2MediaDev_->close();
- return false;
- }
-
- if (imguMediaDev_->open()) {
- cio2MediaDev_->close();
- return false;
- }
-
/*
* FIXME: enabled links in one ImgU instance interfere with capture
* operations on the other one. This can be easily triggered by
@@ -516,9 +503,6 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
ret = registerCameras();
error:
- cio2MediaDev_->close();
- imguMediaDev_->close();
-
return ret == 0;
}
@@ -961,11 +945,6 @@ int ImgUDevice::enableLinks(bool enable)
std::string inputName = name_ + " input";
int ret;
- /* \todo Establish rules to handle media devices open/close. */
- ret = media_->open();
- if (ret)
- return ret;
-
ret = linkSetup(inputName, 0, name_, PAD_INPUT, enable);
if (ret)
goto done;
@@ -981,8 +960,6 @@ int ImgUDevice::enableLinks(bool enable)
ret = linkSetup(name_, PAD_STAT, statName, 0, enable);
done:
- media_->close();
-
return ret;
}
diff --git a/test/media_device/media_device_link_test.cpp b/test/media_device/media_device_link_test.cpp
index 4b8de3bee722e912..3989da43ca9ec30f 100644
--- a/test/media_device/media_device_link_test.cpp
+++ b/test/media_device/media_device_link_test.cpp
@@ -57,12 +57,6 @@ class MediaDeviceLinkTest : public Test
return TestSkip;
}
- if (dev_->open()) {
- cerr << "Failed to open media device at "
- << dev_->deviceNode() << endl;
- return TestFail;
- }
-
return 0;
}
@@ -238,7 +232,6 @@ class MediaDeviceLinkTest : public Test
void cleanup()
{
- dev_->close();
dev_->release();
}
diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.cpp b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
index 21335efbad4d5668..31f3465a7ba3a5b1 100644
--- a/test/v4l2_subdevice/v4l2_subdevice_test.cpp
+++ b/test/v4l2_subdevice/v4l2_subdevice_test.cpp
@@ -51,14 +51,6 @@ int V4L2SubdeviceTest::init()
return TestSkip;
}
- int ret = media_->open();
- if (ret) {
- cerr << "Unable to open media device: " << media_->deviceNode()
- << ": " << strerror(ret) << endl;
- media_->release();
- return TestSkip;
- }
-
MediaEntity *videoEntity = media_->getEntityByName("Scaler");
if (!videoEntity) {
cerr << "Unable to find media entity 'Scaler'" << endl;
@@ -67,7 +59,7 @@ int V4L2SubdeviceTest::init()
}
scaler_ = new V4L2Subdevice(videoEntity);
- ret = scaler_->open();
+ int ret = scaler_->open();
if (ret) {
cerr << "Unable to open video subdevice "
<< scaler_->deviceNode() << endl;
--
2.21.0
More information about the libcamera-devel
mailing list