Skip to content

feat(studio): clip thumbnail modules#2214

Open
ukimsanov wants to merge 1 commit into
studio-dnd/pr22-app-shell-swapfrom
studio-dnd/pr23-clip-thumbnails
Open

feat(studio): clip thumbnail modules#2214
ukimsanov wants to merge 1 commit into
studio-dnd/pr22-app-shell-swapfrom
studio-dnd/pr23-clip-thumbnails

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

What

ImageThumbnail (+tests) and thumbnailUtils (+tests) — frame decode with SVG/AVIF format fallbacks and rounded-corner clipping — plus VideoThumbnail updates.

Why

the decode layer for timeline clip thumbnails, ahead of the visual refresh that renders them.

How

new modules + one modified file; purely presentational.

Test plan

bunx vitest run on both test files; tsc --noEmit; fallow audit clean.


Stack position 23/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary TEMP(studio-dnd) fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.

ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga

@vanceingalls vanceingalls 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.

R1 — Batch E · clip thumbnails

Reviewed as PR 6 of 6 (thumbnail module addition, non-swap). Head SHA 7808aecb41bf55582e58a84ac4e2a17f28f55729.

Verdict: 🟢 LGTM

Presentational addition on top of the new engine. 5 files: 3 net-new (ImageThumbnail.tsx +160, ImageThumbnail.test.tsx +173, thumbnailUtils.{ts,test.ts} +54/+127), 1 tiny modify (VideoThumbnail.tsx +3/-3). Zero interaction with the swap correctness verified for #2211/#2212/#2213.

Verification

  • Landing point: sits atop studio-dnd/pr22-app-shell-swap (#2213 head).
  • Scope: no touches to timeline glue, store, or engine code — purely thumbnail extraction and helper utilities.
  • Coverage: 173 lines of ImageThumbnail tests + 127 lines of util tests = solid coverage for a new module.
  • No deletions, no fallow changes, no cross-package touches.

Nothing to block on. If reviewers upstream in Batch E flag anything, it wouldn't cascade here.

R1 by Via

@ukimsanov ukimsanov force-pushed the studio-dnd/pr22-app-shell-swap branch from 6f9c592 to bc7ce78 Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr23-clip-thumbnails branch 2 times, most recently from 4f76fc2 to ff8f3bf Compare July 11, 2026 08:18
@ukimsanov ukimsanov marked this pull request as ready for review July 11, 2026 09:06

@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.

Re-review — uncaught canvas.toDataURL SecurityError kills thumbnails silently

VideoThumbnail.tsx:122:

const dataUrl = canvas.toDataURL("image/jpeg", 0.6);

video.crossOrigin = "anonymous" is set at line 75, but resolveMediaPreviewUrl (thumbnailUtils.ts:52) passes absolute http(s): URLs through untouched. An external asset whose host doesn't send CORS response headers will taint the canvas → toDataURL throws SecurityError inside the seeked DOM listener.

Failure mode: no try/catch on the toDataURL call. The extractor dies mid-flight, the seeked handler never resolves, extractNext() never advances — the strip stays in shimmer forever with no user-facing error. Not memory-corrupt, not a crash — just silently broken thumbnails.

Reproduces reliably with any external asset whose CDN doesn't serve Access-Control-Allow-Origin. HeyGen assets should be safe (same-origin proxy path), but user-linked or externally-hosted assets from arbitrary sources will trip this.

Fix: wrap drawImage + toDataURL in try/catch. On SecurityError, either fall through to the error handler (which continues to the next timestamp), OR emit a placeholder frame for that timestamp. Either way, keep the extractor loop alive.

Bonus tail-batch nit: AssetCard hover-video has no debounce, no abort, no preload="metadata"

At AssetCard.tsx:190-203, every pointerenter mounts a fresh <video autoPlay> against the preview API. Dragging the cursor across a 40-tile assets grid triggers 40 sequential in-flight fetches; on slow networks this hammers the server and burns decoder init cost.

Fix: add a ~100ms hover-in delay (setTimeout guarded by an in-flight ref), or reuse the VideoFrameThumbnail element.

— Rames Jusso

@ukimsanov ukimsanov force-pushed the studio-dnd/pr22-app-shell-swap branch from 2700f84 to d5242de Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr23-clip-thumbnails branch from ff8f3bf to 47e2df7 Compare July 11, 2026 10:49
What: ImageThumbnail (+tests) and thumbnailUtils (+tests) — frame decode
with SVG/AVIF format fallbacks and rounded-corner clipping — plus
VideoThumbnail updates.

Why: the decode layer for timeline clip thumbnails, ahead of the visual
refresh that renders them.

How: new modules + one modified file; purely presentational.

Test plan: bunx vitest run on both test files; tsc --noEmit; fallow audit
clean.
@ukimsanov ukimsanov force-pushed the studio-dnd/pr23-clip-thumbnails branch from 47e2df7 to 51e0240 Compare July 11, 2026 12:51
@ukimsanov ukimsanov force-pushed the studio-dnd/pr22-app-shell-swap branch from d5242de to 2e1046f Compare July 11, 2026 12:51
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.

4 participants