From 1d2aa5240d2126ca2763eeffd1bd87c69f2b772d Mon Sep 17 00:00:00 2001 From: Leonel Rivas Date: Mon, 12 Oct 2026 17:23:05 -0700 Subject: [PATCH] fix(media-use): transcript-cut video renders without repeated frames or a keyframe warning (#5463) * fix(media-use): transcript-cut video renders without repeated frames or a keyframe warning transcript-cut wrote each kept range to a temporary .mkv, and ffmpeg writes Matroska timestamps in whole milliseconds. A 30 fps frame that belongs at 2/30 s ended up at 0.067, the final copy kept it there, and the renderer showed the previous frame wherever a timestamp was late. The temporary files are now .mov, which holds the frame times exactly and still carries PCM audio, so the audio is encoded once, as before. The cut also gets a keyframe every second. x264 on its own left them several seconds apart, and the renderer warned about the footage this script had just written. Fixes #5417 Co-Authored-By: Claude Opus 5.5 * fix(media-use): transcript-cut ranges start on their first frame with any ffmpeg ffmpeg 7 and later start a kept range that begins between two source frames one frame late, and .mov rounds that lead-in to the millisecond, which put the frames off the grid again. ffmpeg 6 did not do this, so the tests passed locally and failed in CI. Each temporary file is now written at a constant frame rate, so the first frame of a range is at zero whichever ffmpeg runs it. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: James --- skills-manifest.json | 2 +- skills/media-use/scripts/transcript-cut.mjs | 14 +- .../media-use/scripts/transcript-cut.test.mjs | 149 ++++++++++++++++++ 3 files changed, 162 insertions(+), 3 deletions(-) diff --git a/skills-manifest.json b/skills-manifest.json index c62c5e4af4..9d1f272ba0 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -54,7 +54,7 @@ "files": 1 }, "media-use": { - "hash": "4b6ec7ec0ddaf32e", + "hash": "dd6a1116d3d01906", "files": 112 }, "motion-graphics": { diff --git a/skills/media-use/scripts/transcript-cut.mjs b/skills/media-use/scripts/transcript-cut.mjs index 1f232d6ae8..eb0bcbfaf0 100644 --- a/skills/media-use/scripts/transcript-cut.mjs +++ b/skills/media-use/scripts/transcript-cut.mjs @@ -98,7 +98,10 @@ function run() { // with priming silence (~25-35ms for AAC), which concat then bakes in as a // gap at each cut -- a defect distinct from, and surviving, the fades // below. PCM has no priming, so audio is encoded exactly once, at concat. - const ext = args.copy ? extname(outPath) || ".mp4" : isAudioOnly(outPath) ? ".wav" : ".mkv"; + // The video container is .mov, not .mkv: ffmpeg writes Matroska + // timestamps in whole milliseconds, which moves frames off the source's + // frame grid, and the concat copy below carries that into the output. + const ext = args.copy ? extname(outPath) || ".mp4" : isAudioOnly(outPath) ? ".wav" : ".mov"; const out = join(tmpDir, `segment-${String(index).padStart(4, "0")}${ext}`); // --copy stays fade-free (stream copy cannot filter). A segment's true // start/end (index 0's start, the last segment's end) borders nothing @@ -191,12 +194,19 @@ function cutSegment(inputPath, segment, outPath, copy, fade) { ]; if (copy) { argv.push("-c", "copy", "-avoid_negative_ts", "make_zero"); - } else if (extname(outPath).toLowerCase() === ".mkv") { + } else if (extname(outPath).toLowerCase() === ".mov") { // Concat splices raw segment edges together; without a short ramp the // waveform steps discontinuously at every boundary and you hear a click. if (fade) argv.push("-af", fade); // Video intermediate: keep the picture cheap and the audio uncompressed. argv.push("-c:v", "libx264", "-preset", "veryfast", "-crf", "18", "-c:a", "pcm_s16le"); + // A keyframe every second. The renderer seeks into this footage, and x264 + // on its own leaves up to 250 frames between keyframes. + argv.push("-force_key_frames", "expr:gte(t,n_forced)"); + // Constant frame rate from the first frame. Left to itself, ffmpeg 7 starts + // a range that begins between two source frames one frame late, and .mov + // rounds that lead-in to the millisecond. + argv.push("-fps_mode", "cfr"); } else { if (fade) argv.push("-af", fade); argv.push("-c:a", "pcm_s16le"); diff --git a/skills/media-use/scripts/transcript-cut.test.mjs b/skills/media-use/scripts/transcript-cut.test.mjs index 59a4d55c96..95852adb82 100644 --- a/skills/media-use/scripts/transcript-cut.test.mjs +++ b/skills/media-use/scripts/transcript-cut.test.mjs @@ -47,6 +47,155 @@ function readPcm(filePath) { return samples; } +// Three kept ranges, none starting or ending on a frame boundary of any rate below. +const UNALIGNED_KEEP = "0.137-1.913,2.845-4.111,4.9-5.47"; +const UNALIGNED_KEPT_SECONDS = 1.776 + 1.266 + 0.57; + +function cutVideo(t, rate, keep = UNALIGNED_KEEP) { + const { dir, cleanup } = fixture(); + t.after(cleanup); + + const source = join(dir, "source.mp4"); + execFileSync("ffmpeg", [ + "-y", + "-hide_banner", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + `testsrc2=size=160x90:rate=${rate}:duration=6`, + "-f", + "lavfi", + "-i", + `sine=frequency=440:duration=6:sample_rate=${SAMPLE_RATE}`, + "-c:v", + "libx264", + "-preset", + "ultrafast", + "-pix_fmt", + "yuv420p", + "-c:a", + "aac", + source, + ]); + + const transcriptPath = join(dir, "transcript.json"); + writeFileSync(transcriptPath, JSON.stringify({ words: [{ text: "tone", start: 0, end: 6 }] })); + + const output = join(dir, "out.mp4"); + const result = run([ + "--input", + source, + "--transcript", + transcriptPath, + "--keep", + keep, + "--out", + output, + "--json", + ]); + assert.equal(result.status, 0, result.stderr || result.stdout); + return output; +} + +function readVideoTimestamps(filePath) { + const probe = JSON.parse( + execFileSync( + "ffprobe", + [ + "-v", + "error", + "-select_streams", + "v:0", + "-show_entries", + "stream=time_base:packet=pts", + "-of", + "json", + "--", + filePath, + ], + { encoding: "utf8" }, + ), + ); + const [tickNum, tickDen] = probe.streams[0].time_base.split("/").map(Number); + // Packets come in decode order; B-frames put that out of presentation order. + const pts = probe.packets.map((packet) => Number(packet.pts)).sort((a, b) => a - b); + return { tickNum, tickDen, pts }; +} + +for (const rate of ["30", "30000/1001", "60"]) { + test( + `puts every frame of a ${rate} fps video cut on the frame grid, with no gaps`, + { skip: !HAS_FFMPEG }, + (t) => { + const [rateNum, rateDen = 1] = rate.split("/").map(Number); + const { tickNum, tickDen, pts } = readVideoTimestamps(cutVideo(t, rate)); + assert.ok(pts.length > rateNum / rateDen, "expected several seconds of video"); + + // Frame i belongs at exactly i / rate seconds. Compared as integers, so a + // timestamp rounded to the millisecond (0.067 for 2/30) cannot pass. + const offGrid = pts.filter( + (value, index) => value * tickNum * rateNum !== index * rateDen * tickDen, + ); + assert.equal( + offGrid.length, + 0, + `${offGrid.length} of ${pts.length} frames are off the ${rate} fps grid (timestamps in units of ${tickNum}/${tickDen} s)`, + ); + }, + ); +} + +test( + "keeps the keyframes of a video cut no more than a second apart", + { skip: !HAS_FFMPEG }, + (t) => { + // One kept range of almost four seconds, so only the encoder decides where keyframes go. + const output = cutVideo(t, "30", "0.137-4.111,4.9-5.47"); + const probe = JSON.parse( + execFileSync( + "ffprobe", + [ + "-v", + "error", + "-select_streams", + "v:0", + "-show_entries", + "packet=pts_time,flags", + "-of", + "json", + "--", + output, + ], + { encoding: "utf8" }, + ), + ); + const keyframes = probe.packets + .filter((packet) => packet.flags.startsWith("K")) + .map((packet) => Number(packet.pts_time)) + .sort((a, b) => a - b); + const gaps = keyframes.slice(1).map((time, index) => time - keyframes[index]); + const widest = Math.max(...gaps); + // A frame of slack: the forced keyframe is the first frame at or after each second. + assert.ok(widest < 1.05, `keyframes are up to ${widest.toFixed(3)}s apart`); + }, +); + +test( + "does not pad the audio of a video cut with encoder priming at each splice", + { skip: !HAS_FFMPEG }, + (t) => { + const seconds = readPcm(cutVideo(t, "30")).length / SAMPLE_RATE; + // The single AAC encode may round the end up by a frame or two (23 ms each). + // Encoding every segment separately instead adds about 25 ms per splice on top. + assert.ok( + Math.abs(seconds - UNALIGNED_KEPT_SECONDS) < 0.05, + `audio runs ${seconds.toFixed(3)}s for ${UNALIGNED_KEPT_SECONDS.toFixed(3)}s kept`, + ); + }, +); + test( "keeps a spliced tone continuous at every cut, with no raw phase jump or silence gap", { skip: !HAS_FFMPEG },