mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(runtime): clear play guard after hard seek to prevent audio desync on scrub (#639)
* fix(runtime): clear play guard after hard seek to prevent audio desync on scrub When scrubbing the timeline during playback, syncRuntimeMedia detects the offset jump and hard-seeks the media element. But the in-flight play() guard (playRequested WeakSet) from the previous play() call prevented the next sync tick from re-issuing play() — leaving the element paused at the new position for 50-150ms while the GSAP timeline continued advancing. This caused audible audio desync after every scrub. Fix: clear playRequested on the element after a hard seek so the very next sync tick can re-issue play(). Also adds a lint rule (video_audio_double_source) that catches compositions where an unmuted <video> and a separate <audio> point to the same source — a pattern that causes double playback at runtime. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(runtime): detect failed seeks past MP3 buffer and force full fetch Root cause: streaming MP3 with preload="metadata" only buffers the first ~15 seconds. Seeking past the buffered range silently fails — currentTime stays at 0 while the timeline advances, causing permanent audio desync that only a page refresh fixes. Three changes: 1. Move preload="auto" enforcement to run for ALL active elements on every sync tick (not just during play). This catches elements whose preload was overridden after init.ts set it. 2. After a hard seek, check if currentTime actually reached the target. If not (drift > 0.5s), call load() once to force the browser to fully fetch the media and build a complete seek index. 3. Clear the load-retry guard when the clip leaves its active window so re-entry can retry if needed. Reproduced on hyperframes.dev Hermes launch video: vo.mp3 buffered to 15.96s, seeking to 20s failed silently. bg-music.wav (fully buffered) was unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
4a06fb6e84
commit
ea90bbe076
@@ -460,6 +460,44 @@ export const mediaRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> =
|
||||
return findings;
|
||||
},
|
||||
|
||||
// video_audio_double_source — catches audible <video> paired with a separate
|
||||
// <audio> pointing to the same file, which causes double playback at runtime
|
||||
({ tags }) => {
|
||||
const findings: HyperframeLintFinding[] = [];
|
||||
const videoSources = new Map<string, { id?: string; raw: string }>();
|
||||
const audioSources = new Map<string, { id?: string; raw: string }>();
|
||||
|
||||
for (const tag of tags) {
|
||||
if (!readAttr(tag.raw, "data-start")) continue;
|
||||
const src = readAttr(tag.raw, "src");
|
||||
if (!src) continue;
|
||||
const elementId = readAttr(tag.raw, "id") || undefined;
|
||||
if (tag.name === "video") {
|
||||
const isMuted = hasAttrName(tag.raw, "muted");
|
||||
if (!isMuted) {
|
||||
videoSources.set(src, { id: elementId, raw: tag.raw });
|
||||
}
|
||||
} else if (tag.name === "audio") {
|
||||
audioSources.set(src, { id: elementId, raw: tag.raw });
|
||||
}
|
||||
}
|
||||
|
||||
for (const [src, audioInfo] of audioSources) {
|
||||
const videoInfo = videoSources.get(src);
|
||||
if (!videoInfo) continue;
|
||||
findings.push({
|
||||
code: "video_audio_double_source",
|
||||
severity: "error",
|
||||
message: `<audio${audioInfo.id ? ` id="${audioInfo.id}"` : ""}> and <video${videoInfo.id ? ` id="${videoInfo.id}"` : ""}> both point to the same source. The unmuted video already provides audio — the duplicate <audio> will cause double playback and echo.`,
|
||||
elementId: audioInfo.id,
|
||||
fixHint:
|
||||
"Either mute the video (add `muted` attribute) and keep the separate <audio>, or remove the <audio> element and let the video provide its own audio track.",
|
||||
snippet: truncateSnippet(audioInfo.raw),
|
||||
});
|
||||
}
|
||||
return findings;
|
||||
},
|
||||
|
||||
// imperative_media_control
|
||||
findImperativeMediaControlFindings,
|
||||
];
|
||||
|
||||
@@ -204,17 +204,15 @@ describe("syncRuntimeMedia", () => {
|
||||
expect(clip.el.play).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("nudges preload to auto AND calls play() on unbuffered media in one pass", () => {
|
||||
// Assets added after the runtime already bound its metadata listeners
|
||||
// (e.g. a sub-composition that injects a late <audio>) need both the
|
||||
// preload nudge and the play() call from the same sync tick — the
|
||||
// reviewer flagged that these two concerns must not drift.
|
||||
it("forces preload=auto on every active element, not just during play", () => {
|
||||
// Streaming formats (MP3) may arrive with preload="metadata", which only
|
||||
// buffers the first few seconds. Setting preload="auto" on every active
|
||||
// tick catches elements whose preload was overridden after init.ts set it
|
||||
// — and ensures it happens even when paused (e.g. during a seek).
|
||||
const clip = createMockClip({ start: 0, end: 10 });
|
||||
Object.defineProperty(clip.el, "readyState", { value: 0, writable: true });
|
||||
Object.defineProperty(clip.el, "preload", { value: "metadata", writable: true });
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: true, playbackRate: 1 });
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: false, playbackRate: 1 });
|
||||
expect(clip.el.preload).toBe("auto");
|
||||
expect(clip.el.play).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not re-fire play() while a previous play() is in flight", () => {
|
||||
@@ -230,6 +228,77 @@ describe("syncRuntimeMedia", () => {
|
||||
expect(clip.el.play).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("re-issues play() after a hard seek clears the in-flight guard", () => {
|
||||
// A scrub during playback triggers a hard seek (offset jump > 0.5s).
|
||||
// The fix clears the playRequested guard so the very next sync tick can
|
||||
// re-issue play() instead of waiting 50-150ms for the guard to clear
|
||||
// naturally — closing the audible desync gap on timeline scrub.
|
||||
const clip = createMockClip({ start: 0, end: 20, mediaStart: 0 });
|
||||
Object.defineProperty(clip.el, "currentTime", { value: 2, writable: true });
|
||||
// Steady-state playback at t=2
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 2, playing: true, playbackRate: 1 });
|
||||
expect(clip.el.play).toHaveBeenCalledTimes(1);
|
||||
// Scrub to t=15 — hard seek fires, guard should be cleared
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 15, playing: true, playbackRate: 1 });
|
||||
// Next tick: play() should fire again (guard was cleared by the seek)
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 15.02, playing: true, playbackRate: 1 });
|
||||
expect(clip.el.play).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("calls load() once when a seek fails past the buffered range (MP3 partial buffer)", () => {
|
||||
// Streaming MP3 with preload="metadata" only buffers the first ~15s.
|
||||
// When the user seeks to 20s, el.currentTime = 20 silently fails —
|
||||
// currentTime stays at 0. The fix detects this and calls load() once
|
||||
// to trigger a full network fetch.
|
||||
const clip = createMockClip({ start: 0, end: 30, mediaStart: 0 });
|
||||
// Simulate: currentTime is writable but the setter is intercepted
|
||||
// to stay at 0 (simulating failed seek past buffer).
|
||||
let internalTime = 0;
|
||||
Object.defineProperty(clip.el, "currentTime", {
|
||||
get: () => internalTime,
|
||||
set: () => {
|
||||
// Seek silently fails — stays at 0 (MP3 past buffer)
|
||||
},
|
||||
configurable: true,
|
||||
});
|
||||
clip.el.load = vi.fn();
|
||||
// First tick at t=20 — hard seek fires, fails, should call load()
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 20, playing: true, playbackRate: 1 });
|
||||
expect(clip.el.load).toHaveBeenCalledTimes(1);
|
||||
// Second tick — load() should NOT be called again (one-shot guard)
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 20.05, playing: true, playbackRate: 1 });
|
||||
expect(clip.el.load).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not call load() when the seek succeeds", () => {
|
||||
const clip = createMockClip({ start: 0, end: 30, mediaStart: 0 });
|
||||
Object.defineProperty(clip.el, "currentTime", { value: 0, writable: true });
|
||||
clip.el.load = vi.fn();
|
||||
// Seek to 20 — succeeds (currentTime updates)
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 20, playing: true, playbackRate: 1 });
|
||||
expect(clip.el.currentTime).toBe(20);
|
||||
expect(clip.el.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears the load-retry guard when clip deactivates and reactivates", () => {
|
||||
const clip = createMockClip({ start: 0, end: 10, mediaStart: 0 });
|
||||
let internalTime = 0;
|
||||
Object.defineProperty(clip.el, "currentTime", {
|
||||
get: () => internalTime,
|
||||
set: () => {},
|
||||
configurable: true,
|
||||
});
|
||||
clip.el.load = vi.fn();
|
||||
// First activation — seek fails, load() called
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: true, playbackRate: 1 });
|
||||
expect(clip.el.load).toHaveBeenCalledTimes(1);
|
||||
// Deactivate
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 11, playing: true, playbackRate: 1 });
|
||||
// Reactivate — guard was cleared, so load() can fire again
|
||||
syncRuntimeMedia({ clips: [clip], timeSeconds: 5, playing: true, playbackRate: 1 });
|
||||
expect(clip.el.load).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("pauses active clip when not playing", () => {
|
||||
const clip = createMockClip({ start: 0, end: 10 });
|
||||
Object.defineProperty(clip.el, "paused", { value: false, writable: true });
|
||||
|
||||
@@ -78,6 +78,13 @@ export function refreshRuntimeMediaCache(params?: {
|
||||
// inactive so the next activation gets a hard resync on its first tick.
|
||||
const lastOffset = new WeakMap<HTMLMediaElement, number>();
|
||||
|
||||
// Elements that had a seek past their buffered range (common with streaming
|
||||
// MP3 where preload="metadata" only fetches the first few seconds). After
|
||||
// setting preload="auto" and calling load(), we mark the element so subsequent
|
||||
// ticks don't restart the fetch in a loop while the browser downloads data.
|
||||
// Cleared when the clip leaves its active window.
|
||||
const seekLoadRetried = new WeakSet<HTMLMediaElement>();
|
||||
|
||||
// Elements whose play() is in flight. The sync runs on a 50 ms poll and with
|
||||
// a 1–2 s buffer that would fire 20–40 spurious play() calls per element —
|
||||
// noise in devtools and, worse, each `.catch(() => {})` would swallow a real
|
||||
@@ -137,6 +144,13 @@ export function syncRuntimeMedia(params: {
|
||||
}
|
||||
if (clip.volume != null) el.volume = clip.volume;
|
||||
if (shouldMute) el.muted = true;
|
||||
// Ensure full preload for every active media element. Streaming
|
||||
// formats (MP3) may arrive with preload="metadata", which only
|
||||
// buffers the first few seconds and causes seeks to silently fail
|
||||
// past the buffered range. Setting this on every tick is cheap
|
||||
// (no-op when already "auto") and catches elements whose preload
|
||||
// was overridden after init.ts set it.
|
||||
if (el.preload !== "auto") el.preload = "auto";
|
||||
try {
|
||||
// Per-element rate × global transport rate
|
||||
el.playbackRate = clip.playbackRate * params.playbackRate;
|
||||
@@ -177,6 +191,26 @@ export function syncRuntimeMedia(params: {
|
||||
} catch {
|
||||
// ignore browser seek restrictions
|
||||
}
|
||||
// Detect failed seek: if currentTime didn't reach the target,
|
||||
// the browser can't seek past its buffered range. Common with
|
||||
// streaming MP3 where only the first ~15s is cached. Force a
|
||||
// full network fetch via load() so the browser builds a complete
|
||||
// media index. One-shot per element — subsequent sync ticks will
|
||||
// re-attempt the seek once data arrives.
|
||||
if (Math.abs(el.currentTime - relTime) > 0.5 && !seekLoadRetried.has(el)) {
|
||||
seekLoadRetried.add(el);
|
||||
el.load();
|
||||
try {
|
||||
el.currentTime = relTime;
|
||||
} catch {
|
||||
// ignore — the seek will be retried on the next tick
|
||||
}
|
||||
}
|
||||
// After a hard seek, clear the in-flight play guard so the next tick
|
||||
// can re-issue play(). Without this, a seek during playback leaves
|
||||
// the element paused at the new position for 50-150ms (one poll
|
||||
// interval) while the timeline continues — audible desync on scrub.
|
||||
playRequested.delete(el);
|
||||
}
|
||||
if (params.playing && el.paused && !playRequested.has(el)) {
|
||||
// `HTMLMediaElement.play()` is spec'd to queue playback and resolve
|
||||
@@ -190,11 +224,6 @@ export function syncRuntimeMedia(params: {
|
||||
// seconds. The canplay listener was also racey — the event could
|
||||
// fire between `load()` and `addEventListener` attachment, wedging
|
||||
// the element waiting for a callback that never came.
|
||||
//
|
||||
// preload="auto" is already set at bind time in init.ts; the
|
||||
// re-assignment here is defensive for media elements that were
|
||||
// inserted after the runtime bound its listeners.
|
||||
if (el.preload !== "auto") el.preload = "auto";
|
||||
markPlayRequested(el);
|
||||
void el.play().catch((err: unknown) => {
|
||||
// If play() rejects — e.g. autoplay blocked, element removed
|
||||
@@ -219,6 +248,7 @@ export function syncRuntimeMedia(params: {
|
||||
// Clip left its active window — drop the offset baseline so the next
|
||||
// activation (e.g. re-entering a sub-composition) gets a hard resync.
|
||||
lastOffset.delete(el);
|
||||
seekLoadRetried.delete(el);
|
||||
if (!el.paused) el.pause();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user