Repository navigation
fix(core): a looping preview keeps playing when a scene reloads during a hold for data - #7659
Conversation
…g a hold for data A Studio preview unloads scenes far from the playhead. On a long film, the loop's wrap back to the start lands while the first scene is reloading: the film holds for its video, then the scene swap re-seeks with keepPlaying. The seek read the held film as paused, dropped the hold and parked the film at the start, so the loop stopped after its second pass. A seek that keeps playing now counts a hold for data as playing. Co-Authored-By: Claude Opus 5.5 <[email protected]>
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at a0a715a9d15df3b653bdd8b300c22d9cf423f05d.
Need: yes, preserve playback through a buffer-held scene reload · Size: ok · Reuse: existing buffering state and transport.play() · Simpler: already minimal · Structure: fine (checked seekWhenReady/applySeek and transport; no split)
Risk: low — two seek decisions now preserve the existing logical playback state.
No blocking findings. Both seek decisions need the same definition of playing: a stopped clock can represent a buffer hold. Explicit pause still clears that intent, and paused keepPlaying seeks remain paused.
Validation: frozen install and full build passed; vitest run src/runtime --maxWorkers 2 passed all 137 files / 3,179 tests. Four additional fixture probes passed: pause before/after a held keepPlaying seek, four successive buffer-held seeks, and seeking away from a starved clip. Reverting the fix makes the new regression test fail. A routing-only revert also fails; reverting only the applySeek predicate still passes, so the test does not isolate that predicate. Changed-file lint and formatting passed. I did not independently repeat the reported 117-second Chrome walkthrough.
Nit — packages/core/src/runtime/init.buffering.test.ts:286–287: remove the historical “used to stop” clause, retaining the live invariant. The comment-citations check flags that clause; it does not affect runtime correctness.
Verdict: APPROVE
Reasoning: The fix preserves playback intent in both relevant seek paths and retains explicit pause behavior. The regression test reproduces the failure when the fix is reverted.
— Rames
…its test comment Co-Authored-By: Claude Opus 5.5 <[email protected]>
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 756a61a35d69d5d9256a2c3373fe07459cc9fa5b, including the delta from a0a715a9d15df3b653bdd8b300c22d9cf423f05d.
Need: yes, resolve the comment nit while retaining the loop fix · Size: ok · Reuse: playingOrHeldForData shared by both seek decisions · Simpler: already minimal · Structure: fine (checked both helper call sites; no split)
Risk: low — behavior-neutral predicate extraction and comment cleanup.
My previous comment nit is resolved. Mechanically inlining the new helper and restoring the deleted comment produces a byte-identical init.ts to the reviewed revision; the test differs only in comment wording. The closure reads the live clock and buffering array at each call.
Frozen install, changed-file lint/format, comment citations and comment ratchet pass at this head. I did not rerun the runtime suite: the prior 3,179-test run and revert probes were at a0a715a9d, and the executable behavior is unchanged by this delta.
Verdict: APPROVE
Reasoning: No new blockers; the requested comment cleanup is complete and both seek paths retain the reviewed buffer-hold behavior.
— Rames
With loop turned on, a long film in the Studio preview now loops indefinitely. Before this change it wrapped back once, played a second time, then stopped at the start with the playhead frozen at 0:00 while the bar still showed Pause.
What you'll see
Why
The Studio preview unloads scenes that are far from the playhead. In a long film, the opening scene gets unloaded during a full pass. At the next wrap back to the start:
bufferingFor), which is normal.keepPlaying: true.seekWhenReadyandapplySeekdecided "playing" fromclock.isPlaying()alone, and the clock is stopped during a hold for data. So the seek took the paused path: it clearedbufferingForand landed as a paused seek.play()again.The first wrap usually works because the opening scene is still loaded then. That's why the loop stopped "after one loop" and only on long films.
How
A seek with
keepPlayingnow counts a hold for data as playing (clock.isPlaying() || bufferingFor.length > 0), in bothseekWhenReadyandapplySeek. The seek then resumes throughtransport.play(). If the media still isn't ready, the next tick holds again, and it resumes as usual when the data arrives.Test plan
init.buffering.test.ts, "a seek that keeps playing during a hold for data plays on once the track is ready". It fails onmain(the film stays at 0) and passes with the fix.vitest run src/runtimeinpackages/core: 137 files, 3,179 tests pass.🤖 Generated with Claude Code