diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0502cd4865..81fd4e1349 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -483,6 +483,7 @@ jobs: - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22 + - uses: ./.github/actions/install-ffmpeg-linux - name: Discover and run skills tests # We expand the test list via bash so the job fails loudly when the # matcher comes back empty, rather than silently no-op'ing (which diff --git a/packages/cli/src/media-use/lib/transcriptCutFade.mjs b/packages/cli/src/media-use/lib/transcriptCutFade.mjs new file mode 100644 index 0000000000..bc1676074a --- /dev/null +++ b/packages/cli/src/media-use/lib/transcriptCutFade.mjs @@ -0,0 +1,21 @@ +function round3(n) { + return Math.round(Number(n) * 1000) / 1000; +} + +// 30ms in/out ramps kill the click at every concat boundary. A segment shorter +// than 4x the ramp would spend its whole length fading, so scale down there and +// skip entirely on a degenerate one. fadeIn/fadeOut are false at the export's +// own true start/end, where there is no splice to smooth. +export function fadeFilterFor(durationSeconds, { fadeIn, fadeOut }) { + if (!fadeIn && !fadeOut) return null; + const FADE_SECONDS = 0.03; + if (!Number.isFinite(durationSeconds) || durationSeconds <= 0.01) return null; + const d = Math.min(FADE_SECONDS, durationSeconds / 4); + const parts = []; + if (fadeIn) parts.push(`afade=t=in:st=0:d=${round3(d)}`); + if (fadeOut) { + const out = round3(durationSeconds - d); + if (out > 0) parts.push(`afade=t=out:st=${out}:d=${round3(d)}`); + } + return parts.length ? parts.join(",") : null; +} diff --git a/packages/cli/src/media-use/lib/transcriptCutFade.test.mjs b/packages/cli/src/media-use/lib/transcriptCutFade.test.mjs new file mode 100644 index 0000000000..55c97ebf08 --- /dev/null +++ b/packages/cli/src/media-use/lib/transcriptCutFade.test.mjs @@ -0,0 +1,33 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { fadeFilterFor } from "./transcriptCutFade.mjs"; + +test("fades in only at the first segment's trailing splice", () => { + assert.equal(fadeFilterFor(2, { fadeIn: false, fadeOut: true }), "afade=t=out:st=1.97:d=0.03"); +}); + +test("fades both edges of a middle segment's two splices", () => { + assert.equal( + fadeFilterFor(2, { fadeIn: true, fadeOut: true }), + "afade=t=in:st=0:d=0.03,afade=t=out:st=1.97:d=0.03", + ); +}); + +test("fades out only at the last segment's leading splice", () => { + assert.equal(fadeFilterFor(2, { fadeIn: true, fadeOut: false }), "afade=t=in:st=0:d=0.03"); +}); + +test("a single kept segment borders no splice at all", () => { + assert.equal(fadeFilterFor(2, { fadeIn: false, fadeOut: false }), null); +}); + +test("still scales the ramp down on a short segment with both edges faded", () => { + assert.equal( + fadeFilterFor(0.08, { fadeIn: true, fadeOut: true }), + "afade=t=in:st=0:d=0.02,afade=t=out:st=0.06:d=0.02", + ); +}); + +test("skips a degenerate segment even when a splice is requested", () => { + assert.equal(fadeFilterFor(0.005, { fadeIn: true, fadeOut: true }), null); +}); diff --git a/scripts/check-media-use-copy-parity.test.mjs b/scripts/check-media-use-copy-parity.test.mjs index a5bedac0c5..8ebab828d0 100644 --- a/scripts/check-media-use-copy-parity.test.mjs +++ b/scripts/check-media-use-copy-parity.test.mjs @@ -19,6 +19,7 @@ export const MEDIA_USE_COPY_NAMES = [ "prefs-store.mjs", "recipe-store.mjs", "telemetry.mjs", + "transcriptCutFade.mjs", "words.mjs", ]; diff --git a/skills-manifest.json b/skills-manifest.json index 0f5f97ef2a..f0ed394bc9 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -54,8 +54,8 @@ "files": 1 }, "media-use": { - "hash": "e5376453c0d08ac3", - "files": 88 + "hash": "7263711f70b299d6", + "files": 90 }, "motion-graphics": { "hash": "9c2ca2e8c384875d", diff --git a/skills/media-use/scripts/lib/transcriptCutFade.mjs b/skills/media-use/scripts/lib/transcriptCutFade.mjs new file mode 100644 index 0000000000..bc1676074a --- /dev/null +++ b/skills/media-use/scripts/lib/transcriptCutFade.mjs @@ -0,0 +1,21 @@ +function round3(n) { + return Math.round(Number(n) * 1000) / 1000; +} + +// 30ms in/out ramps kill the click at every concat boundary. A segment shorter +// than 4x the ramp would spend its whole length fading, so scale down there and +// skip entirely on a degenerate one. fadeIn/fadeOut are false at the export's +// own true start/end, where there is no splice to smooth. +export function fadeFilterFor(durationSeconds, { fadeIn, fadeOut }) { + if (!fadeIn && !fadeOut) return null; + const FADE_SECONDS = 0.03; + if (!Number.isFinite(durationSeconds) || durationSeconds <= 0.01) return null; + const d = Math.min(FADE_SECONDS, durationSeconds / 4); + const parts = []; + if (fadeIn) parts.push(`afade=t=in:st=0:d=${round3(d)}`); + if (fadeOut) { + const out = round3(durationSeconds - d); + if (out > 0) parts.push(`afade=t=out:st=${out}:d=${round3(d)}`); + } + return parts.length ? parts.join(",") : null; +} diff --git a/skills/media-use/scripts/transcript-cut.mjs b/skills/media-use/scripts/transcript-cut.mjs index d76bd022fb..b7e4f468e0 100644 --- a/skills/media-use/scripts/transcript-cut.mjs +++ b/skills/media-use/scripts/transcript-cut.mjs @@ -6,6 +6,7 @@ import { tmpdir } from "node:os"; import { dirname, extname, join, resolve } from "node:path"; import { parseArgs } from "node:util"; import { compileCutList } from "./lib/cutlist.mjs"; +import { fadeFilterFor } from "./lib/transcriptCutFade.mjs"; import { track } from "./lib/telemetry.mjs"; const { values: args } = parseArgs({ @@ -92,11 +93,23 @@ function run() { try { const parts = segments.map((segment, index) => { - const out = join( - tmpDir, - `segment-${String(index).padStart(4, "0")}${extname(outPath) || ".mp4"}`, - ); - cutSegment(inputPath, segment, out, Boolean(args.copy)); + // Intermediates carry PCM audio, not the final codec. Encoding each + // segment to a lossy codec separately makes the encoder pad every segment + // 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"; + 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 + // kept, so only an interior splice edge gets a ramp. + const fade = args.copy + ? null + : fadeFilterFor(segment.end - segment.start, { + fadeIn: index > 0, + fadeOut: index < segments.length - 1, + }); + cutSegment(inputPath, segment, out, args.copy, fade); return out; }); const listPath = join(tmpDir, "list.txt"); @@ -107,9 +120,16 @@ function run() { // Encode to a sibling temp (same extension so ffmpeg picks the right muxer), // then atomic-rename so a SIGKILL mid-encode can't leave a truncated outPath. const tmpOut = `${outPath}.part${extname(outPath) || ".mp4"}`; + // --copy already produced final-codec segments, so concat can stream-copy. + // Otherwise the PCM intermediates are encoded here, once, for the whole file. + const concatCodecs = args.copy + ? ["-c", "copy"] + : isAudioOnly(outPath) + ? encodeArgsFor(extname(outPath).toLowerCase()) + : ["-c:v", "copy", "-c:a", "aac", "-b:a", "192k", "-movflags", "+faststart"]; execFileSync( "ffmpeg", - ["-y", "-f", "concat", "-safe", "0", "-i", listPath, "-c", "copy", tmpOut], + ["-y", "-f", "concat", "-safe", "0", "-i", listPath, ...concatCodecs, tmpOut], { stdio: "ignore", }, @@ -158,7 +178,7 @@ function run() { console.log(`next: resolve --from ${outPath} --type `); } -function cutSegment(inputPath, segment, outPath, copy) { +function cutSegment(inputPath, segment, outPath, copy, fade) { const argv = [ "-y", "-nostdin", @@ -171,8 +191,15 @@ function cutSegment(inputPath, segment, outPath, copy) { ]; if (copy) { argv.push("-c", "copy", "-avoid_negative_ts", "make_zero"); + } else if (extname(outPath).toLowerCase() === ".mkv") { + // 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"); } else { - argv.push(...encodeArgsFor(extname(outPath).toLowerCase())); + if (fade) argv.push("-af", fade); + argv.push("-c:a", "pcm_s16le"); } argv.push(outPath); execFileSync("ffmpeg", argv, { stdio: "ignore" }); @@ -180,6 +207,10 @@ function cutSegment(inputPath, segment, outPath, copy) { // Codec set per output container. Audio-only outputs must not get the // video-centric aac/x264 set (aac inside .wav breaks timing entirely). +function isAudioOnly(filePath) { + return [".wav", ".mp3", ".m4a", ".aac", ".flac"].includes(extname(filePath).toLowerCase()); +} + function encodeArgsFor(ext) { if (ext === ".wav") return ["-c:a", "pcm_s16le"]; if (ext === ".mp3") return ["-c:a", "libmp3lame", "-q:a", "2"]; diff --git a/skills/media-use/scripts/transcript-cut.test.mjs b/skills/media-use/scripts/transcript-cut.test.mjs new file mode 100644 index 0000000000..99b52dce42 --- /dev/null +++ b/skills/media-use/scripts/transcript-cut.test.mjs @@ -0,0 +1,117 @@ +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +const SCRIPT = fileURLToPath(new URL("./transcript-cut.mjs", import.meta.url)); +const HAS_FFMPEG = + spawnSync("ffmpeg", ["-version"], { stdio: "ignore" }).status === 0 && + spawnSync("ffprobe", ["-version"], { stdio: "ignore" }).status === 0; + +const SAMPLE_RATE = 44100; +// A continuous tone's own slope between samples never exceeds amplitude * 2*pi*f/rate +// (~2054 for 440Hz at full scale here); a raw splice between two independently-cut +// points on the same tone lands at two unrelated phases, so an unfaded join jumps far +// past that. This threshold sits well above normal tone motion and well below a splice. +const MAX_CONTINUOUS_STEP = 6000; +const MIN_SUSPICIOUS_ZERO_RUN = 20; + +function fixture() { + const dir = mkdtempSync(join(tmpdir(), "media-use-transcript-cut-")); + return { dir, cleanup: () => rmSync(dir, { recursive: true, force: true }) }; +} + +function run(args) { + return spawnSync(process.execPath, [SCRIPT, ...args], { encoding: "utf8" }); +} + +function readPcm(filePath) { + const raw = execFileSync("ffmpeg", [ + "-hide_banner", + "-loglevel", + "error", + "-i", + filePath, + "-f", + "s16le", + "-ac", + "1", + "-ar", + String(SAMPLE_RATE), + "-", + ]); + const samples = new Int16Array(raw.buffer, raw.byteOffset, raw.length / 2); + return samples; +} + +test( + "keeps a spliced tone continuous at every cut, with no raw phase jump or silence gap", + { skip: !HAS_FFMPEG }, + (t) => { + const { dir, cleanup } = fixture(); + t.after(cleanup); + + const source = join(dir, "tone.wav"); + execFileSync("ffmpeg", [ + "-y", + "-hide_banner", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + `sine=frequency=440:duration=6:sample_rate=${SAMPLE_RATE}`, + source, + ]); + + const transcriptPath = join(dir, "transcript.json"); + writeFileSync(transcriptPath, JSON.stringify({ words: [{ text: "tone", start: 0, end: 6 }] })); + + const output = join(dir, "out.wav"); + // Two "ugly" removal ranges: neither aligned to the 440Hz period, so the kept + // segments' cut edges land at unrelated phases of the same continuous tone -- + // exactly the shape that clicks without a fade, and the shape jrusso1020's + // review proved this branch never actually fades. + const result = run([ + "--input", + source, + "--transcript", + transcriptPath, + "--remove", + "1.37-1.83,3.29-3.71", + "--out", + output, + "--json", + ]); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const samples = readPcm(output); + assert.ok(samples.length > SAMPLE_RATE, "expected several seconds of audio"); + + let maxStep = 0; + let zeroRun = 0; + let maxZeroRun = 0; + for (let i = 1; i < samples.length; i++) { + const step = Math.abs(samples[i] - samples[i - 1]); + if (step > maxStep) maxStep = step; + if (samples[i] === 0) { + zeroRun++; + if (zeroRun > maxZeroRun) maxZeroRun = zeroRun; + } else { + zeroRun = 0; + } + } + + assert.ok( + maxStep <= MAX_CONTINUOUS_STEP, + `largest sample-to-sample step was ${maxStep}, expected <= ${MAX_CONTINUOUS_STEP} (a raw, unfaded splice)`, + ); + assert.ok( + maxZeroRun < MIN_SUSPICIOUS_ZERO_RUN, + `found a run of ${maxZeroRun} consecutive zero samples, expected < ${MIN_SUSPICIOUS_ZERO_RUN} (a priming-silence gap)`, + ); + }, +);