diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d92d0520..398244f11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **WASAPI**: Output streams now start with real audio immediately instead of undefined content in the render buffer. - **WASAPI**: A stream paused immediately after starting no longer plays silence before real audio on resume. - **WASAPI**: Fix `I64` and `F64` incorrectly reported as supported output formats. +- **WASAPI**: Formats with a `cbSize` that doesn't cover the extension, and configurations whose + derived `WAVEFORMATEX` fields would overflow, are now rejected instead of being read out of + bounds, panicking, or wrapping. +- **WASAPI**: A buffer size or padding count beyond the stream's negotiated bounds, or a capture + packet larger than its buffer, is now rejected with a backend error instead of panicking, + over-reading, or allocating gigabytes. +- **WASAPI**: Empty capture packets are no longer delivered as a null buffer, packets the engine + marks silent now arrive as silence, and a slow data callback no longer leaves `stop()` and + `Drop` unserviced. ## [0.18.2] - 2026-08-16 diff --git a/src/host/aaudio/mod.rs b/src/host/aaudio/mod.rs index 2de6551bb..b30598626 100644 --- a/src/host/aaudio/mod.rs +++ b/src/host/aaudio/mod.rs @@ -1,6 +1,8 @@ //! AAudio backend implementation. //! //! Default backend on Android. +//! +//! The stream-build `timeout` is ignored. use std::{ fmt, diff --git a/src/host/alsa/mod.rs b/src/host/alsa/mod.rs index 8898e3b4a..319efbd71 100644 --- a/src/host/alsa/mod.rs +++ b/src/host/alsa/mod.rs @@ -1,6 +1,9 @@ //! ALSA backend implementation. //! //! Default backend on Linux and BSD systems. +//! +//! The stream-build `timeout` is not a setup bound: it becomes the `poll()` timeout of the +//! stream's run loop, so it applies for the life of the stream, and `None` polls forever. extern crate alsa; #[cfg(feature = "realtime")] diff --git a/src/host/asio/mod.rs b/src/host/asio/mod.rs index 3d91a1818..3d56035b5 100644 --- a/src/host/asio/mod.rs +++ b/src/host/asio/mod.rs @@ -2,6 +2,8 @@ //! //! ASIO is available on Windows with the `asio` feature. //! See the project README for setup instructions. +//! +//! The stream-build `timeout` is ignored. extern crate asio_sys as sys; diff --git a/src/host/audioworklet/mod.rs b/src/host/audioworklet/mod.rs index b25503580..66c489614 100644 --- a/src/host/audioworklet/mod.rs +++ b/src/host/audioworklet/mod.rs @@ -2,6 +2,8 @@ //! //! Available on WebAssembly with the `audioworklet` feature. Requires atomics support. //! See the `audioworklet` example for setup instructions. +//! +//! The stream-build `timeout` is ignored. use std::{ cell::RefCell, diff --git a/src/host/coreaudio/mod.rs b/src/host/coreaudio/mod.rs index 730bc7240..42403504e 100644 --- a/src/host/coreaudio/mod.rs +++ b/src/host/coreaudio/mod.rs @@ -1,6 +1,9 @@ //! CoreAudio backend implementation. //! //! Default backend on macOS, iOS, and tvOS. +//! +//! On macOS the stream-build `timeout` bounds the device rate change made during setup, with a +//! one-second default; on iOS it is ignored. use objc2_core_audio_types::{ AudioStreamBasicDescription, kAudioFormatFlagIsFloat, kAudioFormatFlagIsPacked, diff --git a/src/host/equilibrium.rs b/src/host/equilibrium.rs index 8b4d960c1..660640475 100644 --- a/src/host/equilibrium.rs +++ b/src/host/equilibrium.rs @@ -54,3 +54,52 @@ pub fn fill_equilibrium(buffer: &mut [u8], sample_format: SampleFormat) { } } } + +#[test] +fn test_fill_equilibrium_byte_patterns() { + let mut buf = vec![0u8; 8].into_boxed_slice(); + + // Unsigned 8-bit silence is 0x80, the midpoint of the range. + fill_equilibrium(&mut buf[..], SampleFormat::U8); + assert_eq!(buf[0], 0x80); + assert_eq!(buf[5], 0x80); + + // Signed and float formats rest at zero. + fill_equilibrium(&mut buf[..], SampleFormat::I16); + assert_eq!(buf[0], 0); + assert_eq!(buf[5], 0); + fill_equilibrium(&mut buf[..], SampleFormat::I24); + assert_eq!(buf[0], 0); + assert_eq!(buf[5], 0); + fill_equilibrium(&mut buf[..], SampleFormat::F32); + assert_eq!(buf[0], 0); + assert_eq!(buf[5], 0); + + // DSD silence is the 0x69 pattern. + fill_equilibrium(&mut buf[..], SampleFormat::DsdU8); + assert_eq!(buf[0], 0x69); + assert_eq!(buf[5], 0x69); + + // Multi-byte unsigned formats take the typed path, the only one that casts the buffer to a + // wider pointer: check the value written and that `chunks_exact` accounts for every byte. + fill_equilibrium(&mut buf[..], SampleFormat::U16); + assert!( + buf.chunks_exact(2) + .all(|c| u16::from_ne_bytes(c.try_into().unwrap()) == 0x8000) + ); + fill_equilibrium(&mut buf[..], SampleFormat::U24); + assert!( + buf.chunks_exact(4) + .all(|c| i32::from_ne_bytes(c.try_into().unwrap()) == 0x0080_0000) + ); + fill_equilibrium(&mut buf[..], SampleFormat::U32); + assert!( + buf.chunks_exact(4) + .all(|c| u32::from_ne_bytes(c.try_into().unwrap()) == 0x8000_0000) + ); + fill_equilibrium(&mut buf[..], SampleFormat::U64); + assert!( + buf.chunks_exact(8) + .all(|c| u64::from_ne_bytes(c.try_into().unwrap()) == 0x8000_0000_0000_0000) + ); +} diff --git a/src/host/jack/mod.rs b/src/host/jack/mod.rs index 1315aeda2..073d95b96 100644 --- a/src/host/jack/mod.rs +++ b/src/host/jack/mod.rs @@ -1,6 +1,7 @@ //! JACK backend implementation. //! //! Available on all platforms with the `jack` feature. Requires JACK server and client libraries. +//! The stream-build `timeout` bounds the whole build, including opening the JACK client. extern crate jack; diff --git a/src/host/mod.rs b/src/host/mod.rs index 34a92488d..83cf48caf 100644 --- a/src/host/mod.rs +++ b/src/host/mod.rs @@ -243,8 +243,8 @@ pub(crate) use error_emit::try_emit_error; target_os = "dragonfly", target_os = "freebsd", target_os = "netbsd", - target_os = "windows", target_vendor = "apple", + all(windows, any(feature = "asio", feature = "jack")), all( target_arch = "wasm32", target_os = "unknown", diff --git a/src/host/pipewire/mod.rs b/src/host/pipewire/mod.rs index 412dd3d5c..0d67b1eaa 100644 --- a/src/host/pipewire/mod.rs +++ b/src/host/pipewire/mod.rs @@ -1,3 +1,9 @@ +//! PipeWire backend implementation. +//! +//! Default backend on Linux when PipeWire is available. +//! +//! The stream-build `timeout` bounds stream initialization, waiting two seconds when given `None`. + use std::sync::{ Arc, atomic::{AtomicBool, Ordering}, diff --git a/src/host/pulseaudio/mod.rs b/src/host/pulseaudio/mod.rs index 6a0280b78..e6bcd7836 100644 --- a/src/host/pulseaudio/mod.rs +++ b/src/host/pulseaudio/mod.rs @@ -1,3 +1,9 @@ +//! PulseAudio backend implementation. +//! +//! Default backend on Linux and BSD systems when PipeWire is unavailable. +//! +//! The stream-build `timeout` bounds stream creation. + use std::{ ffi::CString, fmt, diff --git a/src/host/wasapi/device.rs b/src/host/wasapi/device.rs index 0d73be057..7809b5496 100644 --- a/src/host/wasapi/device.rs +++ b/src/host/wasapi/device.rs @@ -205,7 +205,18 @@ pub unsafe fn is_format_supported( Ok(hr.0 == 0) } +// Bytes that follow the `WAVEFORMATEX` header inside a `WAVEFORMATEXTENSIBLE`. The Windows ABI +// fixes this at 22; the assert pins the size derivation below to the ABI value. +const WAVEFORMATEXTENSIBLE_EXTRA_BYTES: u16 = 22; +const _: () = assert!( + mem::size_of::() - mem::size_of::() + == WAVEFORMATEXTENSIBLE_EXTRA_BYTES as usize +); + // Get a cpal Format from a WAVEFORMATEX. +// +// Safety: `waveformatex_ptr` must point to a readable `WAVEFORMATEX` followed by the +// `cbSize` extra bytes its header declares. unsafe fn format_from_waveformatex_ptr( waveformatex_ptr: *const Audio::WAVEFORMATEX, audio_client: &Audio::IAudioClient, @@ -224,6 +235,11 @@ unsafe fn format_from_waveformatex_ptr( (32, Multimedia::WAVE_FORMAT_IEEE_FLOAT) => SampleFormat::F32, (64, Multimedia::WAVE_FORMAT_IEEE_FLOAT) => SampleFormat::F64, (n_bits, KernelStreaming::WAVE_FORMAT_EXTENSIBLE) => { + // The extension is only there to be read if `cbSize` accounts for it. + if unsafe { (*waveformatex_ptr).cbSize } < WAVEFORMATEXTENSIBLE_EXTRA_BYTES { + return None; + } + let waveformatextensible_ptr = waveformatex_ptr as *const Audio::WAVEFORMATEXTENSIBLE; let sub = unsafe { (*waveformatextensible_ptr).SubFormat }; let valid_bits = unsafe { (*waveformatextensible_ptr).Samples.wValidBitsPerSample }; @@ -591,6 +607,9 @@ impl Device { DeviceHandle::Specific(device) => { // can fail if the device has been disconnected since we enumerated it, or if // the device doesn't support playback for some reason + // + // not bounded by `activation_timeout`: `ActivateAudioInterfaceAsync` takes a + // device interface path, and `IMMDevice` only exposes an opaque endpoint ID device .Activate(Com::CLSCTX_ALL, None) .map_err(Error::from)? @@ -722,7 +741,7 @@ impl Device { let usable = is_output || is_format_supported( client, - &waveformat.Format as *const Audio::WAVEFORMATEX, + as_waveformatex_ptr(&waveformat), Audio::AUDCLNT_SHAREMODE_SHARED, )?; if usable { @@ -885,7 +904,7 @@ impl Device { stream_flags, buffer_duration, 0, - &format_attempt.Format, + as_waveformatex_ptr(&format_attempt), None, ) .context("Failed to initialize audio client")?; @@ -893,10 +912,12 @@ impl Device { format_attempt.Format }; - // obtaining the size of the samples buffer in number of frames - let max_frames_in_buffer = audio_client - .GetBufferSize() - .context("Failed to get buffer size")?; + let max_frames_in_buffer = buffer_size_in_frames( + &audio_client, + config.buffer_size, + config.sample_rate, + waveformatex.nBlockAlign, + )?; let period_frames = shared_mode_period_frames(&audio_client, config.sample_rate, max_frames_in_buffer); @@ -990,7 +1011,7 @@ impl Device { | Audio::AUDCLNT_STREAMFLAGS_AUTOCONVERTPCM, buffer_duration, 0, - &format_attempt.Format, + as_waveformatex_ptr(&format_attempt), None, ) .context("Failed to initialize audio client")?; @@ -998,6 +1019,16 @@ impl Device { format_attempt.Format }; + let max_frames_in_buffer = buffer_size_in_frames( + &audio_client, + config.buffer_size, + config.sample_rate, + waveformatex.nBlockAlign, + )?; + + let period_frames = + shared_mode_period_frames(&audio_client, config.sample_rate, max_frames_in_buffer); + // Creating the event that will be signalled whenever we need to submit some samples. let event = Threading::CreateEventA(None, false, false, windows::core::PCSTR(ptr::null())) @@ -1007,14 +1038,6 @@ impl Device { .SetEventHandle(event) .context("Failed to set event handle")?; - // obtaining the size of the samples buffer in number of frames - let max_frames_in_buffer = audio_client - .GetBufferSize() - .context("Failed to get buffer size")?; - - let period_frames = - shared_mode_period_frames(&audio_client, config.sample_rate, max_frames_in_buffer); - // Building a `IAudioRenderClient` that will be used to fill the samples buffer. let render_client = audio_client .GetService::() @@ -1361,7 +1384,8 @@ const WAVEFORMATEXTENSIBLE_SAMPLE_FORMATS: [SampleFormat; 5] = [ // Turns a `Format` into a `WAVEFORMATEXTENSIBLE`. // -// Returns `None` if the WAVEFORMATEXTENSIBLE does not support the given format. +// Returns `None` if the format is unsupported, or if the config does not fit the WAVEFORMATEX +// field widths. fn config_to_waveformatextensible( config: StreamConfig, sample_format: SampleFormat, @@ -1381,8 +1405,9 @@ fn config_to_waveformatextensible( let channels = config.channels; let sample_rate = config.sample_rate; let sample_bytes = sample_format.sample_size() as u16; - let avg_bytes_per_sec = u32::from(channels) * sample_rate * u32::from(sample_bytes); - let block_align = channels * sample_bytes; + // A wide channel count overflows nBlockAlign, and a high sample rate overflows nAvgBytesPerSec. + let block_align = channels.checked_mul(sample_bytes)?; + let avg_bytes_per_sec = sample_rate.checked_mul(u32::from(block_align))?; // wBitsPerSample is the container word size; wValidBitsPerSample is the actual bit depth. // For I24 the container is 32 bits (sample_size() == 4) but only 24 bits are significant. let container_bits = 8 * sample_bytes; @@ -1391,9 +1416,7 @@ fn config_to_waveformatextensible( let cb_size = if format_tag == Audio::WAVE_FORMAT_PCM { 0 } else { - let extensible_size = mem::size_of::(); - let ex_size = mem::size_of::(); - (extensible_size - ex_size) as u16 + WAVEFORMATEXTENSIBLE_EXTRA_BYTES }; let waveformatex = Audio::WAVEFORMATEX { @@ -1433,6 +1456,64 @@ fn config_to_waveformatextensible( Some(waveformatextensible) } +// `cbSize` tells the engine how far to read, so the pointer must carry provenance over the +// whole `WAVEFORMATEXTENSIBLE`, not just its `WAVEFORMATEX` prefix. +fn as_waveformatex_ptr(format: &Audio::WAVEFORMATEXTENSIBLE) -> *const Audio::WAVEFORMATEX { + ptr::from_ref(format).cast() +} + +// WASAPI rounds a shared-mode ring up to a whole number of device periods, so a report somewhat +// above the request is normal; more than `BUFFER_HEADROOM_SECONDS` above it is a driver reporting +// nonsense. The headroom cannot stand alone: `requested` is caller-supplied, so without an +// absolute ceiling a wild request plus a wild report admits a multi-gigabyte buffer. +const BUFFER_HEADROOM_SECONDS: u32 = 10; +const MAX_BUFFER_BYTES: usize = 1 << 30; // 1 GiB + +/// Get the size of the ring buffer WASAPI allocated, rejecting implausible values. +fn buffer_size_in_frames( + audio_client: &Audio::IAudioClient, + buffer_size: BufferSize, + sample_rate: SampleRate, + bytes_per_frame: u16, +) -> Result { + let requested = match buffer_size { + BufferSize::Fixed(frames) => frames, + BufferSize::Default => 0, + }; + let max_frames_in_buffer = + unsafe { audio_client.GetBufferSize() }.context("Failed to get buffer size")?; + let cap = requested.saturating_add(sample_rate.saturating_mul(BUFFER_HEADROOM_SECONDS)); + if max_frames_in_buffer > cap { + let allowance = match buffer_size { + BufferSize::Fixed(_) => { + format!( + "{requested} requested plus {BUFFER_HEADROOM_SECONDS} s at {sample_rate} Hz" + ) + } + BufferSize::Default => { + format!("{BUFFER_HEADROOM_SECONDS} s at {sample_rate} Hz above the engine default") + } + }; + return Err(Error::with_message( + ErrorKind::BackendError, + format!( + "Audio client reported a buffer of {max_frames_in_buffer} frames, more than the \ + {cap} frames allowed ({allowance})" + ), + )); + } + // The stream derives byte counts from these two; keep the product within the address space + // and under the allocation ceiling. + let buffer_bytes = (max_frames_in_buffer as usize).checked_mul(bytes_per_frame as usize); + if buffer_bytes.is_none_or(|bytes| bytes > MAX_BUFFER_BYTES) { + return Err(Error::with_message( + ErrorKind::BackendError, + "Audio buffer size overflows the address space or exceeds the 1 GiB ceiling", + )); + } + Ok(max_frames_in_buffer) +} + /// Get the default device period in frames for a shared-mode stream. fn shared_mode_period_frames( audio_client: &Audio::IAudioClient, @@ -1464,3 +1545,43 @@ fn buffer_size_to_duration(buffer_size: &BufferSize, sample_rate: SampleRate) -> fn buffer_duration_to_frames(buffer_duration: i64, sample_rate: SampleRate) -> FrameCount { ((buffer_duration * sample_rate as i64 * 100 + 500_000_000) / 1_000_000_000) as FrameCount } + +// Tests below pin the overflow guard and field layout of the WAVEFORMATEXTENSIBLE +// conversion; both are visible to real drivers only through a misconfigured stream. +#[test] +fn test_config_to_waveformatextensible_overflow_and_layout() { + let cfg = |channels: u16, sample_rate: SampleRate| StreamConfig { + channels, + sample_rate, + buffer_size: BufferSize::Default, + }; + + // The rate is expressible, but 2ch F64 at 300 MHz overflows the u32 nAvgBytesPerSec field. + assert!(config_to_waveformatextensible(cfg(2, 300_000_000), SampleFormat::F64, None).is_none()); + // 9_000 channels of F64 overflow the u16 nBlockAlign field. + assert!(config_to_waveformatextensible(cfg(9_000, 48_000), SampleFormat::F64, None).is_none()); + + // Plain PCM path: a stereo I16 stream at 48 kHz. + let pcm = config_to_waveformatextensible(cfg(2, 48_000), SampleFormat::I16, None) + .expect("2ch I16 at 48 kHz must convert"); + assert_eq!(pcm.Format.wFormatTag as u32, Audio::WAVE_FORMAT_PCM); + assert_eq!(pcm.Format.nBlockAlign as u32, 4); + // `WAVEFORMATEX` is packed, so read this u32 into a local instead of forming a reference to it. + let avg_bytes_per_sec = pcm.Format.nAvgBytesPerSec; + assert_eq!(avg_bytes_per_sec, 192_000); + assert_eq!(pcm.Format.cbSize as u32, 0); + + // Extensible path: I24 rides in a 32-bit container. + let ext = config_to_waveformatextensible(cfg(2, 48_000), SampleFormat::I24, None) + .expect("2ch I24 at 48 kHz must convert"); + assert_eq!( + ext.Format.wFormatTag as u32, + KernelStreaming::WAVE_FORMAT_EXTENSIBLE + ); + assert_eq!(ext.Format.wBitsPerSample as u32, 32); + assert_eq!( + ext.Format.cbSize as u32, + WAVEFORMATEXTENSIBLE_EXTRA_BYTES as u32 + ); + assert_eq!(unsafe { ext.Samples.wValidBitsPerSample } as u32, 24); +} diff --git a/src/host/wasapi/mod.rs b/src/host/wasapi/mod.rs index 999d1ab86..58c2b7273 100644 --- a/src/host/wasapi/mod.rs +++ b/src/host/wasapi/mod.rs @@ -1,6 +1,10 @@ //! WASAPI backend implementation. //! //! Default backend on Windows. +//! +//! The stream-build `timeout` bounds device activation only, and only for streams built from a +//! default device: a build from a specific device, and the first build after a configuration +//! query (which reuses the client that query activated), are not bounded. use std::io::Error as IoError; diff --git a/src/host/wasapi/stream.rs b/src/host/wasapi/stream.rs index a7723e7f3..5a503a4e7 100644 --- a/src/host/wasapi/stream.rs +++ b/src/host/wasapi/stream.rs @@ -593,7 +593,8 @@ fn process_commands(run_context: &mut RunContext) -> Result { // and may already hold real, unplayed data. let cold_start = match run_context.stream.client_flow { AudioClientFlow::Render { .. } => { - get_available_frames(&run_context.stream)? > 0 + let (available, _) = get_available_frames(&run_context.stream)?; + available > 0 } AudioClientFlow::Capture { .. } => false, }; @@ -668,15 +669,26 @@ fn wait_for_handle_signal(handles: &[Foundation::HANDLE]) -> Result Result { +fn get_available_frames(stream: &StreamInner) -> Result<(FrameCount, FrameCount), Error> { unsafe { let padding = stream .audio_client .GetCurrentPadding() .context("Failed to get current padding")?; - Ok(stream.max_frames_in_buffer - padding) + // Underflowing here would size the render buffer slice from a huge frame count. + let available = stream + .max_frames_in_buffer + .checked_sub(padding) + .ok_or_else(|| { + Error::with_message( + ErrorKind::BackendError, + "IAudioClient::GetCurrentPadding returned more frames than the buffer holds", + ) + })?; + Ok((available, padding)) } } @@ -694,13 +706,13 @@ fn run_input( } let stream = &run_ctxt.stream; - let scratch_len = if stream.sample_format == SampleFormat::I24 { - stream.max_frames_in_buffer as usize * stream.bytes_per_frame as usize / size_of::() - } else { - // The scratch buffer won't be used in this case. - 0 // Vec::with_capacity(0) does not allocate. - }; - let mut scratch_buffer = vec![0; scratch_len].into_boxed_slice(); + // Sized in i64 words, the widest sample, so a buffer served from the scratch through any + // format the callback accepts stays aligned. The I24 conversion shifts its samples through + // it as i32s, and packets the engine marks SILENT are served from it as bytes filled with + // equilibrium -- filling the engine's capture buffer in place would break the contract that + // it is read-only to the client. A zero length allocates no backing storage. + let scratch_len = stream.max_frames_in_buffer as usize * stream.bytes_per_frame as usize; + let mut scratch_buffer = vec![0i64; scratch_len.div_ceil(size_of::())].into_boxed_slice(); loop { match process_commands_and_await_signal(&mut run_ctxt, error_callback) { @@ -872,54 +884,127 @@ fn process_commands_and_await_signal( ControlFlow::Continue(handle_idx != 0) } +/// Releases the packet acquired via `IAudioCaptureClient::GetBuffer` on drop. +/// +/// WASAPI requires every successful `GetBuffer` to be paired with a `ReleaseBuffer`, so the +/// packet must be released on every path out of processing, including errors and panics. +struct CapturePacket<'a> { + capture_client: &'a Audio::IAudioCaptureClient, + frames: u32, +} + +impl CapturePacket<'_> { + /// Releases the packet, surfacing the failure that `Drop` would have to swallow. + fn release(self) -> Result<(), Error> { + let this = mem::ManuallyDrop::new(self); + unsafe { this.capture_client.ReleaseBuffer(this.frames) } + .context("Failed to release capture buffer") + } +} + +impl Drop for CapturePacket<'_> { + fn drop(&mut self) { + unsafe { + let _ = self.capture_client.ReleaseBuffer(self.frames); + } + } +} + // The loop for processing pending input data. fn process_input( stream: &StreamInner, capture_client: Audio::IAudioCaptureClient, data_callback: &mut dyn FnMut(&Data, &CallbackInfo), - scratch_buffer: &mut [i32], + scratch_buffer: &mut [i64], ) -> Result<(), Error> { unsafe { - // Get the available data in the shared buffer. - let mut buffer: *mut u8 = ptr::null_mut(); - let mut flags = mem::MaybeUninit::uninit(); + // `GetNextPacketSize` is implemented by the audio engine and reports an empty packet + // once the ring is drained, but a data callback slower than realtime refills the ring + // while this loop runs, so the drain never reaches zero -- and the bound guarantees + // `run_input` still polls its commands. What is left stays queued. + let max_frames_per_event = stream.max_frames_in_buffer.max(1); + let mut frames_drained: FrameCount = 0; loop { + if frames_drained >= max_frames_per_event { + return Ok(()); + } let mut frames_available = match capture_client.GetNextPacketSize() { Ok(0) => return Ok(()), Ok(f) => f, Err(err) => return Err(Error::from(err)), }; + frames_drained = frames_drained.saturating_add(frames_available); + // Re-initialized every packet: the driver need not write the out-params, and a + // stale buffer from the previous packet would pass for freshly captured data. + let mut buffer: *mut u8 = ptr::null_mut(); + let mut flags: u32 = 0; let mut qpc_position: u64 = 0; let mut device_position: u64 = 0; - let result = capture_client.GetBuffer( + capture_client.GetBuffer( &mut buffer, &mut frames_available, - flags.as_mut_ptr(), + &mut flags, Some(&mut device_position), Some(&mut qpc_position), - ); + )?; + let packet = CapturePacket { + capture_client: &capture_client, + frames: frames_available, + }; + + // An empty packet is reported as `AUDCLNT_S_BUFFER_EMPTY`, a *success* code, so it + // surfaces here rather than as an error. + if frames_available == 0 { + return Ok(()); + } - match result { - // TODO: Can this happen? - Err(e) if e.code() == Audio::AUDCLNT_S_BUFFER_EMPTY => continue, - Err(e) => return Err(Error::from(e)), - Ok(_) => (), + // A non-empty packet without a buffer is a driver bug; fail rather than hand the + // callback a null buffer. + if buffer.is_null() { + return Err(Error::with_message( + ErrorKind::BackendError, + "IAudioCaptureClient::GetBuffer returned a null buffer for a non-empty packet", + )); } - let flags = flags.assume_init(); // The discontinuity flag is undefined on the first GetBuffer after Start, // where device_position is still 0. let xrun = device_position != 0 && flags & Audio::AUDCLNT_BUFFERFLAGS_DATA_DISCONTINUITY.0 as u32 != 0; - debug_assert!(!buffer.is_null()); + // Every length below is derived from this frame count, and the scratch buffer is + // sized for a whole buffer's worth of it. + if frames_available > stream.max_frames_in_buffer { + return Err(Error::with_message( + ErrorKind::BackendError, + "IAudioCaptureClient::GetBuffer returned more frames than the buffer holds", + )); + } let byte_count = frames_available as usize * stream.bytes_per_frame as usize; - let data = if stream.sample_format == SampleFormat::I24 { + // A packet marked SILENT may hold uninitialized data: the engine is not required to + // have written it. Fill equilibrium into the scratch buffer and serve that instead, + // so the callback and the i24 copy see zeros, not stale driver memory -- and the + // engine-owned capture buffer stays untouched, as required below. + let silent = flags & Audio::AUDCLNT_BUFFERFLAGS_SILENT.0 as u32 != 0; + let data = if silent { + // Filling the engine-owned capture buffer would violate its read-only + // contract, so fill the scratch instead and serve that. Bounding the slice + // first keeps a length error a panic rather than UB; viewing the i64 words + // as bytes never increases alignment, so the cast is sound. + let words = &mut scratch_buffer[..byte_count.div_ceil(size_of::())]; + let zeros = slice::from_raw_parts_mut(words.as_mut_ptr().cast::(), byte_count); + fill_equilibrium(zeros, stream.sample_format); + zeros.as_mut_ptr().cast() + } else if stream.sample_format == SampleFormat::I24 { // WASAPI stores i24 in the upper bits - let source_data = - slice::from_raw_parts(buffer.cast(), byte_count / size_of::()); - // use a scratch buffer since the capture buffer isn't meant to be written - let dst = &mut scratch_buffer[..source_data.len()]; + let sample_count = byte_count / size_of::(); + let source_data = slice::from_raw_parts(buffer.cast(), sample_count); + // use a scratch buffer since the capture buffer isn't meant to be written. + // Bounding the slice first keeps a length error a panic rather than UB; + // viewing the i64 words as i32 samples only decreases alignment, so the cast + // is sound and the I24 samples stay 4-aligned. + let words = &mut scratch_buffer[..sample_count.div_ceil(2)]; + let dst = slice::from_raw_parts_mut(words.as_mut_ptr().cast::(), sample_count); dst.copy_from_slice(source_data); for sample in dst.iter_mut() { // On signed integers, >> is an arithmetic shift, @@ -941,10 +1026,33 @@ fn process_input( data_callback(&data, &CallbackInfo { timestamp, xrun }); } - // Release the buffer. - capture_client - .ReleaseBuffer(frames_available) - .context("Failed to release capture buffer")?; + packet.release()?; + } + } +} + +/// Releases the packet acquired via `IAudioRenderClient::GetBuffer` on drop. +/// +/// WASAPI requires every successful `GetBuffer` to be paired with a `ReleaseBuffer`, so the +/// packet must be released on every path out of processing, including errors and panics. +struct RenderPacket<'a> { + render_client: &'a Audio::IAudioRenderClient, + frames: u32, +} + +impl RenderPacket<'_> { + /// Releases the packet, surfacing the failure that `Drop` would have to swallow. + fn release(self) -> Result<(), Error> { + let this = mem::ManuallyDrop::new(self); + unsafe { this.render_client.ReleaseBuffer(this.frames, 0) } + .context("Failed to release render buffer") + } +} + +impl Drop for RenderPacket<'_> { + fn drop(&mut self) { + unsafe { + let _ = self.render_client.ReleaseBuffer(self.frames, 0); } } } @@ -958,12 +1066,12 @@ fn process_output( frames_written: &mut u64, ) -> Result<(), Error> { // The number of frames available for writing. - let frames_available = match get_available_frames(stream)? { - 0 => return Ok(()), // TODO: Can this happen? - n => n, - }; + let (frames_available, padding) = get_available_frames(stream)?; + // The ring can be full; the next audio event fires again once space frees up. + if frames_available == 0 { + return Ok(()); + } - let padding = stream.max_frames_in_buffer - frames_available; let fill_usec = (padding as u64) .saturating_mul(1_000_000) .saturating_div(stream.config.sample_rate as u64) @@ -984,6 +1092,14 @@ fn process_output( unsafe { let buffer = render_client.GetBuffer(frames_available)?; + // Bind the packet before the assert: a failed debug assert panics, and the packet's + // drop is what releases the buffer back to the engine, so it must exist first. The + // capture side constructs its guard immediately after GetBuffer for the same reason. + let mut packet = RenderPacket { + render_client: &render_client, + frames: frames_available, + }; + debug_assert!(!buffer.is_null()); let byte_count = frames_available as usize * stream.bytes_per_frame as usize; @@ -994,7 +1110,15 @@ fn process_output( let len = byte_count / stream.sample_format.sample_size(); let mut data = Data::from_parts(data, len, stream.sample_format); let sample_rate = stream.config.sample_rate; - let timestamp = output_timestamp(stream, sample_rate, clock_frequency, *frames_written)?; + let timestamp = + match output_timestamp(stream, sample_rate, clock_frequency, *frames_written) { + Ok(timestamp) => timestamp, + Err(err) => { + // Release the packet without presenting the unwritten buffer. + packet.frames = 0; + return Err(err); + } + }; // WASAPI exposes no render-side xrun signal. data_callback( &mut data, @@ -1016,7 +1140,7 @@ fn process_output( } } - render_client.ReleaseBuffer(frames_available, 0)?; + packet.release()?; *frames_written += frames_available as u64; } diff --git a/src/host/webaudio/mod.rs b/src/host/webaudio/mod.rs index 422fa26de..330d4680e 100644 --- a/src/host/webaudio/mod.rs +++ b/src/host/webaudio/mod.rs @@ -1,6 +1,8 @@ //! Web Audio backend implementation. //! //! Default backend on WebAssembly. +//! +//! The stream-build `timeout` is ignored. extern crate js_sys; extern crate wasm_bindgen; diff --git a/src/traits.rs b/src/traits.rs index beeff09d2..a73c2e722 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -126,6 +126,12 @@ pub trait HostTrait { /// /// Please note that `Device`s may become invalid if they get disconnected. Therefore, all the /// methods that involve a device return a `Result` allowing the user to handle this case. +/// +/// # Timeouts +/// +/// The `timeout` passed to the stream builders bounds different stages of the stream lifecycle +/// depending on the backend, and some backends ignore it entirely. The host module +/// documentation for each backend says what its `timeout` covers. pub trait DeviceTrait: PartialEq + Eq + Hash + Debug + Display + Send + Sync { /// The iterator type yielding supported input stream formats. type SupportedInputConfigs: Iterator; @@ -370,7 +376,7 @@ pub trait DeviceTrait: PartialEq + Eq + Hash + Debug + Display + Send + Sync { /// * `error_callback` - Called when a stream error occurs (e.g., device disconnected). /// * `timeout` - Time to wait for the backend to initialize the stream. `None` waits /// indefinitely; `Some(duration)` limits how long to wait. Note: not all backends honor - /// this value. + /// this value; see [Timeouts](DeviceTrait#timeouts). /// /// # Errors /// @@ -428,7 +434,7 @@ pub trait DeviceTrait: PartialEq + Eq + Hash + Debug + Display + Send + Sync { /// * `error_callback` - Called when a stream error occurs (e.g., device disconnected). /// * `timeout` - Time to wait for the backend to initialize the stream. `None` waits /// indefinitely; `Some(duration)` limits how long to wait. Note: not all backends honor - /// this value. + /// this value; see [Timeouts](DeviceTrait#timeouts). /// /// # Errors /// @@ -488,7 +494,7 @@ pub trait DeviceTrait: PartialEq + Eq + Hash + Debug + Display + Send + Sync { /// * `error_callback` - Called when a stream error occurs (e.g., device disconnected). /// * `timeout` - Time to wait for the backend to initialize the stream. `None` waits /// indefinitely; `Some(duration)` limits how long to wait. Note: not all backends honor - /// this value. + /// this value; see [Timeouts](DeviceTrait#timeouts). /// /// # Errors /// @@ -535,7 +541,7 @@ pub trait DeviceTrait: PartialEq + Eq + Hash + Debug + Display + Send + Sync { /// * `error_callback` - Called when a stream error occurs (e.g., device disconnected). /// * `timeout` - Time to wait for the backend to initialize the stream. `None` waits /// indefinitely; `Some(duration)` limits how long to wait. Note: not all backends honor - /// this value. + /// this value; see [Timeouts](DeviceTrait#timeouts). /// /// # Errors /// @@ -579,7 +585,7 @@ pub trait DeviceTrait: PartialEq + Eq + Hash + Debug + Display + Send + Sync { /// * `error_callback` - Called when a stream error occurs (e.g., device disconnected). /// * `timeout` - Time to wait for the backend to initialize the stream. `None` waits /// indefinitely; `Some(duration)` limits how long to wait. Note: not all backends honor - /// this value. + /// this value; see [Timeouts](DeviceTrait#timeouts). /// /// # Errors ///