-
Notifications
You must be signed in to change notification settings - Fork 41
refactor(audio): improve device selection and reduce code duplication in sounddevice backend" --body #316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
Thanks for the PR! I’ve reverted to the default setting, which uses the speaker or microphone selected at the system level. |
|
Hi, that sounds good! Finding the default device first is the natural approach. |
Do you want to do it? |
abd9460 to
391262e
Compare
|
Yes, I updated the sound device detection to fall back to the default device first, as you suggested. |
| channel_key = f"max_{device_type}_channels" | ||
|
|
||
| # return default device with appropriate channels | ||
| for idx, dev in enumerate(devices): | ||
| if ( | ||
| name_contains.lower() in dev["name"].lower() | ||
| and dev["max_output_channels"] > 0 | ||
| ): | ||
| if dev.get(channel_key, 0) > 0: | ||
| return idx | ||
| # Return default output device if not found | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't consider the name_contains at all :/
andimarafioti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work in the advertised way. Ie, if you query with a given name, it will still return first the default device if it has an input/output channel.
…unddevice backend The original get_input_device_id() and get_output_device_id() had two issues: 1. Both incorrectly returned sd.default.device[1] (default OUTPUT device) as fallback 2. Nearly identical code duplicated between the two methods This caused failures when: - get_input_device_id() returned an output-only device for recording - get_output_device_id() always returned the same device regardless of type Changes: - Extract shared logic into _find_device_id(name_contains, device_type) helper - Use dynamic channel_key: "max_input_channels" or "max_output_channels" - Proper fallback: search for first device with appropriate channel type - Raise clear RuntimeError if no suitable device exists - Fix typo: logger.warningt → logger.warning This ensures both methods always return valid devices of the correct type.
…rch in sounddevice backend Updated device selection logic to return the first available device with appropriate channels before searching by device name. This provides better compatibility across different audio hardware configurations while maintaining ReSpeaker support as fallback.
…ackend Reorder _find_device_id logic to search by device name first before falling back to the first available device with appropriate channels.
391262e to
ce55b29
Compare
|
You’re right, @andimarafioti. I missed that. I’ve reprioritized _find_device_id() now. Thanks |
|
Hi, could integrate the new changes made by @andimarafioti ? |
Summary
This PR improves audio device selection logic and refactors duplicated code in the sounddevice backend.
Background
The original get_input_device_id() and get_output_device_id() methods had some areas for improvement:
Changes
This PR introduces the following improvements:
Testing
Result
Both methods now consistently return valid devices of the correct type (input or output), which should help prevent issues when audio hardware configurations vary across different systems.