Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions packages/core/src/runtime/clock.steer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { TransportClock } from "./clock";
import { syncRuntimeMedia, type RuntimeMediaClip } from "./media";

// Chromium moves an element's currentTime in steps; the element plays at whatever rate it is given.
function steppingAudio(stepMs: number) {
const el = document.createElement("audio");
document.body.appendChild(el);
let position = 0;
let rate = 1;
let rateWrites = 0;
let seeks = 0;
Object.defineProperties(el, {
paused: { configurable: true, get: () => false },
seeking: { configurable: true, get: () => false },
readyState: { configurable: true, get: () => 4 },
duration: { configurable: true, get: () => 60 },
playbackRate: {
configurable: true,
get: () => rate,
set: (v: number) => {
rateWrites += 1;
rate = v;
},
},
currentTime: {
configurable: true,
get: () => (Math.floor((position * 1000) / stepMs) * stepMs) / 1000,
set: (v: number) => {
seeks += 1;
position = v;
},
},
});
el.play = vi.fn(() => Promise.resolve());
el.pause = vi.fn();
return {
el,
advance: (dtMs: number) => (position += (dtMs / 1000) * rate),
rateWrites: () => rateWrites,
seeks: () => seeks,
};
}

/** Plays one audio clip the clock follows for `seconds` of `hz` display frames, syncing media as the runtime does. */
function play(stepMs: number, hz: number, seconds = 10) {
let ms = 0;
const audio = steppingAudio(stepMs);
const clock = new TransportClock({ nowMs: () => ms, duration: 60 });
const clip: RuntimeMediaClip = {
el: audio.el,
start: 0,
mediaStart: 0,
duration: 60,
end: 60,
volume: null,
playbackRate: 1,
loop: false,
sourceDuration: null,
};
clock.play();
const tickMs = 1000 / hz;
for (let tick = 1; tick <= seconds * hz; tick++) {
ms = tick * tickMs;
audio.advance(tickMs);
clock.attachAudioSource({ el: audio.el, compositionStart: 0, mediaStart: 0 });
syncRuntimeMedia({
clips: [clip],
timeSeconds: clock.now(),
playing: true,
playbackRate: 1,
clockAhead: (el) => clock.aheadOf(el),
getCompositionDuration: () => 60,
});
}
return { rateWrites: audio.rateWrites(), seeks: audio.seeks(), filmSpeed: clock.now() / seconds };
}

describe("a clock that runs between the steps of the audio it follows, as Chromium reports it", () => {
beforeEach(() => {
vi.spyOn(navigator, "userAgent", "get").mockReturnValue(
"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36",
);
});

afterEach(() => {
vi.restoreAllMocks();
document.body.innerHTML = "";
});

it("does not steer that audio against its own lead, so the film keeps its speed", () => {
for (const [stepMs, hz] of [
[45, 60],
[50, 144],
[60, 60],
[70, 60],
[90, 120],
]) {
const run = play(stepMs, hz);
expect(run.rateWrites, `${stepMs} ms steps at ${hz} Hz: rate writes`).toBe(0);
expect(run.seeks, `${stepMs} ms steps at ${hz} Hz: seeks`).toBe(0);
expect(
Math.abs(run.filmSpeed - 1),
`${stepMs} ms steps at ${hz} Hz: film speed`,
).toBeLessThan(0.001);
}
});
});
57 changes: 57 additions & 0 deletions packages/core/src/runtime/clock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,63 @@ describe("TransportClock", () => {
expect(clock.getSource()).toBe("audio");
});

/**
* Plays 1/60 s ticks against an element whose time moves in `stepMs` steps at `speed`; frames are 30 fps. Frames
* count from the 7th tick: until the element's second update the clock does not know its step.
*/
function playStepping(ticks: number, stepMs: number, speed = 1, stallAfterMs = Infinity) {
const { clock, advance, getMs } = createClock({ duration: 60 });
const el = createMockAudioEl(0, false);
clock.play();
clock.attachAudioSource({ el, compositionStart: 0, mediaStart: 0 });
let last = 0;
let skipped = 0;
let heldTicks = 0;
let longestHold = 0;
let mostAhead = 0;
let back = 0;
let previous = 0;
for (let tick = 1; tick <= ticks; tick++) {
advance(1000 / 60);
const played = Math.min(getMs(), stallAfterMs) * speed;
el.currentTime = (Math.floor(played / stepMs) * stepMs) / 1000;
const now = clock.now();
if (now < previous) back += 1;
previous = now;
mostAhead = Math.max(mostAhead, now - el.currentTime);
const frame = Math.floor(now * 30 + 1e-6);
if (tick > 6 && frame > last + 1) skipped += 1;
heldTicks = frame === last ? heldTicks + 1 : 0;
if (tick > 6) longestHold = Math.max(longestHold, heldTicks);
last = frame;
}
return { skipped, longestHold, mostAhead, back, end: clock.now() };
}

