Skip to content

feat: VB-Cable integration for seamless Discord audio routing - #89

Merged
akonopcz merged 13 commits into
developfrom
feature/vb-cable-integration
Dec 30, 2025
Merged

feat: VB-Cable integration for seamless Discord audio routing#89
akonopcz merged 13 commits into
developfrom
feature/vb-cable-integration

Conversation

@akonopcz

@akonopcz akonopcz commented Dec 30, 2025

Copy link
Copy Markdown
Owner

Summary

Complete VB-Cable integration enabling users to share sounds with Discord friends while still being heard themselves.

Features Implemented

1. VB-Cable Detection

  • Automatic detection of VB-Cable devices via cpal enumeration
  • Identifies both CABLE Input (output) and CABLE Output (input) devices
  • Exposes detection status to frontend via check_vb_cable_status command

2. One-Click Installation

  • Downloads VB-Cable Driver Pack45 from official VB-Audio CDN
  • Silent installation with -i -h flags
  • Automatic UAC elevation handling
  • Auto-configures VB-Cable as broadcast device after install

3. Windows Default Device Preservation

  • Saves ALL 4 Windows default audio settings before installation:
    • Render Console (main playback)
    • Render Communications (calls/voice chat)
    • Capture Console (main recording)
    • Capture Communications (calls/voice chat)
  • Restores all defaults after VB-Cable install (prevents VB-Cable becoming system default)
  • Uses com-policy-config crate for Windows IPolicyConfig interface

4. Smart Device Detection Retry

  • After installation, polls for VB-Cable device up to 5 times
  • 1 second delay between retries
  • Returns early as soon as device is detected
  • Handles slow driver initialization gracefully

5. Microphone Routing

  • Routes physical microphone audio to CABLE Input via cpal
  • Friends on Discord hear both the user's voice AND sounds
  • Low latency implementation (100ms ring buffer)
  • Enable/Disable toggle in Settings
  • Persists selection across app restarts
  • Auto-enables on startup if previously enabled

6. VB-Cable Uninstall

  • One-click uninstall button in Settings
  • Stops microphone routing before uninstall
  • Clears VB-Cable settings from app config
  • Uses same installer with -u -h flags

7. User Guidance

  • Help guide for manually disabling unused "CABLE In 16 Ch" device
  • "Open Sound Settings" button opens Windows mmsys.cpl directly
  • Step-by-step instructions (4 steps)

8. Donationware Notice

  • Always-visible attribution to VB-Audio
  • Links to vb-audio.com
  • Required per VB-Audio licensing for software distribution

Related Issues

Closes #39 (VB-Cable Integration)
Closes #83 (Microphone routing: Reduce latency)

Deferred to Future Work

Technical Details

New Rust Module: vbcable/

File Purpose
detection.rs VB-Cable device detection via cpal
installer.rs Download, extract, install/uninstall
default_device.rs Windows COM API for default device management
microphone.rs cpal-based audio routing with ring buffer
mod.rs Module exports

New Tauri Commands

  • check_vb_cable_status - Detection status
  • start_vb_cable_install - Install flow
  • start_vb_cable_uninstall - Uninstall flow
  • save_all_default_devices / restore_all_default_devices - Device preservation
  • wait_for_vb_cable_device - Post-install detection
  • list_microphones - Available capture devices
  • enable_microphone_routing / disable_microphone_routing - Routing control
  • get_microphone_routing_status - Current routing state
  • open_sound_settings - Opens mmsys.cpl

New Dependencies

  • com-policy-config 0.6: Windows IPolicyConfig interface
  • reqwest 0.12: HTTP client (blocking)
  • zip 7.0: ZIP extraction

Test Coverage

  • 13 unit tests in vbcable module
  • Tests for ring buffer, serialization, device manager
  • All 162 project tests pass

Test Plan

  • Fresh install: VB-Cable installs successfully
  • Default devices restored after install
  • VB-Cable auto-selected as broadcast device
  • Microphone routing enables/disables correctly
  • Voice audible on Discord with routing enabled
  • Routing persists across app restart
  • Uninstall removes VB-Cable
  • "Open Sound Settings" opens mmsys.cpl
  • All tests pass: cargo test

- Add com-policy-config 0.6 and windows 0.61 dependencies
- Create vbcable/ module with detection and default device management
- VB-Cable detection via cpal (looks for "CABLE Input")
- Windows default device save/restore via COM IPolicyConfig
- Register 4 new Tauri commands: check_vb_cable_status,
  get_vb_cable_device_name, save_default_audio_device,
  restore_default_audio_device
