macOS: capture RealSense UVC interfaces from UVCAssistant for reliable (multi-)camera streaming#15243
Conversation
…n tools with device-access entitlement
|
Can one of the admins verify this patch? |
sysrsbuild-gh-agentic
left a comment
sysrsbuild-gh-agentic
left a comment
There was a problem hiding this comment.
Automated Code Review — rs-agentic bot
Overall the PR is well-motivated and the problem description is thorough. The core ideas (explicit libusb_detach_kernel_driver before claiming, exception-safe constructor cleanup, outer open/claim retry with backoff) are sound. Several correctness and consistency issues noted below; see inline comments for details.
Missing automated tests: The test plan is entirely manual. No automated tests are provided for:
- The macOS-specific retry/backoff logic in
usb_device_libusb::get_handle - The exception-safety of the
handle_libusbconstructor (cleanup on partial-claim failure) - The CMake codesign step (at minimum a macOS CI build job should be added)
The project's test framework lives in unit-tests/; please add at least a build-level CI job for macOS that verifies the codesign step runs without error (codesign -v -d --entitlements - on a produced binary).
| _first_interface(interface), _context(context), _handle(nullptr) | ||
| { | ||
| auto sts = libusb_open(device, &_handle); | ||
| if (sts == LIBUSB_ERROR_BUSY || sts == LIBUSB_ERROR_ACCESS) |
There was a problem hiding this comment.
🐛 BUG [rs-agentic bot] — libusb_open retries on ACCESS contradict the PR's own rationale
The PR description explicitly states: "Inner claim retry reverted to BUSY-only (tight-looping on ACCESS just re-triggered device resets)." Yet this loop retries libusb_open on both LIBUSB_ERROR_BUSY and LIBUSB_ERROR_ACCESS (up to 20 × 50 ms = 1 s). On macOS, every libusb_open attempt on a device held by UVCAssistant triggers a USBDeviceReEnumerate call internally, so hammering it on ACCESS causes exactly the same perpetual-reset problem described for the claim retry. This loop should be restricted to LIBUSB_ERROR_BUSY only, consistent with the reasoning applied to claim_interface_or_throw.
// Suggested fix: restrict to BUSY only
if (sts == LIBUSB_ERROR_BUSY)
{
auto retry_counter = 20;
do {
...
} while (sts == LIBUSB_ERROR_BUSY && --retry_counter > 0);
}| // with the com.apple.vm.device-access capture entitlement, or disable the camera | ||
| // assistant entirely. This detach maximizes our chances; it is not a guarantee. | ||
| { | ||
| auto detach_sts = libusb_detach_kernel_driver(_handle, interface->get_number()); |
There was a problem hiding this comment.
libusb_detach_kernel_driver only called for the primary interface
libusb_detach_kernel_driver is invoked only for interface->get_number() (the VideoControl interface, typically interface 0). However, UVCAssistant holds the VideoStreaming interfaces (interfaces 1, 2, …) exclusively — those are the ones that actually appear as UsbExclusiveOwner in IORegistry. Triggering capture-mode re-enumeration on interface 0 alone does not guarantee that the OS releases all streaming interfaces fast enough before the claim below.
Consider also calling detach for each associated interface:
for (auto&& assoc : interface->get_associated_interfaces())
{
auto d = libusb_detach_kernel_driver(_handle, assoc->get_number());
if (d != LIBUSB_SUCCESS &&
d != LIBUSB_ERROR_NOT_SUPPORTED &&
d != LIBUSB_ERROR_NOT_FOUND)
LOG_WARNING("failed to capture associated interface " << (int)assoc->get_number() << ": " << d);
}| // will NOT run. Release whatever we managed to claim and close the device | ||
| // here; otherwise the leaked open handle keeps the device busy and defeats | ||
| // the higher-level open retry in usb_device_libusb::get_handle. | ||
| for(auto&& i : interface->get_associated_interfaces()) |
There was a problem hiding this comment.
🐛 BUG [rs-agentic bot] — catch block unconditionally releases ALL associated interfaces, including ones never claimed
If claim_interface_or_throw succeeds for the primary interface but then fails midway through the associated-interface loop (e.g., the 2nd of 4 associated interfaces throws), the catch block still calls libusb_release_interface on all associated interfaces — including those that were never claimed. While libusb_release_interface on an unclaimed interface is benign on most platforms, on macOS it can produce spurious LIBUSB_ERROR_NOT_FOUND error-level log entries that are confusing and could mask real failures.
A more precise cleanup would track which interfaces were successfully claimed:
std::vector<uint8_t> claimed_so_far;
try {
claim_interface_or_throw(interface->get_number());
claimed_so_far.push_back(interface->get_number());
for (auto&& i : interface->get_associated_interfaces()) {
claim_interface_or_throw(i->get_number());
claimed_so_far.push_back(i->get_number());
}
} catch (...) {
for (auto n : claimed_so_far)
libusb_release_interface(_handle, n);
libusb_close(_handle);
_handle = nullptr;
throw;
}| << " after " << max_attempts << " attempts: " << e.what()); | ||
| return nullptr; | ||
| } | ||
| const int delay_ms = 300 * attempt; // 300, 600, 900, 1200 ms backoff |
There was a problem hiding this comment.
300 * attempt and max_attempts = 5 are raw magic numbers. These should be constexpr named constants to make the intent clear and the values easy to tune:
constexpr int k_macos_open_retry_max = 5;
constexpr int k_macos_open_retry_base_ms = 300; // 300, 600, 900, 1200 msAlso note: worst-case total blocking time is 300+600+900+1200 = 3 seconds of synchronous sleep in get_handle. If this is ever called from a UI or time-critical thread, it will cause a visible hang. Worth documenting or adding a LOG_WARNING at the call site.
| } | ||
| catch(const std::exception& e) | ||
| { | ||
| return nullptr; |
There was a problem hiding this comment.
🐛 BUG [rs-agentic bot] — Non-Apple path silently swallows exceptions; e is unused
The #else catch block catches std::exception& e but never logs or uses e — it just returns nullptr. This is a pre-existing issue, but this PR restructured the code around it and could have addressed it. Silent failures here make post-crash debugging very difficult: the caller sees nullptr with no indication of why the handle could not be opened.
At minimum, log the exception:
catch(const std::exception& e)
{
LOG_ERROR("failed to open usb handle for interface " << (int)interface_number << ": " << e.what());
return nullptr;
}| @@ -0,0 +1,83 @@ | |||
| # License: Apache 2.0. See LICENSE file in root directory. | |||
There was a problem hiding this comment.
Per the project's naming conventions (.github/copilot-instructions.md):
Files: kebab-case (e.g.,
backend-v4l2.h,device-model.cpp)
This file is named macos_codesign.cmake (snake_case). It should be renamed to macos-codesign.cmake to be consistent with the project conventions. The include(CMake/macos_codesign.cmake) reference in CMakeLists.txt would need to be updated accordingly.
| COMMAND codesign --force --sign - --entitlements "${LRS_MACOS_DEVICE_ACCESS_ENTITLEMENTS}" --timestamp=none "$<TARGET_FILE:${_exe}>") | ||
| endforeach() | ||
|
|
||
| add_custom_target(lrs-codesign-all ALL ${_cmds} |
There was a problem hiding this comment.
🐛 BUG [rs-agentic bot] — lrs-codesign-all aggregate target does NOT sign binaries on incremental/partial builds
The comment in the function correctly acknowledges this, but it is worth raising as a blocking concern: if a developer rebuilds only one executable (e.g., cmake --build . --target rs-capture), the lrs-codesign-all target is not triggered, and the resulting binary is unsigned. Running it will silently get no capture-entitlement effect on Apple Silicon (SIP disabled), making multi-camera failures appear to be a code regression rather than a missing code-signature.
The lrs_codesign_device_access(target) per-target function is already defined in this same file and attaches a POST_BUILD hook that fires whenever the specific target is rebuilt. Consider using it directly in the relevant CMakeLists.txt files (examples, tools) for each executable target, or at least document the risk prominently for developers:
# In each subdirectory that defines an executable:
lrs_codesign_device_access(rs-multicam)
lrs_codesign_device_access(rs-capture)
# etc.
|
Hi @connorsoohoo, thanks for the contribution! Please make sure to follow our CONTRIBUTING.md guidelines. This PR already targets |
|
@connorsoohoo Thanks for tackling the macOS UVCAssistant contention this is a real, long-standing pain point and the diagnosis in the code comments is excellent. I tested the branch on a full build and hit a blocking issue with the codesigning approach. Environment: macOS 15.6, Apple Silicon (M-series), SIP enabled, no custom boot-args i.e. a stock, default-secured Mac. Blocker: every built tool is SIGKILLed at launch. After a full build, lrs-codesign-all stamps all executables with the restricted com.apple.vm.device-access entitlement over an ad-hoc signature. AMFI rejects that combination and kills the process at execve, before any of our code runs: Smaller notes: lrs_codesign_device_access() is defined but never called dead code. |
5 participants
TL;DR: On macOS, RealSense cameras intermittently fail to stream (
RS2_USB_STATUS_ACCESS, "failed to set power state", or zero frames) — most visibly with multiple cameras — because the system's userspaceUVCAssistantholds the UVC streaming interfaces. This makes the libusb backend capture the device up front and adds a codesigning step so the capture entitlement can authorize it. macOS-only; other platforms are unchanged.Summary
libusb_detach_kernel_driver()before claiming so libusb runs capture-mode re-enumeration up front instead of the lazy auto-detach that races the OS. Made the handle constructor exception-safe — release/close on a failed claim — so a failed attempt no longer leaks an open handle that keeps the device busy. Inner claim retry reverted to BUSY-only (tight-looping on ACCESS just re-triggered device resets).#ifdef __APPLE__): retry the whole open/claim with backoff (300–1200 ms) so a contended bring-up self-heals; other platforms keep the original single-attempt behavior.com.apple.vm.device-access(the entitlement that lets libusb take exclusive ownership of the device). No-op off macOS.Why
macOS auto-claims every UVC camera with
UVCAssistant, a userspace CoreMediaIO system extension that holds the RealSense streaming interfaces exclusively even with no app open (visible in IORegistry asUsbExclusiveOwner = pid …, UVCAssistant). The libusb backend then loseslibusb_claim_interfacewithACCESS. libusb's auto-detach only evicts kernel drivers, so it races the userspace assistant — failures are intermittent and the failing interface alternates run to run. It is worst with multiple cameras (rs-multicam), where bring-up contends across devices, and a killed run could wedge a device until physically replugged.Apple Silicon nuance
The deterministic fix is the
com.apple.vm.device-accesscapture entitlement (the mechanism VMs use for USB passthrough): it routes libusb throughIOServiceAuthorizeand takes the device fromUVCAssistantwithout racing. It is a restricted entitlement — on Apple Silicon with SIP enabled, AMFI ignores a self-signed binary that carries it. The operator mustcsrutil disableonce from Recovery (optionally add theamfi_get_out_of_my_way=1boot-arg). The binary still signs fine with SIP on; the entitlement simply has no effect until SIP is relaxed. Disabling/removingUVCAssistantitself is not viable — it lives on the sealed, read-only system volume. Without the entitlement (plainsudo), the in-process detach + retry improve reliability but cannot guarantee winning the race.Test plan
codesign -d --entitlements - build/Release/rs-multicam); runrs-multicamwith 2× D400 — both stream;ioreg -p IOUSB -r -n "Intel(R) RealSense(TM) Depth Camera ..." -l | grep UsbExclusiveOwnershows the app, notUVCAssistant.rs-captureno longer requires a physical replug.rs-enumerate-devices/rs-capturebehavior unchanged (retry path is#ifdef __APPLE__).