Surface stable device_id in webcam discovery results#16
Merged
Conversation
Return the OS-provided stable identifier as a device_id attribute so users can correlate discovered cameras with physical devices across reboots. Documents per-platform stability caveats (by-id vs by-path on Linux, USB locationID on macOS, friendly name on Windows). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Nicolas Palpacuer (NickPPC)
approved these changes
Apr 13, 2026
sean yu (hexbabe)
suggested changes
Apr 14, 2026
Comment on lines
+127
to
+132
| // label feeds video_path (used to OPEN the camera); deviceID is the stable | ||
| // identifier surfaced as metadata. On Linux they diverge — label becomes the | ||
| // resolved /dev/videoN basename while deviceID stays as the by-id/by-path | ||
| // name from labelParts[0]. On macOS/Windows pion exposes a single string | ||
| // (AVFoundation UID / DirectShow friendly name) that serves both roles, so | ||
| // label == deviceID is intentional, not duplication. |
Contributor
There was a problem hiding this comment.
can we revert to the old comment?
and then put
// OS-provided stable identifier for the device that persists across reboots
above deviceID?
| | **macOS** | `AVCaptureDevice.uniqueID` | Yes for built-in cameras; yes for USB cameras **on the same port** | **No** for USB (Apple derives the ID from USB `locationID` + vendor/product) | | | ||
| | **Windows** | DirectShow friendly name (e.g. `Logitech HD Pro Webcam C920`) | Yes | Yes | Collides for two identical devices. Can change if the driver renames the device. | | ||
|
|
||
| On Linux, the service prefers the most stable identifier available: `by-id` → `by-path` → `videoN`. If you need a serial-number-level identifier for tracking a physical camera across reboots *and* replugs, you must rely on `by-id`, which in turn requires the device to expose a USB serial. |
Contributor
There was a problem hiding this comment.
I don't want the README to get too bloaty over just one new returned field. Can we remove this or shorten it a lot, and keep just the Discovery output section?
|
|
||
| // device_id should be surfaced in Attributes for every result. | ||
| for _, config := range resp { | ||
| test.That(t, config.Attributes["device_id"], test.ShouldEqual, "some_label") |
Contributor
There was a problem hiding this comment.
I'm confused. Label != device_id
why are we asserting that it equals our hardcoded labels str val?
Like why is this test passing. I think it should equal "some_device_id" no?
I think this is likely just a mock fix, but we should def assert against "some_device_id" (and plumb that through our test logic).
| test.That(t, resp, test.ShouldHaveLength, 1) | ||
| test.That(t, resp[0].Attributes["device_id"], test.ShouldEqual, | ||
| "usb-046d_Logitech_Webcam_C920_A1B2C3D4-video-index0") | ||
| cfg, _ := resp[0].ConvertedAttributes.(videosource.WebcamConfig) |
Contributor
There was a problem hiding this comment.
[nit] instead of swallowing the ok, can we assert it is true?
- Restore the original Linux/macOS label comment and add a one-liner above deviceID. - Shorten the README Discovery output section — drop the per-platform stability table. - Plumb a distinct device_id through the mock driver (Linux-style "<stable id>;<device node>" label) so the basic test asserts the split end-to-end. Drops the separate Linux subtest it replaces. - Assert the WebcamConfig type assertion ok bool instead of swallowing it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Return the OS-provided stable identifier as a device_id attribute so users can correlate discovered cameras with physical devices across reboots. Documents per-platform stability caveats (by-id vs by-path on Linux, USB locationID on macOS, friendly name on Windows).