fix(camera): collapse a Brio's IR node into one Linux camera entry

crates/openlogi-camera/src/lib.rs

@@ -315,7 +315,7 @@ fn enumerate_all() -> Vec<Camera> {
 
 #[cfg(target_os = "linux")]
 fn enumerate_all() -> Vec<Camera> {
-    linux::nodes().iter().map(linux::describe).collect()
+    linux::cameras()
 }
 
 #[cfg(not(any(target_os = "macos", target_os = "windows", target_os = "linux")))]

crates/openlogi-camera/src/linux.rs

@@ -31,6 +31,10 @@ pub(crate) struct Node {
     pub(crate) product_id: u16,
     /// USB `iSerialNumber` from sysfs when the device reports one.
     pub(crate) serial_number: Option<String>,
+    /// Canonicalized sysfs directory of the *USB device* (not interface)
+    /// behind this node — shared by every capture node the same physical
+    /// device exposes. See [`cameras`].
+    usb_device: PathBuf,
 }
 
 /// Enumerate every V4L2 capture node, newest-first by node index.
@@ -48,7 +52,8 @@ pub(crate) fn nodes() -> Vec<Node> {
         .filter_map(|entry| {
             let sysfs = entry.path();
             let dev_path = PathBuf::from("/dev").join(entry.file_name());
-            let (vendor_id, product_id) = usb_ids(&sysfs)?;
+            let usb_device = usb_device_sysfs(&sysfs)?;
+            let (vendor_id, product_id) = usb_ids(&usb_device)?;
             if !is_capture_node(&dev_path) {
                 return None;
             }
@@ -58,7 +63,8 @@ pub(crate) fn nodes() -> Vec<Node> {
                 path: dev_path,
                 vendor_id,
                 product_id,
-                serial_number: usb_serial(&sysfs),
+                serial_number: usb_serial(&usb_device),
+                usb_device,
             })
         })
         .collect();
@@ -99,6 +105,100 @@ pub(crate) fn describe(node: &Node) -> Camera {
     }
 }
 
