From d8c2bb87c611cdd02d1868902b7a78c95bbb222c Mon Sep 17 00:00:00 2001 From: dazzatronus Date: Tue, 27 Oct 2026 12:52:26 +1100 Subject: [PATCH] fix: bake html5 clips in playback order from the playhead --- src/components/canvas/players/bake-queue.ts | 65 +++++++++++++ src/components/canvas/players/html5-player.ts | 35 ++++--- tests/bake-queue.test.ts | 91 +++++++++++++++++++ tests/html5-player.test.ts | 87 +++++++++++------- 4 files changed, 229 insertions(+), 49 deletions(-) create mode 100644 src/components/canvas/players/bake-queue.ts create mode 100644 tests/bake-queue.test.ts diff --git a/src/components/canvas/players/bake-queue.ts b/src/components/canvas/players/bake-queue.ts new file mode 100644 index 00000000..255cd1f3 --- /dev/null +++ b/src/components/canvas/players/bake-queue.ts @@ -0,0 +1,65 @@ +/** Where a waiting bake's clip sits on the timeline. Read at each pick, so seeks and edits count. */ +export interface BakeSlot { + start(): number; + end(): number; + playhead(): number; +} + +/** The clip under the playhead goes first, then those ahead in start order, then those already passed. */ +function rank(slot: BakeSlot): [number, number] { + const playhead = slot.playhead(); + const start = slot.start(); + if (start <= playhead && playhead < slot.end()) return [0, start]; + return [start > playhead ? 1 : 2, start]; +} + +export function nextBake(waiting: readonly BakeSlot[]): number { + let best = 0; + let bestRank = rank(waiting[0]); + for (let i = 1; i < waiting.length; i += 1) { + const candidate = rank(waiting[i]); + // Strictly better only, so clips that tie keep their arrival order. + if (candidate[0] < bestRank[0] || (candidate[0] === bestRank[0] && candidate[1] < bestRank[1])) { + best = i; + bestRank = candidate; + } + } + return best; +} + +/** + * Runs bakes one at a time. Each pick waits a task, so clips that join together are all + * waiting when it is made, and it reads the playhead then. + */ +export class BakeQueue { + private readonly waiting: { slot: BakeSlot; run: () => void }[] = []; + private running = false; + private scheduled = false; + + public add(slot: BakeSlot, bake: () => Promise): Promise { + return new Promise((resolve, reject) => { + const run = (): void => { + bake() + .then(resolve, reject) + .finally(() => { + this.running = false; + this.schedule(); + }); + }; + this.waiting.push({ slot, run }); + this.schedule(); + }); + } + + private schedule(): void { + if (this.running || this.scheduled || this.waiting.length === 0) return; + this.scheduled = true; + setTimeout(() => { + this.scheduled = false; + if (this.running || this.waiting.length === 0) return; + const [next] = this.waiting.splice(nextBake(this.waiting.map(({ slot }) => slot)), 1); + this.running = true; + next.run(); + }, 0); + } +} diff --git a/src/components/canvas/players/html5-player.ts b/src/components/canvas/players/html5-player.ts index 9e71ad7e..b1158fe6 100644 --- a/src/components/canvas/players/html5-player.ts +++ b/src/components/canvas/players/html5-player.ts @@ -17,6 +17,7 @@ import { } from "@shotstack/shotstack-canvas"; import * as pixi from "pixi.js"; +import { BakeQueue } from "./bake-queue"; import { computeHtml5CacheKey, html5CacheGet, html5CachePut } from "./html5-cache"; import { createCaptureLoadingGraphic, createPlaceholderGraphic } from "./placeholder-graphic"; @@ -106,7 +107,9 @@ export function bakeScaleFor(onScreenScale: number): number { type Html5Mode = "editing" | "capturing" | "playback" | "stale" | "failed"; export class Html5Player extends Player { - private static captureChain: Promise = Promise.resolve(); + private static bakes = new BakeQueue(); + // Pages still mounting. A bake joins the queue once they have, so the queue orders every clip that loaded together. + private static readonly mounts = new Set>(); private iframe: HTMLIFrameElement | null = null; private renderedWidth: number = 0; @@ -146,6 +149,7 @@ export class Html5Player extends Player { private liveStillFrame: number = -1; private liveStillInFlight: boolean = false; private stillRequest: number | null = null; + private readonly bakeSlot = { start: () => this.getStart(), end: () => this.getEnd(), playhead: () => this.edit.playbackTime }; constructor(edit: Edit, clipConfiguration: ResolvedClip) { super(edit, clipConfiguration, PlayerType.Html5); @@ -256,10 +260,13 @@ export class Html5Player extends Player { // A reload waits on this, so a mount slowed by ingest fetches never runs alongside a second one. private trackMount(mount: Promise): Promise { - this.mounting = mount.finally(() => { + const tracked = mount.finally(() => { this.mounting = null; + Html5Player.mounts.delete(tracked); }); - return this.mounting; + this.mounting = tracked; + Html5Player.mounts.add(tracked); + return tracked; } private async mountAsset(): Promise { @@ -373,29 +380,27 @@ export class Html5Player extends Player { return this.captureInFlight; } const queuedFor = this.contentHash; - const previous = Html5Player.captureChain; // Cached frames cost no bake, so they never wait in the queue behind one. const capture: Promise = this.bakeScale(minScale) .then(scale => this.adoptCachedBake(scale)) - .then( - cached => - cached ?? - previous.then(() => { - // A content change while this waited in the queue would bake the new content at this capture's floor. - if (this.disposed || this.contentHash !== queuedFor) return null; - return this.runCapture(minScale); - }) - ) + .then(cached => cached ?? this.queueBake(queuedFor, minScale)) .finally(() => { // A content change frees the slot, and a newer capture may hold it by now. if (this.captureInFlight === capture) this.captureInFlight = null; }); this.captureInFlight = capture; - // A cache hit settles early, so the next bake waits on the whole queue, not just this capture. - Html5Player.captureChain = Promise.allSettled([previous, capture]).then(() => undefined); return capture; } + private async queueBake(queuedFor: string | null, minScale: number): Promise { + while (Html5Player.mounts.size > 0) await Promise.allSettled([...Html5Player.mounts]); + return Html5Player.bakes.add(this.bakeSlot, async () => { + // A content change while this waited in the queue would bake the new content at this capture's floor. + if (this.disposed || this.contentHash !== queuedFor) return null; + return this.runCapture(minScale); + }); + } + // After a tick, an active clip's global transform is the size the canvas shows it at. An inactive clip's own // transform isn't applied, so its bake reads only the viewport's zoom and may re-bake once the clip is active. private async bakeScale(minScale: number): Promise { diff --git a/tests/bake-queue.test.ts b/tests/bake-queue.test.ts new file mode 100644 index 00000000..2b971c53 --- /dev/null +++ b/tests/bake-queue.test.ts @@ -0,0 +1,91 @@ +import { BakeQueue, nextBake } from "@canvas/players/bake-queue"; + +const at = (start: number, end: number, playhead: () => number = () => 0) => ({ start: () => start, end: () => end, playhead }); + +const settle = () => + new Promise(resolve => { + setTimeout(resolve, 0); + }); + +describe("nextBake", () => { + it("takes the clip under the playhead, then those ahead in start order, then those it has passed", () => { + const playhead = () => 30; + const waiting = [at(54.4, 80.37, playhead), at(0, 24.4, playhead), at(24.4, 54.4, playhead), at(40, 45, playhead)]; + const order: number[] = []; + while (waiting.length > 0) order.push(waiting.splice(nextBake(waiting), 1)[0].start()); + + expect(order).toEqual([24.4, 40, 54.4, 0]); + }); + + it("keeps arrival order between clips that start together", () => { + expect(nextBake([at(10, 20), at(10, 15)])).toBe(0); + }); +}); + +describe("BakeQueue", () => { + it("bakes clips that arrive together from the playhead, whatever their arrival order", async () => { + const queue = new BakeQueue(); + const ran: number[] = []; + await Promise.all([54.4, 24.4, 0].map(start => queue.add(at(start, start + 20), async () => ran.push(start)))); + + expect(ran).toEqual([0, 24.4, 54.4]); + }); + + it("runs one bake at a time", async () => { + const queue = new BakeQueue(); + let running = 0; + let most = 0; + const bake = async () => { + running += 1; + most = Math.max(most, running); + await settle(); + running -= 1; + }; + await Promise.all([0, 10, 20].map(start => queue.add(at(start, start + 10), bake))); + + expect(most).toBe(1); + }); + + it("re-sorts what's waiting when the playhead moves during a bake", async () => { + const queue = new BakeQueue(); + let playhead = 0; + const ran: string[] = []; + let finishFirst = () => {}; + const first = queue.add( + at(0, 10, () => playhead), + () => { + ran.push("A"); + return new Promise(resolve => { + finishFirst = resolve; + }); + } + ); + await settle(); + const rest = [ + queue.add( + at(10, 20, () => playhead), + async () => ran.push("B") + ), + queue.add( + at(20, 30, () => playhead), + async () => ran.push("C") + ) + ]; + playhead = 25; + finishFirst(); + await Promise.all([first, ...rest]); + + expect(ran).toEqual(["A", "C", "B"]); + }); + + it("moves on after a bake fails, and hands the failure to its caller", async () => { + const queue = new BakeQueue(); + const failed = queue.add(at(0, 10), async () => { + throw new Error("bake failed"); + }); + const next = queue.add(at(10, 20), async () => "baked"); + + await expect(failed).rejects.toThrow("bake failed"); + await expect(next).resolves.toBe("baked"); + }); +}); diff --git a/tests/html5-player.test.ts b/tests/html5-player.test.ts index fc1833b3..94696d51 100644 --- a/tests/html5-player.test.ts +++ b/tests/html5-player.test.ts @@ -1,5 +1,6 @@ /** @jest-environment jsdom */ +import { BakeQueue } from "@canvas/players/bake-queue"; import { Html5Player, bakeScaleFor, captureFramesInFlight } from "@canvas/players/html5-player"; import type { Edit } from "@core/edit-session"; import { EditEvent, InternalEvent } from "@core/events/edit-events"; @@ -33,6 +34,14 @@ jest.mock("@canvas/players/player", () => ({ return this.resolvedTiming.length; } + getStart() { + return this.resolvedTiming.start; + } + + getEnd() { + return this.resolvedTiming.start + this.resolvedTiming.length; + } + setResolvedTiming(timing: { start: number; length: number }) { this.resolvedTiming = { ...timing }; this.clipConfiguration.start = timing.start; @@ -119,6 +128,7 @@ function createPlayer() { const edit = { size: { width: 1080, height: 1920 }, isPlaying: false, + playbackTime: 0, getHtml5Prerender: () => "on-load", getInternalEvents: () => events, getResolvedEdit: () => ({ output: { resolution: "hd" } }), @@ -1083,17 +1093,34 @@ it("reports a failed re-bake and keeps the frames it has", async () => { expect(harness.capturedFrames).toBe(frames); }); +type SharedQueue = { bakes: BakeQueue }; + +// Stands in for another clip's bake at the head of the shared queue. +function holdCaptureQueue(): { drain: () => void; restore: () => void } { + const { bakes } = Html5Player as unknown as SharedQueue; + const held = new BakeQueue(); + let drain = () => {}; + const gate = new Promise(resolve => { + drain = resolve; + }); + // Under the playhead from the start, so it runs ahead of any clip that joins after it. + held.add({ start: () => 0, end: () => Infinity, playhead: () => 0 }, () => gate); + Object.assign(Html5Player, { bakes: held }); + return { drain, restore: () => Object.assign(Html5Player, { bakes }) }; +} + +// Earlier tests can leave a bake that never settles on the shared queue. +function emptyCaptureQueue(): () => void { + const { bakes } = Html5Player as unknown as SharedQueue; + Object.assign(Html5Player, { bakes: new BakeQueue() }); + return () => Object.assign(Html5Player, { bakes }); +} + it("drops a re-bake queued for content that has since changed, leaving the newer capture in flight", async () => { const { player, harness } = createStillPlayer(); Object.assign(harness, { capturedFrames: [new Blob(["baked 0"])], capturedHash: "original@0.5", capturedScale: 0.5, mode: "playback" }); shownAt(harness, 0.8); - const { captureChain } = Html5Player as unknown as { captureChain: Promise }; - let drain = () => {}; - Object.assign(Html5Player, { - captureChain: new Promise(resolve => { - drain = resolve; - }) - }); + const queue = holdCaptureQueue(); try { player.update(0, 0); const rebake = harness.captureInFlight!; @@ -1103,7 +1130,7 @@ it("drops a re-bake queued for content that has since changed, leaving the newer await player.reloadAsset(); const newer = harness.captureInFlight; expect(newer).not.toBeNull(); - drain(); + queue.drain(); await rebake; expect(harness.captureInFlight).toBe(newer); @@ -1112,29 +1139,10 @@ it("drops a re-bake queued for content that has since changed, leaving the newer expect(html5CachePut).toHaveBeenCalledWith("changed@0.5", expect.anything()); expect(html5CachePut).not.toHaveBeenCalledWith("changed@1", expect.anything()); } finally { - Object.assign(Html5Player, { captureChain }); + queue.restore(); } }); -// Stands in for another clip's bake at the head of the shared queue. -function holdCaptureQueue(): { drain: () => void; restore: () => void } { - const { captureChain } = Html5Player as unknown as { captureChain: Promise }; - let drain = () => {}; - Object.assign(Html5Player, { - captureChain: new Promise(resolve => { - drain = resolve; - }) - }); - return { drain, restore: () => Object.assign(Html5Player, { captureChain }) }; -} - -// Earlier tests can leave a capture that never settles on the shared queue. -function emptyCaptureQueue(): () => void { - const { captureChain } = Html5Player as unknown as { captureChain: Promise }; - Object.assign(Html5Player, { captureChain: Promise.resolve() }); - return () => Object.assign(Html5Player, { captureChain }); -} - it("adopts cached frames while another clip's bake holds the queue", async () => { const { harness, events } = createStillPlayer(); const queue = holdCaptureQueue(); @@ -1202,15 +1210,26 @@ it("keeps the queue's bakes in order when a cached clip sits between them", asyn } }); -it("leaves no capture's frames on the shared queue once it settles", async () => { - const { harness } = createStillPlayer(); - cacheLib.html5CacheGet.mockResolvedValueOnce({ pngs: [new Blob(["cached 0"])], fps: 1, frameCount: 1 }); +it("bakes the clip that plays first ahead of one whose page loaded first", async () => { + const later = createStillPlayer(); + const opening = createStillPlayer(); + Object.assign(later.player, { getStart: () => 54.4, getEnd: () => 80.37 }); + Object.assign(opening.player, { getStart: () => 0, getEnd: () => 24.4 }); + const baked: string[] = []; + const bake = (name: string) => async () => { + baked.push(name); + return null; + }; + jest.spyOn(later.harness, "runCapture").mockImplementation(bake("later")); + jest.spyOn(opening.harness, "runCapture").mockImplementation(bake("opening")); + jest.spyOn(console, "warn").mockImplementation(() => {}); const restore = emptyCaptureQueue(); try { - harness.beginCapture(); - await until(() => harness.capturedFrames !== null); + later.harness.beginCapture(); + opening.harness.beginCapture(); + await until(() => baked.length === 2); - await expect((Html5Player as unknown as { captureChain: Promise }).captureChain).resolves.toBeUndefined(); + expect(baked).toEqual(["opening", "later"]); } finally { restore(); }