it("moves on every display frame between the element's time updates, never skipping a film frame", () => {
for (const stepMs of [30, 40, 50]) {
const run = playStepping(240, stepMs);
expect(run.skipped, `${stepMs} ms steps: film frames skipped`).toBe(0);
// A 30 fps frame spans two 60 Hz ticks: held for one extra tick, never two.
expect(run.longestHold, `${stepMs} ms steps: ticks a frame is held`).toBeLessThanOrEqual(1);
expect(run.back, `${stepMs} ms steps: reads that went back`).toBe(0);
expect(run.mostAhead, `${stepMs} ms steps: lead over the element`).toBeLessThanOrEqual(
stepMs / 1000 + 1e-9,
);
}
});

it("stops within one of the element's own updates when the element stops moving", () => {
const run = playStepping(120, 40, 1, 1000);
expect(run.end).toBeLessThanOrEqual(1 + 0.04 + 1e-9);
});

it("stays within one update of an element that plays slightly slower than the wall clock", () => {
const run = playStepping(600, 40, 0.98);
expect(run.mostAhead).toBeLessThanOrEqual(0.04 + 1e-9);
expect(run.back).toBe(0);
});

it("names the clock it follows: an audio element, WebAudio, or neither", () => {
const { clock } = createClock({ duration: 10 });
expect(clock.followedClock()).toBe("monotonic");
Expand Down
62 changes: 60 additions & 2 deletions packages/core/src/runtime/clock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,27 @@ const STALL_ADJUSTED_LAG_MS = 33;
const HAVE_FUTURE_DATA = 3;
const AUDIO_START_WAIT_MS = 1500;
const MEDIA_SYNC_STEER_TOLERANCE_SECONDS = 0.04;
const MAX_MEDIA_STEP_SECONDS = 0.25;
const MEDIA_STEPS_KEPT = 8;

type PendingAudioStart = { sinceMs: number; at: number; from: number | null };
type MediaReading = {
el: HTMLMediaElement;
time: number;
steps: number[];
from: number;
fromMs: number;
};

const furthestAhead = (reading: MediaReading | null) =>
reading ? Math.max(0, ...reading.steps) : 0;

function noteStep(reading: MediaReading, time: number): void {
const step = time - reading.time;
if (step > 0 && step <= MAX_MEDIA_STEP_SECONDS)
reading.steps = [...reading.steps.slice(1 - MEDIA_STEPS_KEPT), step];
reading.time = time;
}

export class TransportClock {
private _baseTime = 0;
Expand All @@ -45,6 +64,8 @@ export class TransportClock {
private _lastReadMs: number | null = null;
private _lastNow = 0;
private _audioStart: PendingAudioStart | null = null;
private _reading: MediaReading | null = null;
private _ahead: { el: HTMLMediaElement; seconds: number } | null = null;

constructor(opts?: {
initialTime?: number;
Expand All @@ -59,6 +80,7 @@ export class TransportClock {
}

now(): number {
this._ahead = null;
if (this._playStartMs === null) return this._baseTime;

// Audio-master: drift is impossible because audio IS the clock. Clearing
Expand Down Expand Up @@ -97,12 +119,43 @@ export class TransportClock {
const spec = rate ?? 1;
const time = timeAtSourceTime(spec, el.currentTime - mediaStart) + compositionStart;
const held = this._heldUntilAudioOutputStarts(el.seeking, time);
if (held !== undefined) return held;
if (time >= this._lastNow) return time;
if (held !== undefined) {
this._reading = null;
return held;
}
const between = this._betweenUpdates(el, time);
if (between !== null) return between;
const wrapped = el.loop && this._lastNow - time > timeAtSourceTime(spec, el.duration) / 2;
return !wrapped && el.readyState < HAVE_FUTURE_DATA ? this._lastNow : null;
}

// Chromium moves currentTime in 10-50 ms steps, so reading it as is holds a frame, then jumps. Run on at the play
// rate, kept within [element time, element time + its largest recent step], never back; null when time is behind
// the playhead by more than that step (a late clip, not a step).
private _betweenUpdates(el: HTMLMediaElement, time: number): number | null {
const nowMs = this._nowMs();
const last = this._reading?.el === el ? this._reading : null;
if (time < this._lastNow - furthestAhead(last)) return null;
if (!last) {
this._reading = { el, time, steps: [], from: time, fromMs: nowMs };
return this._runAhead(el, time, Math.max(time, this._lastNow));
}
noteStep(last, time);
const run = last.from + ((nowMs - last.fromMs) / 1000) * this._rate;
const held = Math.min(Math.max(run, time), time + furthestAhead(last));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

blocker: this clock now runs up to one step ahead of the element it follows, but syncRuntimeMedia still rate-steers that element against the clock. Each frame, media.ts:682 computes offset = relTime − el.currentTime, which is now up to one full step. When it passes MEDIA_SYNC_TOLERANCE_SECONDS (40 ms), steerMedia (mediaSteer.ts:55) sets the followed <audio> to 1.03x. That speeds up the element the band is anchored to, so the playhead speeds up with it. On main the offset is about 0, because the clock is the element's time.

To check this, I ran a throwaway vitest probe: this TransportClock plus the real steerMedia, with the element's position moving at its written playbackRate and currentTime floored to N ms steps. Over 10 s of wall time:

steps ticks steered ticks rate writes audio / film speed
45 ms 60 Hz 96/600 64 1.0048x
50 ms 144 Hz 263/1440 398 1.0055x
60 ms 60 Hz 224/600 241 1.0112x
70 ms 60 Hz 275/600 223 1.0137x
90 ms 120 Hz 624/1200 221 1.0156x

Main gives 0 steered ticks and 1.0000x in every case. At 20/30 ms steps, and at 40/50 ms on 60 Hz, the PR is effectively clean, because the lead stays under 40 ms. That matches your manual table, which measured frame cadence but not the element's playbackRate. The body says steps are "30-50 ms or more on many machines", so this is the population the fix targets. They would get audible 1.0↔1.03 rate flips (up to ~40 writes/s at 144 Hz) and a film running 0.5–1.5% fast.

A possible fix: don't steer the clock's own element (clock.audioElement()) by its offset against the clock. Or give media sync the raw element time for that element. Then add a test that runs the clock and steerMedia together, so the two can't drift apart again.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Fixed in 8c55d88: clockAhead takes the lead off for the followed element only. clock.steer.test.ts fails against the old media.ts (45 ms @ 60 Hz: 69 rate writes) and passes now.

if (held !== run) Object.assign(last, { from: held, fromMs: nowMs });
return this._runAhead(el, time, Math.max(held, this._lastNow));
}

private _runAhead(el: HTMLMediaElement, time: number, now: number): number {
this._ahead = { el, seconds: now - time };
return now;
}

aheadOf(el: HTMLMediaElement): number {
return this._ahead?.el === el ? this._ahead.seconds : 0;
}

private _heldUntilAudioOutputStarts(seeking: boolean, time: number): number | null | undefined {
const pending = this._pendingAudioStart();
if (seeking) return pending ? this._lastNow : null;
Expand Down Expand Up @@ -139,6 +192,7 @@ export class TransportClock {
this._playStartMs = this._nowMs();
this._audioStart = { sinceMs: this._playStartMs, at: this._baseTime, from: null };
this._lastNow = this._baseTime;
this._reading = null;
// Not a stall: the gap since the clock was last read (possibly a long
// paused idle) says nothing about lost playback time, since none was
// playing. `_applyStallCorrection` treats null as "nothing to compare
Expand All @@ -151,6 +205,7 @@ export class TransportClock {
if (this._playStartMs === null) return false;
this._baseTime = this.now();
this._playStartMs = null;
this._reading = null;
return true;
}

Expand All @@ -160,6 +215,7 @@ export class TransportClock {
: Math.max(0, timeSeconds);
this._baseTime = clamped;
this._lastNow = clamped;
this._reading = null;
if (this._playStartMs !== null) {
this._playStartMs = this._nowMs();
this._audioStart = { sinceMs: this._playStartMs, at: clamped, from: null };
Expand All @@ -175,6 +231,7 @@ export class TransportClock {

setRate(rate: number): void {
const safe = Number.isFinite(rate) && rate > 0 ? Math.max(0.1, Math.min(5, rate)) : 1;
this._reading = null;
if (this._playStartMs !== null) {
this._baseTime = this.now();
this._playStartMs = this._nowMs();
Expand Down Expand Up @@ -230,6 +287,7 @@ export class TransportClock {
this._lastReadMs = null;
}
this._audioSource = null;
this._reading = null;
}

/** The element the playhead currently follows, if the source is one. */
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/runtime/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3826,6 +3826,7 @@ export function initSandboxRuntimeModular(): void {
forceSync,
keepLateMedia,
cueAheadSeconds,
clockAhead: (el) => clock.aheadOf(el),
clockAdvancing:
clock.getSource() !== "audio" ||
lastSyncedMediaTimeSeconds === null ||
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/runtime/media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ export function syncRuntimeMedia(params: {
* outbound message; further invocations are suppressed by the caller.
*/
onAutoplayBlocked?: () => void;
clockAhead?: (el: HTMLMediaElement) => number;
/** `clipGain` is `data-volume` for a routed clip, played ahead of its FX; 1 otherwise. */
onElementVolume?: (
el: HTMLMediaElement,
Expand Down Expand Up @@ -557,7 +558,7 @@ export function syncRuntimeMedia(params: {
!inWindow &&
(params.timeSeconds >= clip.end || sameInstant(params.timeSeconds, clip.end)) &&
isClipVisibleAt(params.timeSeconds, clip.start, clip.end, params.getCompositionDuration());
const relTime = mediaSourceTimeAt(clip, params.timeSeconds);
const relTime = mediaSourceTimeAt(clip, params.timeSeconds - (params.clockAhead?.(el) ?? 0));
const isHeldVideoTail =
isTerminalVideo ||
(isNonLoopVideo && clip.sourceDuration != null && relTime >= clip.sourceDuration && inWindow);
Expand Down
Loading