-
Notifications
You must be signed in to change notification settings - Fork 440
feat!: add StreamConfigBuilder with cross-platform configuration support #1010
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: master
Are you sure you want to change the base?
Conversation
278e12c
to
5ec0841
Compare
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.
Pull Request Overview
This PR introduces a StreamConfigBuilder
with support for platform-specific audio configuration options, improving ergonomics of stream creation while maintaining full cross-platform compatibility. The builder enables fine-grained control over backend-specific settings without requiring conditional compilation.
Key Changes
- Added
StreamConfigBuilder
with fluent API for configuring audio streams - Extended platform support to include JACK on all compatible platforms
- Introduced platform-specific configuration wrappers (ALSA, JACK, WASAPI)
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/lib.rs |
Core builder implementation and device extensions for improved stream creation API |
src/traits.rs |
Added Clone requirement to DeviceTrait for builder pattern support |
src/platform/mod.rs |
Extended JACK support to macOS and Windows platforms |
src/host/wasapi/mod.rs |
Added WASAPI-specific configuration for exclusive mode support |
src/host/wasapi/device.rs |
Implemented WASAPI configuration methods for stream building |
src/host/jack/mod.rs |
Added JACK-specific configuration for client management |
src/host/jack/device.rs |
Implemented JACK configuration methods with custom client options |
src/host/alsa/mod.rs |
Added ALSA-specific configuration for periods and access types |
Cargo.toml |
Updated feature flags and dependencies for new backend support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pub fn with_buffer_size(mut self, _buffer_size: BufferSize) -> Self { | ||
// Create a new config with the updated buffer size | ||
let supported_config = SupportedStreamConfig::new( | ||
self.config.channels(), | ||
self.config.sample_rate(), | ||
self.config.buffer_size(), |
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.
The _buffer_size
parameter is prefixed with an underscore but is used in the method body, indicating it should not be marked as unused.
pub fn with_buffer_size(mut self, _buffer_size: BufferSize) -> Self { | |
// Create a new config with the updated buffer size | |
let supported_config = SupportedStreamConfig::new( | |
self.config.channels(), | |
self.config.sample_rate(), | |
self.config.buffer_size(), | |
pub fn with_buffer_size(mut self, buffer_size: BufferSize) -> Self { | |
// Create a new config with the updated buffer size | |
let supported_config = SupportedStreamConfig::new( | |
self.config.channels(), | |
self.config.sample_rate(), | |
buffer_size, |
Copilot uses AI. Check for mistakes.
pub fn buffer_size(&self) -> SupportedBufferSize { | ||
self.buffer_size |
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 method signature change from returning &SupportedBufferSize
to SupportedBufferSize
could be a breaking change for existing users who expect a reference.
pub fn buffer_size(&self) -> SupportedBufferSize { | |
self.buffer_size | |
pub fn buffer_size(&self) -> &SupportedBufferSize { | |
&self.buffer_size |
Copilot uses AI. Check for mistakes.
|
||
impl LocalProcessHandler { | ||
#[allow(too_many_arguments)] | ||
#[allow(clippy::too_many_arguments)] |
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.
[nitpick] Consider refactoring this function to use a configuration struct instead of many individual parameters to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
{ | ||
let stream_inner = | ||
self.build_stream_inner(conf, sample_format, alsa::Direction::Capture)?; | ||
self.build_stream_inner(conf, sample_format, alsa::Direction::Capture, None, None)?; |
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.
[nitpick] Using None, None
for optional parameters makes the code unclear. Consider using named parameters or a configuration struct to improve readability.
Copilot uses AI. Check for mistakes.
- Add ergonomic StreamConfigBuilder API for stream configuration - Add platform-specific configuration types: - AlsaStreamConfig: configure periods and access types (RW/MMap modes) - JackStreamConfig: configure client names and port auto-connection - WasapiStreamConfig: configure exclusive/shared mode - Add device-tied stream building methods on SupportedStreamConfig - Add convenience methods: Device::default_input/output_config() - Add platform builder methods: .on_alsa(), .on_jack(), .on_wasapi() - Add jack feature flag for JACK audio backend support - Add audio_thread_priority feature flag for ALSA/WASAPI real-time scheduling - Update documentation with Quick Start guide and cross-platform examples - Maintain backward compatibility with existing Device::build_*_stream APIs This introduces a major ergonomic improvement for cross-platform audio configuration while preserving the existing low-level APIs.
5ec0841
to
cbd8e2a
Compare
This mostly reverts #990 for device compatibility, by letting ALSA calculate the period size from the device default period count. Forcing the period count to 2 caused underruns on some systems.
Note to self: consider making configurable whether CoreAudio follows default audio device changes or not (#1012). |
This PR introduces an
StreamConfigBuilder
with support for platform-specific audio configuration options. This addresses recurring requests for fine-grained control over backend-specific settings while maintaining cross-platform compatibility. At the same time, it improves ergonomics of stream creation.Motivation
There is a recurring interest in exposing platform-specific audio configuration options:
set_buffer_size_near
input_preset
toStreamConfig
#995 - Androidinput_preset
configuration for AEC supportUsers have consistently needed platform-specific control over:
Not yet added:
Currently, these options are either impossible to configure or would require hard-coded defaults or introduction of environment variables.
Solution
All existing APIs remain unchanged. The new builder and platform configuration are purely additive:
As opportunistic refactoring, the following feature flags were re-added:
jack
- Enable JACK audio backend supportaudio_thread_priority
- Enable real-time thread priority for ALSA/WASAPIAnd documentation was updated for readability and correctness.