+/// One [`Camera`] per physical USB device.
+///
+/// A UVC camera can expose more than one *streaming* interface — e.g. the
+/// Brio's second, low-resolution node feeding its IR sensor for Windows
+/// Hello — and each gets its own `/dev/videoN` capture node just like the
+/// main color sensor does (issue #1191). Grouping by the shared USB device
+/// directory and keeping only the highest-resolution node per group turns
+/// that back into one listing entry per physical camera; `node_for_unique_id`
+/// still resolves every node individually, so a secondary node stays
+/// controllable if some other path ever needs it.
+pub(crate) fn cameras() -> Vec<Camera> {
+    let described = nodes().into_iter().map(|node| {
+        let is_color = Device::with_path(&node.path).is_ok_and(|device| has_color_format(&device));
+        (node.usb_device.clone(), describe(&node), is_color)
+    });
+    merge_by_usb_device(described)
+}
+
+/// Collapse `(usb_device, Camera, is_color)` triples to one `Camera` per
+/// distinct `usb_device`, and otherwise preserving first-seen order.
+///
+/// A node's resolution is `None` when its primary format only ever reported
+/// stepwise/continuous frame sizes, or enumeration failed outright — that is
+/// *unknown*, not "0x0". Ranking it below any node with a discrete size would
+/// let a tiny-but-known secondary/IR node (see [`cameras`]) outrank an
+/// unmeasured primary sensor and steal its `unique_id`. So resolution only
+/// ever decides the winner when both sides are known.
+///
+/// When resolution can't decide (either side unknown, or a tie), `is_color`
+/// breaks it instead: a node that reports at least one non-monochrome pixel
+/// format (see [`has_color_format`]) wins over a mono-only IR/depth node,
+/// regardless of which `/dev/videoN` enumerated first — node numbering order
+/// is not a reliable signal (issue: an IR node like `/dev/video10` can sort
+/// before the color node `/dev/video2`). Only when neither resolution nor
+/// color-capability can decide does the first-seen node keep its place, same
+/// as an exact tie.
+fn merge_by_usb_device(nodes: impl IntoIterator<Item = (PathBuf, Camera, bool)>) -> Vec<Camera> {
+    let mut by_device: Vec<(PathBuf, Camera, bool)> = Vec::new();
+    for (usb_device, camera, is_color) in nodes {
+        match by_device.iter_mut().find(|(dev, _, _)| *dev == usb_device) {
+            Some((_, best, best_is_color)) => {
+                let by_resolution = match (camera.max_resolution, best.max_resolution) {
+                    (Some(candidate), Some(current)) => {
+                        Some(resolution_area(candidate) > resolution_area(current))
+                    }
+                    _ => None,
+                };
+                let candidate_wins = by_resolution.unwrap_or(is_color && !*best_is_color);
+                if candidate_wins {
+                    *best = camera;
+                    *best_is_color = is_color;
+                }
+            }
+            None => by_device.push((usb_device, camera, is_color)),
+        }
+    }
+    by_device.into_iter().map(|(_, camera, _)| camera).collect()
+}
+
+/// Whether `device` reports at least one pixel format that isn't a
+/// known monochrome-only V4L2 format.
+///
+/// UVC webcams with a secondary IR/depth sensor (e.g. the Brio's Windows
+/// Hello node, issue #1191) expose it as a plain capture node just like the
+/// primary color sensor, so it can't be told apart by `/dev/videoN` order or
+/// resolution alone. IR sensors report single-channel formats (`GREY`/`Y8`,
+/// `Y10`, `Y12`, `Y16`) where the color sensor reports YUV/RGB/compressed
+/// formats, so this is checked instead of relying on enumeration order. A
+/// node whose format list can't be read is *not* claimed to be a color node.
+fn has_color_format(device: &Device) -> bool {
+    let Ok(formats) = device.enum_formats() else {
+        return false;
+    };
+    formats
+        .iter()
+        .any(|format| !is_monochrome_fourcc(format.fourcc))
+}
+
+/// Whether `fourcc` names a known monochrome-only V4L2 pixel format (as
+/// opposed to a YUV/RGB/Bayer/compressed one carrying color information).
+fn is_monochrome_fourcc(fourcc: FourCC) -> bool {
+    matches!(
+        &fourcc.repr,
+        b"GREY" | b"Y8  " | b"Y10 " | b"Y12 " | b"Y16 "
+    )
+}
+
+/// Pixel count of a resolution, for comparing which capture node is the
+/// primary sensor.
+fn resolution_area(resolution: (u32, u32)) -> u64 {
+    let (w, h) = resolution;
+    u64::from(w) * u64::from(h)
+}
+
 /// The `by-id` symlink for `path` when udev created one (it embeds the USB
 /// serial, so it survives replugging into another port), else the raw node
 /// path. Either way it round-trips through [`node_for_unique_id`].
@@ -116,11 +216,8 @@ fn unique_id_for(path: &Path) -> String {
         .to_string()
 }
 
