From 6e620349009de675250a7e3e753480b85a6df10b Mon Sep 17 00:00:00 2001 From: Paul Elder Date: Sun, 1 Sep 2019 23:53:02 -0400 Subject: libcamera: device_enumerator: fix udev media graph loading dependency When a MediaDevice is enumerated and populated by the DeviceEnumeratorUdev, there is a possibility that the member device nodes of the media graph would not be ready (either not created, or without proper permissions set by udev yet). The MediaDevice is still passed up to the pipeline handler, where an attempt to access the device nodes will fail in EPERM. This whole issue is especially likely to happen when libcamera is run at system init time. To fix this, we first split DeviceEnumerator::addDevice() into three methods: - createDevice() to simply create the MediaDevice - populateMediaDevice() to populate the MediaDevice - addDevice() to pass the MediaDevice up to the pipeline handler DeviceEnumeratorSysfs calls these methods in succession, similar to what it did before when they were all together as addDevice(). DeviceEnumeratorUdev additionally keeps a map of MediaDevices to a list of pending device nodes (plus some other auxillary maps), and a simple list of orphan device nodes. If a v4l device node is ready and there does not exist any MediaDevice node for it, then it goes to the orphan list, otherwise it is initialized and removed from the pending list of the corresponding MediaDevice in the dependency map. When a MediaDevice is populated via DeviceEnumeratorUdev::populateMediaDevice(), it first checks the orphan list to see if the device nodes it needs are there, otherwise it tries to initialize the device nodes and if it fails, then it adds the device nodes it wants to its list in the dependency map. This allows MediaDevice instances to be created and initialized properly with udev when v4l device nodes in the media graph may not be ready when the MediaDevice is populated. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart --- src/libcamera/device_enumerator.cpp | 54 +++++----- src/libcamera/device_enumerator_sysfs.cpp | 35 +++++- src/libcamera/device_enumerator_udev.cpp | 135 +++++++++++++++++++++++- src/libcamera/include/device_enumerator.h | 3 +- src/libcamera/include/device_enumerator_sysfs.h | 6 +- src/libcamera/include/device_enumerator_udev.h | 16 +++ 6 files changed, 216 insertions(+), 33 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index 60c918f0..e76438af 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -195,16 +195,21 @@ DeviceEnumerator::~DeviceEnumerator() */ /** - * \brief Add a media device to the enumerator - * \param[in] deviceNode path to the media device to add + * \brief Create a media device instance + * \param[in] deviceNode path to the media device to create * - * Create a media device for the \a deviceNode, open it, populate its media graph, - * and look up device nodes associated with all entities. Store the media device - * in the internal list for later matching with pipeline handlers. + * Create a media device for the \a deviceNode, open it, and populate its + * media graph. The device enumerator shall then populate the media device by + * associating device nodes with entities using MediaEntity::setDeviceNode(). + * This process is specific to each device enumerator, and the device enumerator + * shall ensure that device nodes are ready to be used (for instance, if + * applicable, by waiting for device nodes to be created and access permissions + * to be set by the system). Once done, it shall add the media device to the + * system with addDevice(). * - * \return 0 on success or a negative error code otherwise + * \return Created media device instance on success, or nullptr otherwise */ -int DeviceEnumerator::addDevice(const std::string &deviceNode) +std::shared_ptr DeviceEnumerator::createDevice(const std::string &deviceNode) { std::shared_ptr media = std::make_shared(deviceNode); @@ -213,34 +218,31 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode) LOG(DeviceEnumerator, Info) << "Unable to populate media device " << deviceNode << " (" << strerror(-ret) << "), skipping"; - return ret; + return nullptr; } LOG(DeviceEnumerator, Debug) << "New media device \"" << media->driver() << "\" created from " << deviceNode; - /* Associate entities to device node paths. */ - for (MediaEntity *entity : media->entities()) { - if (entity->deviceMajor() == 0 && entity->deviceMinor() == 0) - continue; - - std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), - entity->deviceMinor()); - if (deviceNode.empty()) - return -EINVAL; - - ret = entity->setDeviceNode(deviceNode); - if (ret) - return ret; - } + return media; +} +/** + * \brief Add a media device to the enumerator + * \param[in] media media device instance to add + * + * Store the media device in the internal list for later matching with + * pipeline handlers. \a media shall be created with createDevice() first. + * This method shall be called after all members of the entities of the + * media graph have been confirmed to be initialized. + */ +void DeviceEnumerator::addDevice(const std::shared_ptr &media) +{ LOG(DeviceEnumerator, Debug) - << "Added device " << deviceNode << ": " << media->driver(); - - devices_.push_back(std::move(media)); + << "Added device " << media->deviceNode() << ": " << media->driver(); - return 0; + devices_.push_back(media); } /** diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp index f3054d5e..78a7da8d 100644 --- a/src/libcamera/device_enumerator_sysfs.cpp +++ b/src/libcamera/device_enumerator_sysfs.cpp @@ -18,6 +18,7 @@ #include #include "log.h" +#include "media_device.h" namespace libcamera { @@ -32,6 +33,7 @@ int DeviceEnumeratorSysfs::enumerate() { struct dirent *ent; DIR *dir; + int ret = 0; static const char * const sysfs_dirs[] = { "/sys/subsystem/media/devices", @@ -71,11 +73,42 @@ int DeviceEnumeratorSysfs::enumerate() continue; } - addDevice(devnode); + std::shared_ptr media = createDevice(devnode); + if (!media) { + ret = -ENODEV; + break; + } + + if (populateMediaDevice(media) < 0) { + ret = -ENODEV; + break; + } + + addDevice(media); } closedir(dir); + return ret; +} + +int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr &media) +{ + /* Associate entities to device node paths. */ + for (MediaEntity *entity : media->entities()) { + if (entity->deviceMajor() == 0 && entity->deviceMinor() == 0) + continue; + + std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), + entity->deviceMinor()); + if (deviceNode.empty()) + return -EINVAL; + + int ret = entity->setDeviceNode(deviceNode); + if (ret) + return ret; + } + return 0; } diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index 86f6ca18..40853e77 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -7,6 +7,10 @@ #include "device_enumerator_udev.h" +#include +#include +#include + #include #include #include @@ -17,6 +21,7 @@ #include #include "log.h" +#include "media_device.h" namespace libcamera { @@ -57,9 +62,40 @@ int DeviceEnumeratorUdev::init() if (ret < 0) return ret; + ret = udev_monitor_filter_add_match_subsystem_devtype(monitor_, "video4linux", + nullptr); + if (ret < 0) + return ret; + return 0; } +int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev) +{ + const char *subsystem = udev_device_get_subsystem(dev); + if (!subsystem) + return -ENODEV; + + if (!strcmp(subsystem, "media")) { + std::shared_ptr media = + createDevice(udev_device_get_devnode(dev)); + if (!media) + return -ENODEV; + + int ret = populateMediaDevice(media); + if (ret == 0) + addDevice(media); + return 0; + } + + if (!strcmp(subsystem, "video4linux")) { + addV4L2Device(udev_device_get_devnum(dev)); + return 0; + } + + return -ENODEV; +} + int DeviceEnumeratorUdev::enumerate() { struct udev_enumerate *udev_enum = nullptr; @@ -74,6 +110,14 @@ int DeviceEnumeratorUdev::enumerate() if (ret < 0) goto done; + ret = udev_enumerate_add_match_subsystem(udev_enum, "video4linux"); + if (ret < 0) + goto done; + + ret = udev_enumerate_add_match_is_initialized(udev_enum); + if (ret < 0) + goto done; + ret = udev_enumerate_scan_devices(udev_enum); if (ret < 0) goto done; @@ -102,10 +146,12 @@ int DeviceEnumeratorUdev::enumerate() goto done; } - addDevice(devnode); - + ret = addUdevDevice(dev); udev_device_unref(dev); + if (ret < 0) + break; } + done: udev_enumerate_unref(udev_enum); if (ret < 0) @@ -122,6 +168,43 @@ done: return 0; } +int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr &media) +{ + unsigned int pendingNodes = 0; + int ret; + + /* Associate entities to device node paths. */ + for (MediaEntity *entity : media->entities()) { + if (entity->deviceMajor() == 0 && entity->deviceMinor() == 0) + continue; + + std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), + entity->deviceMinor()); + dev_t devnum = makedev(entity->deviceMajor(), + entity->deviceMinor()); + + /* Take device from orphan list first, if it is in the list. */ + if (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) { + if (deviceNode.empty()) + return -EINVAL; + + ret = entity->setDeviceNode(deviceNode); + if (ret) + return ret; + + orphans_.remove(devnum); + continue; + } + + deps_[media].push_back(devnum); + devnumToDevice_[devnum] = media; + devnumToEntity_[devnum] = entity; + pendingNodes++; + } + + return pendingNodes; +} + std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor) { struct udev_device *device; @@ -143,6 +226,48 @@ std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor) return deviceNode; } +/** + * \brief Add a V4L2 device to the media device that it belongs to + * \param[in] devnum major:minor number of V4L2 device to add, as a dev_t + * + * Add V4L2 device identified by \a devnum to the MediaDevice that it belongs + * to, if such a MediaDevice has been created. Otherwise add the V4L2 device + * to the orphan list. If the V4L2 device is added to a MediaDevice, and it is + * the last V4L2 device that the MediaDevice needs, then the MediaDevice is + * added to the DeviceEnumerator, where it is available for pipeline handlers. + * + * \return 0 on success or a negative error code otherwise + */ +int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum) +{ + MediaEntity *entity = devnumToEntity_[devnum]; + if (!entity) { + orphans_.push_back(devnum); + return 0; + } + + std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), + entity->deviceMinor()); + if (deviceNode.empty()) + return -EINVAL; + + int ret = entity->setDeviceNode(deviceNode); + if (ret) + return ret; + + std::shared_ptr media = devnumToDevice_[devnum]; + deps_[media].remove(devnum); + devnumToDevice_.erase(devnum); + devnumToEntity_.erase(devnum); + + if (deps_[media].empty()) { + addDevice(media); + deps_.erase(media); + } + + return 0; +} + void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier) { struct udev_device *dev = udev_monitor_receive_device(monitor_); @@ -153,9 +278,11 @@ void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier) << action << " device " << udev_device_get_devnode(dev); if (action == "add") { - addDevice(deviceNode); + addUdevDevice(dev); } else if (action == "remove") { - removeDevice(deviceNode); + const char *subsystem = udev_device_get_subsystem(dev); + if (subsystem && !strcmp(subsystem, "media")) + removeDevice(deviceNode); } udev_device_unref(dev); diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h index 02aec3bc..c5d26f1a 100644 --- a/src/libcamera/include/device_enumerator.h +++ b/src/libcamera/include/device_enumerator.h @@ -44,7 +44,8 @@ public: std::shared_ptr search(const DeviceMatch &dm); protected: - int addDevice(const std::string &deviceNode); + std::shared_ptr createDevice(const std::string &deviceNode); + void addDevice(const std::shared_ptr &media); void removeDevice(const std::string &deviceNode); private: diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h index 8d3adc91..242b22b2 100644 --- a/src/libcamera/include/device_enumerator_sysfs.h +++ b/src/libcamera/include/device_enumerator_sysfs.h @@ -7,10 +7,13 @@ #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__ #define __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__ +#include #include #include "device_enumerator.h" +class MediaDevice; + namespace libcamera { class DeviceEnumeratorSysfs final : public DeviceEnumerator @@ -20,7 +23,8 @@ public: int enumerate(); private: - std::string lookupDeviceNode(int major, int minor); + int populateMediaDevice(const std::shared_ptr &media); + std::string lookupDeviceNode(int major, int minor) final; }; } /* namespace libcamera */ diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h index 80f9372b..5bdcdea6 100644 --- a/src/libcamera/include/device_enumerator_udev.h +++ b/src/libcamera/include/device_enumerator_udev.h @@ -7,16 +7,23 @@ #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__ #define __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__ +#include +#include +#include #include +#include #include "device_enumerator.h" struct udev; +struct udev_device; struct udev_monitor; namespace libcamera { class EventNotifier; +class MediaDevice; +class MediaEntity; class DeviceEnumeratorUdev : public DeviceEnumerator { @@ -32,8 +39,17 @@ private: struct udev_monitor *monitor_; EventNotifier *notifier_; + std::map, std::list> deps_; + std::map> devnumToDevice_; + std::map devnumToEntity_; + + std::list orphans_; + + int addUdevDevice(struct udev_device *dev); + int populateMediaDevice(const std::shared_ptr &media); std::string lookupDeviceNode(int major, int minor) final; + int addV4L2Device(dev_t devnum); void udevNotify(EventNotifier *notifier); }; -- cgit v1.2.1