fix: keep timestamps monotonic across device and graph changes#1251
Open
roderickvd wants to merge 5 commits into
Open
fix: keep timestamps monotonic across device and graph changes#1251roderickvd wants to merge 5 commits into
roderickvd wants to merge 5 commits into
Conversation
…ng poll A nonzero timeout under 1ms truncated to 0, which ALSA reads as a non-blocking poll. That busy-spins instead of waiting the short interval the caller asked for. Round up to 1ms. An explicit Duration::ZERO still maps to 0, so a deliberate non-blocking poll still works.
… one A frame count and its 100ns duration aren't exact multiples. Truncating on the way out and again on the way back drops common sizes (512, 1024, ...) by a frame, so the supported range and default buffer size came back one short. Round both halves of the round-trip.
A host's reported latency can jump between callbacks: a device reroute, an audio graph reconnection, or a timestamp read that fails and falls back to now() with no latency offset. Folding that into the timestamp can walk the derived instant backward. Wrap the data callbacks in a shared non-decreasing clamp, applied only to the hosts that can actually regress. iOS and JACK didn't account for buffer depth yet, so I added that here too: - iOS: capture and playback now include hardware latency, and refresh it when the audio route changes. - JACK: timestamps now include port latency.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure StreamInstant timestamps remain monotonic across all hosts even when reported latency changes between callbacks (e.g., device reroutes, graph reconnects, transient timestamp read failures), by clamping callback timestamps to never regress.
Changes:
- Add shared “non-decreasing” wrappers for input/output data callbacks and apply them across multiple backends where latency can vary.
- Fix/adjust latency and buffer-depth accounting across hosts (notably iOS route changes + hardware latency, JACK port latency, macOS default output reroute depth refresh).
- Address backend-specific time/buffer issues (ALSA poll timeout rounding; WASAPI buffer size round-trip rounding).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/host/webaudio/mod.rs | Wrap output callback with monotonic clamp to prevent regressions when outputLatency changes. |
| src/host/wasapi/device.rs | Clamp output timestamps; fix buffer size ↔ duration conversions by rounding. |
| src/host/pulseaudio/mod.rs | Wrap input/output callbacks with monotonic clamps to handle latency changes on source/sink switches. |
| src/host/pipewire/device.rs | Wrap input/output callbacks with monotonic clamps to handle graph delay changes. |
| src/host/mod.rs | Introduce shared non-decreasing clamp and callback wrapper helpers. |
| src/host/jack/stream.rs | Include JACK port/hardware latency in timestamps (capture/playback). |
| src/host/jack/device.rs | Apply monotonic clamps around JACK callbacks to prevent regressions on repatching/latency changes. |
| src/host/coreaudio/macos/mod.rs | Extend default-output monitoring to refresh latency/buffer-depth on reroute. |
| src/host/coreaudio/macos/device.rs | Apply monotonic clamp for playback; track/update buffer depth for DefaultOutput reroutes. |
| src/host/coreaudio/ios/session_event_manager.rs | Refresh shared latency depth on route-change notifications. |
| src/host/coreaudio/ios/mod.rs | Apply monotonic clamps; include hardware latency in buffer depth; round frame computations. |
| src/host/audioworklet/mod.rs | Clamp playback timestamps; add polling of outputLatency on main thread and share via atomics. |
| src/host/asio/stream.rs | Apply monotonic clamps for capture/playback on ASIO latency-change events. |
| src/host/alsa/mod.rs | Apply monotonic clamps; round up sub-millisecond poll timeouts to avoid busy polling. |
| src/host/aaudio/mod.rs | Apply monotonic clamps for capture/playback when timestamp reads transiently fail. |
| CHANGELOG.md | Document monotonic timestamp fixes and backend-specific latency/buffer behavior changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
254
to
258
| // rem_frames < rate <= u32::MAX, so rem_frames * 1_000_000_000 < u64::MAX | ||
| let rem_frames = frames as u64 % rate; | ||
| let nanos = rem_frames * 1_000_000_000 / rate; | ||
| // Round to nearest so the duration isn't biased. | ||
| let nanos = (rem_frames * 1_000_000_000 + rate / 2) / rate; | ||
| std::time::Duration::new(secs, nanos as u32) |
Comment on lines
+908
to
+912
| let latency_frames = match callback_latency_frames.load(Ordering::Relaxed) { | ||
| // Fall back to this buffer's own frame count if the depth is unknown (zero). | ||
| 0 => len / channels as usize, | ||
| n => n, | ||
| }; |
Comment on lines
+286
to
+287
| // Keep `playback` monotonic: outputLatency can drop e.g. if the page calls `setSinkId()` to | ||
| // switch output devices), which would pull `playback` backward. |
Comment on lines
+384
to
+386
| // Keep `capture` monotonic: pw_time delay() grows when another client joins | ||
| // needing a larger buffer), which can pull `capture` backward. | ||
| let data_callback = crate::host::monotonic_input_callback(data_callback); |
Comment on lines
+560
to
+562
| // Keep `playback` monotonic: pw_time delay() shrinks when other clients that needed | ||
| // a larger buffer leave the graph, which can pull `playback` backward. | ||
| let data_callback = crate::host::monotonic_output_callback(data_callback); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#1246 and #1247 fixed monotonicity on WASAPI and PipeWire one at a time; this PR takes another pass to ensure no timestamp can regress on any host.
The problem is that a host's reported latency can jump between callbacks (a reroute, a graph reconnection, or a timestamp read that fails and falls back to
now()). Add that to the timestamp and it can step backward, whileStreamInstantis supposed to be strictly monotonic. As solution, on hosts and directions where this can occur, the data callbacks now run through a shared non-decreasing clamp.While I was in here:
Duration::ZEROstill means 0 on purpose.REFERENCE_TIMEround-trip truncated both ways, so 512, 1024 and friends came back a frame short. Rounds now.