-/// Read `idVendor`/`idProduct` from the USB device behind a V4L2 node.
-///
-/// `<sysfs>/device` is the USB *interface*; its parent holds the ids.
-fn usb_ids(sysfs: &Path) -> Option<(u16, u16)> {
-    let usb = usb_device_sysfs(sysfs)?;
+/// Read `idVendor`/`idProduct` from the USB device directory.
+fn usb_ids(usb: &Path) -> Option<(u16, u16)> {
     let vendor = read_trimmed(&usb.join("idVendor"))?;
     let product = read_trimmed(&usb.join("idProduct"))?;
     Some((
@@ -129,9 +226,9 @@ fn usb_ids(sysfs: &Path) -> Option<(u16, u16)> {
     ))
 }
 
-/// USB `iSerialNumber` from the parent USB device, when present and non-empty.
-fn usb_serial(sysfs: &Path) -> Option<String> {
-    let usb = usb_device_sysfs(sysfs)?;
+/// USB `iSerialNumber` from the USB device directory, when present and
+/// non-empty.
+fn usb_serial(usb: &Path) -> Option<String> {
     let serial = read_trimmed(&usb.join("serial"))?;
     let serial = serial.trim();
     // Kernel placeholder when the descriptor has no iSerialNumber.
@@ -213,3 +310,106 @@ fn read_trimmed(path: &Path) -> Option<String> {
         .ok()
         .map(|text| text.trim().to_string())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn camera(name: &str, max_resolution: Option<(u32, u32)>) -> Camera {
+        Camera {
+            name: name.to_string(),
+            unique_id: name.to_string(),
+            serial_number: Some("5091F273".to_string()),
+            vendor_id: 0x046d,
+            product_id: 0x085e,
+            max_resolution,
+            max_fps: None,
+        }
+    }
+
+    #[test]
+    fn brio_ir_node_collapses_into_the_main_capture_node() {
+        // Reproduces issue #1191: the Brio's two capture-capable /dev/videoN
+        // nodes (main sensor + IR sensor for Windows Hello) share one USB
+        // device directory and must collapse to a single listing entry.
+        let usb_device = PathBuf::from("/sys/devices/usb1/1-1");
+        let main = camera("Logitech BRIO", Some((4096, 2160)));
+        let ir = camera("Logitech BRIO", Some((340, 340)));
+
+        let cameras = merge_by_usb_device([
+            (usb_device.clone(), main.clone(), true),
+            (usb_device, ir, false),
+        ]);
+
+        assert_eq!(cameras, vec![main]);
+    }
+
+    #[test]
+    fn distinct_usb_devices_stay_separate() {
+        let one = camera("Logitech BRIO", Some((4096, 2160)));
+        let two = camera("Logitech StreamCam", Some((1920, 1080)));
+
+        let cameras = merge_by_usb_device([
+            (PathBuf::from("/sys/devices/usb1/1-1"), one.clone(), true),
+            (PathBuf::from("/sys/devices/usb1/1-2"), two.clone(), true),
+        ]);
+
+        assert_eq!(cameras, vec![one, two]);
+    }
+
+    #[test]
+    fn resolution_area_compares_pixel_counts() {
+        assert_eq!(resolution_area((340, 340)), 340 * 340);
+        assert!(resolution_area((4096, 2160)) > resolution_area((340, 340)));
+    }
+
+    #[test]
+    fn unknown_resolution_does_not_lose_to_a_known_smaller_node() {
+        // Reproduces the failure mode from the #1234 review: if the primary
+        // node only reports stepwise/continuous frame sizes (or enumeration
+        // fails), `max_resolution` is `None`, not "0x0". It must not be
+        // outranked by a sibling IR/secondary node just because that node
+        // happens to report a small discrete size.
+        let usb_device = PathBuf::from("/sys/devices/usb1/1-1");
+        let primary_unknown = camera("Logitech BRIO", None);
+        let ir = camera("Logitech BRIO", Some((340, 340)));
+
+        let cameras = merge_by_usb_device([
+            (usb_device.clone(), primary_unknown.clone(), true),
+            (usb_device, ir, false),
+        ]);
+
+        assert_eq!(cameras, vec![primary_unknown]);
+    }
+
+    #[test]
+    fn color_node_wins_over_an_ir_node_that_enumerates_first() {
+        // Reproduces the second #1234 review finding: when both the color
+        // and IR node have unknown resolution, `/dev/videoN` enumeration
+        // order is not a reliable tiebreaker — an IR node such as
+        // `/dev/video10` can sort before its sibling color node
+        // `/dev/video2` (nodes() sorts lexicographically by path, and "1" <
+        // "2"). The IR node here is first-seen and would win under plain
+        // "first-seen wins", but `is_color` must override that.
+        let usb_device = PathBuf::from("/sys/devices/usb1/1-1");
+        let ir_seen_first = camera("video10-ir", None);
+        let color_seen_second = camera("video2-color", None);
+
+        let cameras = merge_by_usb_device([
+            (usb_device.clone(), ir_seen_first, false),
+            (usb_device, color_seen_second.clone(), true),
+        ]);
+
+        assert_eq!(cameras, vec![color_seen_second]);
+    }
+
+    #[test]
+    fn is_monochrome_fourcc_recognizes_known_ir_formats() {
+        for code in [b"GREY", b"Y8  ", b"Y10 ", b"Y12 ", b"Y16 "] {
+            assert!(is_monochrome_fourcc(FourCC::new(code)));
+        }
+        for code in [b"YUYV", b"MJPG", b"NV12"] {
+            assert!(!is_monochrome_fourcc(FourCC::new(code)));
+        }
+    }
+}