[kahrendt]

Summary

The sync task's playtime estimate (when the next written frame will finish playing) moves only when it writes audio or the sink reports playback. In steady state a drained sink is not bridged with silence, and sinks report only the audio they were given (the host PortAudio example and ESPHome's I2S speaker both do), so after the sink runs dry mid-stream the estimate stays at the end of the last audio played. The next hard sync is measured against that stale endpoint:

• Server timestamp gap longer than the send-ahead lead (a server keeping a live-input stream open across 20 s of silence): the client logged Lost sync (19964581us off), inserted silence as long as the gap, and then dropped that much of the resumed audio as late.
• Stall with continuous timestamps (the sink ran out because audio arrived or decoded late): the first chunks after it look on time against the stale endpoint and play late uncorrected, up to a sink buffer's worth, until a playback report arrives and the next chunk drops the late prefix.

roles/player/v1.md "Large errors and startup" asks recovery from an underrun to snap to the chunk's timestamp in one shot.

Change

• New SyncTask::starved_playtime_floor(playtime, now): returns now when the estimate trails it by more than HARD_SYNC_THRESHOLD_US, else the estimate.
• handle_synchronize_audio() applies it before computing the error.

Frames not yet written cannot finish before now, so max(estimate, now) is never further from the truth than the estimate. The floor cannot fire while the sink still holds audio, so startup priming, seeks (the aligning keepalive keeps the estimate current) and steady-state sync are unchanged.

The load-time lateness check deliberately keeps the unfloored estimate: it skips whole chunks by their start time, so flooring there would discard partly-late chunks instead of dropping their late prefix.

The floored estimate does not include the sink's hidden latency (about 50 ms of DMA silence on ESPHome after an underrun). The existing re-check of the held chunk after the gap silence drops that remainder once a report arrives, so a stall on such a sink can log two Lost sync/Regained sync pairs instead of one. Lost sync now reports the corrected error (the lead, or the late amount) rather than the gap length.

Tests

• PlayerHardSync.AStarvedEstimateIsFlooredAtNow: table test of the helper (ahead, exactly at the threshold, just over it, far behind).
• PlayerHardSync.AfterAnUnderrunTheGapIsMeasuredFromNow: drives SYNCHRONIZE_AUDIO/TRANSFER_AUDIO without the thread; a chunk due 1 s from now against an estimate 20 s stale gets at most 1 s of silence, then plays whole.

[Copilot]

🟢 Approval recommended

The implementation is focused, documented, and adequately covered by observable behavior tests.

0 open findings

What changed in this PR

Floors stale playback estimates at the current time after sink starvation, preventing oversized hard-sync corrections.

Changes:

• Adds and applies starved_playtime_floor().
• Adds focused unit and integration coverage.
• Documents underrun recovery behavior.

File	Description
src/​sync_task.cpp	Applies and implements the playback-time floor.
src/​sync_task.h	Declares and documents the helper.
tests/​test_player_role.cpp	Tests threshold and underrun recovery behavior.
tests/​CMakeLists.txt	Documents white-box test access.
docs/​playback-sync.md	Describes starvation handling.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.