[{"id":2627,"web_url":"https://patchwork.libcamera.org/comment/2627/","msgid":"<20190908231101.GC4952@pendragon.ideasonboard.com>","date":"2019-09-08T23:11:01","subject":"Re: [libcamera-devel] [PATCH v3] libcamera: device_enumerator: fix\n\tudev media graph loading dependency","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sun, Sep 08, 2019 at 01:00:06AM -0400, Paul Elder wrote:\n> When a MediaDevice is enumerated and populated by the\n> DeviceEnumeratorUdev, there is a possibility that the member device\n> nodes of the media graph would not be ready (either not created, or\n> without proper permissions set by udev yet). The MediaDevice is still\n> passed up to the pipeline handler, where an attempt to access the device\n> nodes will fail in EPERM. This whole issue is especially likely to\n> happen when libcamera is run at system init time.\n> \n> To fix this, we first split DeviceEnumerator::addDevice() into three\n> methods:\n> - createDevice() to simply create the MediaDevice\n> - populateMediaDevice() to populate the MediaDevice\n> - addDevice() to pass the MediaDevice up to the pipeline handler\n> \n> DeviceEnumeratorSysfs calls these methods in succession, similar to what\n> it did before when they were all together as addDevice().\n> \n> DeviceEnumeratorUdev additionally keeps a map of MediaDevices to a list\n> of pending device nodes (plus some other auxillary maps), and a simple\n> list of orphan device nodes. If a v4l device node is ready and there\n> does not exist any MediaDevice node for it, then it goes to the orphan\n> list, otherwise it is initialized and removed from the pending list of\n> the corresponding MediaDevice in the dependency map. When a MediaDevice\n> is populated via DeviceEnumeratorUdev::populateMediaDevice(), it first\n> checks the orphan list to see if the device nodes it needs are there,\n> otherwise it tries to initialize the device nodes and if it fails, then\n> it adds the device nodes it wants to its list in the dependency map.\n> \n> This allows MediaDevice instances to be created and initialized properly\n> with udev when v4l device nodes in the media graph may not be ready when\n> the MediaDevice is populated.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n> Changes in v3:\n> - remove adding any V4L2 nodes that are already ready, to prevent any\n>   potential racing\n> - other minor changes\n> - we're keeing the name populateMediaDevice()\n> \n> Changes in v2:\n> - add documentation\n> - better name for the maps\n> - other minor changes\n> \n> ---\n>  src/libcamera/device_enumerator.cpp           |  54 +++----\n>  src/libcamera/device_enumerator_sysfs.cpp     |  35 ++++-\n>  src/libcamera/device_enumerator_udev.cpp      | 135 +++++++++++++++++-\n>  src/libcamera/include/device_enumerator.h     |   3 +-\n>  .../include/device_enumerator_sysfs.h         |   6 +-\n>  .../include/device_enumerator_udev.h          |  16 +++\n>  6 files changed, 216 insertions(+), 33 deletions(-)\n> \n> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\n> index 60c918f..e76438a 100644\n> --- a/src/libcamera/device_enumerator.cpp\n> +++ b/src/libcamera/device_enumerator.cpp\n> @@ -195,16 +195,21 @@ DeviceEnumerator::~DeviceEnumerator()\n>   */\n>  \n>  /**\n> - * \\brief Add a media device to the enumerator\n> - * \\param[in] deviceNode path to the media device to add\n> + * \\brief Create a media device instance\n> + * \\param[in] deviceNode path to the media device to create\n>   *\n> - * Create a media device for the \\a deviceNode, open it, populate its media graph,\n> - * and look up device nodes associated with all entities. Store the media device\n> - * in the internal list for later matching with pipeline handlers.\n> + * Create a media device for the \\a deviceNode, open it, and populate its\n> + * media graph. The device enumerator shall then populate the media device by\n> + * associating device nodes with entities using MediaEntity::setDeviceNode().\n> + * This process is specific to each device enumerator, and the device enumerator\n> + * shall ensure that device nodes are ready to be used (for instance, if\n> + * applicable, by waiting for device nodes to be created and access permissions\n> + * to be set by the system). Once done, it shall add the media device to the\n> + * system with addDevice().\n>   *\n> - * \\return 0 on success or a negative error code otherwise\n> + * \\return Created media device instance on success, or nullptr otherwise\n>   */\n> -int DeviceEnumerator::addDevice(const std::string &deviceNode)\n> +std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)\n>  {\n>  \tstd::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);\n>  \n> @@ -213,34 +218,31 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode)\n>  \t\tLOG(DeviceEnumerator, Info)\n>  \t\t\t<< \"Unable to populate media device \" << deviceNode\n>  \t\t\t<< \" (\" << strerror(-ret) << \"), skipping\";\n> -\t\treturn ret;\n> +\t\treturn nullptr;\n>  \t}\n>  \n>  \tLOG(DeviceEnumerator, Debug)\n>  \t\t<< \"New media device \\\"\" << media->driver()\n>  \t\t<< \"\\\" created from \" << deviceNode;\n>  \n> -\t/* Associate entities to device node paths. */\n> -\tfor (MediaEntity *entity : media->entities()) {\n> -\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n> -\t\t\tcontinue;\n> -\n> -\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> -\t\t\t\t\t\t\t  entity->deviceMinor());\n> -\t\tif (deviceNode.empty())\n> -\t\t\treturn -EINVAL;\n> -\n> -\t\tret = entity->setDeviceNode(deviceNode);\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> -\t}\n> +\treturn media;\n> +}\n>  \n> +/**\n> + * \\brief Add a media device to the enumerator\n> + * \\param[in] media media device instance to add\n> + *\n> + * Store the media device in the internal list for later matching with\n> + * pipeline handlers. \\a media shall be created with createDevice() first.\n> + * This method shall be called after all members of the entities of the\n> + * media graph have been confirmed to be initialized.\n> + */\n> +void DeviceEnumerator::addDevice(const std::shared_ptr<MediaDevice> &media)\n> +{\n>  \tLOG(DeviceEnumerator, Debug)\n> -\t\t<< \"Added device \" << deviceNode << \": \" << media->driver();\n> -\n> -\tdevices_.push_back(std::move(media));\n> +\t\t<< \"Added device \" << media->deviceNode() << \": \" << media->driver();\n>  \n> -\treturn 0;\n> +\tdevices_.push_back(media);\n>  }\n>  \n>  /**\n> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp\n> index f3054d5..78a7da8 100644\n> --- a/src/libcamera/device_enumerator_sysfs.cpp\n> +++ b/src/libcamera/device_enumerator_sysfs.cpp\n> @@ -18,6 +18,7 @@\n>  #include <unistd.h>\n>  \n>  #include \"log.h\"\n> +#include \"media_device.h\"\n>  \n>  namespace libcamera {\n>  \n> @@ -32,6 +33,7 @@ int DeviceEnumeratorSysfs::enumerate()\n>  {\n>  \tstruct dirent *ent;\n>  \tDIR *dir;\n> +\tint ret = 0;\n>  \n>  \tstatic const char * const sysfs_dirs[] = {\n>  \t\t\"/sys/subsystem/media/devices\",\n> @@ -71,11 +73,42 @@ int DeviceEnumeratorSysfs::enumerate()\n>  \t\t\tcontinue;\n>  \t\t}\n>  \n> -\t\taddDevice(devnode);\n> +\t\tstd::shared_ptr<MediaDevice> media = createDevice(devnode);\n> +\t\tif (!media) {\n> +\t\t\tret = -ENODEV;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (populateMediaDevice(media) < 0) {\n> +\t\t\tret = -ENODEV;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\taddDevice(media);\n>  \t}\n>  \n>  \tclosedir(dir);\n>  \n> +\treturn ret;\n> +}\n> +\n> +int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)\n> +{\n> +\t/* Associate entities to device node paths. */\n> +\tfor (MediaEntity *entity : media->entities()) {\n> +\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> +\t\t\t\t\t\t\t  entity->deviceMinor());\n> +\t\tif (deviceNode.empty())\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tint ret = entity->setDeviceNode(deviceNode);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n>  \treturn 0;\n>  }\n>  \n> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> index 86f6ca1..40853e7 100644\n> --- a/src/libcamera/device_enumerator_udev.cpp\n> +++ b/src/libcamera/device_enumerator_udev.cpp\n> @@ -7,6 +7,10 @@\n>  \n>  #include \"device_enumerator_udev.h\"\n>  \n> +#include <algorithm>\n> +#include <list>\n> +#include <map>\n> +\n>  #include <fcntl.h>\n>  #include <libudev.h>\n>  #include <string.h>\n> @@ -17,6 +21,7 @@\n>  #include <libcamera/event_notifier.h>\n>  \n>  #include \"log.h\"\n> +#include \"media_device.h\"\n>  \n>  namespace libcamera {\n>  \n> @@ -57,9 +62,40 @@ int DeviceEnumeratorUdev::init()\n>  \tif (ret < 0)\n>  \t\treturn ret;\n>  \n> +\tret = udev_monitor_filter_add_match_subsystem_devtype(monitor_, \"video4linux\",\n> +\t\t\t\t\t\t\t      nullptr);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n>  \treturn 0;\n>  }\n>  \n> +int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n> +{\n> +\tconst char *subsystem = udev_device_get_subsystem(dev);\n> +\tif (!subsystem)\n> +\t\treturn -ENODEV;\n> +\n> +\tif (!strcmp(subsystem, \"media\")) {\n> +\t\tstd::shared_ptr<MediaDevice> media =\n> +\t\t\tcreateDevice(udev_device_get_devnode(dev));\n> +\t\tif (!media)\n> +\t\t\treturn -ENODEV;\n> +\n> +\t\tint ret = populateMediaDevice(media);\n> +\t\tif (ret == 0)\n> +\t\t\taddDevice(media);\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tif (!strcmp(subsystem, \"video4linux\")) {\n> +\t\taddV4L2Device(udev_device_get_devnum(dev));\n> +\t\treturn 0;\n> +\t}\n> +\n> +\treturn -ENODEV;\n> +}\n> +\n>  int DeviceEnumeratorUdev::enumerate()\n>  {\n>  \tstruct udev_enumerate *udev_enum = nullptr;\n> @@ -74,6 +110,14 @@ int DeviceEnumeratorUdev::enumerate()\n>  \tif (ret < 0)\n>  \t\tgoto done;\n>  \n> +\tret = udev_enumerate_add_match_subsystem(udev_enum, \"video4linux\");\n> +\tif (ret < 0)\n> +\t\tgoto done;\n> +\n> +\tret = udev_enumerate_add_match_is_initialized(udev_enum);\n> +\tif (ret < 0)\n> +\t\tgoto done;\n> +\n>  \tret = udev_enumerate_scan_devices(udev_enum);\n>  \tif (ret < 0)\n>  \t\tgoto done;\n> @@ -102,10 +146,12 @@ int DeviceEnumeratorUdev::enumerate()\n>  \t\t\tgoto done;\n>  \t\t}\n>  \n> -\t\taddDevice(devnode);\n> -\n> +\t\tret = addUdevDevice(dev);\n>  \t\tudev_device_unref(dev);\n> +\t\tif (ret < 0)\n> +\t\t\tbreak;\n>  \t}\n> +\n>  done:\n>  \tudev_enumerate_unref(udev_enum);\n>  \tif (ret < 0)\n> @@ -122,6 +168,43 @@ done:\n>  \treturn 0;\n>  }\n>  \n> +int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)\n> +{\n> +\tunsigned int pendingNodes = 0;\n> +\tint ret;\n> +\n> +\t/* Associate entities to device node paths. */\n> +\tfor (MediaEntity *entity : media->entities()) {\n> +\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> +\t\t\t\t\t\t\t  entity->deviceMinor());\n> +\t\tdev_t devnum = makedev(entity->deviceMajor(),\n> +\t\t\t\t       entity->deviceMinor());\n> +\n> +\t\t/* Take device from orphan list first, if it is in the list. */\n> +\t\tif (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) {\n> +\t\t\tif (deviceNode.empty())\n> +\t\t\t\treturn -EINVAL;\n> +\n> +\t\t\tret = entity->setDeviceNode(deviceNode);\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\torphans_.remove(devnum);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tdeps_[media].push_back(devnum);\n> +\t\tdevnumToDevice_[devnum] = media;\n> +\t\tdevnumToEntity_[devnum] = entity;\n> +\t\tpendingNodes++;\n> +\t}\n> +\n> +\treturn pendingNodes;\n> +}\n> +\n>  std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)\n>  {\n>  \tstruct udev_device *device;\n> @@ -143,6 +226,48 @@ std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)\n>  \treturn deviceNode;\n>  }\n>  \n> +/**\n> + * \\brief Add a V4L2 device to the media device that it belongs to\n> + * \\param[in] devnum major:minor number of V4L2 device to add, as a dev_t\n> + *\n> + * Add V4L2 device identified by \\a devnum to the MediaDevice that it belongs\n> + * to, if such a MediaDevice has been created. Otherwise add the V4L2 device\n> + * to the orphan list. If the V4L2 device is added to a MediaDevice, and it is\n> + * the last V4L2 device that the MediaDevice needs, then the MediaDevice is\n> + * added to the DeviceEnumerator, where it is available for pipeline handlers.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n> +{\n> +\tMediaEntity *entity = devnumToEntity_[devnum];\n> +\tif (!entity) {\n> +\t\torphans_.push_back(devnum);\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> +\t\t\t\t\t\t  entity->deviceMinor());\n> +\tif (deviceNode.empty())\n> +\t\treturn -EINVAL;\n> +\n> +\tint ret = entity->setDeviceNode(deviceNode);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstd::shared_ptr<MediaDevice> media = devnumToDevice_[devnum];\n> +\tdeps_[media].remove(devnum);\n> +\tdevnumToDevice_.erase(devnum);\n> +\tdevnumToEntity_.erase(devnum);\n> +\n> +\tif (deps_[media].empty()) {\n> +\t\taddDevice(media);\n> +\t\tdeps_.erase(media);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n>  void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)\n>  {\n>  \tstruct udev_device *dev = udev_monitor_receive_device(monitor_);\n> @@ -153,9 +278,11 @@ void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)\n>  \t\t<< action << \" device \" << udev_device_get_devnode(dev);\n>  \n>  \tif (action == \"add\") {\n> -\t\taddDevice(deviceNode);\n> +\t\taddUdevDevice(dev);\n>  \t} else if (action == \"remove\") {\n> -\t\tremoveDevice(deviceNode);\n> +\t\tconst char *subsystem = udev_device_get_subsystem(dev);\n> +\t\tif (subsystem && !strcmp(subsystem, \"media\"))\n> +\t\t\tremoveDevice(deviceNode);\n>  \t}\n>  \n>  \tudev_device_unref(dev);\n> diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h\n> index 02aec3b..c5d26f1 100644\n> --- a/src/libcamera/include/device_enumerator.h\n> +++ b/src/libcamera/include/device_enumerator.h\n> @@ -44,7 +44,8 @@ public:\n>  \tstd::shared_ptr<MediaDevice> search(const DeviceMatch &dm);\n>  \n>  protected:\n> -\tint addDevice(const std::string &deviceNode);\n> +\tstd::shared_ptr<MediaDevice> createDevice(const std::string &deviceNode);\n> +\tvoid addDevice(const std::shared_ptr<MediaDevice> &media);\n>  \tvoid removeDevice(const std::string &deviceNode);\n>  \n>  private:\n> diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h\n> index 8d3adc9..242b22b 100644\n> --- a/src/libcamera/include/device_enumerator_sysfs.h\n> +++ b/src/libcamera/include/device_enumerator_sysfs.h\n> @@ -7,10 +7,13 @@\n>  #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__\n>  #define __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__\n>  \n> +#include <memory>\n>  #include <string>\n>  \n>  #include \"device_enumerator.h\"\n>  \n> +class MediaDevice;\n> +\n>  namespace libcamera {\n>  \n>  class DeviceEnumeratorSysfs final : public DeviceEnumerator\n> @@ -20,7 +23,8 @@ public:\n>  \tint enumerate();\n>  \n>  private:\n> -\tstd::string lookupDeviceNode(int major, int minor);\n> +\tint populateMediaDevice(const std::shared_ptr<MediaDevice> &media);\n> +\tstd::string lookupDeviceNode(int major, int minor) final;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h\n> index 80f9372..5bdcdea 100644\n> --- a/src/libcamera/include/device_enumerator_udev.h\n> +++ b/src/libcamera/include/device_enumerator_udev.h\n> @@ -7,16 +7,23 @@\n>  #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__\n>  #define __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__\n>  \n> +#include <list>\n> +#include <map>\n> +#include <memory>\n>  #include <string>\n> +#include <sys/types.h>\n>  \n>  #include \"device_enumerator.h\"\n>  \n>  struct udev;\n> +struct udev_device;\n>  struct udev_monitor;\n>  \n>  namespace libcamera {\n>  \n>  class EventNotifier;\n> +class MediaDevice;\n> +class MediaEntity;\n>  \n>  class DeviceEnumeratorUdev : public DeviceEnumerator\n>  {\n> @@ -32,8 +39,17 @@ private:\n>  \tstruct udev_monitor *monitor_;\n>  \tEventNotifier *notifier_;\n>  \n> +\tstd::map<std::shared_ptr<MediaDevice>, std::list<dev_t>> deps_;\n> +\tstd::map<dev_t, std::shared_ptr<MediaDevice>> devnumToDevice_;\n> +\tstd::map<dev_t, MediaEntity *> devnumToEntity_;\n> +\n> +\tstd::list<dev_t> orphans_;\n> +\n> +\tint addUdevDevice(struct udev_device *dev);\n> +\tint populateMediaDevice(const std::shared_ptr<MediaDevice> &media);\n>  \tstd::string lookupDeviceNode(int major, int minor) final;\n>  \n> +\tint addV4L2Device(dev_t devnum);\n>  \tvoid udevNotify(EventNotifier *notifier);\n>  };\n>  \n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EEA9660BB2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Sep 2019 01:11:12 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [88.214.162.168])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DD7B623F;\n\tMon,  9 Sep 2019 01:11:09 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1567984272;\n\tbh=Sif4IFloc4yCisLE0uxYS/hCynlmuEma4sGyaj2oy0Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kXLeEDOIhz2rgi1AmVWAHUIXL8wx3KLrmg/356p05QXMQTmvmzfFoFb12X7UraCR7\n\t8kFnodgbUCRmDliIFYgE1PmUeIoze6/l8bd/j2vLRFNhqKgh45TwiWJA6Yp1ZKsy4H\n\tVcyCJEiOWiSTbYcGCDYBF6B3SKDG929VKGiSuRSU=","Date":"Mon, 9 Sep 2019 02:11:01 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190908231101.GC4952@pendragon.ideasonboard.com>","References":"<20190908050006.5565-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190908050006.5565-1-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3] libcamera: device_enumerator: fix\n\tudev media graph loading dependency","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 08 Sep 2019 23:11:13 -0000"}},{"id":2631,"web_url":"https://patchwork.libcamera.org/comment/2631/","msgid":"<5b7a39a9-dde3-0540-65ee-1e7b222b39e5@ideasonboard.com>","date":"2019-09-10T09:36:57","subject":"Re: [libcamera-devel] [PATCH v3] libcamera: device_enumerator: fix\n\tudev media graph loading dependency","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Paul,\n\nThis patch breaks the V4L2 M2M Device operation.\n\nCould you please test and investigate what happened?\n\nYou need to load the vim2m kernel module to enable the test.\n\nPerhaps we should document somewhere that to run the test you must load\nall of:\n  vivid, vimc, vim2m\n\n\nlibcamera$ ./lcdebug ./build/test/v4l2_videodevice/v4l2_m2mdevice\n[10:26:49.846673285]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerUVC\"\n[10:26:49.846917535]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerVimc\"\n[10:26:49.847014380]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerIPU3\"\n[10:26:49.847083634]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerRkISP1\"\n[10:26:49.847211601]  DBG IPAProxy ipa_proxy.cpp:177 Registered proxy\n\"IPAProxyLinux\"\n[10:26:49.852556351]  DBG DeviceEnumerator device_enumerator.cpp:224 New\nmedia device \"uvcvideo\" created from /dev/media0\n[10:26:49.853640081]  DBG DeviceEnumerator device_enumerator.cpp:242\nAdded device /dev/media0: uvcvideo\n[10:26:49.853945949]  DBG DeviceEnumerator device_enumerator.cpp:224 New\nmedia device \"uvcvideo\" created from /dev/media1\n[10:26:49.854690732]  DBG DeviceEnumerator device_enumerator.cpp:242\nAdded device /dev/media1: uvcvideo\n[10:26:49.854961313]  DBG DeviceEnumerator device_enumerator.cpp:224 New\nmedia device \"vim2m\" created from /dev/media4\n[10:26:49.855326614]  DBG DeviceEnumerator device_enumerator.cpp:242\nAdded device /dev/media4: vim2m\n[10:26:49.855602157]  DBG DeviceEnumerator device_enumerator.cpp:224 New\nmedia device \"vimc\" created from /dev/media2\n[10:26:49.858106935]  DBG DeviceEnumerator device_enumerator.cpp:242\nAdded device /dev/media2: vimc\n[10:26:49.858379665]  DBG DeviceEnumerator device_enumerator.cpp:224 New\nmedia device \"vivid\" created from /dev/media3\n[10:26:49.860014947]  DBG DeviceEnumerator device_enumerator.cpp:242\nAdded device /dev/media3: vivid\n[10:26:49.860995086]  DBG DeviceEnumerator device_enumerator.cpp:299\nSuccessful match for media device \"vim2m\"\n[10:26:49.861182547]  ERR V4L2 v4l2_videodevice.cpp:1305 Failed to open\nV4L2 M2M device: No such file or directory\nFailed to open VIM2M device\n\n\n(The deviceNodes for the V4L2 M2M device entities are \"\")\n\n\nlibcamera$ git revert 6e620349009de675250a7e3e753480b85a6df10b\n\nlibcamera$ ./lcdebug ./build/test/v4l2_videodevice/v4l2_m2mdevice\n[10:27:56.365739511]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerUVC\"\n[10:27:56.366391361]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerVimc\"\n[10:27:56.366530058]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerIPU3\"\n[10:27:56.366606163]  DBG Pipeline pipeline_handler.cpp:578 Registered\npipeline handler \"PipelineHandlerRkISP1\"\n[10:27:56.366875927]  DBG IPAProxy ipa_proxy.cpp:177 Registered proxy\n\"IPAProxyLinux\"\n[10:27:56.371587913]  DBG DeviceEnumerator device_enumerator.cpp:219 New\nmedia device \"uvcvideo\" created from /dev/media0\n[10:27:56.372700681]  DBG DeviceEnumerator device_enumerator.cpp:238\nAdded device /dev/media0: uvcvideo\n[10:27:56.373059113]  DBG DeviceEnumerator device_enumerator.cpp:219 New\nmedia device \"uvcvideo\" created from /dev/media1\n[10:27:56.373295221]  DBG DeviceEnumerator device_enumerator.cpp:238\nAdded device /dev/media1: uvcvideo\n[10:27:56.373483256]  DBG DeviceEnumerator device_enumerator.cpp:219 New\nmedia device \"vim2m\" created from /dev/media4\n[10:27:56.373688268]  DBG DeviceEnumerator device_enumerator.cpp:238\nAdded device /dev/media4: vim2m\n[10:27:56.373946704]  DBG DeviceEnumerator device_enumerator.cpp:219 New\nmedia device \"vimc\" created from /dev/media2\n[10:27:56.374664475]  DBG DeviceEnumerator device_enumerator.cpp:238\nAdded device /dev/media2: vimc\n[10:27:56.374821368]  DBG DeviceEnumerator device_enumerator.cpp:219 New\nmedia device \"vivid\" created from /dev/media3\n[10:27:56.375225906]  DBG DeviceEnumerator device_enumerator.cpp:238\nAdded device /dev/media3: vivid\n[10:27:56.375801230]  DBG DeviceEnumerator device_enumerator.cpp:297\nSuccessful match for media device \"vim2m\"\n[10:27:56.376029192]  DBG V4L2 v4l2_videodevice.cpp:444\n/dev/video9[out]: Opened device platform:vim2m: vim2m: vim2m\n[10:27:56.376150346]  DBG V4L2 v4l2_videodevice.cpp:444\n/dev/video9[cap]: Opened device platform:vim2m: vim2m: vim2m\n[10:27:56.376491815]  DBG V4L2 v4l2_videodevice.cpp:824\n/dev/video9[cap]: 4 buffers requested.\n[10:27:56.376549753]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[cap]: Buffer 0 plane 0: length=614400\n[10:27:56.376672692]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[cap]: Buffer 1 plane 0: length=614400\n[10:27:56.376717010]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[cap]: Buffer 2 plane 0: length=614400\n[10:27:56.376775343]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[cap]: Buffer 3 plane 0: length=614400\n[10:27:56.377028570]  DBG V4L2 v4l2_videodevice.cpp:824\n/dev/video9[out]: 4 buffers requested.\n[10:27:56.377072622]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[out]: Buffer 0 plane 0: length=614400\n[10:27:56.377125986]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[out]: Buffer 1 plane 0: length=614400\n[10:27:56.377150123]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[out]: Buffer 2 plane 0: length=614400\n[10:27:56.377167310]  DBG V4L2 v4l2_videodevice.cpp:909\n/dev/video9[out]: Buffer 3 plane 0: length=614400\n[10:27:56.377212874]  DBG V4L2 v4l2_videodevice.cpp:1023\n/dev/video9[cap]: Queueing buffer 0\n[10:27:56.377301766]  DBG V4L2 v4l2_videodevice.cpp:1023\n/dev/video9[cap]: Queueing buffer 1\n[10:27:56.377331186]  DBG V4L2 v4l2_videodevice.cpp:1023\n/dev/video9[cap]: Queueing buffer 2\n\n... Test continues ...\n\n--\nRegards\n\nKieran\n\n\n\nOn 09/09/2019 00:11, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Sun, Sep 08, 2019 at 01:00:06AM -0400, Paul Elder wrote:\n>> When a MediaDevice is enumerated and populated by the\n>> DeviceEnumeratorUdev, there is a possibility that the member device\n>> nodes of the media graph would not be ready (either not created, or\n>> without proper permissions set by udev yet). The MediaDevice is still\n>> passed up to the pipeline handler, where an attempt to access the device\n>> nodes will fail in EPERM. This whole issue is especially likely to\n>> happen when libcamera is run at system init time.\n>>\n>> To fix this, we first split DeviceEnumerator::addDevice() into three\n>> methods:\n>> - createDevice() to simply create the MediaDevice\n>> - populateMediaDevice() to populate the MediaDevice\n>> - addDevice() to pass the MediaDevice up to the pipeline handler\n>>\n>> DeviceEnumeratorSysfs calls these methods in succession, similar to what\n>> it did before when they were all together as addDevice().\n>>\n>> DeviceEnumeratorUdev additionally keeps a map of MediaDevices to a list\n>> of pending device nodes (plus some other auxillary maps), and a simple\n>> list of orphan device nodes. If a v4l device node is ready and there\n>> does not exist any MediaDevice node for it, then it goes to the orphan\n>> list, otherwise it is initialized and removed from the pending list of\n>> the corresponding MediaDevice in the dependency map. When a MediaDevice\n>> is populated via DeviceEnumeratorUdev::populateMediaDevice(), it first\n>> checks the orphan list to see if the device nodes it needs are there,\n>> otherwise it tries to initialize the device nodes and if it fails, then\n>> it adds the device nodes it wants to its list in the dependency map.\n>>\n>> This allows MediaDevice instances to be created and initialized properly\n>> with udev when v4l device nodes in the media graph may not be ready when\n>> the MediaDevice is populated.\n>>\n>> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n>> ---\n>> Changes in v3:\n>> - remove adding any V4L2 nodes that are already ready, to prevent any\n>>   potential racing\n>> - other minor changes\n>> - we're keeing the name populateMediaDevice()\n>>\n>> Changes in v2:\n>> - add documentation\n>> - better name for the maps\n>> - other minor changes\n>>\n>> ---\n>>  src/libcamera/device_enumerator.cpp           |  54 +++----\n>>  src/libcamera/device_enumerator_sysfs.cpp     |  35 ++++-\n>>  src/libcamera/device_enumerator_udev.cpp      | 135 +++++++++++++++++-\n>>  src/libcamera/include/device_enumerator.h     |   3 +-\n>>  .../include/device_enumerator_sysfs.h         |   6 +-\n>>  .../include/device_enumerator_udev.h          |  16 +++\n>>  6 files changed, 216 insertions(+), 33 deletions(-)\n>>\n>> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\n>> index 60c918f..e76438a 100644\n>> --- a/src/libcamera/device_enumerator.cpp\n>> +++ b/src/libcamera/device_enumerator.cpp\n>> @@ -195,16 +195,21 @@ DeviceEnumerator::~DeviceEnumerator()\n>>   */\n>>  \n>>  /**\n>> - * \\brief Add a media device to the enumerator\n>> - * \\param[in] deviceNode path to the media device to add\n>> + * \\brief Create a media device instance\n>> + * \\param[in] deviceNode path to the media device to create\n>>   *\n>> - * Create a media device for the \\a deviceNode, open it, populate its media graph,\n>> - * and look up device nodes associated with all entities. Store the media device\n>> - * in the internal list for later matching with pipeline handlers.\n>> + * Create a media device for the \\a deviceNode, open it, and populate its\n>> + * media graph. The device enumerator shall then populate the media device by\n>> + * associating device nodes with entities using MediaEntity::setDeviceNode().\n>> + * This process is specific to each device enumerator, and the device enumerator\n>> + * shall ensure that device nodes are ready to be used (for instance, if\n>> + * applicable, by waiting for device nodes to be created and access permissions\n>> + * to be set by the system). Once done, it shall add the media device to the\n>> + * system with addDevice().\n>>   *\n>> - * \\return 0 on success or a negative error code otherwise\n>> + * \\return Created media device instance on success, or nullptr otherwise\n>>   */\n>> -int DeviceEnumerator::addDevice(const std::string &deviceNode)\n>> +std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)\n>>  {\n>>  \tstd::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);\n>>  \n>> @@ -213,34 +218,31 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode)\n>>  \t\tLOG(DeviceEnumerator, Info)\n>>  \t\t\t<< \"Unable to populate media device \" << deviceNode\n>>  \t\t\t<< \" (\" << strerror(-ret) << \"), skipping\";\n>> -\t\treturn ret;\n>> +\t\treturn nullptr;\n>>  \t}\n>>  \n>>  \tLOG(DeviceEnumerator, Debug)\n>>  \t\t<< \"New media device \\\"\" << media->driver()\n>>  \t\t<< \"\\\" created from \" << deviceNode;\n>>  \n>> -\t/* Associate entities to device node paths. */\n>> -\tfor (MediaEntity *entity : media->entities()) {\n>> -\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n>> -\t\t\tcontinue;\n>> -\n>> -\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n>> -\t\t\t\t\t\t\t  entity->deviceMinor());\n>> -\t\tif (deviceNode.empty())\n>> -\t\t\treturn -EINVAL;\n>> -\n>> -\t\tret = entity->setDeviceNode(deviceNode);\n>> -\t\tif (ret)\n>> -\t\t\treturn ret;\n>> -\t}\n>> +\treturn media;\n>> +}\n>>  \n>> +/**\n>> + * \\brief Add a media device to the enumerator\n>> + * \\param[in] media media device instance to add\n>> + *\n>> + * Store the media device in the internal list for later matching with\n>> + * pipeline handlers. \\a media shall be created with createDevice() first.\n>> + * This method shall be called after all members of the entities of the\n>> + * media graph have been confirmed to be initialized.\n>> + */\n>> +void DeviceEnumerator::addDevice(const std::shared_ptr<MediaDevice> &media)\n>> +{\n>>  \tLOG(DeviceEnumerator, Debug)\n>> -\t\t<< \"Added device \" << deviceNode << \": \" << media->driver();\n>> -\n>> -\tdevices_.push_back(std::move(media));\n>> +\t\t<< \"Added device \" << media->deviceNode() << \": \" << media->driver();\n>>  \n>> -\treturn 0;\n>> +\tdevices_.push_back(media);\n>>  }\n>>  \n>>  /**\n>> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp\n>> index f3054d5..78a7da8 100644\n>> --- a/src/libcamera/device_enumerator_sysfs.cpp\n>> +++ b/src/libcamera/device_enumerator_sysfs.cpp\n>> @@ -18,6 +18,7 @@\n>>  #include <unistd.h>\n>>  \n>>  #include \"log.h\"\n>> +#include \"media_device.h\"\n>>  \n>>  namespace libcamera {\n>>  \n>> @@ -32,6 +33,7 @@ int DeviceEnumeratorSysfs::enumerate()\n>>  {\n>>  \tstruct dirent *ent;\n>>  \tDIR *dir;\n>> +\tint ret = 0;\n>>  \n>>  \tstatic const char * const sysfs_dirs[] = {\n>>  \t\t\"/sys/subsystem/media/devices\",\n>> @@ -71,11 +73,42 @@ int DeviceEnumeratorSysfs::enumerate()\n>>  \t\t\tcontinue;\n>>  \t\t}\n>>  \n>> -\t\taddDevice(devnode);\n>> +\t\tstd::shared_ptr<MediaDevice> media = createDevice(devnode);\n>> +\t\tif (!media) {\n>> +\t\t\tret = -ENODEV;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\n>> +\t\tif (populateMediaDevice(media) < 0) {\n>> +\t\t\tret = -ENODEV;\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\n>> +\t\taddDevice(media);\n>>  \t}\n>>  \n>>  \tclosedir(dir);\n>>  \n>> +\treturn ret;\n>> +}\n>> +\n>> +int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)\n>> +{\n>> +\t/* Associate entities to device node paths. */\n>> +\tfor (MediaEntity *entity : media->entities()) {\n>> +\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n>> +\t\t\t\t\t\t\t  entity->deviceMinor());\n>> +\t\tif (deviceNode.empty())\n>> +\t\t\treturn -EINVAL;\n>> +\n>> +\t\tint ret = entity->setDeviceNode(deviceNode);\n>> +\t\tif (ret)\n>> +\t\t\treturn ret;\n>> +\t}\n>> +\n>>  \treturn 0;\n>>  }\n>>  \n>> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n>> index 86f6ca1..40853e7 100644\n>> --- a/src/libcamera/device_enumerator_udev.cpp\n>> +++ b/src/libcamera/device_enumerator_udev.cpp\n>> @@ -7,6 +7,10 @@\n>>  \n>>  #include \"device_enumerator_udev.h\"\n>>  \n>> +#include <algorithm>\n>> +#include <list>\n>> +#include <map>\n>> +\n>>  #include <fcntl.h>\n>>  #include <libudev.h>\n>>  #include <string.h>\n>> @@ -17,6 +21,7 @@\n>>  #include <libcamera/event_notifier.h>\n>>  \n>>  #include \"log.h\"\n>> +#include \"media_device.h\"\n>>  \n>>  namespace libcamera {\n>>  \n>> @@ -57,9 +62,40 @@ int DeviceEnumeratorUdev::init()\n>>  \tif (ret < 0)\n>>  \t\treturn ret;\n>>  \n>> +\tret = udev_monitor_filter_add_match_subsystem_devtype(monitor_, \"video4linux\",\n>> +\t\t\t\t\t\t\t      nullptr);\n>> +\tif (ret < 0)\n>> +\t\treturn ret;\n>> +\n>>  \treturn 0;\n>>  }\n>>  \n>> +int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n>> +{\n>> +\tconst char *subsystem = udev_device_get_subsystem(dev);\n>> +\tif (!subsystem)\n>> +\t\treturn -ENODEV;\n>> +\n>> +\tif (!strcmp(subsystem, \"media\")) {\n>> +\t\tstd::shared_ptr<MediaDevice> media =\n>> +\t\t\tcreateDevice(udev_device_get_devnode(dev));\n>> +\t\tif (!media)\n>> +\t\t\treturn -ENODEV;\n>> +\n>> +\t\tint ret = populateMediaDevice(media);\n>> +\t\tif (ret == 0)\n>> +\t\t\taddDevice(media);\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\tif (!strcmp(subsystem, \"video4linux\")) {\n>> +\t\taddV4L2Device(udev_device_get_devnum(dev));\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\treturn -ENODEV;\n>> +}\n>> +\n>>  int DeviceEnumeratorUdev::enumerate()\n>>  {\n>>  \tstruct udev_enumerate *udev_enum = nullptr;\n>> @@ -74,6 +110,14 @@ int DeviceEnumeratorUdev::enumerate()\n>>  \tif (ret < 0)\n>>  \t\tgoto done;\n>>  \n>> +\tret = udev_enumerate_add_match_subsystem(udev_enum, \"video4linux\");\n>> +\tif (ret < 0)\n>> +\t\tgoto done;\n>> +\n>> +\tret = udev_enumerate_add_match_is_initialized(udev_enum);\n>> +\tif (ret < 0)\n>> +\t\tgoto done;\n>> +\n>>  \tret = udev_enumerate_scan_devices(udev_enum);\n>>  \tif (ret < 0)\n>>  \t\tgoto done;\n>> @@ -102,10 +146,12 @@ int DeviceEnumeratorUdev::enumerate()\n>>  \t\t\tgoto done;\n>>  \t\t}\n>>  \n>> -\t\taddDevice(devnode);\n>> -\n>> +\t\tret = addUdevDevice(dev);\n>>  \t\tudev_device_unref(dev);\n>> +\t\tif (ret < 0)\n>> +\t\t\tbreak;\n>>  \t}\n>> +\n>>  done:\n>>  \tudev_enumerate_unref(udev_enum);\n>>  \tif (ret < 0)\n>> @@ -122,6 +168,43 @@ done:\n>>  \treturn 0;\n>>  }\n>>  \n>> +int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)\n>> +{\n>> +\tunsigned int pendingNodes = 0;\n>> +\tint ret;\n>> +\n>> +\t/* Associate entities to device node paths. */\n>> +\tfor (MediaEntity *entity : media->entities()) {\n>> +\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n>> +\t\t\t\t\t\t\t  entity->deviceMinor());\n>> +\t\tdev_t devnum = makedev(entity->deviceMajor(),\n>> +\t\t\t\t       entity->deviceMinor());\n>> +\n>> +\t\t/* Take device from orphan list first, if it is in the list. */\n>> +\t\tif (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) {\n>> +\t\t\tif (deviceNode.empty())\n>> +\t\t\t\treturn -EINVAL;\n>> +\n>> +\t\t\tret = entity->setDeviceNode(deviceNode);\n>> +\t\t\tif (ret)\n>> +\t\t\t\treturn ret;\n>> +\n>> +\t\t\torphans_.remove(devnum);\n>> +\t\t\tcontinue;\n>> +\t\t}\n>> +\n>> +\t\tdeps_[media].push_back(devnum);\n>> +\t\tdevnumToDevice_[devnum] = media;\n>> +\t\tdevnumToEntity_[devnum] = entity;\n>> +\t\tpendingNodes++;\n>> +\t}\n>> +\n>> +\treturn pendingNodes;\n>> +}\n>> +\n>>  std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)\n>>  {\n>>  \tstruct udev_device *device;\n>> @@ -143,6 +226,48 @@ std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)\n>>  \treturn deviceNode;\n>>  }\n>>  \n>> +/**\n>> + * \\brief Add a V4L2 device to the media device that it belongs to\n>> + * \\param[in] devnum major:minor number of V4L2 device to add, as a dev_t\n>> + *\n>> + * Add V4L2 device identified by \\a devnum to the MediaDevice that it belongs\n>> + * to, if such a MediaDevice has been created. Otherwise add the V4L2 device\n>> + * to the orphan list. If the V4L2 device is added to a MediaDevice, and it is\n>> + * the last V4L2 device that the MediaDevice needs, then the MediaDevice is\n>> + * added to the DeviceEnumerator, where it is available for pipeline handlers.\n>> + *\n>> + * \\return 0 on success or a negative error code otherwise\n>> + */\n>> +int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n>> +{\n>> +\tMediaEntity *entity = devnumToEntity_[devnum];\n>> +\tif (!entity) {\n>> +\t\torphans_.push_back(devnum);\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n>> +\t\t\t\t\t\t  entity->deviceMinor());\n>> +\tif (deviceNode.empty())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tint ret = entity->setDeviceNode(deviceNode);\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\tstd::shared_ptr<MediaDevice> media = devnumToDevice_[devnum];\n>> +\tdeps_[media].remove(devnum);\n>> +\tdevnumToDevice_.erase(devnum);\n>> +\tdevnumToEntity_.erase(devnum);\n>> +\n>> +\tif (deps_[media].empty()) {\n>> +\t\taddDevice(media);\n>> +\t\tdeps_.erase(media);\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>>  void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)\n>>  {\n>>  \tstruct udev_device *dev = udev_monitor_receive_device(monitor_);\n>> @@ -153,9 +278,11 @@ void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)\n>>  \t\t<< action << \" device \" << udev_device_get_devnode(dev);\n>>  \n>>  \tif (action == \"add\") {\n>> -\t\taddDevice(deviceNode);\n>> +\t\taddUdevDevice(dev);\n>>  \t} else if (action == \"remove\") {\n>> -\t\tremoveDevice(deviceNode);\n>> +\t\tconst char *subsystem = udev_device_get_subsystem(dev);\n>> +\t\tif (subsystem && !strcmp(subsystem, \"media\"))\n>> +\t\t\tremoveDevice(deviceNode);\n>>  \t}\n>>  \n>>  \tudev_device_unref(dev);\n>> diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h\n>> index 02aec3b..c5d26f1 100644\n>> --- a/src/libcamera/include/device_enumerator.h\n>> +++ b/src/libcamera/include/device_enumerator.h\n>> @@ -44,7 +44,8 @@ public:\n>>  \tstd::shared_ptr<MediaDevice> search(const DeviceMatch &dm);\n>>  \n>>  protected:\n>> -\tint addDevice(const std::string &deviceNode);\n>> +\tstd::shared_ptr<MediaDevice> createDevice(const std::string &deviceNode);\n>> +\tvoid addDevice(const std::shared_ptr<MediaDevice> &media);\n>>  \tvoid removeDevice(const std::string &deviceNode);\n>>  \n>>  private:\n>> diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h\n>> index 8d3adc9..242b22b 100644\n>> --- a/src/libcamera/include/device_enumerator_sysfs.h\n>> +++ b/src/libcamera/include/device_enumerator_sysfs.h\n>> @@ -7,10 +7,13 @@\n>>  #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__\n>>  #define __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__\n>>  \n>> +#include <memory>\n>>  #include <string>\n>>  \n>>  #include \"device_enumerator.h\"\n>>  \n>> +class MediaDevice;\n>> +\n>>  namespace libcamera {\n>>  \n>>  class DeviceEnumeratorSysfs final : public DeviceEnumerator\n>> @@ -20,7 +23,8 @@ public:\n>>  \tint enumerate();\n>>  \n>>  private:\n>> -\tstd::string lookupDeviceNode(int major, int minor);\n>> +\tint populateMediaDevice(const std::shared_ptr<MediaDevice> &media);\n>> +\tstd::string lookupDeviceNode(int major, int minor) final;\n>>  };\n>>  \n>>  } /* namespace libcamera */\n>> diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h\n>> index 80f9372..5bdcdea 100644\n>> --- a/src/libcamera/include/device_enumerator_udev.h\n>> +++ b/src/libcamera/include/device_enumerator_udev.h\n>> @@ -7,16 +7,23 @@\n>>  #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__\n>>  #define __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__\n>>  \n>> +#include <list>\n>> +#include <map>\n>> +#include <memory>\n>>  #include <string>\n>> +#include <sys/types.h>\n>>  \n>>  #include \"device_enumerator.h\"\n>>  \n>>  struct udev;\n>> +struct udev_device;\n>>  struct udev_monitor;\n>>  \n>>  namespace libcamera {\n>>  \n>>  class EventNotifier;\n>> +class MediaDevice;\n>> +class MediaEntity;\n>>  \n>>  class DeviceEnumeratorUdev : public DeviceEnumerator\n>>  {\n>> @@ -32,8 +39,17 @@ private:\n>>  \tstruct udev_monitor *monitor_;\n>>  \tEventNotifier *notifier_;\n>>  \n>> +\tstd::map<std::shared_ptr<MediaDevice>, std::list<dev_t>> deps_;\n>> +\tstd::map<dev_t, std::shared_ptr<MediaDevice>> devnumToDevice_;\n>> +\tstd::map<dev_t, MediaEntity *> devnumToEntity_;\n>> +\n>> +\tstd::list<dev_t> orphans_;\n>> +\n>> +\tint addUdevDevice(struct udev_device *dev);\n>> +\tint populateMediaDevice(const std::shared_ptr<MediaDevice> &media);\n>>  \tstd::string lookupDeviceNode(int major, int minor) final;\n>>  \n>> +\tint addV4L2Device(dev_t devnum);\n>>  \tvoid udevNotify(EventNotifier *notifier);\n>>  };\n>>  \n>> -- \n>> 2.20.1\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0022A60BCF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 10 Sep 2019 11:37:00 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5ADDB2F9;\n\tTue, 10 Sep 2019 11:37:00 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1568108220;\n\tbh=AdAch+OHL2agsiarisOeDz8KkILZd49oiOocqw/3InE=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=SQJFTqD/NR4D2bxD3lZvp7PFCYxc3ngtm/UGC5RJ8fyJaYpHkBwtOJ/fZGUd3fkRt\n\taWfNaAXROYNIXNmelfIXIJZq3k7XH+nv2kzVWGsd7If9LNtaaulybJpIWWAnZZyvSr\n\tixHyUP4gnyTPUVyr/9q0ztCqmWhr/oUzRHhOPwDg=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190908050006.5565-1-paul.elder@ideasonboard.com>\n\t<20190908231101.GC4952@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCJQQYAQoADwIbDAUCWcOUawUJ\n\tB4D+AgAKCRChHkZyEKRh/XJhEACr5iidt/0MZ0rWRMCbZFMWD7D2g6nZeOp+F2zY8CEUW+sd\n\tCDVd9BH9QX9KN5SZo6YtJzMzSzpcx45VwTvtQW0n/6Eujg9EUqblfU9xqvqDmbjEapr5d/OL\n\t21GTALb0owKhA5qDUGEcKGCphpQffKhTNo/BP99jvmJUj7IPSKH97qPypi8/ym8bAxB+uY31\n\tgHTMHf1jMJJ1pRo2tYYPeIIHGDqXBI4sp5GHHF+JcIhgR/e/A6w/dgzHYmQPl2ix5eZYEZbV\n\tTRP+gkX4NV8oHqa/lR+xPOlWElGB57viOSOoWriqxQbFy8XbG1GR8cWlkNtGBGVWaJaSoORP\n\tiowD7irXL91bCyFIqL+7BVk3Jy4uzP744PzE80KwxOp5SQAp9sPzFbgsJrLev90PZySjFHG0\n\twP144DK7nBjOj/J0g9OHVASP1JjK+nw7SDoKnETDIdRC0XmiHXk7TXzPdkvO0UkpHdEPjZUp\n\tWyuc0MqehjR/hTTPt4m/Y14XzEcy6JREIjOrFfUZVho2QpOdv9CNryGdieRTNjUtz463CIaZ\n\tdPBiw9mOMBoNffkn9FIoCjLnAaj9gUAnEHWBZOEviQ5NuyqpeP0YtzI4iaRbSUkYZHej99X3\n\tVmHrdLlMqd/ZgYYbPGSL4AN3FVACb5CxuxEHwo029VcE5U3CSjzqtCoX12tm7A==","Organization":"Ideas on Board","Message-ID":"<5b7a39a9-dde3-0540-65ee-1e7b222b39e5@ideasonboard.com>","Date":"Tue, 10 Sep 2019 10:36:57 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.8.0","MIME-Version":"1.0","In-Reply-To":"<20190908231101.GC4952@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3] libcamera: device_enumerator: fix\n\tudev media graph loading dependency","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 10 Sep 2019 09:37:01 -0000"}},{"id":2639,"web_url":"https://patchwork.libcamera.org/comment/2639/","msgid":"<20190912225532.GD6006@pendragon.ideasonboard.com>","date":"2019-09-12T22:55:32","subject":"Re: [libcamera-devel] [PATCH v3] libcamera: device_enumerator: fix\n\tudev media graph loading dependency","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Sep 10, 2019 at 10:36:57AM +0100, Kieran Bingham wrote:\n> Hi Paul,\n> \n> This patch breaks the V4L2 M2M Device operation.\n> \n> Could you please test and investigate what happened?\n\nI've posted a patch series that should fix this. Could you and Paul have\na look ?\n\n> You need to load the vim2m kernel module to enable the test.\n> \n> Perhaps we should document somewhere that to run the test you must load\n> all of:\n>   vivid, vimc, vim2m\n> \n> \n> libcamera$ ./lcdebug ./build/test/v4l2_videodevice/v4l2_m2mdevice\n> [10:26:49.846673285]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerUVC\"\n> [10:26:49.846917535]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerVimc\"\n> [10:26:49.847014380]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerIPU3\"\n> [10:26:49.847083634]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerRkISP1\"\n> [10:26:49.847211601]  DBG IPAProxy ipa_proxy.cpp:177 Registered proxy\n> \"IPAProxyLinux\"\n> [10:26:49.852556351]  DBG DeviceEnumerator device_enumerator.cpp:224 New\n> media device \"uvcvideo\" created from /dev/media0\n> [10:26:49.853640081]  DBG DeviceEnumerator device_enumerator.cpp:242\n> Added device /dev/media0: uvcvideo\n> [10:26:49.853945949]  DBG DeviceEnumerator device_enumerator.cpp:224 New\n> media device \"uvcvideo\" created from /dev/media1\n> [10:26:49.854690732]  DBG DeviceEnumerator device_enumerator.cpp:242\n> Added device /dev/media1: uvcvideo\n> [10:26:49.854961313]  DBG DeviceEnumerator device_enumerator.cpp:224 New\n> media device \"vim2m\" created from /dev/media4\n> [10:26:49.855326614]  DBG DeviceEnumerator device_enumerator.cpp:242\n> Added device /dev/media4: vim2m\n> [10:26:49.855602157]  DBG DeviceEnumerator device_enumerator.cpp:224 New\n> media device \"vimc\" created from /dev/media2\n> [10:26:49.858106935]  DBG DeviceEnumerator device_enumerator.cpp:242\n> Added device /dev/media2: vimc\n> [10:26:49.858379665]  DBG DeviceEnumerator device_enumerator.cpp:224 New\n> media device \"vivid\" created from /dev/media3\n> [10:26:49.860014947]  DBG DeviceEnumerator device_enumerator.cpp:242\n> Added device /dev/media3: vivid\n> [10:26:49.860995086]  DBG DeviceEnumerator device_enumerator.cpp:299\n> Successful match for media device \"vim2m\"\n> [10:26:49.861182547]  ERR V4L2 v4l2_videodevice.cpp:1305 Failed to open\n> V4L2 M2M device: No such file or directory\n> Failed to open VIM2M device\n> \n> \n> (The deviceNodes for the V4L2 M2M device entities are \"\")\n> \n> \n> libcamera$ git revert 6e620349009de675250a7e3e753480b85a6df10b\n> \n> libcamera$ ./lcdebug ./build/test/v4l2_videodevice/v4l2_m2mdevice\n> [10:27:56.365739511]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerUVC\"\n> [10:27:56.366391361]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerVimc\"\n> [10:27:56.366530058]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerIPU3\"\n> [10:27:56.366606163]  DBG Pipeline pipeline_handler.cpp:578 Registered\n> pipeline handler \"PipelineHandlerRkISP1\"\n> [10:27:56.366875927]  DBG IPAProxy ipa_proxy.cpp:177 Registered proxy\n> \"IPAProxyLinux\"\n> [10:27:56.371587913]  DBG DeviceEnumerator device_enumerator.cpp:219 New\n> media device \"uvcvideo\" created from /dev/media0\n> [10:27:56.372700681]  DBG DeviceEnumerator device_enumerator.cpp:238\n> Added device /dev/media0: uvcvideo\n> [10:27:56.373059113]  DBG DeviceEnumerator device_enumerator.cpp:219 New\n> media device \"uvcvideo\" created from /dev/media1\n> [10:27:56.373295221]  DBG DeviceEnumerator device_enumerator.cpp:238\n> Added device /dev/media1: uvcvideo\n> [10:27:56.373483256]  DBG DeviceEnumerator device_enumerator.cpp:219 New\n> media device \"vim2m\" created from /dev/media4\n> [10:27:56.373688268]  DBG DeviceEnumerator device_enumerator.cpp:238\n> Added device /dev/media4: vim2m\n> [10:27:56.373946704]  DBG DeviceEnumerator device_enumerator.cpp:219 New\n> media device \"vimc\" created from /dev/media2\n> [10:27:56.374664475]  DBG DeviceEnumerator device_enumerator.cpp:238\n> Added device /dev/media2: vimc\n> [10:27:56.374821368]  DBG DeviceEnumerator device_enumerator.cpp:219 New\n> media device \"vivid\" created from /dev/media3\n> [10:27:56.375225906]  DBG DeviceEnumerator device_enumerator.cpp:238\n> Added device /dev/media3: vivid\n> [10:27:56.375801230]  DBG DeviceEnumerator device_enumerator.cpp:297\n> Successful match for media device \"vim2m\"\n> [10:27:56.376029192]  DBG V4L2 v4l2_videodevice.cpp:444\n> /dev/video9[out]: Opened device platform:vim2m: vim2m: vim2m\n> [10:27:56.376150346]  DBG V4L2 v4l2_videodevice.cpp:444\n> /dev/video9[cap]: Opened device platform:vim2m: vim2m: vim2m\n> [10:27:56.376491815]  DBG V4L2 v4l2_videodevice.cpp:824\n> /dev/video9[cap]: 4 buffers requested.\n> [10:27:56.376549753]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[cap]: Buffer 0 plane 0: length=614400\n> [10:27:56.376672692]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[cap]: Buffer 1 plane 0: length=614400\n> [10:27:56.376717010]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[cap]: Buffer 2 plane 0: length=614400\n> [10:27:56.376775343]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[cap]: Buffer 3 plane 0: length=614400\n> [10:27:56.377028570]  DBG V4L2 v4l2_videodevice.cpp:824\n> /dev/video9[out]: 4 buffers requested.\n> [10:27:56.377072622]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[out]: Buffer 0 plane 0: length=614400\n> [10:27:56.377125986]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[out]: Buffer 1 plane 0: length=614400\n> [10:27:56.377150123]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[out]: Buffer 2 plane 0: length=614400\n> [10:27:56.377167310]  DBG V4L2 v4l2_videodevice.cpp:909\n> /dev/video9[out]: Buffer 3 plane 0: length=614400\n> [10:27:56.377212874]  DBG V4L2 v4l2_videodevice.cpp:1023\n> /dev/video9[cap]: Queueing buffer 0\n> [10:27:56.377301766]  DBG V4L2 v4l2_videodevice.cpp:1023\n> /dev/video9[cap]: Queueing buffer 1\n> [10:27:56.377331186]  DBG V4L2 v4l2_videodevice.cpp:1023\n> /dev/video9[cap]: Queueing buffer 2\n> \n> ... Test continues ...\n> \n> --\n> Regards\n> \n> Kieran\n> \n> \n> \n> On 09/09/2019 00:11, Laurent Pinchart wrote:\n> > Hi Paul,\n> > \n> > Thank you for the patch.\n> > \n> > On Sun, Sep 08, 2019 at 01:00:06AM -0400, Paul Elder wrote:\n> >> When a MediaDevice is enumerated and populated by the\n> >> DeviceEnumeratorUdev, there is a possibility that the member device\n> >> nodes of the media graph would not be ready (either not created, or\n> >> without proper permissions set by udev yet). The MediaDevice is still\n> >> passed up to the pipeline handler, where an attempt to access the device\n> >> nodes will fail in EPERM. This whole issue is especially likely to\n> >> happen when libcamera is run at system init time.\n> >>\n> >> To fix this, we first split DeviceEnumerator::addDevice() into three\n> >> methods:\n> >> - createDevice() to simply create the MediaDevice\n> >> - populateMediaDevice() to populate the MediaDevice\n> >> - addDevice() to pass the MediaDevice up to the pipeline handler\n> >>\n> >> DeviceEnumeratorSysfs calls these methods in succession, similar to what\n> >> it did before when they were all together as addDevice().\n> >>\n> >> DeviceEnumeratorUdev additionally keeps a map of MediaDevices to a list\n> >> of pending device nodes (plus some other auxillary maps), and a simple\n> >> list of orphan device nodes. If a v4l device node is ready and there\n> >> does not exist any MediaDevice node for it, then it goes to the orphan\n> >> list, otherwise it is initialized and removed from the pending list of\n> >> the corresponding MediaDevice in the dependency map. When a MediaDevice\n> >> is populated via DeviceEnumeratorUdev::populateMediaDevice(), it first\n> >> checks the orphan list to see if the device nodes it needs are there,\n> >> otherwise it tries to initialize the device nodes and if it fails, then\n> >> it adds the device nodes it wants to its list in the dependency map.\n> >>\n> >> This allows MediaDevice instances to be created and initialized properly\n> >> with udev when v4l device nodes in the media graph may not be ready when\n> >> the MediaDevice is populated.\n> >>\n> >> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> >> ---\n> >> Changes in v3:\n> >> - remove adding any V4L2 nodes that are already ready, to prevent any\n> >>   potential racing\n> >> - other minor changes\n> >> - we're keeing the name populateMediaDevice()\n> >>\n> >> Changes in v2:\n> >> - add documentation\n> >> - better name for the maps\n> >> - other minor changes\n> >>\n> >> ---\n> >>  src/libcamera/device_enumerator.cpp           |  54 +++----\n> >>  src/libcamera/device_enumerator_sysfs.cpp     |  35 ++++-\n> >>  src/libcamera/device_enumerator_udev.cpp      | 135 +++++++++++++++++-\n> >>  src/libcamera/include/device_enumerator.h     |   3 +-\n> >>  .../include/device_enumerator_sysfs.h         |   6 +-\n> >>  .../include/device_enumerator_udev.h          |  16 +++\n> >>  6 files changed, 216 insertions(+), 33 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp\n> >> index 60c918f..e76438a 100644\n> >> --- a/src/libcamera/device_enumerator.cpp\n> >> +++ b/src/libcamera/device_enumerator.cpp\n> >> @@ -195,16 +195,21 @@ DeviceEnumerator::~DeviceEnumerator()\n> >>   */\n> >>  \n> >>  /**\n> >> - * \\brief Add a media device to the enumerator\n> >> - * \\param[in] deviceNode path to the media device to add\n> >> + * \\brief Create a media device instance\n> >> + * \\param[in] deviceNode path to the media device to create\n> >>   *\n> >> - * Create a media device for the \\a deviceNode, open it, populate its media graph,\n> >> - * and look up device nodes associated with all entities. Store the media device\n> >> - * in the internal list for later matching with pipeline handlers.\n> >> + * Create a media device for the \\a deviceNode, open it, and populate its\n> >> + * media graph. The device enumerator shall then populate the media device by\n> >> + * associating device nodes with entities using MediaEntity::setDeviceNode().\n> >> + * This process is specific to each device enumerator, and the device enumerator\n> >> + * shall ensure that device nodes are ready to be used (for instance, if\n> >> + * applicable, by waiting for device nodes to be created and access permissions\n> >> + * to be set by the system). Once done, it shall add the media device to the\n> >> + * system with addDevice().\n> >>   *\n> >> - * \\return 0 on success or a negative error code otherwise\n> >> + * \\return Created media device instance on success, or nullptr otherwise\n> >>   */\n> >> -int DeviceEnumerator::addDevice(const std::string &deviceNode)\n> >> +std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)\n> >>  {\n> >>  \tstd::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);\n> >>  \n> >> @@ -213,34 +218,31 @@ int DeviceEnumerator::addDevice(const std::string &deviceNode)\n> >>  \t\tLOG(DeviceEnumerator, Info)\n> >>  \t\t\t<< \"Unable to populate media device \" << deviceNode\n> >>  \t\t\t<< \" (\" << strerror(-ret) << \"), skipping\";\n> >> -\t\treturn ret;\n> >> +\t\treturn nullptr;\n> >>  \t}\n> >>  \n> >>  \tLOG(DeviceEnumerator, Debug)\n> >>  \t\t<< \"New media device \\\"\" << media->driver()\n> >>  \t\t<< \"\\\" created from \" << deviceNode;\n> >>  \n> >> -\t/* Associate entities to device node paths. */\n> >> -\tfor (MediaEntity *entity : media->entities()) {\n> >> -\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n> >> -\t\t\tcontinue;\n> >> -\n> >> -\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> >> -\t\t\t\t\t\t\t  entity->deviceMinor());\n> >> -\t\tif (deviceNode.empty())\n> >> -\t\t\treturn -EINVAL;\n> >> -\n> >> -\t\tret = entity->setDeviceNode(deviceNode);\n> >> -\t\tif (ret)\n> >> -\t\t\treturn ret;\n> >> -\t}\n> >> +\treturn media;\n> >> +}\n> >>  \n> >> +/**\n> >> + * \\brief Add a media device to the enumerator\n> >> + * \\param[in] media media device instance to add\n> >> + *\n> >> + * Store the media device in the internal list for later matching with\n> >> + * pipeline handlers. \\a media shall be created with createDevice() first.\n> >> + * This method shall be called after all members of the entities of the\n> >> + * media graph have been confirmed to be initialized.\n> >> + */\n> >> +void DeviceEnumerator::addDevice(const std::shared_ptr<MediaDevice> &media)\n> >> +{\n> >>  \tLOG(DeviceEnumerator, Debug)\n> >> -\t\t<< \"Added device \" << deviceNode << \": \" << media->driver();\n> >> -\n> >> -\tdevices_.push_back(std::move(media));\n> >> +\t\t<< \"Added device \" << media->deviceNode() << \": \" << media->driver();\n> >>  \n> >> -\treturn 0;\n> >> +\tdevices_.push_back(media);\n> >>  }\n> >>  \n> >>  /**\n> >> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp\n> >> index f3054d5..78a7da8 100644\n> >> --- a/src/libcamera/device_enumerator_sysfs.cpp\n> >> +++ b/src/libcamera/device_enumerator_sysfs.cpp\n> >> @@ -18,6 +18,7 @@\n> >>  #include <unistd.h>\n> >>  \n> >>  #include \"log.h\"\n> >> +#include \"media_device.h\"\n> >>  \n> >>  namespace libcamera {\n> >>  \n> >> @@ -32,6 +33,7 @@ int DeviceEnumeratorSysfs::enumerate()\n> >>  {\n> >>  \tstruct dirent *ent;\n> >>  \tDIR *dir;\n> >> +\tint ret = 0;\n> >>  \n> >>  \tstatic const char * const sysfs_dirs[] = {\n> >>  \t\t\"/sys/subsystem/media/devices\",\n> >> @@ -71,11 +73,42 @@ int DeviceEnumeratorSysfs::enumerate()\n> >>  \t\t\tcontinue;\n> >>  \t\t}\n> >>  \n> >> -\t\taddDevice(devnode);\n> >> +\t\tstd::shared_ptr<MediaDevice> media = createDevice(devnode);\n> >> +\t\tif (!media) {\n> >> +\t\t\tret = -ENODEV;\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\n> >> +\t\tif (populateMediaDevice(media) < 0) {\n> >> +\t\t\tret = -ENODEV;\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\n> >> +\t\taddDevice(media);\n> >>  \t}\n> >>  \n> >>  \tclosedir(dir);\n> >>  \n> >> +\treturn ret;\n> >> +}\n> >> +\n> >> +int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)\n> >> +{\n> >> +\t/* Associate entities to device node paths. */\n> >> +\tfor (MediaEntity *entity : media->entities()) {\n> >> +\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n> >> +\t\t\tcontinue;\n> >> +\n> >> +\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> >> +\t\t\t\t\t\t\t  entity->deviceMinor());\n> >> +\t\tif (deviceNode.empty())\n> >> +\t\t\treturn -EINVAL;\n> >> +\n> >> +\t\tint ret = entity->setDeviceNode(deviceNode);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\t}\n> >> +\n> >>  \treturn 0;\n> >>  }\n> >>  \n> >> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> >> index 86f6ca1..40853e7 100644\n> >> --- a/src/libcamera/device_enumerator_udev.cpp\n> >> +++ b/src/libcamera/device_enumerator_udev.cpp\n> >> @@ -7,6 +7,10 @@\n> >>  \n> >>  #include \"device_enumerator_udev.h\"\n> >>  \n> >> +#include <algorithm>\n> >> +#include <list>\n> >> +#include <map>\n> >> +\n> >>  #include <fcntl.h>\n> >>  #include <libudev.h>\n> >>  #include <string.h>\n> >> @@ -17,6 +21,7 @@\n> >>  #include <libcamera/event_notifier.h>\n> >>  \n> >>  #include \"log.h\"\n> >> +#include \"media_device.h\"\n> >>  \n> >>  namespace libcamera {\n> >>  \n> >> @@ -57,9 +62,40 @@ int DeviceEnumeratorUdev::init()\n> >>  \tif (ret < 0)\n> >>  \t\treturn ret;\n> >>  \n> >> +\tret = udev_monitor_filter_add_match_subsystem_devtype(monitor_, \"video4linux\",\n> >> +\t\t\t\t\t\t\t      nullptr);\n> >> +\tif (ret < 0)\n> >> +\t\treturn ret;\n> >> +\n> >>  \treturn 0;\n> >>  }\n> >>  \n> >> +int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n> >> +{\n> >> +\tconst char *subsystem = udev_device_get_subsystem(dev);\n> >> +\tif (!subsystem)\n> >> +\t\treturn -ENODEV;\n> >> +\n> >> +\tif (!strcmp(subsystem, \"media\")) {\n> >> +\t\tstd::shared_ptr<MediaDevice> media =\n> >> +\t\t\tcreateDevice(udev_device_get_devnode(dev));\n> >> +\t\tif (!media)\n> >> +\t\t\treturn -ENODEV;\n> >> +\n> >> +\t\tint ret = populateMediaDevice(media);\n> >> +\t\tif (ret == 0)\n> >> +\t\t\taddDevice(media);\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tif (!strcmp(subsystem, \"video4linux\")) {\n> >> +\t\taddV4L2Device(udev_device_get_devnum(dev));\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\treturn -ENODEV;\n> >> +}\n> >> +\n> >>  int DeviceEnumeratorUdev::enumerate()\n> >>  {\n> >>  \tstruct udev_enumerate *udev_enum = nullptr;\n> >> @@ -74,6 +110,14 @@ int DeviceEnumeratorUdev::enumerate()\n> >>  \tif (ret < 0)\n> >>  \t\tgoto done;\n> >>  \n> >> +\tret = udev_enumerate_add_match_subsystem(udev_enum, \"video4linux\");\n> >> +\tif (ret < 0)\n> >> +\t\tgoto done;\n> >> +\n> >> +\tret = udev_enumerate_add_match_is_initialized(udev_enum);\n> >> +\tif (ret < 0)\n> >> +\t\tgoto done;\n> >> +\n> >>  \tret = udev_enumerate_scan_devices(udev_enum);\n> >>  \tif (ret < 0)\n> >>  \t\tgoto done;\n> >> @@ -102,10 +146,12 @@ int DeviceEnumeratorUdev::enumerate()\n> >>  \t\t\tgoto done;\n> >>  \t\t}\n> >>  \n> >> -\t\taddDevice(devnode);\n> >> -\n> >> +\t\tret = addUdevDevice(dev);\n> >>  \t\tudev_device_unref(dev);\n> >> +\t\tif (ret < 0)\n> >> +\t\t\tbreak;\n> >>  \t}\n> >> +\n> >>  done:\n> >>  \tudev_enumerate_unref(udev_enum);\n> >>  \tif (ret < 0)\n> >> @@ -122,6 +168,43 @@ done:\n> >>  \treturn 0;\n> >>  }\n> >>  \n> >> +int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)\n> >> +{\n> >> +\tunsigned int pendingNodes = 0;\n> >> +\tint ret;\n> >> +\n> >> +\t/* Associate entities to device node paths. */\n> >> +\tfor (MediaEntity *entity : media->entities()) {\n> >> +\t\tif (entity->deviceMajor() == 0 && entity->deviceMinor() == 0)\n> >> +\t\t\tcontinue;\n> >> +\n> >> +\t\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> >> +\t\t\t\t\t\t\t  entity->deviceMinor());\n> >> +\t\tdev_t devnum = makedev(entity->deviceMajor(),\n> >> +\t\t\t\t       entity->deviceMinor());\n> >> +\n> >> +\t\t/* Take device from orphan list first, if it is in the list. */\n> >> +\t\tif (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) {\n> >> +\t\t\tif (deviceNode.empty())\n> >> +\t\t\t\treturn -EINVAL;\n> >> +\n> >> +\t\t\tret = entity->setDeviceNode(deviceNode);\n> >> +\t\t\tif (ret)\n> >> +\t\t\t\treturn ret;\n> >> +\n> >> +\t\t\torphans_.remove(devnum);\n> >> +\t\t\tcontinue;\n> >> +\t\t}\n> >> +\n> >> +\t\tdeps_[media].push_back(devnum);\n> >> +\t\tdevnumToDevice_[devnum] = media;\n> >> +\t\tdevnumToEntity_[devnum] = entity;\n> >> +\t\tpendingNodes++;\n> >> +\t}\n> >> +\n> >> +\treturn pendingNodes;\n> >> +}\n> >> +\n> >>  std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)\n> >>  {\n> >>  \tstruct udev_device *device;\n> >> @@ -143,6 +226,48 @@ std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)\n> >>  \treturn deviceNode;\n> >>  }\n> >>  \n> >> +/**\n> >> + * \\brief Add a V4L2 device to the media device that it belongs to\n> >> + * \\param[in] devnum major:minor number of V4L2 device to add, as a dev_t\n> >> + *\n> >> + * Add V4L2 device identified by \\a devnum to the MediaDevice that it belongs\n> >> + * to, if such a MediaDevice has been created. Otherwise add the V4L2 device\n> >> + * to the orphan list. If the V4L2 device is added to a MediaDevice, and it is\n> >> + * the last V4L2 device that the MediaDevice needs, then the MediaDevice is\n> >> + * added to the DeviceEnumerator, where it is available for pipeline handlers.\n> >> + *\n> >> + * \\return 0 on success or a negative error code otherwise\n> >> + */\n> >> +int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n> >> +{\n> >> +\tMediaEntity *entity = devnumToEntity_[devnum];\n> >> +\tif (!entity) {\n> >> +\t\torphans_.push_back(devnum);\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tstd::string deviceNode = lookupDeviceNode(entity->deviceMajor(),\n> >> +\t\t\t\t\t\t  entity->deviceMinor());\n> >> +\tif (deviceNode.empty())\n> >> +\t\treturn -EINVAL;\n> >> +\n> >> +\tint ret = entity->setDeviceNode(deviceNode);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tstd::shared_ptr<MediaDevice> media = devnumToDevice_[devnum];\n> >> +\tdeps_[media].remove(devnum);\n> >> +\tdevnumToDevice_.erase(devnum);\n> >> +\tdevnumToEntity_.erase(devnum);\n> >> +\n> >> +\tif (deps_[media].empty()) {\n> >> +\t\taddDevice(media);\n> >> +\t\tdeps_.erase(media);\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >>  void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)\n> >>  {\n> >>  \tstruct udev_device *dev = udev_monitor_receive_device(monitor_);\n> >> @@ -153,9 +278,11 @@ void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)\n> >>  \t\t<< action << \" device \" << udev_device_get_devnode(dev);\n> >>  \n> >>  \tif (action == \"add\") {\n> >> -\t\taddDevice(deviceNode);\n> >> +\t\taddUdevDevice(dev);\n> >>  \t} else if (action == \"remove\") {\n> >> -\t\tremoveDevice(deviceNode);\n> >> +\t\tconst char *subsystem = udev_device_get_subsystem(dev);\n> >> +\t\tif (subsystem && !strcmp(subsystem, \"media\"))\n> >> +\t\t\tremoveDevice(deviceNode);\n> >>  \t}\n> >>  \n> >>  \tudev_device_unref(dev);\n> >> diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h\n> >> index 02aec3b..c5d26f1 100644\n> >> --- a/src/libcamera/include/device_enumerator.h\n> >> +++ b/src/libcamera/include/device_enumerator.h\n> >> @@ -44,7 +44,8 @@ public:\n> >>  \tstd::shared_ptr<MediaDevice> search(const DeviceMatch &dm);\n> >>  \n> >>  protected:\n> >> -\tint addDevice(const std::string &deviceNode);\n> >> +\tstd::shared_ptr<MediaDevice> createDevice(const std::string &deviceNode);\n> >> +\tvoid addDevice(const std::shared_ptr<MediaDevice> &media);\n> >>  \tvoid removeDevice(const std::string &deviceNode);\n> >>  \n> >>  private:\n> >> diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h\n> >> index 8d3adc9..242b22b 100644\n> >> --- a/src/libcamera/include/device_enumerator_sysfs.h\n> >> +++ b/src/libcamera/include/device_enumerator_sysfs.h\n> >> @@ -7,10 +7,13 @@\n> >>  #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__\n> >>  #define __LIBCAMERA_DEVICE_ENUMERATOR_SYSFS_H__\n> >>  \n> >> +#include <memory>\n> >>  #include <string>\n> >>  \n> >>  #include \"device_enumerator.h\"\n> >>  \n> >> +class MediaDevice;\n> >> +\n> >>  namespace libcamera {\n> >>  \n> >>  class DeviceEnumeratorSysfs final : public DeviceEnumerator\n> >> @@ -20,7 +23,8 @@ public:\n> >>  \tint enumerate();\n> >>  \n> >>  private:\n> >> -\tstd::string lookupDeviceNode(int major, int minor);\n> >> +\tint populateMediaDevice(const std::shared_ptr<MediaDevice> &media);\n> >> +\tstd::string lookupDeviceNode(int major, int minor) final;\n> >>  };\n> >>  \n> >>  } /* namespace libcamera */\n> >> diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h\n> >> index 80f9372..5bdcdea 100644\n> >> --- a/src/libcamera/include/device_enumerator_udev.h\n> >> +++ b/src/libcamera/include/device_enumerator_udev.h\n> >> @@ -7,16 +7,23 @@\n> >>  #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__\n> >>  #define __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__\n> >>  \n> >> +#include <list>\n> >> +#include <map>\n> >> +#include <memory>\n> >>  #include <string>\n> >> +#include <sys/types.h>\n> >>  \n> >>  #include \"device_enumerator.h\"\n> >>  \n> >>  struct udev;\n> >> +struct udev_device;\n> >>  struct udev_monitor;\n> >>  \n> >>  namespace libcamera {\n> >>  \n> >>  class EventNotifier;\n> >> +class MediaDevice;\n> >> +class MediaEntity;\n> >>  \n> >>  class DeviceEnumeratorUdev : public DeviceEnumerator\n> >>  {\n> >> @@ -32,8 +39,17 @@ private:\n> >>  \tstruct udev_monitor *monitor_;\n> >>  \tEventNotifier *notifier_;\n> >>  \n> >> +\tstd::map<std::shared_ptr<MediaDevice>, std::list<dev_t>> deps_;\n> >> +\tstd::map<dev_t, std::shared_ptr<MediaDevice>> devnumToDevice_;\n> >> +\tstd::map<dev_t, MediaEntity *> devnumToEntity_;\n> >> +\n> >> +\tstd::list<dev_t> orphans_;\n> >> +\n> >> +\tint addUdevDevice(struct udev_device *dev);\n> >> +\tint populateMediaDevice(const std::shared_ptr<MediaDevice> &media);\n> >>  \tstd::string lookupDeviceNode(int major, int minor) final;\n> >>  \n> >> +\tint addV4L2Device(dev_t devnum);\n> >>  \tvoid udevNotify(EventNotifier *notifier);\n> >>  };\n> >>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8A850600E9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 13 Sep 2019 00:55:41 +0200 (CEST)","from pendragon.ideasonboard.com (bl10-204-24.dsl.telepac.pt\n\t[85.243.204.24])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 34FE333A;\n\tFri, 13 Sep 2019 00:55:40 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1568328941;\n\tbh=rHOyhjEr2Ap+W5iUvsbQ6rqiiFVwk23bAyv/MyBAhzk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mTU1SdBlz4Nwmgs3YHPqkr9K9VRPXQwmAQ85KozByRCI1KDEk8NpdeXRrgXmF8iAD\n\tHwC5wsfv119r8RBwEFzqk7xd3xOHQcEX1bfTxRKYwtDGXcuZ82YoaeyyeVA0GFJSvF\n\txEmFYm/UyMU57BbwEzrY8z13YQKEl/01Ntsrv2hc=","Date":"Fri, 13 Sep 2019 01:55:32 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190912225532.GD6006@pendragon.ideasonboard.com>","References":"<20190908050006.5565-1-paul.elder@ideasonboard.com>\n\t<20190908231101.GC4952@pendragon.ideasonboard.com>\n\t<5b7a39a9-dde3-0540-65ee-1e7b222b39e5@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<5b7a39a9-dde3-0540-65ee-1e7b222b39e5@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3] libcamera: device_enumerator: fix\n\tudev media graph loading dependency","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 12 Sep 2019 22:55:41 -0000"}}]