Skip to content

Conversation

richiemcilroy
Copy link
Member

@richiemcilroy richiemcilroy commented Oct 16, 2025

This pull request updates the logic for selecting and configuring audio input devices in the microphone feed code.

Fixes the issue of AirPods not showing in the list of available devices.

Summary by CodeRabbit

  • Bug Fixes
    • Improved microphone device compatibility by refining audio format detection and sample rate selection logic. The system now supports a wider range of audio devices with intelligent fallback to alternative sample rates when preferred options are unavailable.

Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

The microphone device selection logic is refactored to dynamically determine suitable audio sample rates instead of requiring fixed 48kHz support. A new helper function selects optimal rates (48kHz or 44.1kHz) within device constraints, falling back to the device's maximum supported rate.

Changes

Cohort / File(s) Summary
Microphone device configuration
crates/recording/src/feeds/microphone.rs
Added SupportedStreamConfigRange import. Replaced filtering logic with map-based approach that validates ffmpeg format support and applies select_sample_rate() helper. New select_sample_rate() function selects 48kHz or 44.1kHz if available within device range; otherwise uses max supported rate. Result now returns dynamically-adjusted config instead of forced 48kHz.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With ears perked up and whiskers all aquiver,
We sample the rates that devices deliver!
From forty-four upward to forty-eight's call,
A flexible hop—no fixed rate thralls all.
The microphone logic hops truly refined!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the main change by indicating an improvement in device configuration selection specifically for the microphone feed, matching the PR’s updates to audio input device logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch audio-selection-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
crates/recording/src/feeds/microphone.rs (1)

159-169: Helper function logic is sound.

The select_sample_rate function implements a sensible fallback strategy:

  1. Prefers 48kHz (industry standard for professional audio)
  2. Falls back to 44.1kHz (CD audio quality standard)
  3. Uses device's max rate if neither preferred rate is supported

The boundary checks correctly ensure the selected rate falls within the device's min_sample_rate and max_sample_rate range.

Consider adding a doc comment to explain the rate selection strategy:

+/// Selects an optimal sample rate for the given config.
+///
+/// Prefers 48kHz or 44.1kHz if supported within the device's range,
+/// otherwise falls back to the device's maximum supported rate.
 fn select_sample_rate(config: &SupportedStreamConfigRange) -> cpal::SampleRate {
     const PREFERRED_RATES: [u32; 2] = [48_000, 44_100];
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1ca80a and d35649f.

📒 Files selected for processing (1)
  • crates/recording/src/feeds/microphone.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.

Files:

  • crates/recording/src/feeds/microphone.rs
crates/*/src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Rust crates should place tests within the src/ and/or a sibling tests/ directory for each crate inside crates/*.

Files:

  • crates/recording/src/feeds/microphone.rs
🧬 Code graph analysis (1)
crates/recording/src/feeds/microphone.rs (1)
crates/media-info/src/lib.rs (2)
  • ffmpeg_sample_format_for (277-287)
  • rate (118-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (2)
crates/recording/src/feeds/microphone.rs (2)

5-5: LGTM! Import is necessary for the new helper function.

The SupportedStreamConfigRange import is correctly added to support the parameter type in the new select_sample_rate helper function.


150-154: Good refactor! The dynamic sample rate selection is more flexible.

The refactored logic correctly replaces the rigid 48kHz requirement with a flexible approach that:

  1. Filters configs by FFmpeg format compatibility
  2. Dynamically selects an appropriate sample rate for each compatible config
  3. Maintains the existing quality-based sorting (highest sample size and max rate first)

This should successfully include devices like AirPods that may support 44.1kHz or other standard rates instead of 48kHz.

To confirm this resolves the AirPods issue, please verify that AirPods (or similar devices without 48kHz support) now appear in the device list after these changes.

@richiemcilroy richiemcilroy merged commit 548d5ba into main Oct 16, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant