Repository navigation
fix(#91): a VOD stream plays out to its end, then reports done+ended - #113
Merged
Merged
Conversation
The player reported `done`/emitted `ended` while roughly a segment of audio was still queued-but-unplayed, and it fed the worklet in one burst that overflowed the fixed 16-second ring and silently dropped audio — so the last ~4 s of a VOD clip never played and the host could not tell a finished stream from a freeze. Pace PCM posting to the worklet by the play-ahead so the ring never overflows, then drain to completion (play-ahead ~0) before setting the persistent `done` flag and emitting `ended` exactly once. New browser tests assert each fixture's audio plays out within 1 s of the advertised EXTINF total and that `done`/`ended` fire; all said tests fail on the unfixed player. Browser gate: 21 passed (14 pre-existing + 7 new), Rust gate 157/157.
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.
Closes #91. Delegated to
deepseek-v4-flashvia crush; $0.04 for the session. I verified every claim independently — details below.Root cause — two compounding faults, both measured
_pumpAudioInnerdrained all decoded PCM and posted it to the worklet in one tick. A clip decodes far faster than it plays, so the ring overflowed and silently dropped audio. Measured:audioSecfroze at 16.00 withahead=2.87— about the ring's capacity, i.e. 2.87 s of fed audio physically unplayable.donewas never reached while buffered audio was still queued. The end-of-stream path flushed and emittedendedwhile roughly a segment remained posted-but-unplayed. Anddonewas a transient on the emitted stat object, so a later stats emission cleared it — the first observed run froze ataudioSec=14.88,done=false, permanently.That second half is the worse one: a host could not distinguish "stream finished" from "player froze".
Fix
_AUDIO_LEAD_Sof play-ahead, so the ring cannot overflow.done— set as a field on_statsso a later emission cannot clear it;endedfires exactly once.Verification — I re-ran all of it, and negative-controlled the tests
cargo fmt --all --checkclippy --workspace --all-targets -- -D warningscargo nextest run --workspacebun test(packages/player)tsc --noEmit -p packages/playerbun run test:streamsThe new tests genuinely bite. I reverted only
skyfire-player.jsto its pre-fix state, kept the tests, and re-ran:The timing difference is the tell: unfixed, the tests short-circuit in ~2 s because the old code declared the stream over almost immediately. Fixed, each takes the clip's full ~19 s because it now actually waits for playout. A test that passed by accident could not show that asymmetry.
Measured on
arte(advertised 18.76 s): before 14.88–16.01 s played withdone=false; after 18.85 s,done=true,endedonce,ahead→0.03,queued=0. Within 0.1 s of advertised, against a 1 s criterion.Scope
Additive only —
packages/player/skyfire-player.jsand 70 new lines inweb/tests/streams.spec.mjs. No existing test was modified, which I checked by diffing rather than trusting the report.Note on the oracle
The assertion is against the playlist's own advertised total (sum of
EXTINF), computed per-stream at test time, not a hardcoded number. That was chosen deliberately so it cannot be satisfied by tuning a constant, and it covers all seven fixtures rather than the two I originally measured.🤖 Generated with Claude Code
https://claude.ai/code/session_01Qsz3AufTr5z6MPZJzkoSCT