Patch Detail
Show a patch.
GET /api/1.1/patches/25388/?format=api
{ "id": 25388, "url": "https://patchwork.libcamera.org/api/1.1/patches/25388/?format=api", "web_url": "https://patchwork.libcamera.org/patch/25388/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20251208110035.248881-3-barnabas.pocze@ideasonboard.com>", "date": "2025-12-08T11:00:35", "name": "[RFC,v2,3/3] libcamera: device_enumerator_udev: Handle duplicate devices", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "ce5af863359161ffe04c5f3cdc75a0fd7b0eef31", "submitter": { "id": 216, "url": "https://patchwork.libcamera.org/api/1.1/people/216/?format=api", "name": "Barnabás Pőcze", "email": "barnabas.pocze@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/25388/mbox/", "series": [ { "id": 5642, "url": "https://patchwork.libcamera.org/api/1.1/series/5642/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5642", "date": "2025-12-08T11:00:33", "name": "[RFC,v2,1/3] libcamera: device_enumerator_udev: Add `override`", "version": 2, "mbox": "https://patchwork.libcamera.org/series/5642/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/25388/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/25388/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B58DDBDDFC\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 8 Dec 2025 11:00:45 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B4FC461406;\n\tMon, 8 Dec 2025 12:00:43 +0100 (CET)", "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 9B4D8613F7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 8 Dec 2025 12:00:40 +0100 (CET)", "from pb-laptop.local (185.221.142.68.nat.pool.zt.hu\n\t[185.221.142.68])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 321F0C7A;\n\tMon, 8 Dec 2025 11:58:21 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sUuDe+3n\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765191501;\n\tbh=D1liksjrLRPHgt0evjnO37S4z3Yg2269EbVTF3HMU3Q=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=sUuDe+3nirVoYftnNVI1V8CijNdGJtp1xt3Q9IulbybP/PnXaV1tC0zae8F93TmLu\n\tyNhmLE78v3NBIh6sUY1HZ4bEdw03n16qPZyGslBBcCVm/jWTWQSKDyGDI2nSq58Dyx\n\ttOE6hpAK7Hz6/WSIsMkSOd+ld/wUiGVDj0xmuW4Y=", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>", "Subject": "[RFC PATCH v2 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices", "Date": "Mon, 8 Dec 2025 12:00:35 +0100", "Message-ID": "<20251208110035.248881-3-barnabas.pocze@ideasonboard.com>", "X-Mailer": "git-send-email 2.52.0", "In-Reply-To": "<20251208110035.248881-1-barnabas.pocze@ideasonboard.com>", "References": "<20251208110035.248881-1-barnabas.pocze@ideasonboard.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "It is possible that same device is processed multiple times, leading to\nmultiple `MediaDevice`s being instantiated, mostly likely leading to\na fatal error:\n\n Trying to register a camera with a duplicated ID xyzw...\n\nThere is a time window after the `udev_monitor` has been created in `init()`\nand the first (and only) enumeration done in `enumerate()`. If e.g. a UVC\ncamera is connected in this time frame, then it is possible that it will be\nprocessed both by the `udevNotify()` and the initial `enumerate()`, leading\nto the fatal error. This can be reproduced as follows:\n\n 1. $ gdb --args cam -m\n 2. (gdb) break libcamera::DeviceEnumeratorUdev::enumerate\n 3. (gdb) run\n 4. when the breakpoint is hit, connect a usb camera\n 5. (gdb) continue\n 6. observe fatal error\n\nTo address this, keep track of the devnums of all devices reported by\nudev, and reject devices with already known devnums. This ensures that\nthe same device won't be reported multiple times (assuming that udev\nreports \"add\" / \"remove\" events in the correct order).\n\nCloses: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\nSigned-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nchanges in v2:\n * address review comments\n\nv1: https://patchwork.libcamera.org/patch/25336/\n---\n .../internal/device_enumerator_udev.h | 3 ++\n src/libcamera/device_enumerator_udev.cpp | 38 ++++++++++++++-----\n 2 files changed, 32 insertions(+), 9 deletions(-)\n\n--\n2.52.0", "diff": "diff --git a/include/libcamera/internal/device_enumerator_udev.h b/include/libcamera/internal/device_enumerator_udev.h\nindex c2f7154b6b..958e27a2f1 100644\n--- a/include/libcamera/internal/device_enumerator_udev.h\n+++ b/include/libcamera/internal/device_enumerator_udev.h\n@@ -13,6 +13,7 @@\n #include <set>\n #include <string>\n #include <sys/types.h>\n+#include <unordered_set>\n\n #include <libcamera/base/class.h>\n\n@@ -59,6 +60,7 @@ private:\n \tLIBCAMERA_DISABLE_COPY_AND_MOVE(DeviceEnumeratorUdev)\n\n \tint addUdevDevice(struct udev_device *dev);\n+\tvoid removeUdevDevice(struct udev_device *dev);\n \tint populateMediaDevice(MediaDevice *media, DependencyMap *deps);\n \tstd::string lookupDeviceNode(dev_t devnum);\n\n@@ -70,6 +72,7 @@ private:\n \tEventNotifier *notifier_;\n\n \tstd::set<dev_t> orphans_;\n+\tstd::unordered_set<dev_t> devices_;\n \tstd::list<MediaDeviceDeps> pending_;\n \tstd::map<dev_t, MediaDeviceDeps *> devMap_;\n };\ndiff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\nindex 4e20a3cc0c..3195dd06e2 100644\n--- a/src/libcamera/device_enumerator_udev.cpp\n+++ b/src/libcamera/device_enumerator_udev.cpp\n@@ -76,6 +76,21 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n \tif (!subsystem)\n \t\treturn -ENODEV;\n\n+\t/*\n+\t * Record that udev reported the given devnum. And reject if it has\n+\t * already been seen (e.g. device added between udev monitor creation\n+\t * in `init()` and `enumerate()`). This record is kept even if later\n+\t * in this function an error is encountered. Only a \"remove\" event\n+\t * from udev should erase it from `devices_`.\n+\t */\n+\tconst dev_t devnum = udev_device_get_devnum(dev);\n+\tif (devnum == makedev(0, 0))\n+\t\treturn -ENODEV;\n+\n+\tconst auto [it, inserted] = devices_.insert(devnum);\n+\tif (!inserted)\n+\t\treturn -EEXIST;\n+\n \tif (!strcmp(subsystem, \"media\")) {\n \t\tstd::unique_ptr<MediaDevice> media =\n \t\t\tcreateDevice(udev_device_get_devnode(dev));\n@@ -111,13 +126,22 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)\n \t}\n\n \tif (!strcmp(subsystem, \"video4linux\")) {\n-\t\taddV4L2Device(udev_device_get_devnum(dev));\n+\t\taddV4L2Device(devnum);\n \t\treturn 0;\n \t}\n\n \treturn -ENODEV;\n }\n\n+void DeviceEnumeratorUdev::removeUdevDevice(struct udev_device *dev)\n+{\n+\tconst char *subsystem = udev_device_get_subsystem(dev);\n+\tif (subsystem && !strcmp(subsystem, \"media\"))\n+\t\tremoveDevice(udev_device_get_devnode(dev));\n+\n+\tdevices_.erase(udev_device_get_devnum(dev));\n+}\n+\n int DeviceEnumeratorUdev::enumerate()\n {\n \tstruct udev_enumerate *udev_enum = nullptr;\n@@ -341,18 +365,14 @@ void DeviceEnumeratorUdev::udevNotify()\n \t}\n\n \tstd::string_view action(udev_device_get_action(dev));\n-\tstd::string_view deviceNode(udev_device_get_devnode(dev));\n\n \tLOG(DeviceEnumerator, Debug)\n-\t\t<< action << \" device \" << deviceNode;\n+\t\t<< action << \" device \" << udev_device_get_devnode(dev);\n\n-\tif (action == \"add\") {\n+\tif (action == \"add\")\n \t\taddUdevDevice(dev);\n-\t} else if (action == \"remove\") {\n-\t\tconst char *subsystem = udev_device_get_subsystem(dev);\n-\t\tif (subsystem && !strcmp(subsystem, \"media\"))\n-\t\t\tremoveDevice(std::string(deviceNode));\n-\t}\n+\telse if (action == \"remove\")\n+\t\tremoveUdevDevice(dev);\n\n \tudev_device_unref(dev);\n }\n", "prefixes": [ "RFC", "v2", "3/3" ] }