Skip to content

fix(engine): local renders lay out width-only videos like the browser - #5771

Merged
miguel-heygen merged 2 commits into
mainfrom
cloudrender/video-hint-box
Oct 17, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
cloudrender/video-hint-box

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What changed

A local hyperframes render drew a <video> that the page sizes with only a CSS width (or only a height, or only a max-width) in the wrong box. A 200px-wide 16:9 clip got a 200x360 box with the picture centred inside it, so it sat lower than in the browser preview and in distributed chunk renders, which draw it at 200x112.

Cause: local renders pass the probed size of every extracted video to the engine, and applyVideoMetadataHints wrote it as width/height attributes. Those attributes are CSS size hints, so whichever axis the page left unset took the hinted pixel value instead of following the video's shape.

The hint exists for videos whose metadata Chrome never loads (frames are injected, so Chrome would otherwise lay them out at 300x150). It now gives such a video a transparent SVG poster of the probed size instead. Chrome takes a video's natural size from its poster until the video's own metadata loads, so every CSS combination lays out exactly as it does with the real video. The explicit aspect-ratio fallback is gone for the same reason. A video that already has a poster is left alone.

contain: size + contain-intrinsic-size was tried first and rejected: it matches the simple cases but shrinks a bordered width:100% flex item (style-9-prod's layout) by the border width.

What I measured

  • New real-Chrome test frameCapture-videoMetadataHints.browser.test.ts: six 640x360 videos with no metadata (no size, width only, height only, both sizes with object-fit, max-width only, bordered flex item) must get the boxes Chrome gives the same markup once a real 640x360 video's metadata has loaded (measured separately: 640x360, 200x112.5, 160x90, 200x300, 100x56.25, 416x241).
    • On origin/main (with only the export added) it fails: width only 200x360, height only 400x90, max-width only 100x360, bordered flex item 416x376.
    • With the fix: passes 3 runs in a row. Mutation (contain-size approach instead of the poster): fails on no size and the bordered flex item.
  • frameCapture.test.ts: 55 passed. Engine typecheck, oxlint and oxfmt clean.
  • Render of a two-video page (width-only and height-only clips): before, the local frame put both clips in the wrong box; after, the local frame and a distributed chunk frame show the same layout (PSNR 50 dB, the encoder difference only).
  • Every producer regression fixture with a video (28): loaded each output/compiled.html in headless Chrome and compared every video's box with old hints, new hints and no hints, with media loading and with media blocked. With media loading, old and new give the same box in all 28, so no baseline should move. With media blocked, new matches the loaded box in all 28; old was off for style-9-prod (1920x1087 instead of 1936x1096).
  • Producer regression harness (Docker test image built from this branch) on the video-layout fixtures: render-video-overlay-stretch, style-9-prod, portrait-edge-bleed, sub-composition-video, pip-video-late-host, video-hfid-no-id, style-4-prod: 7 of 7 passed against the committed baselines. No baseline changed.

What I did NOT exercise

  • A video that already has an authored poster keeps that poster, so if its metadata never loads it lays out at the poster's shape (same as in a browser before the video loads).
  • The test cannot observe the wait for the poster to decode: Chrome had applied the poster by the next CDP call even without it. A same-task probe showed the box is still 300x150 straight after setting the poster and correct after the decode, which is why the wait stays.
  • Windows and macOS renders; the full regression suite (only the video fixtures listed above ran).

@github-actions

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 173 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 33aae6b3. The only change since 6c26d10d renames the poster variable and drops a comment. Swapping the width/height attribute hints for a transparent SVG poster is the right fix. The attributes were acting as CSS sizes, so a video whose page set only one dimension (or only max-width) had its other axis pinned to the probed pixel size. A poster sets the video's natural size, which is what Chrome would use once the real metadata loads, so every CSS combination lays out the same way it does in the preview.

What I checked:

  • Callers: all three call sites at frameCapture.ts:2509, :2665 and :3459 await the now-async evaluate, so the poster decode finishes before capture starts.
  • Removed attributes and fallback: nothing in engine, core runtime or producer reads the hinted width/height attributes or the removed aspect-ratio fallback. The frame-injection overlay in screenshotService.ts (~line 798) sizes the <img> from getBoundingClientRect, so it picks up the corrected box with no other change.
  • Authored posters: a video that already has a poster is skipped, so authored posters are never overwritten. The PR body states the trade-off.
  • Test: the browser test pins the six boxes Chrome gives a 640x360 video once its metadata has loaded, including the bordered flex item that defeated the contain: size approach. The PR body reports it failing on main for four of them, so it does guard against this regression.

Non-blocking:

  1. image.decode() rejects if a poster fails to decode, which would make Promise.all throw and abort page setup. With a well-formed SVG data URL built from validated finite positive numbers that shouldn't happen. Promise.allSettled would still make the worst case a wrong box instead of a failed render.
  2. The poster is a real poster attribute on the live element. Studio's scene-swap URL comparison (sceneUrls in core/src/runtime/init.ts) and the asset-error diagnostics both read poster. That doesn't matter in a render page, but it's worth remembering if this hint ever runs in a preview context.

No live run on my side; I relied on the PR body's measured numbers and the browser test.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 17, 2026
Merged via the queue into main with commit fc41fa2 Oct 17, 2026
75 checks passed
@miguel-heygen
miguel-heygen deleted the cloudrender/video-hint-box branch October 17, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants