Repository navigation
fix(wasapi): draining stops and silent capture buffers - #1379
roderickvd wants to merge 5 commits into
Conversation
LastExceed
left a comment
LastExceed
left a comment
There was a problem hiding this comment.
Sorry for the late reply, I kinda forgot about this.
Regarding the removal of 64-bit formats: We did previously establish (here) that the built-in resampler does not support them, but I still don't see anything preventing a driver from supporting any arbitrary format it wants natively, since WASAPIs buffers are untyped. This may be limited to exclusive mode (as is the case with DSD, since the sound server doesn't know how to mix that), but since we do plan to support exclusive mode in the near future, and we're really just talking about a few extra match branches here, it may be worth keeping 64-bit format support around.
| #[cfg(any(windows, target_vendor = "apple"))] | ||
| pub(crate) fn wait_for_drain(window: std::time::Duration, timeout: Option<std::time::Duration>) { | ||
| let wait = timeout.map_or(window, |t| window.min(t)); | ||
| if !wait.is_zero() { |
There was a problem hiding this comment.
I know this is a pre-existing function, but maybe rename it to sleep_at_most() or something else that doesn't explicitly specify draining, since it isn't functionally limited to that.
| let scratch_len = (stream.max_frames_in_buffer as usize * stream.bytes_per_frame as usize) | ||
| .div_ceil(size_of::<i32>()); |
There was a problem hiding this comment.
Maybe use SampleFormat::sample_size() instead of size_of::<i32>() here
There was a problem hiding this comment.
Less magic is more better. Should be OK with I24 right?
There was a problem hiding this comment.
The first sentence jokingly confirms that your proposal is more expressive.
The second sentence seeks confirmation as to whether your proposal will work across 24-bit sample formats.
There was a problem hiding this comment.
Got it. Language barrier strikes again😅
Yes, I24 will work fine, as that function returns size_of::<i32>() for I24/U24. might be worth adding a comment there, to clarify that this is intentional.
This branch has not been deployed
2 participants
Following the discussion at #1357 I had a closer look at WASAPI and did see that stops are at risk of draining short. Though draining stops will never be perfect on WASAPI, this at least improves that.
While at it I also removed dead 64-bit format conversions, and quickly addressed #1376.