Skip to content

Commit 6c7a25e

Browse files
committed
fix(media): look ffprobe up on PATH before ffmpeg's sibling
The TSDoc claimed fluent-ffmpeg's order (FFPROBE_PATH, then PATH, then ffmpeg's directory) while the code checked the sibling second, so a stray or unusable file next to the ffmpeg binary would be cached and mask a working PATH install for every probe. Match the documented order. Pinned by a test in its own file: the resolved path memoizes at module scope, so precedence is only observable in a module no other test has resolved in. It mocks existsSync as well as execSync, without which the sibling never exists on the test host and the two orderings are indistinguishable.
1 parent 4560e5a commit 6c7a25e

2 files changed

Lines changed: 70 additions & 4 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* Standalone file: the resolved ffprobe path is memoized at module scope, so
3+
* the first resolution in a process wins. Testing precedence therefore needs a
4+
* module whose memo no other test has populated.
5+
*
6+
* @vitest-environment node
7+
*/
8+
import { describe, expect, it, vi } from 'vitest'
9+
10+
const { execSyncMock, execFileMock, existsSyncMock } = vi.hoisted(() => ({
11+
execSyncMock: vi.fn(),
12+
execFileMock: vi.fn(),
13+
existsSyncMock: vi.fn(),
14+
}))
15+
16+
vi.mock('node:child_process', () => ({
17+
execSync: execSyncMock,
18+
execFile: execFileMock,
19+
}))
20+
21+
vi.mock('node:fs', () => ({
22+
existsSync: existsSyncMock,
23+
}))
24+
25+
import { runFfmpegOperation } from '@/lib/media/ffmpeg'
26+
27+
describe('ffprobe lookup precedence', () => {
28+
it('prefers ffprobe on PATH over a sibling of the ffmpeg binary', async () => {
29+
// ffmpeg resolves into a directory whose ffprobe sibling may be stray or
30+
// unusable; a real PATH entry must win. Both lookups go through execSync,
31+
// so they are distinguished by the command.
32+
execSyncMock.mockImplementation((cmd: string) => {
33+
if (cmd.includes('ffprobe')) return '/usr/bin/ffprobe\n'
34+
if (cmd.includes('ffmpeg')) return '/opt/broken/ffmpeg\n'
35+
throw new Error(`unexpected command: ${cmd}`)
36+
})
37+
// The sibling exists on disk — without this the test cannot tell the two
38+
// orderings apart, because a non-existent sibling is skipped either way.
39+
existsSyncMock.mockImplementation((p: string) => p === '/opt/broken/ffprobe')
40+
execFileMock.mockImplementation((_bin, _args, _opts, cb) => {
41+
cb(null, JSON.stringify({ format: {}, streams: [] }), '')
42+
return {}
43+
})
44+
45+
await runFfmpegOperation('probe', [{ buffer: Buffer.from('media'), mimeType: 'video/mp4' }])
46+
47+
expect(execFileMock.mock.calls[0][0]).toBe('/usr/bin/ffprobe')
48+
expect(execFileMock.mock.calls[0][0]).not.toBe('/opt/broken/ffprobe')
49+
})
50+
})

apps/sim/lib/media/ffmpeg.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,20 @@ function ensureFfmpeg(): void {
4545
}
4646
}
4747

48+
function lookupOnPath(binary: string): string | null {
49+
try {
50+
const cmd = process.platform === 'win32' ? `where ${binary}` : `which ${binary}`
51+
return execSync(cmd, { encoding: 'utf-8' }).trim().split('\n')[0] || null
52+
} catch {
53+
return null
54+
}
55+
}
56+
4857
/**
49-
* Mirrors fluent-ffmpeg's resolution order (FFPROBE_PATH, then PATH, then
50-
* ffmpeg's own directory) so replacing its ffprobe call does not narrow where
51-
* the binary may live for self-hosters.
58+
* Mirrors fluent-ffmpeg's resolution order — FFPROBE_PATH, then PATH, then
59+
* ffmpeg's own directory — so replacing its ffprobe call does not narrow where
60+
* the binary may live for self-hosters. PATH outranks the sibling deliberately:
61+
* a stray or unusable file next to ffmpeg must not mask a working install.
5262
*/
5363
function resolveFfprobePath(): string {
5464
if (ffprobePath) return ffprobePath
@@ -60,8 +70,14 @@ function resolveFfprobePath(): string {
6070
return ffprobePath
6171
}
6272

73+
const onPath = lookupOnPath(binary)
74+
if (onPath) {
75+
ffprobePath = onPath
76+
return ffprobePath
77+
}
78+
6379
// Deliberately initFfmpegPath, not ensureFfmpeg: a missing ffmpeg must not
64-
// stop a probe, since ffprobe may still be on PATH.
80+
// stop a probe when ffprobe is installed on its own.
6581
initFfmpegPath()
6682
const sibling = ffmpegPath ? path.join(path.dirname(ffmpegPath), binary) : undefined
6783
ffprobePath = sibling && existsSync(sibling) ? sibling : binary

0 commit comments

Comments
 (0)