-
Notifications
You must be signed in to change notification settings - Fork 474
wasapi: Enable resampling and rate adjustment #1097
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
This allows non-native sample rates to be resampled by the WASAPI server. This should not have any meaningful performance impact, many pieces of software and middleware like SDL do this already.
0d0178e to
961323e
Compare
|
|
caf69e2 to
721517f
Compare
721517f to
c3ea12a
Compare
|
I have tested this and this works, it allows me to request custom sampling rates in windows that was otherwise locked to only one single sampling rate conversion. This solves issue #593 that has been open for almost 5 years now. @roderickvd you might remember me from the Rust Audio discord, i was ranting about the resampling issue on Windows and you recommended me to use rodio instead. I can vouch that this PR works, you might be interested to take a look |
kaezrr
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.
Tested on a windows VM, works as intended, solves #593
roderickvd
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.
Thank you for your contribution!
I'd like that we altogether think about this in the context of API and expectations. Let's say that a device supports 48 and 192 kHz, and is configured to use 192 kHz by default. Now when we request 48 kHz it's indeed the question if we should:
a. fail like we do currently
b. silently resample
Rationale for A: we request 48 kHz explicitly and can't get that, so that's a failure.
Rationale for B: out-of-the-box experience for users.
Don't take this as a final decision, but currently I'm leaning towards A (keeping as-is). When users want resampling, they can do so explicitly. For example, by:
- catching an open stream error and falling back to the default stream configuration instead, then doing resampling themselves
- using Rodio which is intended as a batteries-included audio crate, unlike cpal which is more low-level
- using extension traits in the future (see #1074 for an example), that could allow users to explicitly opt-in to resampling behavior
While we're considering this, please find my review comments inline. Please add a changelog entry as well.
|
Apparently PulseAudio transparently converts formats already, and PipeWire requires you to attach SPAs to your stream for conversion regardless, so I don't see the particular issue with automatic resampling here.
Resampling can be annoying to implement, and APOs might do a better job at it (by, for example, using the sound hardware).
I don't think low-level APIs should just ignore features implemented by the APIs beneath them (look right below).
Once implemented, sure thing. |
this should allow resampling sample rates that are too quirky for native hardware by the WASAPI server 👍