{"id":25336,"url":"https://patchwork.libcamera.org/api/1.1/patches/25336/?format=json","web_url":"https://patchwork.libcamera.org/patch/25336/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>","date":"2025-12-02T13:58:17","name":"[RFC,v1,3/3] libcamera: device_enumerator_udev: Handle duplicate devices","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"4ef323e3506ed48c3e286e1a91de2b42a084b3ae","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/1.1/people/216/?format=json","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/25336/mbox/","series":[{"id":5632,"url":"https://patchwork.libcamera.org/api/1.1/series/5632/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5632","date":"2025-12-02T13:58:15","name":"[RFC,v1,1/3] libcamera: device_enumerator_udev: Add `override`","version":1,"mbox":"https://patchwork.libcamera.org/series/5632/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/25336/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/25336/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 1C460C326B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  2 Dec 2025 13:58:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 83E2E60C8A;\n\tTue,  2 Dec 2025 14:58:26 +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 F2A0B60C85\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Dec 2025 14:58:21 +0100 (CET)","from pb-laptop.local (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DBDE53E6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Dec 2025 14:56:07 +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=\"GjsheDcT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764683767;\n\tbh=jPB0+IUU8YzDDMW47FMXdlxIgmHC2QlYdky5+D22dpw=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=GjsheDcTFxe6jVp2MImDh8kdpWsnDBFHlc2Ges3AMChkSf/VVvOZ7sUm9pd0zuViY\n\tJUi05xvcBPE4p1XlB3GIuzSD9cT/GfXYiil2Y8gLS0CNZz6HfF+EvZnqS4dvPMcKv5\n\t55Ag7pXWJLdqUHmozKHWSaJj+/DnjWxG/siM3ACE=","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Subject":"[RFC PATCH v1 3/3] libcamera: device_enumerator_udev: Handle\n\tduplicate devices","Date":"Tue,  2 Dec 2025 14:58:17 +0100","Message-ID":"<20251202135817.1607250-3-barnabas.pocze@ideasonboard.com>","X-Mailer":"git-send-email 2.52.0","In-Reply-To":"<20251202135817.1607250-1-barnabas.pocze@ideasonboard.com>","References":"<20251202135817.1607250-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\nThis can most easily happen as a result of multiple `enumerate()` calls,\nhowever, currently `enumerate()` is only ever called once, so that won't\ntrigger the issue.\n\nNonetheless, it is still possible to trigger this issue due to the fact\nthat there is a time window after the `udev_monitor` has been created in\n`init()` and the first (and only) enumeration done in `enumerate()`. If\ne.g. a UVC camera is connected in this time frame, then it is possible\nthat it will be processed both by the `udevNotify()` and the initial\n`enumerate()`, leading to the fatal error. This can be reproduced as\nfollows:\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\nany number of `enumerate()` calls and hotplug events will work correctly\n(assuming that udev reports \"add\" / \"remove\" events correctly).\n\nCloses: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293\nSigned-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n---\n .../internal/device_enumerator_udev.h         |  3 ++\n src/libcamera/device_enumerator_udev.cpp      | 38 ++++++++++++++-----\n 2 files changed, 32 insertions(+), 9 deletions(-)","diff":"diff --git a/include/libcamera/internal/device_enumerator_udev.h b/include/libcamera/internal/device_enumerator_udev.h\nindex c2f7154b6b..3de7c37104 100644\n--- a/include/libcamera/internal/device_enumerator_udev.h\n+++ b/include/libcamera/internal/device_enumerator_udev.h\n@@ -11,6 +11,7 @@\n #include <map>\n #include <memory>\n #include <set>\n+#include <unordered_set>\n #include <string>\n #include <sys/types.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..406e59b360 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 already\n+\t * been seen (e.g. multiple `enumerate()` calls, device added between udev\n+\t * monitor creation in `init()` and `enumerate()`). This record is kept even\n+\t * if later in this function an error is encountered. Only a \"remove\" event from\n+\t * 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","v1","3/3"]}