- Update STATE.md and ROADMAP.md for Phase 2 progress
Problem:
- Users had to manually download and install VB-Cable
- Installation changed Windows default audio devices without restoration
- No visual feedback during installation process

Solution:
- Created installer.rs with download, ZIP extraction, and UAC-elevated launch
- Save/restore ALL 4 Windows default audio devices (render/capture x console/communications)
- VbCableSettings component with 8-step installation flow and status feedback
- Auto-select VB-Cable as broadcast device after installation

Backend (Rust):
- Added installer.rs with ShellExecuteExW for UAC elevation
- Extended default_device.rs with SavedDefaults for all 4 device types
- New commands: save_all_default_devices, restore_all_default_devices
- Added reqwest (blocking), zip 7.0, open crate dependencies

Frontend (React):
- Created VbCableSettings.tsx with install button and status display
- Added SavedDefaults TypeScript interface
- Integrated VbCableSettings in Settings.tsx
- 02-03: Donationware notice (license requirement) + smart retry logic
- 02-04: Microphone routing ("Abhören") for Discord voice + sounds
- 02-05: Disable unused "CABLE In 16 Ch" device
- Add always-visible donationware notice for VB-Audio license compliance
- Implement smart retry logic (5 attempts x 1s) for device detection
- Replace static 3s delay with wait_for_vb_cable_device command
- Early return when device found, faster installation on quick systems
- Add microphone enumeration (excludes VB-Cable devices)
- Implement audio routing via cpal background thread with ring buffer
- Add Enable/Disable toggle in VbCableSettings UI
- Persist microphone routing settings across app restarts
- Auto-enable routing on startup if previously enabled
- Convert all UI strings to English
Problem:
- Ring buffer was sized for 1 second, causing noticeable voice delay
- No buffer prefill caused audio glitches at routing start

Solution:
- Reduce buffer to 100ms (good balance between latency and stability)
- Add buffer prefill with silence to prevent startup underruns
- Remove unused code (is_vb_cable_installed, restore_default method)

Related to #83
Problem:
- Auto-disable of CABLE In 16ch device requires admin rights
- SetupAPI reports success but device remains enabled
- Users have no way to uninstall VB-Cable from within the app

Solution:
- Removed device_control.rs (feature abandoned due to admin requirements)
- Added VB-Cable uninstall button in Settings
- Added step-by-step guide for manually disabling CABLE In 16ch
- Created issue #88 for future uninstaller integration

Backend (Rust):
- Added uninstall_vbcable() function using -u -h flags
- Added start_vb_cable_uninstall command
- Removed device control commands and Windows features

Frontend (React):
- Added uninstall button with progress feedback
- Added help section with manual disable instructions
- Opens Windows sound settings for easy access
Problem:
- VB-Cable module had only 8 unit tests
- Coverage for default_device.rs was at 9.68%

Solution:
- Added 5 new tests for SavedDefaults and DefaultDeviceManager
- Total vbcable tests now: 13
@codecov

codecov Bot commented Dec 30, 2025

Copy link
Copy Markdown

Problem:
- VbCableSettings.tsx had 0% test coverage
- codecov/patch check failed due to missing tests

Solution:
- Added 14 component tests covering:
  - Render states (installed/not installed)
  - Install/uninstall buttons
  - Microphone dropdown and selection
  - Enable/disable routing functionality
  - Sound settings link
  - Donationware notice
@akonopcz akonopcz self-assigned this Dec 30, 2025
@akonopcz akonopcz added enhancement New feature or request feature New features labels Dec 30, 2025
This was linked to issues Dec 30, 2025
@akonopcz akonopcz added this to the v1.0 Beta milestone Dec 30, 2025
- Added disable routing tests (4)
- Added install flow tests (3)
- Added uninstall flow tests (2)
- Added error handling tests (2)
- Added website link tests (2)

Total: 27 tests (up from 14)
@akonopcz

Copy link
Copy Markdown
Owner Author

@claude

@claude

claude Bot commented Dec 30, 2025

Copy link
Copy Markdown

Claude finished @dranelixx's task in 2m 12s —— View job


Comprehensive Code Review

I've thoroughly reviewed this VB-Cable integration PR. Overall, this is excellent work - the implementation is well-structured, follows project conventions, and includes good test coverage. Below are my findings organized by severity.


🎯 Summary

