Repository navigation
feat: bundle ffmpeg binaries - #62
Conversation
📝 WalkthroughWalkthroughAdds ffmpeg-static and ffprobe-static to package.json, resolves their packaged binary paths at module load in src/domain/media_process.ts (preferring PATH via Bun.which, then requiring package-provided binaries, falling back to "ffmpeg"/"ffprobe"), updates runFfmpeg/runFfprobe to use resolved commands, changes inspectSourceDimensions in src/domain/prepare_image.ts to use ffprobe -show_streams JSON, and updates AGENTS.md/README.md plus adds docs/plans/bundle-ffmpeg-binaries.md. Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 17: Update the sentence "FFmpeg and ffprobe are installed through `bun
install`." to clarify the prerequisite and fallback: state that while bun may
install FFmpeg/ffprobe on supported platforms, the application will also attempt
to locate binaries via the system PATH as a fallback; explicitly instruct users
to ensure either bun-installed binaries are available or that ffmpeg/ffprobe are
present on PATH for unsupported platforms or alternative installations.
Reference the existing wording "FFmpeg and ffprobe are installed through `bun
install`" and the concept "PATH fallback" when making the edit.
In `@README.md`:
- Line 22: Update the README sentence that currently states "FFmpeg and ffprobe
are installed through `bun install`" to reflect the actual behavior: document
that bundled FFmpeg/ffprobe binaries provided by the package are preferred and
will be used when available, but the system PATH is still checked and serves as
a fallback if bundled binaries are not present; edit the line text accordingly
so it clearly communicates "bundled binaries preferred; PATH fallback"
semantics.
In `@src/domain/media_process.ts`:
- Around line 1-14: The module currently imports ffmpeg-static and resolves
ffprobe-static at top-level so any require/import failure prevents falling back;
wrap both resolutions in guarded try-catch helper logic: create a small function
that attempts to import/require ffmpeg-static (referencing ffmpegPath) and
returns its path or undefined on error, and likewise require("ffprobe-static")
(referencing ffprobeStatic) and safely read .path only if present; then set
FFMPEG_CMD = resolved ffmpeg path ?? "ffmpeg" and FFPROBE_CMD = resolved ffprobe
path (string) ?? "ffprobe". Ensure these guarded resolves happen before
exporting MediaProcessResult and avoid throwing on import failures so the
fallback strings are reachable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 746eca27-bcf5-4600-a903-40b743306f95
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
AGENTS.mdREADME.mddocs/plans/bundle-ffmpeg-binaries.mdpackage.jsonsrc/domain/media_process.tssrc/domain/prepare_image.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,ts,jsx,tsx}: Prioritize using plain JavaScript/TypeScript instead of libraries for fundamental algorithms (e.g., use Array methods instead of lodash)
Use functional programming patterns and immutable data structures in JavaScript/TypeScript code
Files:
src/domain/media_process.tssrc/domain/prepare_image.ts
src/domain/media_process.ts
📄 CodeRabbit inference engine (AGENTS.md)
FFmpeg/ffprobe process boundary should be packaged in
src/domain/media_process.ts
Files:
src/domain/media_process.ts
src/domain/prepare_image.ts
📄 CodeRabbit inference engine (AGENTS.md)
Source probing, layout selection, and FFmpeg export logic should be implemented in
src/domain/prepare_image.ts
Files:
src/domain/prepare_image.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: edhor1608/passepartout
Timestamp: 2026-05-17T15:36:39.503Z
Learning: Before submitting PRs, run `bun run check` to execute typecheck, Biome lint, unit tests, and the v1 integration test
Learnt from: CR
Repo: edhor1608/passepartout
Timestamp: 2026-05-17T15:36:39.503Z
Learning: Use `bun install --frozen-lockfile` to install dependencies
Learnt from: CR
Repo: edhor1608/passepartout
Timestamp: 2026-05-17T15:36:39.503Z
Learning: For feature branches, update the relevant markdown in `docs/plans/` with the problem, decisions, commands run, and lessons learned
🔇 Additional comments (5)
docs/plans/bundle-ffmpeg-binaries.md (1)
1-23: LGTM!AGENTS.md (1)
51-51: LGTM!package.json (1)
25-29: LGTM!src/domain/media_process.ts (1)
30-35: LGTM!src/domain/prepare_image.ts (1)
71-71: LGTM!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e8beea54e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8e8beea to
f490398
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/domain/media_process.ts (1)
13-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd empty-string guards to FFmpeg/ffprobe path resolution.
The library export formats are correct—
ffmpeg-staticreturns a string directly andffprobe-staticreturns an object with a.pathproperty. However, both resolver functions should check for non-empty strings. An empty string would bypass the?? "ffmpeg"and?? "ffprobe"fallbacks (since empty string is defined) and cause a runtime failure when passed toBun.spawnSync.Update the type guards to include
.length > 0:Fix: Add empty-string checks
function resolveFfmpegPath(): string | undefined { try { const path = require("ffmpeg-static") as unknown; - return typeof path === "string" ? path : undefined; + return typeof path === "string" && path.length > 0 ? path : undefined; } catch { return undefined; } } function resolveFfprobePath(): string | undefined { try { const ffprobeStatic = require("ffprobe-static") as { path?: unknown }; - return typeof ffprobeStatic.path === "string" ? ffprobeStatic.path : undefined; + return typeof ffprobeStatic.path === "string" && ffprobeStatic.path.length > 0 + ? ffprobeStatic.path + : undefined; } catch { return undefined; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/domain/media_process.ts` around lines 13 - 29, The current resolveFfmpegPath and resolveFfprobePath functions return any string including the empty string which can bypass the fallback to "ffmpeg"/"ffprobe" and break Bun.spawnSync; update both type guards to ensure non-empty strings by checking typeof === "string" && path.length > 0 (for resolveFfmpegPath) and typeof ffprobeStatic.path === "string" && ffprobeStatic.path.length > 0 (for resolveFfprobePath) so they return undefined for empty strings and allow the fallback to be used.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/domain/prepare_image.ts`:
- Line 71: The ffprobe -show_entries string is invalid because side_data_list
can't be explicitly requested; update the ffprobe args used in prepare_image.ts
so getRotation() can find rotation data: either replace the current entry string
("stream=width,height,side_data_list:stream_tags=rotate") with a call that uses
-show_streams (e.g., include "streams" or use the equivalent API option) so
side_data_list is present, or restrict to only rotation via
"stream_side_data=rotation" (and keep stream_tags if needed); ensure the ffprobe
invocation and the parsing in getRotation() still match the JSON shape returned.
---
Duplicate comments:
In `@src/domain/media_process.ts`:
- Around line 13-29: The current resolveFfmpegPath and resolveFfprobePath
functions return any string including the empty string which can bypass the
fallback to "ffmpeg"/"ffprobe" and break Bun.spawnSync; update both type guards
to ensure non-empty strings by checking typeof === "string" && path.length > 0
(for resolveFfmpegPath) and typeof ffprobeStatic.path === "string" &&
ffprobeStatic.path.length > 0 (for resolveFfprobePath) so they return undefined
for empty strings and allow the fallback to be used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 003b3d01-9166-4a42-9f23-51ff5817da48
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
AGENTS.mdREADME.mddocs/plans/bundle-ffmpeg-binaries.mdpackage.jsonsrc/domain/media_process.tssrc/domain/prepare_image.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,ts,jsx,tsx}: Prioritize using plain JavaScript/TypeScript instead of libraries for fundamental algorithms (e.g., use Array methods instead of lodash)
Use functional programming patterns and immutable data structures in JavaScript/TypeScript code
Files:
src/domain/prepare_image.tssrc/domain/media_process.ts
src/domain/prepare_image.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement source probing, layout selection, and FFmpeg export logic in
src/domain/prepare_image.ts
Files:
src/domain/prepare_image.ts
src/domain/media_process.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement FFmpeg and ffprobe process boundary in the
src/domain/media_process.tsmodule
Files:
src/domain/media_process.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: edhor1608/passepartout
Timestamp: 2026-05-17T15:42:51.605Z
Learning: Use `bun run check` before PRs to run typecheck, Biome lint, unit tests, and the v1 integration test
Learnt from: CR
Repo: edhor1608/passepartout
Timestamp: 2026-05-17T15:42:51.605Z
Learning: Install dependencies using `bun install --frozen-lockfile`
Learnt from: CR
Repo: edhor1608/passepartout
Timestamp: 2026-05-17T15:42:51.605Z
Learning: For feature branches, update the relevant markdown in `docs/plans/` with the problem, decisions, commands run, and lessons learned
🔇 Additional comments (6)
docs/plans/bundle-ffmpeg-binaries.md (1)
1-23: LGTM!AGENTS.md (1)
17-17: LGTM!Also applies to: 51-51
README.md (1)
22-22: LGTM!src/domain/media_process.ts (1)
1-11: LGTM!Also applies to: 46-46, 50-50
src/domain/prepare_image.ts (1)
64-92: LGTM!Also applies to: 222-238
package.json (1)
26-29: Package versions are stable with no known vulnerabilities.Both
ffmpeg-static(^5.3.0) andffprobe-static(^3.1.0) are available on NPM and have no known security advisories.
f490398 to
a356c39
Compare
9583e55 to
f0447e7
Compare
9cfa709 to
8dedf53
Compare
Merge activity
|
f0447e7 to
7eb1c34
Compare
1 participant
Note
Bundle ffmpeg and ffprobe binaries as fallbacks when not found on PATH
ffmpeg-staticandffprobe-staticas runtime dependencies in package.jsonrunFfmpegandrunFfprobein media_process.ts to resolve binaries with PATH-first priority, falling back to bundled binaries, then bare command namesinspectSourceDimensionsin prepare_image.ts to use-show_streamsinstead of the previous-show_entriesflag combinationffmpeg-staticandffprobe-staticpackages, which include platform-specific binaries and increase install size significantlyMacroscope summarized f0447e7.
Summary by CodeRabbit
New Features
Documentation
Improvements