Skip to content

fix(documentary-montage): render real footage (Soundtrack guard + cuts→scenes)#242

Open
1gassner wants to merge 1 commit into
calesthio:mainfrom
1gassner:fix/documentary-montage-footage-render
Open

fix(documentary-montage): render real footage (Soundtrack guard + cuts→scenes)#242
1gassner wants to merge 1 commit into
calesthio:mainfrom
1gassner:fix/documentary-montage-footage-render

Conversation

@1gassner

Copy link
Copy Markdown

What

Two fixes that make the documentary-montage (CinematicRenderer) Remotion
render path produce visible footage from a schema-compliant edit_decisions.
Today such a render either crashes (on a music opt-out) or produces a 30s black
video.

Fix 1 — CinematicRenderer crashes when music/soundtrack has no src

The render sites guard the object ({music ? <Soundtrack src={music.src}/> : null}),
but a music opt-out passes a truthy-but-srcless object (e.g. {"source":"none"}),
so music.src is undefinedresolveAsset(undefined).startsWith(...) aborts
the entire render.

  • Guard on .src: {music?.src ? … } / {soundtrack?.src ? … }
  • Fail-soft resolveAsset: if (!src) return ""

Fix 2 — documentary-montage renders a black video (cuts never mapped to scenes)

renderer_family: documentary-montage routes to the CinematicRenderer
composition, which consumes a scenes[] array. But edit_decisions carry
cuts (per schemas/artifacts/edit_decisions.schema.json), and
_remotion_render never builds scenes from them → the composition falls back
to its empty default scenes → 30s black output.

  • Build video scenes from the staged cuts (startSeconds cumulative,
    durationSeconds = out − in), gated to CinematicRenderer compositions so
    Explainer/others are untouched.

Relationship to #238 (please read)

The cuts→scenes mapping depends on the local asset staging from #238 (the
public/_om_assets staging). That staging is included in this branch's
video_compose.py diff because it isn't merged yet. This PR should be based
on / rebased onto #238
(or merged after it). CinematicRenderer.tsx is fully
independent of #238.

Verification

  • Full test suite: 426 passed / 8 skipped, no regressions.
  • End-to-end: a schema-valid cuts edit_decisions with music:{source:"none"}
    now renders non-black 1080p footage (frame inspected) — previously black.

🤖 Generated with Claude Code

…s→scenes)

Two fixes that make the documentary-montage (CinematicRenderer) Remotion
render path produce visible footage from a schema-compliant edit_decisions:

1. CinematicRenderer.tsx — the music/soundtrack render sites guarded the
   object ({music ? ...}) but then passed `.src` (undefined for a music
   opt-out like {source:"none"}) into <Soundtrack>, crashing the whole render
   at resolveAsset(undefined).startsWith. Guard on `.src` (music?.src /
   soundtrack?.src) + fail-soft resolveAsset(!src -> "").

2. video_compose.py (_remotion_render) — edit_decisions carry `cuts`, but the
   CinematicRenderer composition consumes a `scenes[]` array. Nothing mapped
   cuts -> scenes, so the composition fell back to its empty default scenes and
   rendered a 30s black video (the "local rendering broken" symptom). Build
   video scenes from the staged cuts, gated to CinematicRenderer compositions
   (Explainer etc. untouched).

NOTE on attribution: the staging code in video_compose.py is from calesthio#238
(@calesthio), applied locally and not yet merged; the cuts->scenes mapping
depends on it. This change should be based on / rebased onto calesthio#238.

Verified: full suite 426 passed / 8 skipped; a schema `cuts` edit_decisions
with a music opt-out renders non-black 1080p footage (frame inspected).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@1gassner 1gassner requested a review from calesthio as a code owner June 30, 2026 14:54

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

Disclosure first: I authored #238 (local zero-key asset staging), which this PR overlaps, so please weigh my review with that in mind. The core contribution here is genuinely useful and I'd like to see it land — the notes below are about scoping it so it composes cleanly.

What's valuable and correct

  • The resolveAsset fail-soft (if (!src) return "") and the music?.src / soundtrack?.src guards are a real crash fix — a truthy-but-srcless music opt-out ({"source":"none"}) genuinely blows up the whole render at undefined.startsWith. Good catch.
  • The cuts→scenes mapping for CinematicRenderer is the real fix for the black-video symptom and is the part I'd most want merged — documentary-montage/cinematic-trailer consume scenes[], and nothing was translating cuts into them.

Please address before merge

  1. The _stage_asset block is a verbatim re-implementation of #238 and will hard-conflict. This branch is based on pre-#238 upstream, so it re-introduces the same public/_om_assets staging (cuts + audio.narration/audio.music). If #238 merges first this whole hunk conflicts; if this merges first it silently reverts the follow-up. Worse, it's the v1 of that block — the maintainer-requested #238 update also stages the top-level soundtrack.src / music.src that CinematicRenderer actually reads (CinematicRenderer.tsx:480,491). Since this PR stages only the nested Explainer-shape audio, a real (non-opt-out) local music/soundtrack path still isn't staged and will hit the same file:// render rejection. Suggest rebasing on current main and dropping the staging hunk entirely, keeping only the cuts→scenes mapping + the resolveAsset/guard fixes.

  2. LOCAL_PATCHES_2026-06-30.md should not be in the PR. It's local patch notes at the repo root — an unrelated artifact (the PR checklist calls this out). The commit message / PR body already capture the rationale.

  3. cuts→scenes hardcodes kind: "video". A still-image cut (image source) becomes a kind:"video" scene, which CinematicRenderer will try to play as footage. Consider deriving kind from the cut (e.g. image vs. video by cut.type/extension) so mixed still+motion montages render correctly.

Happy to coordinate on the #238 overlap so we don't double-land the staging code — the cuts→scenes work is the piece #238 doesn't cover and is worth keeping.

@calesthio calesthio left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the documentary-montage fix. The core bug is real and valuable: CinematicRenderer should not crash on a truthy music/soundtrack object without src, and cuts need to be adapted to scenes for documentary/cinematic renderers.\n\nI’m requesting changes because the current branch still carries the unresolved issues from the earlier review:\n\n- It includes a local patch-note artifact at the repo root (LOCAL_PATCHES_2026-06-30.md), which should not land as part of the product diff.\n- It reimplements the #238 local-asset staging block instead of depending on/rebasing after the canonical staging fix. That creates conflict risk and currently misses the later #238 follow-ups such as top-level cinematic soundtrack.src / music.src staging and stale staged-copy refresh.\n- The cuts→scenes adapter hardcodes kind: 'video', so image cuts or mixed still/motion montages can be handed to CinematicRenderer as video scenes. Please derive kind from cut metadata or source type.\n\nBest path: rebase after the local asset staging PR is settled, drop the duplicated staging hunk and local notes file, then keep the CinematicRenderer guard plus a tested cuts→scenes adapter. That would preserve the high-value part of this PR without conflicting with the staging work.

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.

3 participants