Strengths:

  • ✅ Modular architecture with clear separation of concerns
  • ✅ Comprehensive logging following project standards
  • ✅ Good test coverage (13 unit tests, frontend component tests)
  • ✅ Proper error handling throughout
  • ✅ Follows AGENTS.md conventions (snake_case, doc comments, etc.)
  • ✅ Smart features (retry logic, device preservation, auto-restore)

Areas for Improvement:

  • ⚠️ Test coverage gaps in critical paths (installer, commands)
  • ⚠️ Some unsafe Windows API usage needs validation
  • ⚠️ Minor performance considerations

🔴 Critical Issues

1. Unsafe Windows API - Missing Error Validation (src-tauri/src/vbcable/installer.rs:165-182)

The launch_installer and launch_uninstaller functions don't validate if sei.hProcess is valid before waiting:

if !sei.hProcess.is_invalid() {
    // Wait for installer
    let wait_result = unsafe { WaitForSingleObject(sei.hProcess, INFINITE) };
    // ...
} else {
    warn!("No process handle returned - installer may have failed to start");
}

Problem: If the user cancels the UAC prompt, the function returns Ok(()) despite installation failure.

Solution: Check the process handle validity and return an error if it's invalid:

if sei.hProcess.is_invalid() {
    error!("No process handle - installation cancelled or failed");
    return Err("Installation cancelled or failed to start".to_string());
}

2. Blocking Operations on Main Thread (src-tauri/src/vbcable/installer.rs:188-202, 291-319)

Both install_vbcable() and uninstall_vbcable() perform blocking network I/O and wait for process completion synchronously:

pub fn install_vbcable() -> Result<(), String> {
    let zip_path = download_vbcable()?;  // Blocking HTTP download
    let installer_path = extract_installer(&zip_path)?;
    launch_installer(&installer_path)?;  // Blocks until installer completes
    // ...
}

Problem: This freezes the UI during installation (though the PR description mentions "Windows will show a driver approval dialog", so blocking may be intentional).

Recommendation: Document this behavior clearly in the function doc comments, or consider using Tauri's async command pattern if non-blocking is desired.


⚠️ High Priority Issues

3. Test Coverage Gaps - Critical Paths (Codecov: 30.59% patch coverage)

Critical functionality lacks integration/end-to-end tests:

  • installer.rs: 0% coverage (189 lines uncovered)
  • commands/vbcable.rs: 0% coverage (64 lines uncovered)
  • default_device.rs: 38.23% coverage (126 lines uncovered)

Issue: Unit tests verify serialization and data structures, but the actual Windows API calls, installer flow, and device detection aren't tested.

Recommendation:

  • Add integration tests that mock Windows API calls (using test doubles/fixtures)
  • Add tests for error paths (network failure, UAC cancellation, device not found)
  • Consider adding feature-flagged tests that run on Windows CI

4. Ring Buffer Memory Safety (src-tauri/src/vbcable/microphone.rs:98-134)

The ring buffer implementation uses raw index arithmetic without bounds checking in hot path:

fn write(&mut self, samples: &[f32]) {
    for &sample in samples {
        self.buffer[self.write_pos] = sample;
        self.write_pos = (self.write_pos + 1) % self.capacity;
    }
}

Issue: While the modulo operation prevents out-of-bounds access, there's a potential for write/read pointer collision if buffer size is too small.

Current Mitigation: Buffer is sized to ~100ms latency (line 208), which should prevent issues in practice.

Recommendation: Add a debug assertion to detect buffer overrun:

fn write(&mut self, samples: &[f32]) {
    for &sample in samples {
        self.buffer[self.write_pos] = sample;
        let next_write = (self.write_pos + 1) % self.capacity;
        debug_assert_ne!(next_write, self.read_pos, "Ring buffer overrun");
        self.write_pos = next_write;
    }
}

5. Error Handling - Partial Restore Failure (src-tauri/src/vbcable/default_device.rs:88-130)

restore_all_defaults collects errors but returns a generic error message:

if errors.is_empty() {
    Ok(())
} else {
    Err(format!("Some devices failed to restore: {}", errors.join(", ")))
}

Issue: The frontend can't distinguish between "all failed" vs "some failed". Users may not know which device to manually fix.

Recommendation: Return a structured error or log specific failures prominently so users know what to check in Windows settings.

6. Microphone Routing - No Sample Rate Validation (src-tauri/src/vbcable/microphone.rs:175-193)

The code logs sample rates but doesn't validate they match:

