Skip to content

Fix speaker audio desync by removing server-side pacing and adding sample-offset sync#2412

Closed
joviah wants to merge 4 commits intocc-tweaked:mc-1.20.xfrom
joviah:fix/speaker-audio-sync
Closed

Fix speaker audio desync by removing server-side pacing and adding sample-offset sync#2412
joviah wants to merge 4 commits intocc-tweaked:mc-1.20.xfrom
joviah:fix/speaker-audio-sync

Conversation

@joviah
Copy link
Copy Markdown

@joviah joviah commented Apr 6, 2026

Summary

Fixes #1874 and #2120.

  • Removed server-side wall-clock pacing in DfpwmState. Audio is now sent to the client on the next server tick after Lua pushes it. The existing single-slot buffer design provides natural flow control.
  • Added sample-offset timestamps to EncodedAudio packets. The server tracks cumulative samples sent per stream.
  • Added client-side drift detection in DfpwmStream. The client tracks consumed samples and compares against incoming offsets. Small drift self-corrects; drift beyond 500ms triggers a hard reset (queue clear + resume from latest packet).

Why not improve the estimation?

The server fundamentally cannot model client playback accurately — network latency, sound engine scheduling, volume=0, and the 8-speaker Minecraft limit all cause the estimate to drift. Rather than building a better estimate, this PR removes the estimation entirely and lets the client self-correct using real data.

Edge cases covered

Test plan

  • Unit tests for EncodedAudio sampleOffset serialization roundtrip
  • Unit tests for DfpwmState sampleOffset tracking and isPlaying() timeout
  • Unit tests for DfpwmStream drift detection: normal flow, hard reset, stale packet drop, small drift, read stall, stream restart
  • Clean boot test on 1.20.1 Forge (no crashes or errors)
  • Manual test: reproduction script from speakers.playAudio sometimes plays only on the next call #1874
  • Manual test: volume=0 then increase mid-stream
  • Manual test: >8 speakers

Files changed

  • EncodedAudio.java — added sampleOffset field + VarLong serialization + withSampleOffset()
  • DfpwmState.java — removed clientEndTime/CLIENT_BUFFER, added sampleOffset counter, simplified shouldSendPending(), updated isPlaying() to use timeout
  • SpeakerPeripheral.java — updated shouldSendPending() call site
  • DfpwmStream.java — added sync state (consumedSamples, bufferedSamples, streamBaseOffset), drift detection in push(), sample tracking in read(), synchronized access, debug logging
  • Updated all 3 existing test files + added new test cases

joviah and others added 4 commits March 20, 2026 13:02
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Send audio eagerly on next tick instead of estimating client playback.
Track cumulative sample offset per stream for client-side sync.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Track consumed and buffered samples against server-provided offsets.
Hard reset when drift exceeds 500ms, drop stale packets.
Synchronized push()/read() for thread safety.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@SquidDev
Copy link
Copy Markdown
Member

SquidDev commented Apr 7, 2026

Ah, I had not realised you were planning to use Claude to write this. CC:T does not accept LLM-generated code — I have updated the contributing documentation to more accurately reflect this.

The server tracks cumulative samples sent per stream. [...] Removed server-side wall-clock pacing in DfpwmState

I think tagging each audio packet with the expected time it will play, and then dropping/pad the audio buffer as needed makes sense — I've debated doing something similar with global timer in an attempt to fix #2412, but that's a bit trickier than using a per-stream timer.

However, I think any approach here needs to use the time (or at least the sound engine's notion of time), rather than just the sample count. If the sound engine itself is not reading the the audio stream (which is the underlying problem of #2108 or #1313), then this fix does nothing.

I think the server has to remain the source of truth here — the implementation here allows you the server to send an infinite amount of audio data to the client, potentially OOMing them — it's just the way DfpwmStream handles providing data to the sound engine needs improving.

@SquidDev SquidDev closed this Apr 7, 2026
@joviah
Copy link
Copy Markdown
Author

joviah commented Apr 7, 2026

No worries! Thanks!

@joviah joviah deleted the fix/speaker-audio-sync branch April 7, 2026 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

speakers.playAudio sometimes plays only on the next call

2 participants