Repository navigation
perf(studio): scale timeline strip frames before drawing them - #7651
Merged
Merged
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Approving at 072a3702. I found no blocker in the code. The PR captures check ("Require Before and After captures") is red on this head and needs captures or an exemption before merge.
What I verified
thumbnailFrameSink.test.tsandthumbnailVideoDecoder.test.tspass at head (50/50).- The "fails without the fix:" test fails on base, but run as-is the failure is not the one the body describes. The new mock drops
CanvasSink, so the old decoder fails withNo "CanvasSink" export is defined on the "mediabunny" mock. I added the oldCanvasSinkmock back next toVideoSampleSinkand it then fails on the real point:createImageBitmapis never called, so the full frame goes through the canvas draw. The test holds either way. Consider keeping aCanvasSinkstub in the mock so the failure on main says why. - Resource handling: the
VideoFramefromtoVideoFrame()is closed in.finallyeven whencreateImageBitmaprejects. The bitmap is closed after the draw. The sample is closed infinally, which also runs when the decode loop stops iterating early (abort or timeout). - No distortion:
targetcomes fromtargetDimensions(info.aspect), so its aspect already matches the frame. Thecontainpath, which resizes the whole frame intotarget, can't letterbox or stretch beyond rounding. Turned and flipped samples keepdrawWithFit, which applies the rotation asCanvasSinkdid.
Non-blocking
- The test "keeps the reader when an offscreen frame will not encode" was deleted. The
HTMLCanvasElementencode-failure case ("keeps the reader when a clip's frame will not encode") still covers retaining the reader. TheOffscreenCanvasconvertToBlobbranch is now only reachable without adocument, and no test covers it. - Each frame now gets a new canvas, where
CanvasSinkusedpoolSize: 1. That's fine at strip sizes. I'm noting it in case strip counts get large. - Browser support:
createImageBitmap(VideoFrame, sx, sy, sw, sh, { resizeWidth, resizeHeight })is fine in Chromium. As the body says, Safari and Firefox weren't exercised.
Contributor
miguel-heygen
changed the title
miguel-heygen
enabled auto-merge
November 14, 2026 03:04
Merged
1 of 2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Timeline video strips now get each frame cropped and scaled by the browser (
createImageBitmapwith the strip's size) instead of drawing the full-size decoded frame onto a canvas. The scaling still runs on the main thread on a box with no GPU (about 140 ms per delete in a CPU profile), so this makes the work smaller, not off-thread.Why
Drawing a 3456x2234 frame onto a strip-sized canvas costs 100 ms or more of main thread per frame. While strips fill in, an edit's requests (a delete is about eight, one after another) each wait their turn behind those draws, so deleting a clip on a 4K timeline takes well over half a second to land.
Related work
Refs the Studio timeline delete being slow on 4K clips. Leaves the paused-frame decoder alone: switching it off changed nothing.
How
thumbnailFrameSink.ts:scaledFramesreads Mediabunny samples (VideoSampleSink), crops (cover) and scales each straight frame increateImageBitmap, and draws the small bitmap on a strip-sized canvas.drawWithFit, which applies the turn.thumbnailVideoDecoder.tsuses it in place ofCanvasSink; everything downstream (encode, caches, cleanup) is unchanged.Before
Timeline strips of two 4K clips, drawn the old way.
After
The same strips with this change: same frames, same crop.
What I measured
Cold delete of a 3456x2234, 60 fps, 45 Mbps clip with thumbnails on, in Studio on a shared 2-core Linux box with no GPU (load 2 to 16). Five deletes per run, three runs per build, original and fix alternating, same fixture and build otherwise. Time from pressing Delete to the clip leaving the timeline (median per run):
Main-thread long tasks in the 1.5 s after the press, mean per delete: original 480 ms, this PR 210 ms. With thumbnails switched off in the UI the same delete takes 181 to 269 ms.
Known limit: this does not bring the cold delete under 300 ms on this box every time (run 2 is 680 ms); the box is shared and the spread inside a run is wide.
Test plan
fails without the fix:
thumbnailVideoDecoder.test.ts"never hands a full-size frame to a canvas draw" (fails on the oldCanvasSinkpath, where the full frame is drawn).thumbnailFrameSink.test.ts(cover crop centred, contain whole frame, turned and flipped frames use the sample draw, frames and samples closed when the scale fails); the decoder test mock now yields video samples. Both files pass 3 runs in a row.What I did NOT exercise