info!("Input config: {} Hz, {} channels, {:?}",
    input_config.sample_rate().0, input_config.channels(), ...);
info!("Output config: {} Hz, {} channels, {:?}",
    output_config.sample_rate().0, output_config.channels(), ...);

Issue: Deferred to #82 (mic routing sample rate mismatch). If sample rates differ, audio will play at wrong speed.

Current State: Acknowledged as future work. Fine for this PR, but should be prioritized for next iteration.


💡 Medium Priority Issues

7. Logging Consistency - Use of warn! for Expected Cases (src-tauri/src/vbcable/microphone.rs:365)

} else {
    warn!("No active microphone routing to disable");
}

Issue: Per AGENTS.md, warn! is for "recoverable issues". Disabling when nothing is active is a no-op, not a warning.

Recommendation: Use debug! instead:

debug!("No active microphone routing to disable");

8. Frontend - Duplicate State Management (src/components/settings/VbCableSettings.tsx:21-24)

const [selectedMicrophone, setSelectedMicrophone] = useState<string>("");
const [isRoutingActive, setIsRoutingActive] = useState(false);

Issue: Microphone routing state is tracked in both React state AND settings context. This can lead to sync issues.

Recommendation: Derive selectedMicrophone and isRoutingActive from settings context as the single source of truth, or clearly document why both are needed.

9. Installer Cleanup - Silent Failure (src-tauri/src/vbcable/installer.rs:196-198)

if let Err(e) = fs::remove_file(&zip_path) {
    warn!("Failed to cleanup ZIP file: {}", e);
}

Issue: Cleanup failures are logged but not surfaced to the user. Over time, temp files could accumulate.

Recommendation: Add a periodic cleanup mechanism or surface warnings to the UI (e.g., "Installation succeeded, but temp files remain").

10. COM Initialization Pattern (src-tauri/src/vbcable/default_device.rs:139-146, 199-206)

let hr = CoInitializeEx(None, COINIT_MULTITHREADED);
let we_initialized_com = hr.is_ok();
if hr.is_err() && hr != windows::core::HRESULT(0x80010106u32 as i32) {
    // ...
}

Issue: Magic number 0x80010106 (RPC_E_CHANGED_MODE) is used twice without a constant.

Recommendation: Define a constant:

const RPC_E_CHANGED_MODE: i32 = 0x80010106u32 as i32;

📝 Low Priority / Suggestions

11. Hardcoded URL (src-tauri/src/vbcable/installer.rs:15-16)

const VBCABLE_DOWNLOAD_URL: &str =
    "https://download.vb-audio.com/Download_CABLE/VBCABLE_Driver_Pack45.zip";

Suggestion: If VB-Audio updates the driver, you'll need to update the URL. Consider making this configurable or adding a version check.

12. Frontend - Magic Numbers (src/components/settings/VbCableSettings.tsx:208, 261)

setInstallStep("");

Suggestion: Use a constant for clarity:

const INSTALL_STEP_NONE = "";
setInstallStep(INSTALL_STEP_NONE);

13. Documentation - Missing Context (src-tauri/src/vbcable/microphone.rs:1-7)

The module doc comment is excellent, but doesn't mention the 100ms latency tradeoff (reduced from 1s per #83).

Suggestion: Add a note about latency tuning in the module doc.

14. Test Naming (src-tauri/src/vbcable/microphone.rs:449-457)

#[test]
fn test_list_capture_devices() {
    let devices = list_capture_devices();
    for (_, name) in &devices {
        assert!(!name.to_lowercase().contains("cable"));
    }
}

Suggestion: This test doesn't actually verify the function works (could return empty list). Rename to test_list_capture_devices_excludes_cable to clarify intent.


✅ What's Done Well

  1. Excellent Logging: All functions use tracing with appropriate levels (info for user actions, debug for flow, warn for issues).

  2. Smart Retry Logic: The wait_for_vb_cable function (detection.rs:76-98) handles driver initialization delays elegantly.

  3. Defensive Programming: Ring buffer prefill (microphone.rs:112-118) prevents audio glitches - industry best practice.

  4. Comprehensive Settings Integration: VB-Cable settings persist across restarts (settings.rs:34-39, lib.rs:416-430).

  5. User Guidance: The "CABLE In 16 Ch" tip in the UI (VbCableSettings.tsx:352-374) shows attention to user experience.

  6. Type Safety: VbCableStatus enum uses tagged unions for type-safe status representation.

  7. Frontend Tests: Component tests cover key scenarios (installed/not installed, routing enable/disable).


📋 Recommendations Summary

Before Merge:

  1. ✅ Fix Critical Issue 1 (UAC cancellation handling)
  2. ✅ Document blocking behavior in installer functions (Issue 2)
  3. ⚠️ Address High Priority 5 (error handling clarity) or document as future work

Post-Merge (Future PRs):

  1. Add integration tests for installer and Windows API calls (Issue 3)
  2. Implement sample rate validation/resampling (Issue 6, already tracked as Mic routing: Handle sample rate mismatch #82)
  3. Consider ring buffer overflow detection (Issue 4)
  4. Clean up minor issues (7-14)

🎓 Adherence to AGENTS.md

Conventions Followed:

  • Rust: snake_case, doc comments, tracing logs, explicit error handling
  • TypeScript: Functional components, hooks, TypeScript strict mode
  • Git: Conventional commits format
  • Modularity: Clean separation (vbcable/ module structure)

⚠️ Minor Deviations:

  • Some private functions (>20 lines) lack doc comments (e.g., find_capture_device, find_cable_input_device)

📊 Coverage Analysis

Current patch coverage: 30.59% (660 lines uncovered)

Breakdown:

  • installer.rs: 0% (file I/O and Windows API - hard to test)
  • microphone.rs: 27.30% (cpal streams - requires audio hardware)
  • default_device.rs: 38.23% (COM APIs - Windows-specific)
  • VbCableSettings.tsx: 75.17% (good coverage for UI component)

Verdict: Coverage is low due to hardware/OS dependencies. This is acceptable for this PR given the complexity of mocking Windows APIs and audio devices. However, prioritize adding mockable integration tests in a follow-up.


✅ Final Verdict

This PR is ready to merge with minor fixes to Critical Issue 1 (UAC handling). The implementation is solid, well-architected, and follows project conventions. The low test coverage is primarily due to hardware/OS dependencies, which is understandable for a first iteration.

Suggested Next Steps:

  1. Fix UAC cancellation error handling (5 minutes)
  2. Merge to develop
  3. Create follow-up issue for integration tests (3 above)
  4. Monitor user feedback for edge cases

Great work on this complex feature! 🚀


| Branch: feature/vb-cable-integration

Problem:
- launch_installer returned Ok(()) even when user cancelled UAC prompt
- This caused incorrect success status in UI

Solution:
- Return Err() when process handle is invalid (UAC cancelled)
- Changed log level from warn to error for this case
- Ok() only returned after successful process completion
Ring Buffer:
- Add overflow detection with warning log
- Add overflow_logged flag to prevent log spam

Error Handling:
- Return structured RestoreResult instead of generic error
- Frontend shows specific failed devices to user
- Add RestoreResult/RestoreFailure types

Code Quality:
- Change warn! to debug! for expected no-op case
- Define RPC_E_CHANGED_MODE constant (remove magic number)
- Add latency documentation to microphone module
- Rename test to clarify intent (test_list_capture_devices_excludes_cable)

Tests:
- Add RestoreResult serialization tests
- Update frontend mock for new RestoreResult type
@akonopcz

Copy link
Copy Markdown
Owner Author

Code Review Response

Addressed in this PR

  • Critical 1: UAC cancellation now returns error (d2ef8e3)
  • High 4: Ring buffer overflow detection added
  • High 5: Structured RestoreResult with per-device failure info
  • Medium 7: Changed warn! to debug! for no-op case
  • Medium 10: Defined RPC_E_CHANGED_MODE constant
  • Low 13: Added latency documentation to microphone module
  • Low 14: Renamed test to test_list_capture_devices_excludes_cable

Deferred to separate issues

Intentionally not addressed

  • 11 (Hardcoded URL): VB-Cable Pack45 has been stable for years. Current error handling already catches download failures and shows "Manual Download" button. Adding version checks would require scraping VB-Audio's website (fragile) or maintaining an external config (complexity not justified).
  • 12 (Magic string ""): Too minor. setInstallStep("") is self-explanatory in context. Adding a constant like INSTALL_STEP_NONE would add noise without improving readability.

@akonopcz
akonopcz merged commit 97ecc2a into develop Dec 30, 2025
5 checks passed
@akonopcz
akonopcz deleted the feature/vb-cable-integration branch December 30, 2025 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request feature New features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mic routing: Reduce latency VB-Cable Integration

1 participant