From 4ffddd16157a5bd8497117fece8affd7e09daae6 Mon Sep 17 00:00:00 2001 From: isamu Date: Tue, 12 May 2026 04:42:59 +0900 Subject: [PATCH] fix: cherry-pick four upstream bug fixes (scientific notation / loglevel prefix / filenames copy / codecData init) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picks up four small, self-contained bug fixes from upstream fluent-ffmpeg open PRs that the original maintainer hasn't merged. Each addresses a real-world failure mode that this fork inherited verbatim, has a narrow patch surface, and is shipped with a regression test that pins the upstream issue number in its comment. ### upstream #1131 — scientific notation in seek / duration args Calling `seekInput(1e-7)`, `duration(1e-7)`, `seekOutput(...)` or `setDuration(...)` with a sufficiently small (or large) finite number passed the value through `String(n)`, which falls back to scientific notation (`'1e-7'`). ffmpeg's CLI parser rejects that form. Numeric args now flow through a new `utils.formatNumberForCall(n)` helper built on `Intl.NumberFormat('en-US', { useGrouping: false, maximumFractionDigits: 10 })` so the argv token is always a fixed-point string. De facto behaviour change: the internal option list stores these args as `string` instead of preserving the input `number`; the eventual spawn argv is unchanged (both forms become `'10'` at spawn time via String()). Five existing tests that asserted on the element type were updated to the new `string` shape. ### upstream #928 — `-loglevel +level` blocks the `progress` event When the user added `-loglevel +level` to the ffmpeg command, every stderr line gained a `[info]` / `[warning]` / `[error]` prefix. The old `parseProgressLine` required every space-separated token to look like `key=value`; the leading `[info]` has no `=` so the entire line was rejected and no `progress` event ever fired. Strip a leading `[]` prefix before key=value splitting. ### upstream #1017 — `filenames` event leaks internal array The screenshot pipeline emitted `this.emit('filenames', filenames)` where `filenames` was the same array reference the pipeline kept using to drive `output(...)` / `seek(...)` calls. A consumer that stored the array (or appended to it) corrupted the in-flight screenshot generation. Emit a defensive `[...filenames]` copy. ### upstream #1301 — codecData.audio_details / video_details undefined for single-stream inputs For an input with only an audio stream (or only a video stream), the counterpart `*_details` field on the `codecData` event payload was `undefined`. Consumers destructuring `audio_details.length` or `video_details[0]` crashed. Initialise both arrays to `[]` at `Input #N` time; the populated side is replaced when the matching `Stream #N:M: Audio:` / `Video:` line arrives. ## Verification - yarn format / lint / build / typecheck: clean - yarn test: 502 passing / 0 failing / 9 skipped (492 before; ten new regression tests cover the four fixes — five for the number formatter, three for the loglevel prefix, two for the codecData init.) The filenames defensive-copy change has no targeted unit test because the existing screenshot-pipeline tests already exercise the emit codepath; the one-line spread is self-evidently defensive and any test that previously depended on receiving the same reference would have surfaced. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 7 +++ lib/options/inputs.ts | 6 ++- lib/options/output.ts | 6 ++- lib/recipes.ts | 6 ++- lib/utils.ts | 32 ++++++++++- test/args.test.ts | 4 +- test/duration-input.test.ts | 14 ++--- test/utils.test.ts | 104 +++++++++++++++++++++++++++++++++++- 8 files changed, 163 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8cab6e3..daa2ea9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to `@modernized/fluent-ffmpeg` are documented here. Format f ## [Unreleased] +### Fixed + +- **Scientific-notation in `seekInput` / `setStartTime` / `durationInput` / `setInputDuration` / `seek` / `seekOutput` / `duration` / `setDuration` / `withDuration`.** A numeric argument that JavaScript stringifies to scientific notation (e.g. `duration(1e-7)` → `'1e-7'`) was passed verbatim to ffmpeg, which rejects the form. Numeric inputs now flow through a new `utils.formatNumberForCall()` helper (built on `Intl.NumberFormat('en-US', { useGrouping: false, maximumFractionDigits: 10 })`) so the argv token is always a fixed-point string. Mirrors upstream [fluent-ffmpeg#1131](https://github.com/fluent-ffmpeg/node-fluent-ffmpeg/pull/1131). **De facto behaviour change**: the internal option list now always stores these args as `string` (e.g. `['-t', '10']`) instead of preserving the input `number` (`['-t', 10]`). The eventual ffmpeg argv is unchanged (both forms become `'10'` at spawn time); only consumers that introspect `cmd._inputs[i].options.get()` or `cmd._getArguments()` and assert on the element type see the difference. +- **`-loglevel +level` blocked the `progress` event.** When the user added `-loglevel +level`, every stderr line gained a `[info]` / `[warning]` / `[error]` prefix and `parseProgressLine` rejected the whole line (the leading bracketed token has no `=`). The parser now strips a leading `[]` prefix before key=value splitting, so `progress` events fire under that loglevel too. Mirrors upstream [fluent-ffmpeg#928](https://github.com/fluent-ffmpeg/node-fluent-ffmpeg/pull/928). +- **`filenames` event leaked the internal screenshot-pipeline array.** A consumer that stored or mutated the array received from `cmd.on('filenames', ...)` corrupted the in-flight screenshot generation. Emit a defensive `[...filenames]` copy. Mirrors upstream [fluent-ffmpeg#1017](https://github.com/fluent-ffmpeg/node-fluent-ffmpeg/pull/1017). +- **`codecData.audio_details` / `video_details` were `undefined` for inputs missing the corresponding stream.** Consumers destructuring `audio_details.length` (or similar) on an audio-only / video-only input crashed. Both arrays are now initialised to `[]` at `Input #N` time and replaced when their `Stream #N:M:` line matches. Mirrors upstream [fluent-ffmpeg#1301](https://github.com/fluent-ffmpeg/node-fluent-ffmpeg/pull/1301). + ## [0.1.4] - 2026-05-11 ### Fixed diff --git a/lib/options/inputs.ts b/lib/options/inputs.ts index 43779462..aa3c1c7e 100644 --- a/lib/options/inputs.ts +++ b/lib/options/inputs.ts @@ -87,7 +87,8 @@ function applyInputsOptions(proto: FfmpegCommandPrototype): void { if (!this._currentInput) { throw new Error('No input specified'); } - this._currentInput.options('-ss', seek); + const value = typeof seek === 'number' ? utils.formatNumberForCall(seek) : seek; + this._currentInput.options('-ss', value); return this; }; @@ -102,7 +103,8 @@ function applyInputsOptions(proto: FfmpegCommandPrototype): void { if (!this._currentInput) { throw new Error('No input specified'); } - this._currentInput.options('-t', duration); + const value = typeof duration === 'number' ? utils.formatNumberForCall(duration) : duration; + this._currentInput.options('-t', value); return this; }; diff --git a/lib/options/output.ts b/lib/options/output.ts index daf5f837..5f665e5f 100644 --- a/lib/options/output.ts +++ b/lib/options/output.ts @@ -78,7 +78,8 @@ function applyOutputOptions(proto: FfmpegCommandPrototype): void { }; proto.seekOutput = proto.seek = function (this: FfmpegCommandThis, seek: string | number) { - this._currentOutput!.options('-ss', seek); + const value = typeof seek === 'number' ? utils.formatNumberForCall(seek) : seek; + this._currentOutput!.options('-ss', value); return this; }; @@ -86,7 +87,8 @@ function applyOutputOptions(proto: FfmpegCommandPrototype): void { proto.setDuration = proto.duration = function (this: FfmpegCommandThis, duration: string | number) { - this._currentOutput!.options('-t', duration); + const value = typeof duration === 'number' ? utils.formatNumberForCall(duration) : duration; + this._currentOutput!.options('-t', value); return this; }; diff --git a/lib/recipes.ts b/lib/recipes.ts index 96a3a00b..741b1444 100644 --- a/lib/recipes.ts +++ b/lib/recipes.ts @@ -328,7 +328,11 @@ function applyRecipes(proto: FfmpegCommandPrototype): void { const size = await computeSizeForTokens(pattern, resolvedSize, getMetadata); pattern = replaceSizeTokens(pattern, size); const filenames = generateFilenames(pattern, config.timemarks); - this.emit('filenames', filenames); + // Defensive copy: consumers that store / mutate the emitted + // array must not affect the internal `filenames` list used to + // drive the screenshot pipeline below. Mirrors upstream + // fluent-ffmpeg#1017. + this.emit('filenames', [...filenames]); await ensureDirectory(config.folder!); return filenames; })().then( diff --git a/lib/utils.ts b/lib/utils.ts index 21c01a76..33ee8380 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -149,10 +149,16 @@ function tryStartInput(line: string, state: Required): boolean { if (!match) return false; state.inInput = true; state.inputIndex += 1; + // Initialise audio_details / video_details up-front so consumers that + // destructure them on the `codecData` event always see an array, even + // for inputs that have no audio or no video stream. Mirrors upstream + // fluent-ffmpeg#1301. state.inputStack[state.inputIndex] = { format: match[1], audio: '', + audio_details: [], video: '', + video_details: [], duration: '', }; return true; @@ -210,8 +216,15 @@ function extractCodecData( return false; } +// `-loglevel +level` prepends `[level]` (e.g. `[info]`, `[warning]`, +// `[error]`) to every stderr line. Strip it before key=value parsing +// so progress events still fire under that loglevel. Mirrors upstream +// fluent-ffmpeg#928. +const loglevelPrefixRegexp = /^\[[a-z]+\]\s*/; + function parseProgressLine(line: string): Record | null { - const trimmed = line.replace(/=\s+/g, '=').trim(); + const stripped = line.replace(loglevelPrefixRegexp, ''); + const trimmed = stripped.replace(/=\s+/g, '=').trim(); const progress: Record = {}; const allValid = trimmed.split(' ').every((part) => { const [key, value] = part.split('=', 2); @@ -222,6 +235,22 @@ function parseProgressLine(line: string): Record | null { return allValid ? progress : null; } +// Avoid scientific notation when forwarding numeric seek / duration +// arguments to ffmpeg — ffmpeg's CLI rejects values like `1e-7`. +// Plain-number formatting with up to 10 fractional digits matches +// upstream fluent-ffmpeg#1131 (covers high-sample-rate audio / +// frame-accurate seeks without losing precision). +const FF_NUMBER_MAX_FRACTION_DIGITS = 10; +const flatNumberFormatter = new Intl.NumberFormat('en-US', { + useGrouping: false, + maximumFractionDigits: FF_NUMBER_MAX_FRACTION_DIGITS, +}); + +function formatNumberForCall(value: number): string { + if (!Number.isFinite(value)) return String(value); + return flatNumberFormatter.format(value); +} + // Coerce ffmpeg's progress-line numeric fields to a finite number. // Modern ffmpeg emits `bitrate=N/A`, `size=N/A`, etc. for several common // scenarios (output to pipe before first frame, copy codec, fragmented mp4, @@ -333,6 +362,7 @@ const utils = { makeFilterStrings, which: whichCached, timemarkToSeconds, + formatNumberForCall, extractCodecData, extractProgress, extractError, diff --git a/test/args.test.ts b/test/args.test.ts index 4e5128fa..629bf12b 100644 --- a/test/args.test.ts +++ b/test/args.test.ts @@ -375,7 +375,7 @@ describe('Command', () => { assert.ok(!err); assert.ok(args!.indexOf('-loop') !== -1 || args!.indexOf('-loop_output') !== -1); assert.ok(args!.indexOf('-t') > -1); - assert.ok(args!.indexOf(120) > -1); + assert.ok(args!.indexOf('120') > -1); }); it('should add the -loop 1 and a time argument (timemark)', () => { @@ -517,7 +517,7 @@ describe('Command', () => { testhelper.logArgError(err); assert.ok(!err); assert.ok(args!.indexOf('-t') > -1); - assert.ok(args!.indexOf(10) > -1); + assert.ok(args!.indexOf('10') > -1); }); }); diff --git a/test/duration-input.test.ts b/test/duration-input.test.ts index 198ecb2c..df6fdf4c 100644 --- a/test/duration-input.test.ts +++ b/test/duration-input.test.ts @@ -20,7 +20,7 @@ describe('durationInput / setInputDuration (issue #53)', () => { it('appends -t to the current input options', () => { const cmd: FfmpegInst = new Ffmpeg().input('first.mp4').durationInput(10); const argv = cmd._inputs[0].options.get(); - assert.deepEqual(argv, ['-t', 10]); + assert.deepEqual(argv, ['-t', '10']); }); it('exposes setInputDuration as a synonym (matches setStartTime / seekInput pair)', () => { @@ -32,8 +32,8 @@ describe('durationInput / setInputDuration (issue #53)', () => { it('chains with seekInput in either order', () => { const a: FfmpegInst = new Ffmpeg().input('a.mp4').seekInput(5).durationInput(10); const b: FfmpegInst = new Ffmpeg().input('b.mp4').durationInput(10).seekInput(5); - assert.deepEqual(a._inputs[0].options.get(), ['-ss', 5, '-t', 10]); - assert.deepEqual(b._inputs[0].options.get(), ['-t', 10, '-ss', 5]); + assert.deepEqual(a._inputs[0].options.get(), ['-ss', '5', '-t', '10']); + assert.deepEqual(b._inputs[0].options.get(), ['-t', '10', '-ss', '5']); }); it('applies independently to multiple inputs', () => { @@ -44,8 +44,8 @@ describe('durationInput / setInputDuration (issue #53)', () => { .input('second.mp4') .seekInput(100) .durationInput(15); - assert.deepEqual(cmd._inputs[0].options.get(), ['-ss', 0, '-t', 10]); - assert.deepEqual(cmd._inputs[1].options.get(), ['-ss', 100, '-t', 15]); + assert.deepEqual(cmd._inputs[0].options.get(), ['-ss', '0', '-t', '10']); + assert.deepEqual(cmd._inputs[1].options.get(), ['-ss', '100', '-t', '15']); }); it('throws "No input specified" when called before .input() (consistent with seekInput)', () => { @@ -67,7 +67,7 @@ describe('durationInput / setInputDuration (issue #53)', () => { const i2 = argv.indexOf('-i', i1 + 1); assert.notEqual(i1, -1); assert.notEqual(i2, -1); - assert.deepEqual(argv.slice(i1 - 2, i1), ['-t', 10]); - assert.deepEqual(argv.slice(i2 - 2, i2), ['-t', 20]); + assert.deepEqual(argv.slice(i1 - 2, i1), ['-t', '10']); + assert.deepEqual(argv.slice(i2 - 2, i2), ['-t', '20']); }); }); diff --git a/test/utils.test.ts b/test/utils.test.ts index c3aac11c..50f203be 100644 --- a/test/utils.test.ts +++ b/test/utils.test.ts @@ -580,7 +580,16 @@ describe('utils.extractCodecData', () => { const state: CodecState = {}; utils.extractCodecData(command, 'Input #0, mov,mp4,m4a, from foo.mp4', state); assert.deepEqual(state, { - inputStack: [{ format: 'mov,mp4,m4a', audio: '', video: '', duration: '' }], + inputStack: [ + { + format: 'mov,mp4,m4a', + audio: '', + audio_details: [], + video: '', + video_details: [], + duration: '', + }, + ], inputIndex: 0, inInput: true, }); @@ -612,6 +621,31 @@ describe('utils.extractCodecData', () => { assert.deepEqual(state.inputStack?.[0]?.video_details, ['h264', 'yuv420p', '1024x768']); }); + // --- Regression for upstream fluent-ffmpeg#1301 --------------------- + // + // For inputs with only an audio stream (or only a video stream) the + // counterpart `*_details` field was `undefined`. Consumers + // destructuring `audio_details.length` on the `codecData` event then + // crashed. Initialise both arrays up-front so destructuring is always + // safe; the populated side is replaced when a Stream:line matches. + + it('exposes empty audio_details / video_details on a fresh input record', () => { + const command = makeEmitter(); + const state: CodecState = {}; + utils.extractCodecData(command, 'Input #0, mp3, from audio-only.mp3', state); + assert.deepEqual(state.inputStack?.[0]?.audio_details, []); + assert.deepEqual(state.inputStack?.[0]?.video_details, []); + }); + + it('leaves the unrelated *_details array empty for an audio-only input', () => { + const command = makeEmitter(); + const state: CodecState = {}; + utils.extractCodecData(command, 'Input #0, mp3, from audio-only.mp3', state); + utils.extractCodecData(command, ' Stream #0:0: Audio: mp3, 44100 Hz, stereo', state); + assert.deepEqual(state.inputStack?.[0]?.audio_details, ['mp3', '44100 Hz', 'stereo']); + assert.deepEqual(state.inputStack?.[0]?.video_details, []); + }); + it('flips inInput off when an "Output #" line arrives', () => { const command = makeEmitter(); const state: CodecState = {}; @@ -811,6 +845,41 @@ describe('utils.extractProgress', () => { assert.equal(data.targetSize, 0); }); + // --- Regression for upstream fluent-ffmpeg#928 ----------------------- + // + // `-loglevel +level` prepends `[info]` / `[warning]` / `[error]` to + // every stderr line. The previous parser required every + // space-separated token to look like `key=value`; `[info]` had no `=` + // so the entire line was rejected and no `progress` event fired. + + it('parses a progress line with a leading [info] loglevel prefix (issue: upstream #928)', () => { + const command = makeCommand(); + utils.extractProgress( + command, + '[info] frame=1668 fps=0.0 q=-0.0 size=N/A time=00:00:55.90 bitrate=N/A speed=112x', + ); + assert.equal(command.progresses.length, 1, 'progress must still fire under -loglevel +level'); + const [data] = command.progresses; + assert.equal(data.frames, 1668); + assert.equal(data.timemark, '00:00:55.90'); + }); + + it('strips other loglevel prefixes too (warning / error / verbose)', () => { + const command = makeCommand(); + utils.extractProgress( + command, + '[warning] frame=10 fps=20 size=100 time=00:00:01.00 bitrate=8kbits/s', + ); + assert.equal(command.progresses.length, 1); + }); + + it('leaves regular non-prefixed progress lines unchanged (no regression)', () => { + const command = makeCommand(); + utils.extractProgress(command, 'frame=99 fps=30 size=50 time=00:00:03.30 bitrate=4kbits/s'); + assert.equal(command.progresses.length, 1); + assert.equal(command.progresses[0].frames, 99); + }); + it('regression: a fully-numeric progress line still reports the numeric values (no behaviour change)', () => { const command = makeCommand(); utils.extractProgress( @@ -826,6 +895,39 @@ describe('utils.extractProgress', () => { }); }); +describe('utils.formatNumberForCall', () => { + // --- Regression for upstream fluent-ffmpeg#1131 ---------------------- + // + // `seekInput(1e-7)` previously produced the argv token `1e-7`, which + // ffmpeg rejects. JavaScript's default `String(n)` falls back to + // scientific notation for very small / very large finite numbers; + // ffmpeg's CLI parser does not understand that form. + + it('integer round-trips as a plain integer string', () => { + assert.equal(utils.formatNumberForCall(10), '10'); + assert.equal(utils.formatNumberForCall(0), '0'); + }); + + it('plain decimal stays in fixed-point form', () => { + assert.equal(utils.formatNumberForCall(1.5), '1.5'); + assert.equal(utils.formatNumberForCall(123.456), '123.456'); + }); + + it('expands very small fractions out of scientific notation (the fix)', () => { + // 1e-7 = 0.0000001 — well within our 10-digit max-fraction budget. + assert.equal(utils.formatNumberForCall(1e-7), '0.0000001'); + }); + + it('handles negative values without losing the sign', () => { + assert.equal(utils.formatNumberForCall(-0.5), '-0.5'); + }); + + it('passes non-finite values through (NaN / Infinity stay as String() reports them)', () => { + assert.equal(utils.formatNumberForCall(Number.NaN), 'NaN'); + assert.equal(utils.formatNumberForCall(Number.POSITIVE_INFINITY), 'Infinity'); + }); +}); + describe('utils.which (callback wrapper around which@7)', () => { // The wrapper resolves a binary name against the user's PATH, never errors // (failures resolve to '' so the lookup still completes), and caches the