fix(camera): collapse a Brio's IR node into one Linux camera entry - #1234
fix(camera): collapse a Brio's IR node into one Linux camera entry#12344ni1ak wants to merge 2 commits into
Conversation
Greptile SummaryThe PR groups Linux V4L2 nodes by their canonical USB-device sysfs directory and retains one camera entry per device. The revised unknown-resolution fallback still depends on incidental device-node ordering, so it can retain an IR endpoint instead of the primary color endpoint.
Confidence Score: 4/5This PR should not merge until deduplication identifies the primary color endpoint without relying on incidental device-node ordering. The attempted fix for unknown primary resolution still preserves whichever sibling sorts first, allowing a secondary or IR endpoint to become the capture and control identifier. Files Needing Attention: crates/openlogi-camera/src/linux.rs
|
| Filename | Overview |
|---|---|
| crates/openlogi-camera/src/linux.rs | Adds USB-device grouping and resolution-based winner selection, but unknown resolution leaves endpoint selection dependent on lexicographic /dev/videoN ordering. |
| crates/openlogi-camera/src/lib.rs | Switches Linux enumeration to the new grouped camera list without otherwise changing platform behavior. |
Reviews (4): Last reviewed commit: "fix(camera): treat an unknown max_resolu..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
832f3dc to
dc9bf93
Compare
davidbudnick
added
type: bug
On Linux, the Brio exposes two capture-capable /dev/videoN nodes over UVC: the main color sensor and a low-resolution node feeding its IR sensor for Windows Hello. VIDIOC_ENUM_FMT reports capture formats on both, so linux.rs::is_capture_node() classified both as camera nodes and enumerate_cameras() listed the same physical webcam twice, with only one of the two entries carrying real controls. Group V4L2 nodes by their shared USB device sysfs directory (not the per-interface sysfs entry VIDIOC_QUERYCAP exposes) and keep only the highest-resolution node per group, so one physical camera again yields one Camera. node_for_unique_id() still resolves every node individually, so a secondary node stays reachable if some other code path needs it. Fixes AprilNEA#1191
merge_by_usb_device picked the highest-resolution node in each USB device group to decide which /dev/videoN capture node represents the camera. resolution_area(None) returned 0, so a primary sensor whose frame-size enumeration only reports stepwise/continuous ranges (or fails outright) lost to any sibling node with a discrete size — including a Brio's tiny IR sensor — and the returned unique_id pointed preview/control operations at the wrong node. Resolution now only decides the winner when both nodes report a known size; an unknown resolution no longer outranks, nor is outranked by, a known one, and the first-seen node keeps its place instead.
f012613 to
e64b04f
Compare
| if let (Some(candidate), Some(current)) = | ||
| (camera.max_resolution, best.max_resolution) | ||
| && resolution_area(candidate) > resolution_area(current) | ||
| { | ||
| *best = camera; | ||
| } |
There was a problem hiding this comment.
When the primary color node has an unknown resolution and an IR node sorts first—for example, /dev/video10 before /dev/video2—merge_by_usb_device() preserves the IR node. Its unique_id then becomes the capture and control identifier, so preview and control operations open the secondary sensor while the primary color endpoint disappears from enumeration.
Knowledge Base Used: Camera control integration
2 participants
Summary
/dev/videoNnodes: the main color sensor and a low-resolution node feeding its IR sensor (used for Windows Hello-style auth). Both nodes passVIDIOC_ENUM_FMT's capture-format check inlinux.rs::is_capture_node(), so both were previously classified as camera nodes and listed separately — matching the reporter'sopenlogi listoutput (4096x2160@120and340x340@30, same vendor/product id and serial).Changes
crates/openlogi-camera/src/linux.rs:Nodenow also carries the canonicalized sysfs directory of the underlying USB device (not the per-node USB interfaceVIDIOC_QUERYCAPexposes), shared by every capture node one physical camera exposes.cameras()/merge_by_usb_device(), which group described nodes by that shared USB device directory and keep only the highest-resolution entry per group.node_for_unique_id()(used by the control and capture-stream paths) is untouched and still resolves every node individually, so a secondary node (e.g. the IR node) stays independently addressable if some other code path ever needs it.crates/openlogi-camera/src/lib.rs: Linux'senumerate_all()now callslinux::cameras()instead of mappinglinux::nodes()directly.This is Linux-only, behind
#[cfg(target_os = "linux")]— macOS (AVFoundation) and Windows (DirectShow/Media Foundation) enumeration paths are untouched.Testing
cargo test -p openlogi-camera— includes new testsbrio_ir_node_collapses_into_the_main_capture_node,distinct_usb_devices_stay_separate,resolution_area_treats_unknown_resolution_as_smallest.cargo fmt --all -- --checkcargo tree --workspace --target all --invert openlogi-camera→openlogi-camera,openlogi-cli,openlogi,openlogi-desktop,openlogi-permissions):cargo clippy -p openlogi-camera -p openlogi-cli -p openlogi -p openlogi-permissions --all-targets -- -D warningscargo clippy -p openlogi-desktop --all-targets -- -D warningscargo test -p openlogi-camera -p openlogi-cli -p openlogi -p openlogi-permissions -p openlogi-desktopopenlogi listoutput (same vendor/product id and serial, differing max resolution).linux.rsbehindcfg(target_os = "linux"), so it cannot affect those platforms' enumeration paths.Fixes #1191