fix: bake html5 clips in playback order from the playhead (#264)

Author: dazzatronus

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<T>(slot: BakeSlot, bake: () => Promise<T>): Promise<T> {
+		return new Promise<T>((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);
+	}
+}

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<unknown> = 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<Promise<void>>();
 
 	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<void>): Promise<void> {
-		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<void> {
@@ -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<Blob[] | null> = 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<Blob[] | null> {
+		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<number> {

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<void>(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");
+	});
+});

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<void>(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<unknown> };
-	let drain = () => {};
-	Object.assign(Html5Player, {
-		captureChain: new Promise<void>(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<unknown> };
-	let drain = () => {};
-	Object.assign(Html5Player, {
-		captureChain: new Promise<void>(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<unknown> };
-	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<unknown> }).captureChain).resolves.toBeUndefined();
+		expect(baked).toEqual(["opening", "later"]);
 	} finally {
 		restore();
 	}