Skip to content

Commit 22b9ff3

Browse files
authored
Reduce audible startup sync correction (#255)
## Summary This reduces audible pitch shift / warble during Sendspin client playback startup and other stream transitions. The main issue appears to be that the client can start playback with a consistent initial sync offset, then correct that offset quite aggressively using sample insert/drop correction. On my Linux endpoint this was especially noticeable at the beginning of tracks. Some discussion of this occurred at #107, however it was indeterminate and difficult to pin down the cause. With analysis and help from GPT5.5, with my full understanding and detailed review, this PR makes three related changes: - Fixes the drop-frame correction path so it discards one input frame and outputs the following frame, instead of repeating the previous output frame while consuming two input frames (bug fix) - Reduces the maximum correction rate from +/-4% to +/-0.2% (much more reliably below audible threshold) - Adds a short startup grace period before sync correction begins, so DAC/time-sync estimates can settle before the client starts inserting or dropping samples (most impactful improvement; basically correction was correcting issues that were not present simply due to lack of data) ## Analysis I was hearing pitch shift and warble on a fully up-to-date sendspin endpoint, most noticeably at playback start and during track changes. Looking through `sendspin/audio.py`, the most suspicious path was the sync correction logic. The previous drop correction branch did this: 1. Read one frame. 2. Read another frame. 3. Output the previous frame again. That effectively produced a duplicate-then-skip pattern, which is more audible than a simple one-frame drop. Separately, the correction loop allowed up to +/-4% playback speed correction over a 2 second target window. On real playback that is enough to sound like pitch movement, especially right after startup when the first sync estimate is still settling. This was changed to a maximum +/-0.2% correction over an 8 second window, which is more conservative, but still within reasonable sync delay expectations (counting to 8 will help provide confidence; if we believe users would reasonably be OK with out-of-sync clients converging within 8 seconds, then this is a reasonable default). Importantly, this is well below the threshold of audible pitch shift or warble while still providing a means to converge. In any case, if the sync is too far out, a reanchor will be triggered. Additionally, a 750ms sync correction delay was added; this does not delay audible playback, it only suppresses sample insert/drop correction briefly after playback enters the PLAYING state. That gives the DAC timing and clock-sync estimates a short window to settle before the client starts making speed adjustments based on them. In practice this avoids reacting to the first unstable startup measurements while still allowing scheduled playback to begin on time. ## Real Endpoint Comparison I tested this on my actual Sendspin Linux endpoint using the same daemon config, audio device, and negotiated format: - Device: HiFiBerry DAC+ - Format: `flac:48000:24:2` - Server: Music Assistant - Output latency reported by PortAudio: ~42.7 ms The original version repeatedly started streams around `-42 ms` sync error, then corrected aggressively. Original observed debug stats: - Underflows: `0` - Reanchors: `0` - Warnings/errors: `0` - Speed range: `98.29%` to `100.11%` - Max inserted frames per 1s log window: `821` - Max dropped frames per 1s log window: `55` Patched observed debug stats: - Underflows: `0` - Reanchors: `0` - Warnings/errors: `0` - Speed range: `99.78%` to `100.04%` - Max inserted frames per 1s log window: `106` - Max dropped frames per 1s log window: `21` The startup offset is still visible, but correction is much less aggressive and avoids the previous drop-frame artifact. ## Tests Added focused regression tests for: - Drop correction discarding one frame without repeating the previous output frame. - Startup correction grace period suppressing immediate insert/drop correction. Local verification: ```bash uv run --extra test ruff check sendspin/audio.py tests/test_audio.py uv run --extra test mypy sendspin uv run --extra test pytest ``` Results: ```text All checks passed Success: no issues found in 30 source files 91 passed ``` Testing for a day in real world scenarios has also been very successful.
1 parent e15f174 commit 22b9ff3

2 files changed

Lines changed: 95 additions & 11 deletions

File tree

sendspin/audio.py

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ class AudioPlayer:
109109
"""Maximum DAC-to-loop time ratio to prevent wild extrapolation."""
110110

111111
# Sync error correction: playback speed adjustment range
112-
_MAX_SPEED_CORRECTION: Final[float] = 0.04
113-
"""Maximum playback speed deviation for sync correction (0.04 = ±4% speed variation)."""
112+
_MAX_SPEED_CORRECTION: Final[float] = 0.002
113+
"""Maximum playback speed deviation for sync correction (0.002 = ±0.2% speed variation)."""
114114

115115
# Sync error correction: secondary thresholds (rarely need adjustment)
116116
_CORRECTION_DEADBAND_US: Final[int] = 2_000
@@ -133,8 +133,10 @@ class AudioPlayer:
133133
"""Minimum threshold for updating start time to avoid churn (5ms)."""
134134

135135
# Sync correction planning
136-
_CORRECTION_TARGET_SECONDS: Final[float] = 2.0
137-
"""Target window to fix sync error through micro-corrections (2 seconds)."""
136+
_CORRECTION_TARGET_SECONDS: Final[float] = 8.0
137+
"""Target window to fix sync error through micro-corrections (8 seconds)."""
138+
_CORRECTION_START_GRACE_US: Final[int] = 750_000
139+
"""Delay sync corrections after startup so DAC/time-sync estimates can settle."""
138140

139141
def __init__(
140142
self,
@@ -211,6 +213,7 @@ def __init__(
211213
# Scheduled start anchoring
212214
self._scheduled_start_loop_time_us: int | None = None
213215
self._scheduled_start_dac_time_us: int | None = None
216+
self._playback_started_loop_time_us: int = 0
214217

215218
# Server timeline cursor for the next input frame to be consumed
216219
self._server_ts_cursor_us: int = 0
@@ -384,6 +387,7 @@ def clear(self) -> None:
384387
self._last_dac_calibration_time_us = 0
385388
self._scheduled_start_loop_time_us = None
386389
self._scheduled_start_dac_time_us = None
390+
self._playback_started_loop_time_us = 0
387391
self._server_ts_cursor_us = 0
388392
self._server_ts_cursor_remainder = 0
389393
self._first_server_timestamp_us = None
@@ -531,15 +535,19 @@ def _audio_callback( # noqa: PLR0915
531535
# Handle correction event if at boundary
532536
if frames_remaining > 0:
533537
if drop_counter <= 0 and drop_every_n > 0:
534-
# Drop frame: read EXTRA frame to advance cursor faster
535-
_ = self._read_one_input_frame() # Read frame we're replacing
536-
_ = self._read_one_input_frame() # Read frame we're DROPPING
538+
# Drop one input frame, then output the following frame. This
539+
# advances the source cursor by two frames while rendering one,
540+
# avoiding the old duplicate-then-skip artifact.
541+
_ = self._read_one_input_frame()
542+
replacement_frame = self._read_one_input_frame()
543+
if replacement_frame is None:
544+
replacement_frame = self._last_output_frame
537545
drop_counter = drop_every_n
538546
self._frames_dropped_since_log += 1
539-
# Output last frame instead (don't output either frame we read)
540547
output_buffer[bytes_written : bytes_written + frame_size] = (
541-
self._last_output_frame
548+
replacement_frame
542549
)
550+
self._last_output_frame = replacement_frame
543551
bytes_written += frame_size
544552
frames_remaining -= 1
545553
insert_counter -= 1
@@ -1035,14 +1043,20 @@ def _handle_start_gating(
10351043
// self._MICROSECONDS_PER_SECOND
10361044
)
10371045
self._skip_input_frames(frames_to_drop)
1038-
self._playback_state = PlaybackState.PLAYING
1046+
self._set_playing()
10391047

10401048
# If we've reached/overrun the scheduled time, arm playback
10411049
if current_time_us >= target_time_us:
1042-
self._playback_state = PlaybackState.PLAYING
1050+
self._set_playing()
10431051

10441052
return bytes_written
10451053

1054+
def _set_playing(self) -> None:
1055+
"""Transition to PLAYING and start the correction grace window once."""
1056+
if self._playback_state != PlaybackState.PLAYING:
1057+
self._playback_started_loop_time_us = self._now_us()
1058+
self._playback_state = PlaybackState.PLAYING
1059+
10461060
def _update_correction_schedule(self, error_us: int) -> None:
10471061
"""Plan occasional sample drop/insert to correct sync error.
10481062
@@ -1062,6 +1076,13 @@ def _update_correction_schedule(self, error_us: int) -> None:
10621076

10631077
abs_err = abs(self._sync_error_filtered_us)
10641078

1079+
if self._playback_started_loop_time_us:
1080+
since_start_us = self._now_us() - self._playback_started_loop_time_us
1081+
if since_start_us < self._CORRECTION_START_GRACE_US:
1082+
self._insert_every_n_frames = 0
1083+
self._drop_every_n_frames = 0
1084+
return
1085+
10651086
# Do nothing within deadband
10661087
if abs_err <= self._CORRECTION_DEADBAND_US:
10671088
self._insert_every_n_frames = 0

tests/test_audio.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
from __future__ import annotations
2+
3+
from types import SimpleNamespace
4+
5+
from sendspin.audio import AudioPlayer, PlaybackState, _QueuedChunk
6+
7+
8+
class _NoCallbackStatus:
9+
input_underflow = False
10+
output_underflow = False
11+
12+
def __bool__(self) -> bool:
13+
return False
14+
15+
16+
def test_drop_correction_discards_one_frame_without_repeating_previous() -> None:
17+
now_us = 0
18+
19+
def now() -> int:
20+
return now_us
21+
22+
player = AudioPlayer(lambda ts: ts, lambda ts: ts, now_us=now)
23+
player._format = SimpleNamespace( # noqa: SLF001
24+
sample_rate=48_000,
25+
channels=1,
26+
bit_depth=16,
27+
frame_size=2,
28+
)
29+
player._playback_state = PlaybackState.PLAYING # noqa: SLF001
30+
player._drop_every_n_frames = 1 # noqa: SLF001
31+
player._frames_until_next_drop = 1 # noqa: SLF001
32+
player._queue.put( # noqa: SLF001
33+
_QueuedChunk(
34+
server_timestamp_us=0,
35+
audio_data=b"\x01\x00\x02\x00\x03\x00",
36+
)
37+
)
38+
39+
out = bytearray(4)
40+
player._audio_callback( # noqa: SLF001
41+
memoryview(out),
42+
frames=2,
43+
time=SimpleNamespace(outputBufferDacTime=0.0),
44+
status=_NoCallbackStatus(),
45+
)
46+
47+
assert bytes(out) == b"\x01\x00\x03\x00"
48+
49+
50+
def test_sync_correction_waits_for_startup_grace_period() -> None:
51+
now_us = 1_000_000
52+
53+
def now() -> int:
54+
return now_us
55+
56+
player = AudioPlayer(lambda ts: ts, lambda ts: ts, now_us=now)
57+
player._format = SimpleNamespace(sample_rate=48_000) # noqa: SLF001
58+
player._playback_started_loop_time_us = now_us # noqa: SLF001
59+
60+
player._update_correction_schedule(50_000) # noqa: SLF001
61+
62+
assert player._drop_every_n_frames == 0 # noqa: SLF001
63+
assert player._insert_every_n_frames == 0 # noqa: SLF001

0 commit comments

Comments
 (0)