Skip to content

fix(player): pause parent audio proxy on seek to prevent stutter loop#890

Merged
terencecho merged 1 commit into
mainfrom
fix/player-seek-pauses-parent-audio-proxy
May 16, 2026
Merged

fix(player): pause parent audio proxy on seek to prevent stutter loop#890
terencecho merged 1 commit into
mainfrom
fix/player-seek-pauses-parent-audio-proxy

Conversation

@terencecho
Copy link
Copy Markdown
Contributor

Summary

  • seek() only called seekAll() under parent audio ownership, leaving the <audio> proxy playing while the timeline froze at the new seek target.
  • The periodic mirrorTime drift-correction (parent-media.ts) would then yank currentTime back to the timeline position every ~80ms of accumulated drift, producing an audible stutter loop while the video frame stayed frozen.
  • Fix: make seek() symmetric with pause() — pause the parent proxy before seeking it.

Repro

  1. Use the player in an environment where the runtime posts media-autoplay-blocked (mobile / autoplay-restricted contexts), promoting audio ownership to "parent".
  2. Start playback with audio.
  3. Click anywhere on the scrubber while playing.
  4. Before: audio stutters in a short loop while the video frame is frozen.
  5. After: audio cleanly pauses at the new seek position.

Test plan

  • Added regression test `seek() while playing pauses parent proxy (prevents mirrorTime stutter loop)` in `hyperframes-player.test.ts`.
  • `pnpm --filter @hyperframes/player test` — 110/110 pass.
  • Manual repro on a device where ownership flips to `parent`.

🤖 Generated with Claude Code

`seek()` only called `seekAll()` when audio ownership was promoted to
the parent frame, leaving the `<audio>` proxy playing. With the
timeline frozen at the new seek target, the periodic `mirrorTime`
drift-correction would yank `currentTime` back each tick, producing
an audible audio stutter loop while the visual frame stayed frozen.

Make `seek()` symmetric with `pause()` for the parent-owned audio
path: pause before seeking the proxy.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean symmetric-with-pause fix for the mirrorTime stutter loop.

Audited

The root-cause framing is precise: seek() set this._paused = true and called seekAll(target), but never called pauseAll() on the parent audio proxy. So the proxy kept playing past the now-frozen timeline target, and the periodic mirrorTime drift-correction (parent-media.ts) yanked currentTime back every ~80ms of accumulated drift → audible stutter loop while the video frame stayed frozen.

The fix is exactly the missing call:

if (this._media.audioOwner === "parent") {
  this._media.pauseAll();      // ← added
  this._media.seekAll(timeInSeconds);
}

The inline comment captures the failure mode in one sentence: "leaving the proxy playing turns the next mirrorTime drift-correction tick into a perpetual seek→play→drift→seek stutter loop." That's exactly the right thing to pin for future readers. ✓

The fix is scoped to audioOwner === "parent" — the runtime-owned path isn't touched. That's correct because runtime-owned audio lives inside the iframe and is driven by the runtime's own seek-handling logic; the parent-proxy path is specifically for autoplay-blocked / mobile contexts where audio plays from the parent's <audio> element. The bug only manifests under that ownership, and the fix is scoped to it.

Test coverage

The new regression test is right-shaped:

player._promoteToParentProxy?.();
player.play();
mockAudio.pause.mockClear();   // isolate from play()'s incidental work
player.seek(12.5);
expect(mockAudio.pause).toHaveBeenCalled();
expect(mockAudio.currentTime).toBe(12.5);

Pins both the pause call AND the seek target. The mockClear() between play and seek isolates the assertion from any pause calls play() might have made. ✓

Body claim verification

  • "seek() only called seekAll() under parent audio ownership, leaving the proxy playing" — verified by the diff ✓
  • "mirrorTime drift-correction (parent-media.ts) would then yank currentTime back to the timeline position every ~80ms" — citation to out-of-diff code; plausible mechanism
  • "Fix: make seek() symmetric with pause() — pause the parent proxy before seeking it" — verified ✓

Non-blocking note

Path-enum for other seekAll-without-pauseAll call sites: the bug pattern is "set audio proxy currentTime while the parent's mirrorTime drift-correction is still running." Worth a quick grep for other entry points that could trigger the same shape — frameStep, stepFrame, programmatic currentTime setters, or any other public method that mutates timeline state under audioOwner === "parent". If there are other callers of seekAll (or direct audio.currentTime = ... writes), they may have the same drift-loop bug at a different entry point. The pause-before-seek invariant should be centralized — either every seekAll call site does the pause defensively (current pattern), or seekAll itself calls pauseAll first (cleaner contract). Cosmetic for this PR; worth a follow-up audit.

CI

Preflight, Detect changes, Format, Semantic PR title, File size check, regression all green so far. Test, Typecheck, Lint, Build, CLI smoke (required), Perf:*, Analyze (*) in-progress on this commit. No failures.

Test plan checkbox "Manual repro on a device where ownership flips to parent" still unchecked — the regression test pins the unit behavior; the manual integration verification on a mobile/autoplay-blocked device is the remaining gate. Worth doing once before merge but not blocking.

Review by Rames Jusso (pr-review)

@terencecho terencecho merged commit c003ef6 into main May 16, 2026
37 checks passed
@terencecho terencecho deleted the fix/player-seek-pauses-parent-audio-proxy branch May 16, 2026 06:20
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.

2 participants