fix(engine): a video held past its media end keeps following its animation (#6424)

* fix(engine): a video held past its media end keeps following its animation

Fixes #4049.

* fix(engine): drop two comments the names already carry

packages/engine/src/services/frameCapture.ts

@@ -4569,8 +4569,8 @@ async function captureAccumulatedFrame(
   await seekPageTimeline(session.page, frameTime, undefined);
   const totals = { seekMs: Date.now() - eventfulSeekStart, beforeCaptureMs: 0, screenshotMs: 0 };
   if (session.onBeforeCapture) {
-    // The injector copies a video's own style only when its frame changes: inject at frameTime
-    // first so the held frame carries frameTime's style, not the first sample's.
+    // Sub-samples leave a video's style alone: inject at frameTime first so the held frame
+    // carries frameTime's style, not the first sample's.
     const injectStart = Date.now();
     session.captureStep = "injecting video frames";
     await session.onBeforeCapture(session.page, frameTime, frameTime);

packages/engine/src/services/screenshotService.ts

@@ -643,25 +643,35 @@ export async function ensureRenderFrameSiblings(page: Page): Promise<void> {
  * cache entry for a frame that never reached the page, which would otherwise
  * short-circuit the next inject at the same frameIndex and leave the host's
  * first visible frame blank.
+ *
+ * `restyleIds` keep their current picture and only re-copy the video's style.
  */
 export async function injectVideoFramesBatch(
   page: Page,
   updates: Array<{ videoId: string; dataUri: string }>,
+  restyleIds: readonly string[] = [],
 ): Promise<string[]> {
-  if (updates.length === 0) return [];
+  if (updates.length === 0 && restyleIds.length === 0) return [];
   return withVideoFrameSources(page, updates, (resolved) =>
-    injectResolvedVideoFramesBatch(page, resolved),
+    injectResolvedVideoFramesBatch(page, resolved, restyleIds),
   );
 }
 
 async function injectResolvedVideoFramesBatch(
   page: Page,
   updates: VideoFrameUpdate[],
+  restyleIds: readonly string[],
 ): Promise<string[]> {
+  // Build the sibling id with core's function rather than a template here,
+  // so the id the readers look up has exactly one definition.
+  const items: Array<Partial<VideoFrameUpdate> & { videoId: string; frameId: string }> = [
+    ...updates,
+    ...restyleIds.map((videoId) => ({ videoId })),
+  ].map((item) => ({ ...item, frameId: renderFrameIdForRenderId(item.videoId) }));
   return await page.evaluate(
     // fallow-ignore-next-line complexity
     async (
-      items: Array<VideoFrameUpdate & { frameId: string }>,
+      items: Array<Partial<VideoFrameUpdate> & { videoId: string; frameId: string }>,
       visualProperties: string[],
       colorGradingSourceHiddenAttr: string,
     ) => {
@@ -723,6 +733,8 @@ async function injectResolvedVideoFramesBatch(
 
         let img = video.nextElementSibling as HTMLImageElement | null;
         const hasImg = img !== null && img.classList.contains("__render_frame__");
+        const restyleOnly = item.dataUri === undefined;
+        if (restyleOnly && !hasImg) continue;
 
         if (isVisualAncestorHidden(video)) {
           // Don't paint a frame over a hidden host — if an existing replacement
@@ -812,7 +824,7 @@ async function injectResolvedVideoFramesBatch(
         // Publish after full decode so raster cannot race it with a reduced JPEG decode.
         img.style.setProperty("visibility", "hidden", "important");
         img.decoding = "sync";
-        if (img.getAttribute("src") !== item.dataUri) {
+        if (item.dataUri !== undefined && img.getAttribute("src") !== item.dataUri) {
           if (item.crossOrigin) img.crossOrigin = item.crossOrigin;
           else img.removeAttribute("crossorigin");
           img.src = item.dataUri;
@@ -835,22 +847,20 @@ async function injectResolvedVideoFramesBatch(
         // GSAP-controlled value.
         video.style.setProperty("visibility", "hidden", "important");
         video.style.setProperty("pointer-events", "none", "important");
-        injectedIds.push(item.videoId);
+        if (!restyleOnly) injectedIds.push(item.videoId);
       }
       if (pendingDecodes.length > 0) {
         await Promise.all(pendingDecodes);
       }
       for (const img of imagesToPublish) img.style.visibility = "visible";
-      if (injectedIds.length > 0) {
+      if (imagesToPublish.length > 0) {
         const redraw = (window as Window & { __hf?: { colorGrading?: { redraw?: () => void } } })
           .__hf?.colorGrading?.redraw;
         redraw?.();
       }
       return injectedIds;
     },
-    // Build the sibling id with core's function rather than a template here,
-    // so the id the readers look up has exactly one definition.
-    updates.map((update) => ({ ...update, frameId: renderFrameIdForRenderId(update.videoId) })),
+    items,
     [...MEDIA_VISUAL_STYLE_PROPERTIES],
     COLOR_GRADING_SOURCE_HIDDEN_ATTR,
   );

packages/engine/src/services/videoFrameInjector.test.ts

@@ -231,18 +231,22 @@ describe("createVideoFrameInjector cache hygiene against page-side skips", () =>
     injectVideoFramesBatchMock.mockResolvedValueOnce([]);
     await hook!(fakePage, 0);
     expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(1);
-    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(fakePage, [
-      { videoId: "pip", dataUri: "data:image/png;base64,fake-/p" },
-    ]);
+    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(
+      fakePage,
+      [{ videoId: "pip", dataUri: "data:image/png;base64,fake-/p" }],
+      [],
+    );
 
     // Second call: same frameIndex, but the previous call did not really
     // paint. The cache must NOT short-circuit; the inject must run again.
     injectVideoFramesBatchMock.mockResolvedValueOnce(["pip"]);
     await hook!(fakePage, 0);
     expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(2);
-    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(fakePage, [
-      { videoId: "pip", dataUri: "data:image/png;base64,fake-/p" },
-    ]);
+    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(
+      fakePage,
+      [{ videoId: "pip", dataUri: "data:image/png;base64,fake-/p" }],
+      [],
+    );
   });
 
   it("does cache normally when the page reports the id as injected", async () => {
@@ -265,7 +269,21 @@ describe("createVideoFrameInjector cache hygiene against page-side skips", () =>
     expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(1);
 
     await hook!(fakePage, 0);
-    // Cache hit — no second inject for the same frameIndex.
+    // Cache hit: the picture is not sent again, only the video's style is re-copied.
+    expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(2);
+    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(fakePage, [], ["pip"]);
+  });
+
+  it("leaves a held video's style alone on a motion-blur sub-sample", async () => {
+    const fakePage = { evaluate: async () => undefined } as unknown as Page;
+    const hook = createVideoFrameInjector(
+      fakeTable({ videoId: "pip", framePath: "/p", frameIndex: 5 }),
+      { frameSrcResolver: inlineResolver },
+    );
+
+    injectVideoFramesBatchMock.mockResolvedValueOnce(["pip"]);
+    await hook!(fakePage, 0.5, 0.5);
+    await hook!(fakePage, 0.48, 0.5);
     expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(1);
   });
 
@@ -283,13 +301,16 @@ describe("createVideoFrameInjector cache hygiene against page-side skips", () =>
     await fresh!(truthPage, 0);
     // The fresh hook paints its own page even though the first hook already painted frame 5.
     expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(2);
-    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(truthPage, [
-      { videoId: "pip", dataUri: "data:image/png;base64,fake-/p" },
-    ]);
+    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(
+      truthPage,
+      [{ videoId: "pip", dataUri: "data:image/png;base64,fake-/p" }],
+      [],
+    );
 
     await hook!(renderPage, 0);
-    // The first hook still remembers frame 5 on its page.
-    expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(2);
+    // The first hook still remembers frame 5 on its page: it only re-copies the style.
+    expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(3);
+    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(renderPage, [], ["pip"]);
   });
 
   it("reinjects frame zero when a whole-chunk retry uses a fresh hook and page", async () => {

packages/engine/src/services/videoFrameInjector.ts

@@ -215,14 +215,19 @@ export function createVideoFrameInjector(
     }
 
     const updates: Array<{ videoId: string; dataUri: string; frameIndex: number }> = [];
+    const restyleIds: string[] = [];
     const activeIds = new Set<string>();
+    const isBlurSubSample = heldVideoTime !== undefined && heldVideoTime !== time;
     if (activePayloads.size > 0) {
       const pendingReads: Array<Promise<{ videoId: string; dataUri: string; frameIndex: number }>> =
         [];
       for (const [videoId, payload] of activePayloads) {
         activeIds.add(videoId);
         const lastFrameIndex = lastInjectedFrameByVideo.get(videoId);
-        if (lastFrameIndex === payload.frameIndex) continue;
+        if (lastFrameIndex === payload.frameIndex) {
+          if (!isBlurSubSample) restyleIds.push(videoId);
+          continue;
+        }
         pendingReads.push(
           frameCache
             .get(payload.framePath)
@@ -239,7 +244,7 @@ export function createVideoFrameInjector(
     }
 
     await syncVideoFrameVisibility(page, Array.from(activeIds));
-    if (updates.length > 0) {
+    if (updates.length > 0 || restyleIds.length > 0) {
       // Only record cache entries for videos the page actually painted.
       // injectVideoFramesBatch skips any video whose visual ancestor is
       // hidden (sub-comp host out-of-window) and returns the subset of ids
@@ -250,6 +255,7 @@ export function createVideoFrameInjector(
         await injectVideoFramesBatch(
           page,
           updates.map((u) => ({ videoId: u.videoId, dataUri: u.dataUri })),
+          restyleIds,
         ),
       );
       for (const update of updates) {

packages/producer/src/services/render/motionBlurVideo.integration.test.ts

@@ -1,5 +1,6 @@
 // Motion blur over a <video>, rendered for real (#5144): held video rows match blur-off, the
-// bar smears, and a sub-composition cut keeps its video. Needs Chrome + ffmpeg: integration lane.
+// bar smears, and a sub-composition cut keeps its video. A video held past its media end keeps
+// following its style (#4049). Needs Chrome + ffmpeg: integration lane.
 
 import { spawnSync } from "node:child_process";
 import {
@@ -81,6 +82,40 @@ const CUT_FPS = 24;
 const CUT_FRAME = 24;
 const CUT_FRAME_COUNT = 48;
 
+// A 0.2 s white clip in a 0.5 s slot, fading out over the whole slot on black (#4049).
+const HELD_MEDIA_SECONDS = 0.2;
+const HELD_COMPOSITION = `<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8" />
+  <style>
+    * { margin: 0; padding: 0; box-sizing: border-box; }
+    body { overflow: hidden; }
+    #root { background: #000; }
+    #clip {
+      position: absolute; left: 0; top: 0; width: ${WIDTH}px; height: ${HEIGHT}px;
+      animation: fade ${DURATION_SECONDS}s linear both;
+    }
+    @keyframes fade { from { opacity: 1; } to { opacity: 0; } }
+  </style>
+</head>
+<body>
+  <div
+    id="root"
+    data-composition-id="held-video"
+    data-start="0"
+    data-duration="${DURATION_SECONDS}"
+    data-width="${WIDTH}"
+    data-height="${HEIGHT}"
+    data-fps="${FPS}"
+    data-no-timeline
+  >
+    <video id="clip" class="clip" src="assets/white.mp4" data-start="0" data-duration="${DURATION_SECONDS}" data-track-index="0" muted playsinline></video>
+  </div>
+</body>
+</html>
+`;
+
 let root: string;
 
 async function renderFrames(
@@ -237,3 +272,60 @@ describe.skipIf(!HAS_FFMPEG)("motion blur over video — real render (#5144)", (
     }
   });
 });
+
+describe.skipIf(!HAS_FFMPEG)("a video held past its media end — real render (#4049)", () => {
+  let heldDir: string;
+  let plain: Buffer[];
+  let blurred: Buffer[];
+
+  beforeAll(async () => {
+    root = mkdtempSync(join(tmpdir(), "hf-held-video-"));
+    heldDir = join(root, "held");
+    mkdirSync(join(heldDir, "assets"), { recursive: true });
+    writeFileSync(join(heldDir, "index.html"), HELD_COMPOSITION);
+    const clip = spawnSync(
+      FFMPEG,
+      [
+        "-v",
+        "error",
+        "-f",
+        "lavfi",
+        "-i",
+        `color=c=white:size=${WIDTH}x${HEIGHT}:rate=${FPS}:duration=${HELD_MEDIA_SECONDS}`,
+        "-pix_fmt",
+        "yuv420p",
+        "-c:v",
+        "libx264",
+        "-y",
+        join(heldDir, "assets", "white.mp4"),
+      ],
+      { encoding: "utf-8" },
+    );
+    if (clip.status !== 0) throw new Error(`white clip fixture failed: ${clip.stderr}`);
+    plain = await renderFrames("held-plain", undefined, heldDir);
+    blurred = await renderFrames("held-blurred", { samplesPerFrame: 4 }, heldDir);
+  }, 600_000);
+
+  afterAll(() => {
+    if (root) rmSync(root, { recursive: true, force: true });
+  });
+
+  it("keeps following the fade after the media ends", () => {
+    expect(plain).toHaveLength(FRAME_COUNT);
+    const levels = plain.map(meanLevel);
+    const lastMediaFrame = Math.floor(HELD_MEDIA_SECONDS * FPS);
+    for (let i = lastMediaFrame; i < FRAME_COUNT - 1; i++) {
+      expect(levels[i + 1]!, `frame ${i + 1}`).toBeLessThan(levels[i]!);
+    }
+    expect(levels[FRAME_COUNT - 1]!).toBeLessThan(levels[0]! * 0.15);
+  });
+
+  it("gives every motion-blur sample the output frame's fade", () => {
+    expect(blurred).toHaveLength(FRAME_COUNT);
+    for (let i = 0; i < FRAME_COUNT; i++) {
+      expect(rows(blurred[i]!, 0, HEIGHT).equals(rows(plain[i]!, 0, HEIGHT)), `frame ${i}`).toBe(
+        true,
+      );
+    }
+  });
+});