From ef4f997a7266e42e177dd741ca905da599c80bb2 Mon Sep 17 00:00:00 2001 From: Brian Madison Date: Sun, 26 Jul 2026 23:53:02 -0500 Subject: [PATCH] Release 3.1.0: review the cut on a virtual timeline, not a render Reviewing a cut no longer means waiting for an encode. All of this came out of one real 20-minute 4K project whose gate-2 preview took 22 minutes to render, twice, because the first one failed validation at the finish line. New edl_to_ffconcat.py writes the EDL as a virtual timeline (ffconcat, and optionally an mpv EDL) that plays the cut with no encode at all. On a 379-segment 16-minute cut it resolves in 33ms against 22 minutes for the equivalent render, verified pixel-identical to source ground truth at four points including random seeks. mc-cut now builds that at gate 2 and renders no file; a file is rendered only when something needs a file. Preview proxies are now all-intra, which is a correctness fix rather than a speed tweak: the concat demuxer can only cut on keyframes, so against a long-GOP proxy the timeline silently plays the wrong frames while reporting the exact right duration. Measured at an 8.333s GOP against segments averaging 2.51s, every segment began up to 8.3s early. Proxy paths and sidecars now carry the encode family and a recipe so an older proxy can never be reused as if it were intra. Two EDL bugs that only appear on long cuts are now cutting rules: boundaries must be quantized to the source frame grid (sub-frame times accumulate past the output gate's tolerance beyond ~150 segments), and a trailing silence must be truncated rather than tightened (which otherwise scrapes a junk segment off the end of the file). Applying approved cuts now says out loud that everything derived from the EDL is stale, including beat anchors once the beats stage has run, with an explicit regeneration order and a required anchor re-check before graphics continue. A concat stream copy renders the same frames ~470x faster but is deliberately not the preview path: it emits duplicate DTS on sub-frame-length segments and fails render_preview's own decode validation. It is kept, tested and documented for callers that accept the caveat. --- .claude-plugin/marketplace.json | 2 +- CHANGELOG.md | 29 +++ skills/mc-cut/SKILL.md | 47 ++++- skills/mc-cut/references/rendering.md | 44 +++- skills/mc-cut/scripts/composite_core.py | 122 +++++++++-- skills/mc-cut/scripts/edl_to_ffconcat.py | 193 ++++++++++++++++++ skills/mc-cut/scripts/render_preview.py | 34 ++- .../scripts/tests/test-edl_to_ffconcat.py | 169 +++++++++++++++ .../scripts/tests/test-render_preview.py | 49 ++++- 9 files changed, 662 insertions(+), 27 deletions(-) create mode 100644 skills/mc-cut/scripts/edl_to_ffconcat.py create mode 100644 skills/mc-cut/scripts/tests/test-edl_to_ffconcat.py diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index eaa67c7..25c90ca 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -19,7 +19,7 @@ "name": "bmad-manticore", "source": "./", "description": "AI video production pipeline: brain dump to a rough cut sitting in your editor, in your own words, with approval gates at every taste decision.", - "version": "3.0.0", + "version": "3.1.0", "author": { "name": "Brian (BMad) Madison" }, diff --git a/CHANGELOG.md b/CHANGELOG.md index e75e1ed..e088617 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,35 @@ All notable changes to BMad Manticore are documented here. Dates are ISO (YYYY-MM-DD). +## 3.1.0 - Unreleased + +Reviewing a cut no longer means waiting for a render. Everything here came out of one real 20-minute 4K project whose gate-2 preview took 22 minutes to encode — twice, because the first one failed validation at the finish line. + +### You do not render a file to review a cut any more + +- New `edl_to_ffconcat.py` writes the EDL as a **virtual timeline** — an ffconcat playlist (and optionally an mpv EDL) that plays the cut with no encode at all: `ffplay -f concat -safe 0 -i cut/preview.ffconcat`. On a 379-segment 16-minute cut the timeline resolves in 33ms against 22 minutes for the equivalent render, and was verified pixel-identical (PSNR inf) to source ground truth at four points including random seeks. It carries video and audio together, so A/V sync is reviewable — which an audio-only preview cannot show. +- mc-cut now builds the virtual timeline at gate 2 and does **not** render a preview file. Rendering a full preview before the creator has approved a single call was the most expensive habit the stage had. A file gets rendered when something actually needs a file: a composited preview once overlays exist, something to share, or the gate-4 final. +- `render_preview.py --proxy-only` builds the proxy and exits, so the timeline can be built without a render. + +### Preview proxies are all-intra, and that is a correctness fix + +- `build_proxy_command` now encodes proxies all-intra (`-g 1 -keyint_min 1 -sc_threshold 0`). This is not a speed tweak: the concat demuxer can only cut on keyframes, so against a long-GOP proxy the virtual timeline silently plays the WRONG frames while still reporting the exact right duration. Measured at an 8.333s GOP against segments averaging 2.51s (163 of 379 under 2s), every segment began up to 8.3s early — a wrong cut that passes every check except a frame comparison. `edl_to_ffconcat.py` refuses a non-intra source unless you pass `--allow-long-gop`. +- Proxy paths carry the encode family (`-720p-intra.mp4`) and sidecars now record a recipe, so a proxy built by an older version can never be reused as if it were intra. A sidecar with no recipe line is stale by definition. +- The cost is disk: roughly 10x a long-GOP proxy (698MB vs 65MB for 20 minutes at 720p), built once per source and reused by every later preview and render. + +### Two EDL bugs that only appear on long cuts + +- **EDL boundaries must be quantized to the source frame grid.** They come from the audio map at 0.1s granularity and from word timestamps, so they are sub-frame by default; ffmpeg rounds each `trim` to the nearest frame and the error accumulates. A 379-segment cut rendered 954.766s against an expected 952.460s and the output gate correctly refused to publish it — the cut was fine, only the arithmetic disagreed. Past roughly 150 segments this happens every time. Now a cutting rule, with the tail-clamp for boundaries that round past `source_duration`. +- **A trailing silence must be truncated, not tightened.** Tightening the final silence at both ends like an interior gap leaves a junk segment scraped off the absolute end of the file, past the last detected silence and past `source_duration` once frame-aligned. Now a cutting rule: end the final segment on a real tail inside the verified silence. + +### Applying approved cuts invalidates everything downstream, loudly + +- New section in mc-cut spelling out that the moment `cut/edl.json` is rewritten, every derived artifact is stale — the virtual timeline, the FCPXML, the edited transcript, the preview, the boundary frames, and **every beat anchor in `beats/beats.md`**. Nothing on disk announces this: a stale FCPXML imports cleanly and a stale beat table renders overlays that land off their phrase, discovered after the graphics are paid for. The regeneration order is now explicit, `verify_edl.py` runs first and its non-zero exit stops the rest, and when a beat table already exists the anchor check must re-run and be reported before graphics continue. + +### Not shipped, and why + +- A concat-demuxer **stream copy** off the intra proxy renders the same frames ~470x faster (2.8s vs 22min, 340x realtime). It is not the preview path: it emits duplicate DTS wherever a segment runs only a frame or two, and the file then fails `render_preview.py`'s own decode validation — 125 errors on that cut, and 3 with every timestamp remedy applied (`+genpts`, `avoid_negative_ts`, `video_track_timescale`, `fps_mode passthrough`, `muxdelay 0`), never zero, from just 2 sub-0.1s segments. Publishing it would mean loosening a correctness gate to buy a speed number. `composite_core.build_streamcopy_command` keeps it, tested and documented, for a caller that wants a scratch file and accepts the caveat. The speed moved to the virtual timeline instead, which is both faster and correct. + ## 3.0.0 - Unreleased The big release: one motion-graphics engine with the full HyperFrames toolkit behind it, cross-platform support, a final render that only re-does what changed, delivery polish (loudness, captions, OBS alpha), and a cut stage rebuilt from the ground up after its first real project. Upgrading from 1.x is a clean reinstall (see README): your brand, voice bible, and format profiles live in your studio folder, not in `_bmad/`, so they survive and onboarding picks them back up. diff --git a/skills/mc-cut/SKILL.md b/skills/mc-cut/SKILL.md index 6be17e9..4aadd27 100644 --- a/skills/mc-cut/SKILL.md +++ b/skills/mc-cut/SKILL.md @@ -83,7 +83,21 @@ uv run {skill-root}/scripts/edited_transcript.py transcript/words.json --edl cut Both clean and source timecodes come from here; never convert between them by hand. Run the editorial pass on that transcript per `{skill-root}/references/editorial-pass.md`, writing `cut/editorial-review.md` from `{skill-root}/assets/editorial-review-template.md`. Nothing it recommends is auto-applied. RE-RECORD items are the one exception to "the cut applies the calls": there is no pickup re-entry path, so they hand over as a shoot list and the cut proceeds without them. -Write `cut/cutplan.md` carrying both tiers, each call with its timestamp and the quoted words. Routine silence trims group into one line. Always itemize section re-reads, bloopers and every content-tier recommendation, whatever their size. Set `approvals.cutplan = "pending"`, present it, and STOP for gate 2. +Write `cut/cutplan.md` carrying both tiers, each call with its timestamp and the quoted words. Routine silence trims group into one line. Always itemize section re-reads, bloopers and every content-tier recommendation, whatever their size. + +Then build the VIRTUAL TIMELINE and give the creator that to watch. Do NOT render a file for gate 2. + +``` +uv run {skill-root}/scripts/edl_to_ffconcat.py cut/edl.json -o cut/preview.ffconcat --source renders/proxy/-p-intra.mp4 +``` + +They watch it with no render at all: + +``` +ffplay -f concat -safe 0 -i cut/preview.ffconcat +``` + +Set `approvals.cutplan = "pending"`, present it, and STOP for gate 2. ## Apply the approved calls @@ -95,9 +109,36 @@ uv run {skill-root}/scripts/snap_spans.py cut/approved-spans.json --audio-map cu Back up the prior EDL to `cut/edl.pre-editorial.json`, rewrite `cut/edl.json` from the snapped spans, re-run `verify_edl.py`, and append the APPLIED section to `cut/editorial-review.md`. +## THE EDL CHANGED, SO EVERYTHING DERIVED FROM IT IS NOW A LIE + +The moment `cut/edl.json` is rewritten, every artifact built from the previous one is stale: the virtual timeline, the FCPXML, the edited transcript, the preview, the boundary frames, and — once the beats stage has run — **every beat anchor in `beats/beats.md`**. Nothing on disk announces this. A stale FCPXML imports cleanly. A stale beat table renders overlays that land off their phrase, and that is discovered after the graphics are paid for. + +So regenerate the whole derived set together, in this order, every single time the EDL changes: + +``` +uv run {skill-root}/scripts/verify_edl.py cut/edl.json --audio-map cut/audio-map.json --words transcript/words.json -o cut/edl-check.json +uv run {skill-root}/scripts/edited_transcript.py transcript/words.json --edl cut/edl.json -o cut/edited-transcript.md -j cut/edited-words.json +uv run {skill-root}/scripts/edl_to_ffconcat.py cut/edl.json -o cut/preview.ffconcat --source +uv run {skill-root}/scripts/edl_to_fcpxml.py cut/edl.json -o cut/rough.fcpxml +``` + +`verify_edl.py` runs FIRST and its non-zero exit stops the rest: never regenerate derived artifacts from an EDL that has not re-passed. + +**If `beats/beats.md` exists, the beat table is now suspect.** Beat times are derived from the EDL, so a cut applied after the beats stage moves every anchor downstream of it. Re-run mc-beats' anchor check and report the result to the creator before any graphics work continues: + +``` +uv run {skill-root}/../mc-beats/scripts/verify_anchors.py beats/beats.md --edl cut/edl.json --words transcript/words.json -o beats/anchor-check.json +``` + +A non-zero exit means the beat table must go back to mc-beats before mc-graphics runs again. Say so plainly rather than letting the graphics stage build against moved anchors. + ## Deliver -After approval, and again after every later re-approval that changes the cut: render the preview, export the timeline, and regenerate every other derived artifact together. `{skill-root}/references/rendering.md` carries the commands, the config wiring and the staleness check. Inspect the boundary frames for what they can see, black frames and straddles, up to 3 retries per cut. They see less than they appear to: on the corrupted project every frame looked clean while the cut underneath was built on the hole. +**Do not render a file to review a cut.** The virtual timeline above is what the creator watches at gate 2 and after every re-approval: it costs nothing, it is frame-exact, and it carries video AND audio so A/V sync is reviewable. Rendering a full preview before a single call has been approved is the single most expensive habit this stage ever had — on a 379-segment 16-minute cut it was 22 minutes, paid before the creator had said yes to anything. + +Render a FILE only when something actually needs a file: a composited preview once overlays exist, something to upload or share, or the gate-4 final. `{skill-root}/references/rendering.md` carries the commands, the config wiring and the staleness check. + +Whenever a file does get rendered, inspect the boundary frames for what they can see, black frames and straddles, up to 3 retries per cut. They see less than they appear to: on the corrupted project every frame looked clean while the cut underneath was built on the hole. Chapters or log notes written against source timecode remap onto the edited timeline with `uv run {skill-root}/scripts/remap_timecode.py cut/edl.json --direction orig-to-clean --chapters -o `, and `--direction clean-to-orig` maps back. @@ -114,6 +155,8 @@ Final render: when the project reaches the final stage, offer the final-quality ## Cutting rules (non-negotiable) - Never cut inside a word. Edges land inside an audio-verified silence, which makes this structural rather than aspirational. Pad 30 to 200 ms. +- **Quantize every EDL boundary to the source frame grid** (`round(t * fps) / fps`) before writing `cut/edl.json`. Boundaries come from the audio map at 0.1s granularity and from word timestamps, so they are sub-frame by default; ffmpeg rounds each `trim` to the nearest frame and the error ACCUMULATES. A 379-segment cut rendered 954.766s against an expected 952.460s and the output gate correctly refused to publish it — the cut was fine, only the arithmetic disagreed. Past roughly 150 segments this happens every time. Max shift from quantizing is half a frame (16.67ms at 30fps), far inside the 200ms silence pad, and `verify_edl.py` re-passes. Clamp any boundary that rounds past `source_duration`. +- **Truncate a TAIL silence, never tighten it.** The trailing silence after the last word is not a gap between two utterances. Tightening it at both ends leaves a junk segment scraped off the absolute end of the file, past the last detected silence and past `source_duration` once frame-aligned. End the final segment on a real tail (roughly 0.5s after the last word), inside the verified silence. - 30 ms audio fades on every cut boundary (`fade_ms` in the EDL). - Never shrink or letterbox the source video to make room for graphics; overlays composite over the full frame in safe zones. Nothing enforces this one, and the beats and graphics stages inherit whatever canvas this stage leaves them. diff --git a/skills/mc-cut/references/rendering.md b/skills/mc-cut/references/rendering.md index 89dc361..b003e1a 100644 --- a/skills/mc-cut/references/rendering.md +++ b/skills/mc-cut/references/rendering.md @@ -15,7 +15,40 @@ order rather than on any stated rule. So the flags strings are an escape hatch for what the config does not model, such as `--segment-target-seconds`. Never put `--height`, `--crf`, or the loudness flags in them. -## Preview +## The default is NOT to render + +Reviewing a cut does not need a rendered file. The virtual timeline plays the +EDL directly — no encode, no wait, frame-exact, video and audio together: + +``` +uv run {skill-root}/scripts/render_preview.py cut/edl.json -o renders/preview.mp4 --proxy-only +uv run {skill-root}/scripts/edl_to_ffconcat.py cut/edl.json -o cut/preview.ffconcat \ + --source renders/proxy/-720p-intra.mp4 +ffplay -f concat -safe 0 -i cut/preview.ffconcat +``` + +`--proxy-only` builds the all-intra proxy and stops. Add `--mpv cut/preview.mpv.edl` +to `edl_to_ffconcat.py` for creators who have mpv, which scrubs better. + +Measured on a 379-segment 16-minute cut: the timeline resolves in 33ms against +22 minutes for the equivalent render, and was verified pixel-identical (PSNR +inf) to source ground truth at four points including random seeks. + +**The all-intra proxy is a correctness precondition, not an optimisation.** The +concat demuxer can only cut on keyframes. Against a long-GOP proxy (8.333s GOP +vs segments averaging 2.51s) every segment starts up to 8.3s early while the +total duration still matches the EDL exactly — a wrong cut that passes every +check except a frame comparison. `edl_to_ffconcat.py` refuses a non-intra +source unless you pass `--allow-long-gop`. + +The proxy costs ~10x the disk of a long-GOP one (698MB vs 65MB for 20 minutes +at 720p) and is built once per source, then reused by every later preview and +render. + +## Preview file, when you actually need a file + +Only when something needs to be a file: a composited preview once overlays +exist, something to upload or share, or the gate-4 final. ``` uv run {skill-root}/scripts/render_preview.py cut/edl.json \ @@ -26,6 +59,15 @@ uv run {skill-root}/scripts/render_preview.py cut/edl.json \ Defaults to 720p CRF 28 when `[render]` leaves them unset. Never loudness-normalized. Check `"validated": true` in the summary. +This path re-encodes. A concat stream copy off the intra proxy is ~470x faster +(2.8s vs 22min measured) and is genuinely the same frames, but it emits +duplicate DTS wherever a segment runs only a frame or two, and the file then +fails this script's own decode validation — 125 errors on that cut, and 3 with +every timestamp remedy applied, never zero, from just 2 sub-0.1s segments. +`composite_core.build_streamcopy_command` keeps it, tested, for a caller that +wants a scratch file and accepts the caveat. It is not the review path because +the virtual timeline is faster AND correct. + Composited, once the graphics stage has rendered overlays into `graphics/`, add: ``` diff --git a/skills/mc-cut/scripts/composite_core.py b/skills/mc-cut/scripts/composite_core.py index e421e3a..6202251 100644 --- a/skills/mc-cut/scripts/composite_core.py +++ b/skills/mc-cut/scripts/composite_core.py @@ -360,50 +360,140 @@ def build_lane_composite_command(base, lane_files, output, crf=28, return argv -def proxy_path(proxy_dir, source, height): +def proxy_path(proxy_dir, source, height, intra=True): """Where a source's preview proxy lives (pure). - Named by the source stem plus height so a project's proxies are readable - on disk; freshness is decided by the sidecar digest, not the name. + Named by the source stem, height and encode family so a project's proxies + are readable on disk AND an all-intra proxy can never collide with a + long-GOP one built by an older version. Freshness is still decided by the + sidecar, not the name. """ - return Path(proxy_dir) / f"{Path(source).stem}-{height}p.mp4" + suffix = "-intra" if intra else "" + return Path(proxy_dir) / f"{Path(source).stem}-{height}p{suffix}.mp4" -def proxy_is_fresh(proxy, source): - """True when a proxy exists and was built from this exact source (pure-ish). +def proxy_is_fresh(proxy, source, recipe=None): + """True when this proxy was built from this source UNDER THIS RECIPE. The sidecar records the source's content digest at build time, so a re-recorded take with the same filename correctly invalidates its proxy. + + It also records the proxy RECIPE, because content is only half of + freshness: a proxy built by an older version is long-GOP, and the stream + copy lane is silently WRONG against it (right duration, wrong frames). A + sidecar with no recipe line predates the intra lane and is therefore + stale by definition. """ + recipe = PROXY_RECIPE if recipe is None else recipe proxy = Path(proxy) sidecar = proxy.with_name(proxy.name + ".src") if not proxy.is_file() or not sidecar.is_file(): return False try: - return sidecar.read_text(encoding="utf-8").strip() == \ - content_digest(source, cheap=True) + lines = sidecar.read_text(encoding="utf-8").splitlines() except OSError: return False + if not lines: + return False + if lines[0].strip() != content_digest(source, cheap=True): + return False + stored = lines[1].strip() if len(lines) > 1 else "" + return stored == recipe + +# The proxy recipe. Bumping this string invalidates every existing proxy via +# the sidecar, which is what you want whenever the encode settings change: +# a proxy built under an older recipe is silently wrong for the current one. +PROXY_RECIPE = "intra-v1" -def build_proxy_command(source, out, height, crf=26, preset="veryfast"): + +def build_proxy_command(source, out, height, crf=26, preset="veryfast", + intra=True): """ffmpeg argv transcoding a source to a preview proxy (pure). One linear pass. Audio is re-encoded rather than copied so the proxy is seekable and self-contained; the preview's audio comes from here too, and the final render never touches proxies. + + ALL-INTRA (intra=True, the default) is what makes the fast preview lane + possible, and it is not an optimisation detail -- it is a correctness + precondition: + + - Every frame is a keyframe, so the concat demuxer's inpoint/outpoint are + frame-exact. Against a long-GOP proxy they are NOT: measured at an + 8.333s GOP against segments averaging 2.51s, every segment began up to + 8.3s early while total duration still matched the EDL exactly, so the + wrong cut looked right in every check except a frame comparison. + - Because every cut lands on a keyframe by definition, the preview can be + a STREAM COPY instead of a re-encode. Measured on a 379-segment 16-min + cut: 22 minutes of filter_complex re-encode became 2.8 seconds of + remux, at 340x realtime. + + The cost is disk: all-intra 720p runs roughly 10x a long-GOP proxy (698MB + vs 65MB for 20 minutes). That is the trade this lane makes deliberately -- + the proxy is built once per source and reused by every later preview, + while the re-encode was paid on every single iteration. + """ + cmd = ["ffmpeg", "-y", "-hide_banner", "-v", "error", "-i", str(source), + "-vf", f"scale=-2:{height}", "-c:v", "libx264", "-preset", preset, + "-crf", str(crf), "-pix_fmt", "yuv420p"] + if intra: + # -g 1 alone is not enough: x264 can still emit non-IDR frames, and + # scenecut detection inserts its own keyframes on top. All three + # together give exactly one I-frame per frame. + cmd += ["-g", "1", "-keyint_min", "1", "-sc_threshold", "0"] + cmd += ["-c:a", "aac", "-b:a", "160k", "-movflags", "+faststart", + str(out)] + return cmd + + +def build_streamcopy_command(concat_file, out): + """ffmpeg argv remuxing an ffconcat virtual timeline into a file (pure). + + No filtergraph and no encoder: the segments are cut on keyframes so their + packets are copied straight through. Valid ONLY against an all-intra + source (see build_proxy_command) and ONLY when nothing needs compositing + -- an overlay changes pixels, which forces a re-encode. + + The EDL's fade_ms is deliberately not applied here. Audio fades exist to + stop a click when a cut lands mid-waveform, and verify_edl already + guarantees every boundary rests in audio-verified silence, so there is no + discontinuity for a fade to hide. Measured across 8 random joins on a + 379-segment cut: unfaded peaks ran -31 to -44 dB against speech peaking + at -9 dB, and tracked the faded render within 0.5 dB on most joins. If a + future format ever cuts outside silence, this assumption dies with it. + + NOT WIRED INTO render_preview, and the reason is worth keeping: + This is ~470x faster than the encode (2.8s vs 22min measured on a + 379-segment 16-minute cut, 340x realtime) and it is genuinely the + same frames. But the concat demuxer emits DUPLICATE DTS wherever a + segment runs only a frame or two, and the resulting file fails + render_preview's decode validation. On that same cut: 125 decode + errors plain, and 3 with every timestamp remedy tried (+genpts, + avoid_negative_ts make_zero, video_track_timescale 30000, fps_mode + passthrough, muxdelay 0). Never zero. Only 2 of 379 segments were + under 0.1s, which is all it takes. + + Publishing that file would mean loosening a correctness gate to buy a + speed number, so the speed moved instead of the gate: gate 2 review + runs on the virtual timeline (edl_to_ffconcat.py), which is never + muxed and therefore cannot have this problem, and which verifies + frame-exact. Kept here, tested, and available for a caller that wants + a scratch file and accepts the caveat. """ - return ["ffmpeg", "-y", "-hide_banner", "-v", "error", "-i", str(source), - "-vf", f"scale=-2:{height}", "-c:v", "libx264", "-preset", preset, - "-crf", str(crf), "-pix_fmt", "yuv420p", "-c:a", "aac", - "-b:a", "160k", "-movflags", "+faststart", str(out)] + return ["ffmpeg", "-y", "-hide_banner", "-v", "error", + "-f", "concat", "-safe", "0", "-i", str(concat_file), + "-c", "copy", "-movflags", "+faststart", str(out)] -def write_proxy_sidecar(proxy, source): - """Record which source build this proxy, for proxy_is_fresh.""" +def write_proxy_sidecar(proxy, source, recipe=None): + """Record which source and which recipe built this proxy, for + proxy_is_fresh. Line 1 is the source digest, line 2 the recipe.""" + recipe = PROXY_RECIPE if recipe is None else recipe proxy = Path(proxy) proxy.with_name(proxy.name + ".src").write_text( - content_digest(source, cheap=True) + "\n", encoding="utf-8") + content_digest(source, cheap=True) + "\n" + recipe + "\n", + encoding="utf-8") def proxied_edl(edl, mapping): diff --git a/skills/mc-cut/scripts/edl_to_ffconcat.py b/skills/mc-cut/scripts/edl_to_ffconcat.py new file mode 100644 index 0000000..3da6f87 --- /dev/null +++ b/skills/mc-cut/scripts/edl_to_ffconcat.py @@ -0,0 +1,193 @@ +#!/usr/bin/env python3 +# /// script +# requires-python = ">=3.11" +# /// +"""Write the VIRTUAL TIMELINE for an EDL: an ffconcat playlist (and optionally +an mpv EDL) that plays the cut with no render at all. + +Usage: + uv run {skill-root}/scripts/edl_to_ffconcat.py cut/edl.json \ + -o cut/preview.ffconcat [--source ] [--mpv cut/preview.mpv.edl] + +Why this exists: + The cut stage used to make the creator wait for a full re-encode before + they could watch anything. On a 16-minute cut with 379 segments that was + ~22 minutes, and it happened at gate 2, BEFORE a single call had been + approved. An NLE does not work that way: it plays a virtual timeline, + seeking into the source and jumping between segments live. + + This is that timeline, in a file. ffplay/ffmpeg read it directly: + + ffplay -f concat -safe 0 -i cut/preview.ffconcat + + Zero render, instant, seekable, video AND audio (so A/V sync is + reviewable, which an audio-only preview cannot show). + +THE KEYFRAME CONSTRAINT, which is the whole reason this needs a matching proxy: + The concat demuxer's `inpoint` can only begin at a KEYFRAME. Against a + normal long-GOP encode this silently produces the WRONG CUT: measured on a + 720p proxy with an 8.333s GOP against segments averaging 2.51s (163 of 379 + under 2s), every segment started up to 8.3s early. Total duration still + matched the EDL to the millisecond, because `outpoint` clips the length -- + so the failure is invisible unless you compare frames. + + Against an ALL-INTRA source every frame is a keyframe, so every cut point + is exact. Verified pixel-identical (PSNR inf) to source ground truth at + four points including random seeks. + + So: point this at an all-intra proxy, which is what composite_core's + build_proxy_command now produces. Pointing it at a long-GOP master or an + old proxy gives a plausible-looking, wrong timeline. --require-intra + (default) refuses rather than letting that happen. + +Contract: + input edl.json: {source, source_duration, segments[]}; each segment + {source, start, end, ...} in seconds against its source. + output -o: an ffconcat v1.0 playlist, one file/inpoint/outpoint triple + per segment, with absolute source paths. + --mpv optionally also write an mpv EDL (`# mpv EDL v0`, one + file,start,length line per segment, %len%-quoted paths). + --source override the source path every segment points at (e.g. swap the + master for its proxy). Default: each segment's own source. + summary json.dumps on stdout: {ok, segments, duration_seconds, output, + mpv, all_intra}. + +Exit codes: 0 ok, 1 source unreadable or not all-intra (with +--require-intra), 2 usage error. + +STATUS: implemented (pure logic covered by +scripts/tests/test-edl_to_ffconcat.py). +""" + +import argparse +import json +import subprocess +import sys +from pathlib import Path + + +def ffconcat_lines(segments, source_override=None): + """The ffconcat v1.0 body for these segments (pure).""" + out = ["ffconcat version 1.0"] + for seg in segments: + src = source_override or seg["source"] + out.append(f"file '{src}'") + out.append(f"inpoint {float(seg['start']):.6f}") + out.append(f"outpoint {float(seg['end']):.6f}") + return out + + +def mpv_edl_lines(segments, source_override=None): + """The mpv EDL v0 body for these segments (pure). + + mpv takes file,START,LENGTH (not end), and quotes a path as + %% so commas in filenames cannot split the record. + """ + out = ["# mpv EDL v0"] + for seg in segments: + src = str(source_override or seg["source"]) + start = float(seg["start"]) + length = float(seg["end"]) - start + out.append(f"%{len(src.encode())}%{src},{start:.6f},{length:.6f}") + return out + + +def is_all_intra(path, probe_seconds=30): + """True when every sampled video frame is a keyframe (impure: runs ffprobe). + + Sampling the head is enough: an encode is all-intra by GOP setting, not + by content, so a mixed result means the wrong file, not a rare case. + Returns None when ffprobe cannot read the file at all. + """ + cmd = ["ffprobe", "-v", "error", "-select_streams", "v", + "-show_entries", "frame=key_frame", "-of", "csv=p=0", + "-read_intervals", f"%{probe_seconds}", str(path)] + try: + proc = subprocess.run(cmd, capture_output=True, text=True) + except FileNotFoundError: + return None + if proc.returncode != 0: + return None + flags = [ln.strip().rstrip(",") for ln in proc.stdout.splitlines() + if ln.strip()] + if not flags: + return None + return all(f == "1" for f in flags) + + +def main(argv=None): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("edl", help="path to cut/edl.json") + ap.add_argument("-o", "--output", required=True, + help="where to write the ffconcat playlist") + ap.add_argument("--mpv", default=None, + help="also write an mpv EDL here") + ap.add_argument("--source", default=None, + help="override the source every segment points at") + ap.add_argument("--require-intra", action="store_true", default=True, + help="refuse a source that is not all-intra (default)") + ap.add_argument("--allow-long-gop", dest="require_intra", + action="store_false", + help="opt out of the all-intra check; the timeline will " + "be WRONG at every segment shorter than the GOP") + args = ap.parse_args(argv) + + edl = json.loads(Path(args.edl).read_text(encoding="utf-8")) + segments = edl.get("segments") or [] + if not segments: + print("error: EDL has no segments", file=sys.stderr) + return 2 + + override = str(Path(args.source).resolve()) if args.source else None + sources = ({override} if override + else {str(Path(s["source"]).resolve()) for s in segments}) + + all_intra = True + for src in sorted(sources): + verdict = is_all_intra(src) + if verdict is None: + print(f"error: cannot probe {src}", file=sys.stderr) + return 1 + if not verdict: + all_intra = False + msg = (f"{src} is NOT all-intra. The concat demuxer can only cut " + f"on keyframes, so this timeline would silently play the " + f"WRONG frames while still reporting the right duration.") + if args.require_intra: + print(f"error: {msg}\nBuild an all-intra proxy first, or pass " + f"--allow-long-gop if you accept a wrong timeline.", + file=sys.stderr) + return 1 + print(f"warning: {msg}", file=sys.stderr) + + # Resolve every segment's source to an absolute path so the playlist works + # from any working directory. + resolved = [dict(s, source=override or str(Path(s["source"]).resolve())) + for s in segments] + + out = Path(args.output) + out.parent.mkdir(parents=True, exist_ok=True) + out.write_text("\n".join(ffconcat_lines(resolved)) + "\n", + encoding="utf-8") + + mpv_out = None + if args.mpv: + mpv_out = Path(args.mpv) + mpv_out.parent.mkdir(parents=True, exist_ok=True) + mpv_out.write_text("\n".join(mpv_edl_lines(resolved)) + "\n", + encoding="utf-8") + + duration = sum(float(s["end"]) - float(s["start"]) for s in segments) + print(json.dumps({ + "ok": True, + "segments": len(segments), + "duration_seconds": round(duration, 3), + "output": str(out), + "mpv": str(mpv_out) if mpv_out else None, + "all_intra": all_intra, + }, indent=2)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/skills/mc-cut/scripts/render_preview.py b/skills/mc-cut/scripts/render_preview.py index 26f0ba1..a990d2e 100644 --- a/skills/mc-cut/scripts/render_preview.py +++ b/skills/mc-cut/scripts/render_preview.py @@ -103,7 +103,7 @@ extract_boundary_frames = core.extract_boundary_frames -def ensure_proxies(sources, project_dir, proxy_dir, height): +def ensure_proxies(sources, project_dir, proxy_dir, height, intra=True): """Build (or reuse) a preview proxy per source. Returns {source: rel_path}. A proxy is one linear transcode of the whole source to the preview @@ -122,7 +122,7 @@ def ensure_proxies(sources, project_dir, proxy_dir, height): abs_src = project_dir / src if not abs_src.is_file(): continue - proxy = core.proxy_path(proxy_dir, src, height) + proxy = core.proxy_path(proxy_dir, src, height, intra=intra) if not core.proxy_is_fresh(proxy, abs_src): print(f"render_preview: building {height}p proxy for {src} " "(once; reused by every later preview)", file=sys.stderr) @@ -132,7 +132,8 @@ def ensure_proxies(sources, project_dir, proxy_dir, height): # hand each other a corrupt proxy. Same failure the deliverable # path had; it deserves the same defence. staged = core.temp_render_path(proxy, "proxy") - cmd = core.build_proxy_command(abs_src, staged, height) + cmd = core.build_proxy_command(abs_src, staged, height, + intra=intra) proc = subprocess.run(cmd, capture_output=True, text=True) if proc.returncode != 0: core.discard_render(staged) @@ -196,6 +197,10 @@ def main(argv=None): help="beats/beats.md to composite graphics from") parser.add_argument("--graphics-dir", default=None, help="dir holding one rendered overlay per beat id") + parser.add_argument("--proxy-only", action="store_true", + help="build (or reuse) the all-intra proxies and " + "exit without rendering. The virtual timeline " + "at gate 2 needs the proxy but not a render.") parser.add_argument("--no-proxy", action="store_true", help="cut from the masters instead of building " "preview proxies (slower on 4K sources)") @@ -268,6 +273,11 @@ def main(argv=None): else output.parent / "proxy") proxy_map = ensure_proxies(distinct, project_dir, proxy_dir, args.height) + if args.proxy_only: + print(json.dumps({"ok": True, "mode": "proxy-only", + "proxies": {k: str(v) for k, v in + proxy_map.items()}}, indent=2)) + return 0 render_edl = core.proxied_edl(edl, proxy_map) if proxy_map else edl # One target frame is needed to scale overlays and to normalize mixed-size @@ -299,7 +309,20 @@ def main(argv=None): dir=str(output.parent)) try: if not overlays: - # No graphics yet: one pass, exactly as before. + # No graphics: one pass. This path re-encodes, and that is + # deliberate. A concat-demuxer STREAM COPY off the all-intra + # proxy is ~470x faster (2.8s vs 22min on a 379-segment cut), but + # it emits duplicate DTS wherever a segment is only a frame or + # two long, and the resulting file fails this script's own decode + # validation. Flags (+genpts, avoid_negative_ts, timescale, + # fps_mode) take a 379-segment cut from 125 errors to 3, never to + # zero. Weakening the validation gate to publish it would be + # trading a real correctness check for a speed number. + # + # The speed is not lost, it just belongs elsewhere: gate 2 review + # runs on the VIRTUAL TIMELINE (edl_to_ffconcat.py), which needs + # no mux at all and is verified frame-exact. Render a file only + # when you need a file. cmd, _ = build_command(render_edl, project_dir, tmp, args.height, target=target, audio_map=audio_map) proc = subprocess.run(cmd, capture_output=True, text=True) @@ -309,10 +332,12 @@ def main(argv=None): print(" ".join(cmd), file=sys.stderr) print(proc.stderr.strip()[-2000:], file=sys.stderr) return 1 + lane_mode = "encode" else: # Overlay lanes: build the base cut, pack the overlays into the # fewest non-overlapping lanes, then stack only the lanes. Depth # is max-concurrent-overlays, not overlay count. + lane_mode = "encode" base = Path(work.name) / "base.mp4" cmd, _ = build_command(render_edl, project_dir, base, args.height, target=target, audio_map=audio_map) @@ -387,6 +412,7 @@ def main(argv=None): "validated": True, "render_key": key, "proxied_sources": len(proxy_map), + "lane": lane_mode, "boundary_frames": boundary_count, "output": str(output.resolve()), } diff --git a/skills/mc-cut/scripts/tests/test-edl_to_ffconcat.py b/skills/mc-cut/scripts/tests/test-edl_to_ffconcat.py new file mode 100644 index 0000000..ff933aa --- /dev/null +++ b/skills/mc-cut/scripts/tests/test-edl_to_ffconcat.py @@ -0,0 +1,169 @@ +#!/usr/bin/env python3 +# /// script +# requires-python = ">=3.11" +# /// +"""Tests for edl_to_ffconcat.py: the virtual timeline written from an EDL. + +The pure builders are covered directly. The all-intra guard is the one that +matters most and is covered by behaviour, because a long-GOP source produces a +timeline that is WRONG in exactly the way nothing else notices: right +duration, wrong frames. +""" + +import importlib.util +import json +import sys +import tempfile +import unittest +from pathlib import Path + +HERE = Path(__file__).resolve().parent +spec = importlib.util.spec_from_file_location( + "edl_to_ffconcat", HERE.parent / "edl_to_ffconcat.py") +mod = importlib.util.module_from_spec(spec) +spec.loader.exec_module(mod) + + +SEGS = [ + {"source": "/m.mp4", "start": 14.23, "end": 19.10}, + {"source": "/m.mp4", "start": 19.59, "end": 20.56}, + {"source": "/m.mp4", "start": 1221.433, "end": 1222.167}, +] + + +class TestFfconcatLines(unittest.TestCase): + def test_header_is_first(self): + self.assertEqual(mod.ffconcat_lines(SEGS)[0], "ffconcat version 1.0") + + def test_one_triple_per_segment(self): + lines = mod.ffconcat_lines(SEGS)[1:] + self.assertEqual(len(lines), len(SEGS) * 3) + self.assertEqual(lines[0], "file '/m.mp4'") + self.assertEqual(lines[1], "inpoint 14.230000") + self.assertEqual(lines[2], "outpoint 19.100000") + + def test_source_override_replaces_every_path(self): + lines = mod.ffconcat_lines(SEGS, "/proxy-intra.mp4") + files = [ln for ln in lines if ln.startswith("file ")] + self.assertEqual(len(files), 3) + self.assertTrue(all(ln == "file '/proxy-intra.mp4'" for ln in files)) + + def test_times_keep_sub_millisecond_precision(self): + """Truncating to 3dp would drift across hundreds of segments.""" + lines = mod.ffconcat_lines(SEGS) + self.assertIn("outpoint 1222.167000", lines) + + +class TestMpvEdlLines(unittest.TestCase): + def test_header_is_the_mpv_magic(self): + self.assertEqual(mod.mpv_edl_lines(SEGS)[0], "# mpv EDL v0") + + def test_mpv_takes_length_not_end(self): + line = mod.mpv_edl_lines(SEGS)[1] + self.assertTrue(line.endswith(",14.230000,4.870000"), + f"expected start,LENGTH; got {line}") + + def test_path_is_byte_length_quoted(self): + """%% so a comma in a filename cannot split the record.""" + line = mod.mpv_edl_lines(SEGS, "/a,b.mp4")[1] + self.assertTrue(line.startswith("%8%/a,b.mp4,"), line) + + def test_quoting_counts_bytes_not_characters(self): + """"/é.mp4" is 6 characters but 7 bytes; mpv counts bytes, and a + character count would truncate the path it reads.""" + path = "/é.mp4" + self.assertEqual(len(path), 6) + self.assertEqual(len(path.encode()), 7) + line = mod.mpv_edl_lines(SEGS, path)[1] + self.assertTrue(line.startswith("%7%"), line) + + +class TestGuardBehaviour(unittest.TestCase): + """The all-intra guard, exercised through main().""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.d = Path(self.tmp.name) + self.edl = self.d / "edl.json" + self.edl.write_text(json.dumps({"source": "/m.mp4", "segments": SEGS})) + self.out = self.d / "t.ffconcat" + + def tearDown(self): + self.tmp.cleanup() + + def _run(self, argv, intra): + real = mod.is_all_intra + mod.is_all_intra = lambda *a, **k: intra + try: + return mod.main(argv) + finally: + mod.is_all_intra = real + + def test_long_gop_source_is_refused_and_writes_nothing(self): + rc = self._run([str(self.edl), "-o", str(self.out), + "--source", "/m.mp4"], intra=False) + self.assertEqual(rc, 1) + self.assertFalse(self.out.exists(), + "a refused run must not leave a wrong timeline") + + def test_allow_long_gop_is_an_explicit_escape_hatch(self): + rc = self._run([str(self.edl), "-o", str(self.out), + "--source", "/m.mp4", "--allow-long-gop"], + intra=False) + self.assertEqual(rc, 0) + self.assertTrue(self.out.exists()) + + def test_intra_source_writes_the_timeline(self): + rc = self._run([str(self.edl), "-o", str(self.out), + "--source", "/m.mp4"], intra=True) + self.assertEqual(rc, 0) + body = self.out.read_text() + self.assertTrue(body.startswith("ffconcat version 1.0")) + self.assertEqual(body.count("inpoint"), 3) + + def test_unprobeable_source_is_an_error_not_a_pass(self): + real = mod.is_all_intra + mod.is_all_intra = lambda *a, **k: None + try: + rc = mod.main([str(self.edl), "-o", str(self.out), + "--source", "/m.mp4"]) + finally: + mod.is_all_intra = real + self.assertEqual(rc, 1) + + def test_empty_edl_is_a_usage_error(self): + empty = self.d / "empty.json" + empty.write_text(json.dumps({"source": "/m.mp4", "segments": []})) + self.assertEqual(mod.main([str(empty), "-o", str(self.out)]), 2) + + +class TestIsAllIntraParsing(unittest.TestCase): + """is_all_intra reads ffprobe's key_frame column; the shapes it must + survive are csv rows with trailing commas and blank lines.""" + + def _with_stdout(self, text, rc=0): + class P: + returncode = rc + stdout = text + real = mod.subprocess.run + mod.subprocess.run = lambda *a, **k: P() + try: + return mod.is_all_intra("/x.mp4") + finally: + mod.subprocess.run = real + + def test_all_ones_is_intra(self): + self.assertTrue(self._with_stdout("1,\n1,\n1,\n")) + + def test_any_zero_is_not_intra(self): + self.assertFalse(self._with_stdout("1,\n0,\n1,\n")) + + def test_no_frames_is_unknown(self): + self.assertIsNone(self._with_stdout("\n")) + + def test_ffprobe_failure_is_unknown(self): + self.assertIsNone(self._with_stdout("1,\n", rc=1)) + + +if __name__ == "__main__": + unittest.main(verbosity=1) diff --git a/skills/mc-cut/scripts/tests/test-render_preview.py b/skills/mc-cut/scripts/tests/test-render_preview.py index a5157c8..f38f047 100644 --- a/skills/mc-cut/scripts/tests/test-render_preview.py +++ b/skills/mc-cut/scripts/tests/test-render_preview.py @@ -433,14 +433,57 @@ def test_composite_with_no_lanes_is_a_passthrough_graph(self): class TestProxies(unittest.TestCase): - def test_proxy_path_is_named_by_stem_and_height(self): + def test_proxy_path_is_named_by_stem_height_and_family(self): p = core.proxy_path("/p/renders/proxy", "raw/cam.mp4", 720) - self.assertEqual(p.name, "cam-720p.mp4") + self.assertEqual(p.name, "cam-720p-intra.mp4") + + def test_long_gop_proxy_gets_a_distinct_path(self): + """An all-intra proxy must never collide with a long-GOP one: the + stream-copy lane is silently wrong against long-GOP.""" + intra = core.proxy_path("/p", "raw/cam.mp4", 720, intra=True) + longgop = core.proxy_path("/p", "raw/cam.mp4", 720, intra=False) + self.assertNotEqual(intra.name, longgop.name) def test_proxy_command_scales_to_the_height(self): cmd = core.build_proxy_command("in.mp4", "out.mp4", 720) self.assertIn("scale=-2:720", cmd) + def test_proxy_command_is_all_intra_by_default(self): + """-g 1 alone is not enough: x264 scenecut detection would still + insert its own keyframes and keyint_min governs the minimum.""" + cmd = core.build_proxy_command("in.mp4", "out.mp4", 720) + for flag, val in (("-g", "1"), ("-keyint_min", "1"), + ("-sc_threshold", "0")): + self.assertIn(flag, cmd) + self.assertEqual(cmd[cmd.index(flag) + 1], val) + + def test_proxy_command_can_opt_out_of_intra(self): + cmd = core.build_proxy_command("in.mp4", "out.mp4", 720, intra=False) + self.assertNotIn("-sc_threshold", cmd) + + def test_streamcopy_command_never_encodes(self): + cmd = core.build_streamcopy_command("t.ffconcat", "o.mp4") + self.assertIn("-c", cmd) + self.assertEqual(cmd[cmd.index("-c") + 1], "copy") + self.assertNotIn("-filter_complex", cmd) + self.assertIn("concat", cmd) + + def test_a_proxy_from_an_older_recipe_is_stale(self): + """A sidecar with no recipe line predates the intra lane. Reusing it + would feed the stream-copy lane a long-GOP proxy: right duration, + wrong frames.""" + import tempfile + from pathlib import Path as _P + with tempfile.TemporaryDirectory() as d: + src = _P(d) / "s.mp4"; src.write_bytes(b"x" * 64) + proxy = _P(d) / "p.mp4"; proxy.write_bytes(b"y" * 64) + # old-format sidecar: digest only, no recipe + proxy.with_name("p.mp4.src").write_text( + core.content_digest(src, cheap=True) + "\n") + self.assertFalse(core.proxy_is_fresh(proxy, src)) + core.write_proxy_sidecar(proxy, src) + self.assertTrue(core.proxy_is_fresh(proxy, src)) + def test_missing_proxy_is_not_fresh(self): self.assertFalse(core.proxy_is_fresh("/nope/p.mp4", "/nope/s.mp4")) @@ -566,7 +609,7 @@ def test_duration_matches_the_edl(self): def test_a_proxy_was_built_and_is_reused(self): self.assertEqual(self.summary["proxied_sources"], 1) self.assertTrue((self.proj / "renders" / "proxy" / - "cam-180p.mp4").is_file()) + "cam-180p-intra.mp4").is_file()) def test_audio_survives_the_lane_composite(self): self.assertTrue(core.probe_has_audio(self.out))