Repository navigation
milestone 6: add export-video ffmpeg baseline with snapshots - #6
Conversation
![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: e0c07041dc
ℹ️ 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".
| "yuv420p", | ||
| "-movflags", | ||
| "+faststart", | ||
| "-an", |
There was a problem hiding this comment.
The ffmpeg invocation always includes -an, so every exported file is muted regardless of whether the source has audio. This causes silent outputs for normal story/reel inputs with sound, which is a functional regression for production use even when the transcode otherwise succeeds. Audio should be preserved (or made optional via an explicit mute flag) instead of being dropped unconditionally.
Useful? React with 👍 / 👎.
| @@ -0,0 +1 @@ | |||
| {"ffmpeg_filter":"scale=1080:1920:force_original_aspect_ratio=increase,crop=1080:1920","fps":30,"input_path":"/Users/jonas/repos/passepartout/tests/fixtures/images/portrait_video_360x640.mp4","output_path":"/Users/jonas/repos/passepartout/tests/fixtures/exports/e2e_export_video_reliable_reel_portrait.mp4","selected_profile":"reliable_reel_safe","target_resolution":"1080x1920","video_codec":"h264","white_canvas_enabled":false} | |||
There was a problem hiding this comment.
This snapshot hardcodes machine-specific absolute paths (/Users/jonas/...) for input_path and output_path, while the e2e test compares the full JSON payload verbatim. Any checkout whose repo root differs will produce different resolved paths and fail the snapshot even when export behavior is correct, so these path fields need normalization (for example, converting to repo-relative values).
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughIntroduces a video export feature: new TypeScript types Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/phase1_knowledge.md (1)
29-36:⚠️ Potential issue | 🟡 MinorVary repeated “Added” sentence starters for readability.
Static analysis flags repetition in this section; a small rewording will improve flow.
✍️ Suggested rewording
-- Added real image fixtures in `tests/fixtures/images` (PPM) and used them in visual tests. -- Added media inspector baseline for PPM (`P3` + `P6`) and wired analyze e2e snapshot tests. -- Added raster fixture generation (`sips`) and analyze e2e coverage for PNG/JPEG fixtures. -- Added video fixture generation (`ffmpeg`) and analyze coverage for MP4/MOV fixtures. -- Added export integration/e2e snapshot tests with fixture outputs in `tests/fixtures/exports`. -- Added export-video integration/e2e snapshot tests and dedicated export-video snapshot fixtures. +- Added real image fixtures in `tests/fixtures/images` (PPM) and used them in visual tests. +- Introduced a media inspector baseline for PPM (`P3` + `P6`) and wired analyze e2e snapshot tests. +- Implemented raster fixture generation (`sips`) and analyze e2e coverage for PNG/JPEG fixtures. +- Implemented video fixture generation (`ffmpeg`) and analyze coverage for MP4/MOV fixtures. +- Added export integration/e2e snapshot tests with fixture outputs in `tests/fixtures/exports`. +- Added export-video integration/e2e snapshot tests and dedicated export-video snapshot fixtures.
🤖 Fix all issues with AI agents
In `@src/cli/export_video.ts`:
- Around line 33-58: The switch handling for options (--out, --mode, --surface,
--workflow, --canvas-profile, --crf) assigns the next token to variables (out,
mode, surface, workflow, canvasProfile, crf) without validating that next is a
real value; update the parser around those cases to first check that next is
defined and does not startWith('--') and if it fails, throw or print a clear
"Missing value for --<option>" error (use the option name from the current
case), otherwise assign; for --crf additionally validate Number.parseInt(next)
is not NaN and error "Invalid numeric value for --crf" when parsing fails.
Ensure these checks occur before incrementing i and before casting to
Mode/Surface/Workflow/CanvasProfile.
In `@src/domain/export_video.ts`:
- Around line 93-95: The error thrown when FFmpeg fails should include more
context: update the failure branch that checks proc.exitCode in export_video.ts
to include proc.spawnargs (the full command and arguments), proc.exitCode, and
the stderr output (proc.stderr.toString().trim()) in the thrown Error message so
logs show the exact command run and the exit code alongside the FFmpeg stderr
for easier debugging.
In `@tests/e2e/export_video.snapshots.e2e.test.ts`:
- Around line 1-29: The test currently trims and string-compares the last stdout
line, which is brittle; use the existing parseJsonStdout helper to parse CLI
JSON output and compare objects instead: call parseJsonStdout(result.stdout)
after runExportVideoCli(scenario.args) to obtain the actual object, read and
JSON.parse the expected file (join(snapshotDir, `${scenario.id}.json`)) into an
object, and assert deep equality (e.g., expect(actualObj).toEqual(expectedObj));
update references in this test (export_video.snapshots.e2e.test.ts) where
result, snapshotDir, and scenario.id are used to replace the string matching
logic with parseJsonStdout-based object comparison.
In `@tests/fixtures/e2e/generate-export-video-snapshots.ts`:
- Around line 42-47: The code only checks payload?.startsWith("{") but may still
write malformed JSON; update the block around payload, payload parsing and
writeFileSync to validate by attempting JSON.parse(payload) inside a try-catch
before calling writeFileSync (referencing the payload variable, writeFileSync
call, join(snapshotDir, `${testCase.id}.json`) and testCase.id); on parse
failure throw a clear Error (including testCase.id and the parse error message)
or skip writing so tests fail with a descriptive error instead of producing
invalid snapshot files.
- Around line 17-23: The code can access an undefined value when "--out" is the
last arg; update the block that uses outIndex/testCase.args so it fails fast on
a malformed test case: inside the outIndex >= 0 branch check if outIndex ===
testCase.args.length - 1 and throw a clear Error (including testCase identifier
if available) indicating a missing value for "--out"; otherwise read outputPath
= testCase.args[outIndex + 1] and continue to rmSync(join(repoRoot, outputPath),
{ force: true }) as before. Ensure you reference the existing variables
outIndex, testCase.args, outputPath, rmSync and repoRoot when making the change.
- Around line 25-34: Wrap the Bun.spawnSync call for the test case with a
timeout option (e.g., timeout: 30000) so a hung child won't block the script;
update the Bun.spawnSync invocation that produces proc to include the timeout
property, then handle the timeout/failure by checking proc.exitCode (and any
timeout-specific indicators) and throw a descriptive Error including testCase.id
and proc.stderr/toString or a timeout message if the command timed out. Ensure
you modify the Bun.spawnSync call and the subsequent failure branch that
currently references proc.exitCode and proc.stderr.
In
`@tests/fixtures/e2e/snapshots/export_video/export-video-experimental-story.json`:
- Line 1: The snapshot contains absolute paths in the JSON fields "input_path"
and "output_path" which break portability; update the snapshot generation or
normalization step used for
tests/fixtures/e2e/snapshots/export_video/export-video-experimental-story.json
so that it emits repo-relative paths (or a stable placeholder) instead of
absolute filesystem paths—for example, strip the system-dependent prefix and
store paths relative to the repo root or replace with a normalized token when
producing the snapshot (adjust the snapshot serializer or CLI code that writes
these fields).
In `@tests/integration/export_video.integration.test.ts`:
- Around line 6-7: The fixtures directory variable name fixtures currently
points to a folder named "images" but the tests use video files
(portrait_video_360x640.mp4, landscape_video_640x360.mov); update the folder
name or variable to avoid confusion — either rename the actual directory from
"images" to "media" (or "fixtures/media") or change the variable value
(fixtures) to point to the correct media folder, and update any related
references (e.g., where portrait_video_360x640.mp4 and
landscape_video_640x360.mov are loaded) so the path is accurate and clearly
indicates it contains video media.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
docs/phase1_knowledge.mdpackage.jsonsrc/cli/export_video.tssrc/domain/export_video.tssrc/types/contracts.tstests/e2e/export_video.snapshots.e2e.test.tstests/fixtures/e2e/export_video_cases.jsontests/fixtures/e2e/generate-export-video-snapshots.tstests/fixtures/e2e/snapshots/export_video/export-video-experimental-story.jsontests/fixtures/e2e/snapshots/export_video/export-video-reliable-feed-landscape-white.jsontests/fixtures/e2e/snapshots/export_video/export-video-reliable-reel-portrait.jsontests/helpers/cli.tstests/integration/export_video.integration.test.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/cli/export_video.ts (3)
src/types/contracts.ts (5)
ExportVideoInput(131-140)Mode(1-1)Surface(2-2)Workflow(4-4)CanvasProfile(5-5)src/domain/export_video.ts (1)
exportVideo(37-107)src/domain/recommend.ts (1)
stableStringify(104-106)
src/domain/export_video.ts (4)
src/types/contracts.ts (2)
ExportVideoInput(131-140)ExportVideoOutput(142-151)src/domain/media_inspector.ts (1)
inspectMedia(255-287)src/domain/recommend.ts (1)
recommend(34-87)src/domain/rules.ts (1)
parseResolution(19-27)
tests/e2e/export_video.snapshots.e2e.test.ts (1)
tests/helpers/cli.ts (1)
runExportVideoCli(43-45)
🪛 LanguageTool
docs/phase1_knowledge.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... outputs in tests/fixtures/exports. - Added export-video integration/e2e snapshot t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (12)
package.json (1)
6-25: Scripts wiring looks good.
The new export-video and fixture scripts integrate cleanly with the CLI and snapshot tooling.tests/fixtures/e2e/export_video_cases.json (1)
1-13: Test cases are well-structured.
Consistent use of--jsonand relative fixture paths aligns with the snapshot harness.tests/helpers/cli.ts (2)
11-14: Command union update looks good.
43-45: New export-video CLI helper is consistent with existing wrappers.tests/integration/export_video.integration.test.ts (2)
16-55: Well-structured integration test with good coverage.The test properly validates the full export workflow: CLI execution, file creation, and metadata verification via the analyze command. The assertions on resolution, orientation, and codec ensure the FFmpeg pipeline produces correct output.
57-83: Good coverage for white-canvas export scenario.This test validates the white-canvas feature with the feed surface, checking both the target resolution and the
white_canvas_enabledflag in the output. The test correctly exercises the--canvas-profileargument.src/cli/export_video.ts (2)
68-93: Validation logic is well-structured.The validation for mode, surface, workflow, canvas profile, and CRF values is comprehensive. The error messages are descriptive and will help users correct invalid inputs.
104-116: CLI main flow is clean and straightforward.The main function properly orchestrates argument parsing, domain logic invocation, and output formatting. The conditional JSON output via
stableStringifyensures deterministic output for testing.src/domain/export_video.ts (3)
8-35: Well-implemented FFmpeg filter builder with proper validation.The
buildFilterfunction correctly handles two scenarios:
- White canvas mode: scales to inner dimensions preserving aspect ratio, centers with padding, then adds outer margins
- Standard mode: scales to fill target dimensions and crops to exact size
The validation for non-positive inner dimensions (lines 20-22) prevents invalid FFmpeg commands.
65-91: Verify that stripping audio (-an) is intentional.The FFmpeg command includes
-anwhich removes audio from the output. If the input video contains audio that should be preserved, this would need adjustment. Confirm this is intentional per the product requirements.
37-51: Good use of domain composition.The function properly orchestrates media inspection, recommendation generation, and resolution parsing. The
sourceRatiocalculation frommedia.width / media.heightensures accurate aspect ratio handling for the recommendation engine.src/types/contracts.ts (1)
131-151: Type definitions are well-aligned with existing patterns.The
ExportVideoInputandExportVideoOutputtypes follow the established conventions fromExportImageInput/ExportImageOutput. The video-specific fields (crf,video_codec,fps) are appropriately typed for their use cases.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (proc.exitCode !== 0) { | ||
| throw new Error(`ffmpeg video export failed: ${proc.stderr.toString().trim()}`); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Error message could include more context for debugging.
When FFmpeg fails, including the full command or key parameters would help diagnose issues. Currently only stderr is included.
💡 Enhanced error message
if (proc.exitCode !== 0) {
- throw new Error(`ffmpeg video export failed: ${proc.stderr.toString().trim()}`);
+ throw new Error(
+ `ffmpeg video export failed (input: ${inputPath}, filter: ${filter}): ${proc.stderr.toString().trim()}`
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (proc.exitCode !== 0) { | |
| throw new Error(`ffmpeg video export failed: ${proc.stderr.toString().trim()}`); | |
| } | |
| if (proc.exitCode !== 0) { | |
| throw new Error( | |
| `ffmpeg video export failed (input: ${inputPath}, filter: ${filter}): ${proc.stderr.toString().trim()}` | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@src/domain/export_video.ts` around lines 93 - 95, The error thrown when
FFmpeg fails should include more context: update the failure branch that checks
proc.exitCode in export_video.ts to include proc.spawnargs (the full command and
arguments), proc.exitCode, and the stderr output (proc.stderr.toString().trim())
in the thrown Error message so logs show the exact command run and the exit code
alongside the FFmpeg stderr for easier debugging.
| const payload = lines[lines.length - 1]; | ||
| if (!payload?.startsWith("{")) { | ||
| throw new Error(`no json payload for ${testCase.id}`); | ||
| } | ||
|
|
||
| writeFileSync(join(snapshotDir, `${testCase.id}.json`), `${payload}\n`, "utf8"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing JSON parse validation before writing snapshot.
The code validates that the payload starts with { but doesn't wrap the write in a try-catch. If the payload is malformed JSON (starts with { but is invalid), the snapshot file will contain invalid JSON that will cause test failures with unclear error messages.
🛡️ Proposed fix to validate JSON before writing
const payload = lines[lines.length - 1];
if (!payload?.startsWith("{")) {
throw new Error(`no json payload for ${testCase.id}`);
}
- writeFileSync(join(snapshotDir, `${testCase.id}.json`), `${payload}\n`, "utf8");
+ try {
+ JSON.parse(payload); // validate JSON structure
+ } catch {
+ throw new Error(`invalid json payload for ${testCase.id}: ${payload}`);
+ }
+ writeFileSync(join(snapshotDir, `${testCase.id}.json`), `${payload}\n`, "utf8");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const payload = lines[lines.length - 1]; | |
| if (!payload?.startsWith("{")) { | |
| throw new Error(`no json payload for ${testCase.id}`); | |
| } | |
| writeFileSync(join(snapshotDir, `${testCase.id}.json`), `${payload}\n`, "utf8"); | |
| const payload = lines[lines.length - 1]; | |
| if (!payload?.startsWith("{")) { | |
| throw new Error(`no json payload for ${testCase.id}`); | |
| } | |
| try { | |
| JSON.parse(payload); // validate JSON structure | |
| } catch { | |
| throw new Error(`invalid json payload for ${testCase.id}: ${payload}`); | |
| } | |
| writeFileSync(join(snapshotDir, `${testCase.id}.json`), `${payload}\n`, "utf8"); |
🤖 Prompt for AI Agents
In `@tests/fixtures/e2e/generate-export-video-snapshots.ts` around lines 42 - 47,
The code only checks payload?.startsWith("{") but may still write malformed
JSON; update the block around payload, payload parsing and writeFileSync to
validate by attempting JSON.parse(payload) inside a try-catch before calling
writeFileSync (referencing the payload variable, writeFileSync call,
join(snapshotDir, `${testCase.id}.json`) and testCase.id); on parse failure
throw a clear Error (including testCase.id and the parse error message) or skip
writing so tests fail with a descriptive error instead of producing invalid
snapshot files.
| const fixtures = join(import.meta.dir, "..", "fixtures", "images"); | ||
| const outDir = join(import.meta.dir, "..", "fixtures", "exports"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Fixture directory named "images" but contains video files.
The fixture directory is named images but the tests reference video files (portrait_video_360x640.mp4, landscape_video_640x360.mov). Consider renaming to media or fixtures for clarity, or this may be intentional for shared image/video fixtures.
🤖 Prompt for AI Agents
In `@tests/integration/export_video.integration.test.ts` around lines 6 - 7, The
fixtures directory variable name fixtures currently points to a folder named
"images" but the tests use video files (portrait_video_360x640.mp4,
landscape_video_640x360.mov); update the folder name or variable to avoid
confusion — either rename the actual directory from "images" to "media" (or
"fixtures/media") or change the variable value (fixtures) to point to the
correct media folder, and update any related references (e.g., where
portrait_video_360x640.mp4 and landscape_video_640x360.mov are loaded) so the
path is accurate and clearly indicates it contains video media.
da22ae8 to
14f11b4
Compare
e0c0704 to
b693ecc
Compare
There was a problem hiding this comment.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
docs/phase1_knowledge.mdpackage.jsonsrc/cli/export_video.tssrc/domain/export_video.tssrc/types/contracts.tstests/e2e/export_video.snapshots.e2e.test.tstests/fixtures/e2e/export_video_cases.jsontests/fixtures/e2e/generate-export-video-snapshots.tstests/fixtures/e2e/snapshots/export_video/export-video-experimental-story.jsontests/fixtures/e2e/snapshots/export_video/export-video-reliable-feed-landscape-white.jsontests/fixtures/e2e/snapshots/export_video/export-video-reliable-reel-portrait.jsontests/helpers/cli.tstests/integration/export_video.integration.test.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/domain/export_video.ts (4)
src/types/contracts.ts (2)
ExportVideoInput(131-140)ExportVideoOutput(142-151)src/domain/media_inspector.ts (1)
inspectMedia(322-354)src/domain/recommend.ts (1)
recommend(41-94)src/domain/rules.ts (1)
parseResolution(34-42)
tests/e2e/export_video.snapshots.e2e.test.ts (1)
tests/helpers/cli.ts (2)
runExportVideoCli(43-45)parseJsonStdout(47-64)
src/cli/export_video.ts (3)
src/types/contracts.ts (5)
ExportVideoInput(131-140)Mode(1-1)Surface(2-2)Workflow(4-4)CanvasProfile(5-5)src/domain/export_video.ts (1)
exportVideo(37-115)src/domain/recommend.ts (1)
stableStringify(111-113)
tests/integration/export_video.integration.test.ts (1)
tests/helpers/cli.ts (3)
runExportVideoCli(43-45)runAnalyzeCli(35-37)parseJsonStdout(47-64)
tests/fixtures/e2e/generate-export-video-snapshots.ts (1)
tests/helpers/cli.ts (1)
parseJsonStdout(47-64)
🪛 LanguageTool
docs/phase1_knowledge.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... outputs in tests/fixtures/exports. - Added export-video integration/e2e snapshot t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (18)
docs/phase1_knowledge.md (1)
14-14: LGTM!Documentation accurately reflects the new export-video baseline functionality and test coverage added in this PR. The updates to the "What Was Implemented," "What Was Tried," and "Deferred" sections are clear and consistent with existing formatting.
Also applies to: 36-36, 41-41
package.json (1)
10-10: LGTM!New script entries follow the established naming conventions and align with existing patterns like
export-imageandfixtures:e2e:export.Also applies to: 24-24
tests/fixtures/e2e/snapshots/export_video/export-video-reliable-reel-portrait.json (1)
1-1: LGTM!Snapshot uses relative paths for portability and the ffmpeg filter chain (scale with aspect ratio preservation, then crop) aligns with the expected reel export behavior.
tests/fixtures/e2e/snapshots/export_video/export-video-experimental-story.json (1)
1-1: LGTM!The previous absolute path issue has been addressed. Paths are now relative, making the snapshot portable across environments.
tests/fixtures/e2e/export_video_cases.json (1)
1-14: LGTM!Test cases provide good coverage across:
- Modes: reliable and experimental
- Surfaces: reel, feed, story
- Orientations: portrait and landscape
- White canvas: enabled and disabled
The case definitions align with their corresponding snapshot fixtures.
tests/fixtures/e2e/snapshots/export_video/export-video-reliable-feed-landscape-white.json (1)
1-1: LGTM!The ffmpeg filter chain correctly implements white canvas letterboxing with proper centering math:
(1080-994)/2=43for horizontal offset and(1350-918)/2=216for vertical offset. The multi-stage padding approach maintains aspect ratio while fitting content within the feed container.tests/e2e/export_video.snapshots.e2e.test.ts (2)
15-24: LGTM!The normalization function correctly handles absolute-to-relative path conversion, ensuring snapshot portability across environments. The conditional check for leading
/prevents unnecessary transformations on already-relative paths.
1-14: LGTM!Test structure is clean: loads cases from fixture JSON, executes CLI for each scenario, validates exit codes, and compares normalized payloads against snapshots. The use of
parseJsonStdouthelper addresses the previous review feedback about robust JSON extraction.Also applies to: 26-38
tests/helpers/cli.ts (1)
11-14: LGTM!The type union extension and new
runExportVideoCliwrapper follow the established pattern of the existing CLI helpers (runRecommendCli,runAnalyzeCli,runExportImageCli). The type-safe command union ensures compile-time validation of CLI commands.Also applies to: 43-45
tests/fixtures/e2e/generate-export-video-snapshots.ts (1)
1-53: LGTM!The implementation addresses all prior review feedback:
- Proper validation for
--outwithout a value (lines 21-23)- Timeout added to
Bun.spawnSyncto prevent CI hangs (line 32)- JSON validation is handled by
parseJsonStdouthelper which already includes try/catch validation before returningThe path normalization logic (lines 43-48) correctly converts absolute paths to relative paths for stable snapshots across environments.
src/types/contracts.ts (1)
130-151: LGTM!The new
ExportVideoInputandExportVideoOutputtypes are well-designed:
- Consistent with existing
ExportImageInput/ExportImageOutputpatternscrfis appropriately optional (defaults handled in domain layer)video_codecandfpsare required in output, ensuring complete metadata for exported videostests/integration/export_video.integration.test.ts (1)
16-111: LGTM!The integration tests provide solid coverage:
- Verifies reliable portrait reel export produces correct dimensions (1080x1920) and codec (h264)
- Validates white-canvas export uses the correct feed canvas target (1080x1350)
- Tests CLI error handling for missing
--modeand--crfvaluesThe pattern of using
runAnalyzeClion the output file to verify actual encoded dimensions is a robust verification approach.src/cli/export_video.ts (2)
13-111: LGTM!The argument parsing is well-implemented:
- Validates that value-requiring options (
--out,--mode,--surface,--workflow,--canvas-profile,--crf) have actual values (not missing or another flag)- Validates enum values against allowed sets
- CRF range validation (0-51) is correct for H.264
122-138: LGTM!Clean main function structure with proper error handling:
- Uses
stableStringifyfor deterministic JSON output (important for snapshot testing)- Consistent error message formatting and exit code behavior
src/domain/export_video.ts (4)
8-35: LGTM!The
buildFilterfunction correctly constructs FFmpeg filter chains:
- White-canvas path: scales to inner dimensions preserving aspect ratio, centers content with white padding, then adds margin padding
- Default path: scales up preserving aspect ratio, then crops to exact target dimensions
- Properly validates that inner frame dimensions are positive
65-97: LGTM!The FFmpeg invocation is well-structured:
- Uses
-yto overwrite without prompting- Maps video and optional audio streams correctly (
0:v:0,0:a?)- Codec choices (libx264, aac) and pixel format (yuv420p) ensure broad compatibility
+faststartenables progressive download for web playback- 60-second timeout prevents indefinite hangs
99-103: Enhanced error context addresses prior feedback.The error message now includes both the input path and filter string, making FFmpeg failures easier to diagnose.
49-49: No action needed — height is already validated before the division.The
inspectMediafunction guarantees thatmedia.height > 0beforeexport_videouses it. All dimension readers (readPpmSize,readPngSize,readJpegSize,readVideoMetadata) validate that dimensions are greater than 0 and throw errors if not. Additionally,formatAspect—called withininspectMediaat line 348—validatesheight > 0and throws if the check fails. By the timeinspectMediareturns, height is guaranteed to be positive, making the division at line 49 safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/e2e/export_video.snapshots.e2e.test.ts`:
- Line 35: Replace the JSON.stringify-based equality check with Jest's deep
equality matcher: change the assertion that currently calls
expect(JSON.stringify(actual)).toBe(JSON.stringify(expected)) to use
expect(actual).toEqual(expected) so failures produce clear diffs; locate this
assertion in the export video e2e test (the expect line in
tests/e2e/export_video.snapshots.e2e.test.ts) and update it accordingly.
In `@tests/integration/export_video.integration.test.ts`:
- Around line 6-7: The fixtures directory is misnamed "images" while it contains
video files; update the path used by the fixtures constant in
tests/integration/export_video.integration.test.ts from "images" to "media" (and
rename the on-disk directory accordingly), and likewise update any other tests
or imports that reference the fixtures constant or the old directory (e.g.,
files portrait_video_360x640.mp4 and landscape_video_640x360.mov) so all
references point to the new "media" directory.
14f11b4 to
f62b08c
Compare
b693ecc to
510c50f
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
docs/phase1_knowledge.mdpackage.jsonsrc/cli/export_video.tssrc/domain/export_video.tssrc/types/contracts.tstests/e2e/export_video.snapshots.e2e.test.tstests/fixtures/e2e/export_video_cases.jsontests/fixtures/e2e/generate-export-video-snapshots.tstests/fixtures/e2e/snapshots/export_video/export-video-experimental-story.jsontests/fixtures/e2e/snapshots/export_video/export-video-reliable-feed-landscape-white.jsontests/fixtures/e2e/snapshots/export_video/export-video-reliable-reel-portrait.jsontests/helpers/cli.tstests/integration/export_video.integration.test.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/cli/export_video.ts (3)
src/types/contracts.ts (5)
ExportVideoInput(140-149)Mode(2-2)Surface(5-5)Workflow(11-11)CanvasProfile(14-14)src/domain/export_video.ts (1)
exportVideo(37-115)src/domain/recommend.ts (1)
stableStringify(111-113)
tests/integration/export_video.integration.test.ts (1)
tests/helpers/cli.ts (3)
runExportVideoCli(43-45)runAnalyzeCli(35-37)parseJsonStdout(47-64)
src/domain/export_video.ts (4)
src/types/contracts.ts (2)
ExportVideoInput(140-149)ExportVideoOutput(151-160)src/domain/media_inspector.ts (1)
inspectMedia(380-412)src/domain/recommend.ts (1)
recommend(41-94)src/domain/rules.ts (1)
parseResolution(34-42)
tests/fixtures/e2e/generate-export-video-snapshots.ts (2)
tests/helpers/types.ts (1)
ExportCase(1-4)tests/helpers/cli.ts (1)
parseJsonStdout(47-64)
tests/e2e/export_video.snapshots.e2e.test.ts (2)
tests/helpers/types.ts (1)
ExportCase(1-4)tests/helpers/cli.ts (2)
runExportVideoCli(43-45)parseJsonStdout(47-64)
🪛 LanguageTool
docs/phase1_knowledge.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... outputs in tests/fixtures/exports. - Added export-video integration/e2e snapshot t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (16)
tests/fixtures/e2e/snapshots/export_video/export-video-reliable-reel-portrait.json (1)
1-1: LGTM!The snapshot correctly captures the expected output for a portrait video export to reel surface: scale-and-crop filter for 1080x1920, h264 codec, 30 fps, and white canvas disabled. The structure aligns with the
ExportVideoOutputcontract.tests/fixtures/e2e/snapshots/export_video/export-video-reliable-feed-landscape-white.json (1)
1-1: LGTM!The white-canvas snapshot correctly demonstrates the three-stage filter pipeline: scale to inner frame (994x918) with aspect preservation, center-pad the inner frame, then pad to target resolution (1080x1350) with white margins. The margin offsets (43, 216) are consistent with the inner frame dimensions.
src/types/contracts.ts (1)
139-160: LGTM!The new
ExportVideoInputandExportVideoOutputtypes follow the established patterns fromExportImageInput/ExportImageOutput, appropriately substitutingcrfforqualityand adding video-specific fields (video_codec,fps). The type definitions are consistent and well-structured.src/domain/export_video.ts (3)
8-35: LGTM!The
buildFilterfunction correctly implements two distinct filter strategies:
- White canvas:
decreaseaspect ratio to fit within inner frame, then center-pad, then apply margins with white background.- Standard:
increaseaspect ratio to cover target, then crop to exact dimensions.The validation at lines 20-22 properly guards against invalid margin configurations.
37-58: LGTM!The setup correctly:
- Defaults
crfto 23 (standard H.264 quality setting)- Passes
sourceRatiofrom actual media dimensions to the recommendation engine- Chains media inspection → recommendation → resolution parsing → filter building
105-115: LGTM!The return object correctly maps all fields to the
ExportVideoOutputcontract. Returning the original (non-resolved) paths maintains consistency with the input and enables path normalization in test fixtures.src/cli/export_video.ts (1)
122-140: LGTM!The main function follows a clean CLI pattern: parse arguments, execute domain logic, and output results in the requested format. Error handling properly reports the message and exits with code 1.
docs/phase1_knowledge.md (1)
14-14: LGTM!Documentation accurately captures the new
export-videoCLI baseline with deterministic profile-driven output sizing for MP4/H.264.package.json (2)
10-10: LGTM!The new
export-videoscript follows the established naming convention and correctly points to the CLI entrypoint.
24-24: LGTM!The fixture generation script follows the existing
fixtures:e2e:*naming pattern.tests/fixtures/e2e/export_video_cases.json (1)
1-14: LGTM!The test cases provide good coverage across the feature matrix:
- Modes: reliable and experimental
- Surfaces: reel, feed, and story
- Orientations: portrait and landscape
- White canvas: enabled (feed_compat) and disabled
Each case includes
--jsonfor machine-readable output validation against snapshots.tests/fixtures/e2e/snapshots/export_video/export-video-experimental-story.json (1)
1-1: Snapshot looks consistent and repo‑relative.Fields are stable and paths are already normalized, which keeps the snapshot portable.
tests/e2e/export_video.snapshots.e2e.test.ts (1)
22-33: Snapshot comparison flow looks solid.Parsing JSON output and comparing normalized objects will keep the tests resilient to formatting noise.
tests/helpers/cli.ts (1)
11-45: LGTM: export-video CLI helper wired correctly.The wrapper keeps the test helpers consistent with other CLI entrypoints.
tests/integration/export_video.integration.test.ts (1)
16-111: Integration coverage is strong.Success paths plus explicit error cases make this suite comprehensive for the new CLI.
tests/fixtures/e2e/generate-export-video-snapshots.ts (1)
13-47: Snapshot generator looks robust.Cleanup, timeout, JSON parsing, and path normalization provide stable snapshot generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/phase1_knowledge.md`:
- Around line 34-36: The three consecutive bullets starting with "Added" ("Added
video fixture generation (`ffmpeg`) and analyze coverage for MP4/MOV fixtures.",
"Added export integration/e2e snapshot tests with fixture outputs in
`tests/fixtures/exports`.", "Added export-video integration/e2e snapshot tests
and dedicated export-video snapshot fixtures.") should be rephrased to vary
sentence structure—e.g., change one to "Introduced export-video integration/e2e
snapshot tests...", another to "Added video fixture generation..." or "Included
export integration/e2e snapshot tests..." or combine items into a single
sentence—update those three bullet lines so they no longer all start with
"Added" while preserving the original meaning.
In `@src/cli/export_video.ts`:
- Around line 90-104: Replace the hardcoded arrays used to validate mode,
surface, workflow, and canvasProfile with the shared constants exported from
contracts.ts (MODES, SURFACES, WORKFLOWS, CANVAS_PROFILES): update the checks
that currently use ["reliable", "experimental"], ["feed", "story", "reel"],
["app_direct", "api_scheduler", "unknown"], and ["feed_compat",
"feed_app_direct"] to use MODES.includes(mode), SURFACES.includes(surface),
WORKFLOWS.includes(workflow), and CANVAS_PROFILES.includes(canvasProfile)
respectively, and add the appropriate import for those constants at the top of
src/cli/export_video.ts.
In `@src/domain/export_video.ts`:
- Around line 96-97: The hard-coded 60_000ms timeout in the exportVideo call
should be made configurable: add an optional timeout parameter to the
exportVideo function signature (e.g., timeoutMs?: number) and use that value in
place of the literal (falling back to 60_000 if undefined), or read a
process/env-level default if you prefer; update any callers to pass a larger
timeout for big/HD exports and adjust tests that assume the 60s default. Ensure
the unique symbol exportVideo and the place where timeout: 60_000 is set are
updated together so the timeout is pluggable and documented in the function
signature.
In `@tests/e2e/export_video.snapshots.e2e.test.ts`:
- Around line 11-19: normalizeExportVideoPayload currently only normalizes POSIX
absolute paths (starting with "/"), so Windows drive-letter paths like "C:\..."
slip through; update the checks for input_path and output_path in
normalizeExportVideoPayload to also detect Windows absolute paths (e.g., match
/^[A-Za-z]:[\\/]/) and use a replacement that tolerates both forward and
backslashes (e.g., replace the leading path via a regex like
/^.*tests[\/\\]fixtures[\/\\]/ to "tests/fixtures/") so Windows snapshots
normalize the same as POSIX.
| - Added video fixture generation (`ffmpeg`) and analyze coverage for MP4/MOV fixtures. | ||
| - Added export integration/e2e snapshot tests with fixture outputs in `tests/fixtures/exports`. | ||
| - Added export-video integration/e2e snapshot tests and dedicated export-video snapshot fixtures. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor style: Consider varying sentence structure.
Three consecutive bullet points begin with "Added". Consider rephrasing for variety, e.g., "Introduced export-video integration/e2e snapshot tests..." or restructuring the list.
🧰 Tools
🪛 LanguageTool
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... outputs in tests/fixtures/exports. - Added export-video integration/e2e snapshot t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/phase1_knowledge.md` around lines 34 - 36, The three consecutive bullets
starting with "Added" ("Added video fixture generation (`ffmpeg`) and analyze
coverage for MP4/MOV fixtures.", "Added export integration/e2e snapshot tests
with fixture outputs in `tests/fixtures/exports`.", "Added export-video
integration/e2e snapshot tests and dedicated export-video snapshot fixtures.")
should be rephrased to vary sentence structure—e.g., change one to "Introduced
export-video integration/e2e snapshot tests...", another to "Added video fixture
generation..." or "Included export integration/e2e snapshot tests..." or combine
items into a single sentence—update those three bullet lines so they no longer
all start with "Added" while preserving the original meaning.
| if (!["reliable", "experimental"].includes(mode)) { | ||
| throw new Error(`Invalid mode: ${mode}`); | ||
| } | ||
|
|
||
| if (!["feed", "story", "reel"].includes(surface)) { | ||
| throw new Error(`Invalid surface: ${surface}`); | ||
| } | ||
|
|
||
| if (!["app_direct", "api_scheduler", "unknown"].includes(workflow)) { | ||
| throw new Error(`Invalid workflow: ${workflow}`); | ||
| } | ||
|
|
||
| if (canvasProfile && !["feed_compat", "feed_app_direct"].includes(canvasProfile)) { | ||
| throw new Error(`Invalid canvas profile: ${canvasProfile}`); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use imported constants for validation instead of hardcoded arrays.
The validation checks duplicate the allowed values that are already defined as constants in contracts.ts (MODES, SURFACES, WORKFLOWS, CANVAS_PROFILES). Using these constants ensures consistency if the allowed values change.
♻️ Proposed refactor to use shared constants
+import {
+ MODES,
+ SURFACES,
+ WORKFLOWS,
+ CANVAS_PROFILES,
+ type CanvasProfile,
+ type ExportVideoInput,
+ type Mode,
+ type Surface,
+ type Workflow,
+} from "../types/contracts";
- if (!["reliable", "experimental"].includes(mode)) {
+ if (!MODES.includes(mode)) {
throw new Error(`Invalid mode: ${mode}`);
}
- if (!["feed", "story", "reel"].includes(surface)) {
+ if (!SURFACES.includes(surface)) {
throw new Error(`Invalid surface: ${surface}`);
}
- if (!["app_direct", "api_scheduler", "unknown"].includes(workflow)) {
+ if (!WORKFLOWS.includes(workflow)) {
throw new Error(`Invalid workflow: ${workflow}`);
}
- if (canvasProfile && !["feed_compat", "feed_app_direct"].includes(canvasProfile)) {
+ if (canvasProfile && !CANVAS_PROFILES.includes(canvasProfile)) {
throw new Error(`Invalid canvas profile: ${canvasProfile}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!["reliable", "experimental"].includes(mode)) { | |
| throw new Error(`Invalid mode: ${mode}`); | |
| } | |
| if (!["feed", "story", "reel"].includes(surface)) { | |
| throw new Error(`Invalid surface: ${surface}`); | |
| } | |
| if (!["app_direct", "api_scheduler", "unknown"].includes(workflow)) { | |
| throw new Error(`Invalid workflow: ${workflow}`); | |
| } | |
| if (canvasProfile && !["feed_compat", "feed_app_direct"].includes(canvasProfile)) { | |
| throw new Error(`Invalid canvas profile: ${canvasProfile}`); | |
| } | |
| if (!MODES.includes(mode)) { | |
| throw new Error(`Invalid mode: ${mode}`); | |
| } | |
| if (!SURFACES.includes(surface)) { | |
| throw new Error(`Invalid surface: ${surface}`); | |
| } | |
| if (!WORKFLOWS.includes(workflow)) { | |
| throw new Error(`Invalid workflow: ${workflow}`); | |
| } | |
| if (canvasProfile && !CANVAS_PROFILES.includes(canvasProfile)) { | |
| throw new Error(`Invalid canvas profile: ${canvasProfile}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/export_video.ts` around lines 90 - 104, Replace the hardcoded arrays
used to validate mode, surface, workflow, and canvasProfile with the shared
constants exported from contracts.ts (MODES, SURFACES, WORKFLOWS,
CANVAS_PROFILES): update the checks that currently use ["reliable",
"experimental"], ["feed", "story", "reel"], ["app_direct", "api_scheduler",
"unknown"], and ["feed_compat", "feed_app_direct"] to use MODES.includes(mode),
SURFACES.includes(surface), WORKFLOWS.includes(workflow), and
CANVAS_PROFILES.includes(canvasProfile) respectively, and add the appropriate
import for those constants at the top of src/cli/export_video.ts.
| timeout: 60_000, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider making the timeout configurable for large video files.
The 60-second timeout may be insufficient for longer or higher-resolution video exports. Large files could exceed this limit during encoding.
💡 Optional: Add timeout as an input parameter
export type ExportVideoInput = {
file: string;
out: string;
mode: Mode;
surface: Surface;
workflow?: Workflow;
whiteCanvas?: boolean;
canvasProfile?: CanvasProfile;
crf?: number;
+ timeout?: number;
};Then in exportVideo:
- timeout: 60_000,
+ timeout: input.timeout ?? 60_000,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/domain/export_video.ts` around lines 96 - 97, The hard-coded 60_000ms
timeout in the exportVideo call should be made configurable: add an optional
timeout parameter to the exportVideo function signature (e.g., timeoutMs?:
number) and use that value in place of the literal (falling back to 60_000 if
undefined), or read a process/env-level default if you prefer; update any
callers to pass a larger timeout for big/HD exports and adjust tests that assume
the 60s default. Ensure the unique symbol exportVideo and the place where
timeout: 60_000 is set are updated together so the timeout is pluggable and
documented in the function signature.
| function normalizeExportVideoPayload(payload: Record<string, unknown>): Record<string, unknown> { | ||
| const normalized = structuredClone(payload) as { input_path?: string; output_path?: string }; | ||
| if (typeof normalized.input_path === "string" && normalized.input_path.startsWith("/")) { | ||
| normalized.input_path = normalized.input_path.replace(/^.*tests\/fixtures\//, "tests/fixtures/"); | ||
| } | ||
| if (typeof normalized.output_path === "string" && normalized.output_path.startsWith("/")) { | ||
| normalized.output_path = normalized.output_path.replace(/^.*tests\/fixtures\//, "tests/fixtures/"); | ||
| } | ||
| return normalized as Record<string, unknown>; |
There was a problem hiding this comment.
Handle Windows absolute paths in snapshot normalization.
Right now normalization only triggers for paths starting with “/”, so Windows paths like C:\... won’t be normalized and can fail snapshots on Windows runners.
🛠️ Suggested fix
function normalizeExportVideoPayload(payload: Record<string, unknown>): Record<string, unknown> {
const normalized = structuredClone(payload) as { input_path?: string; output_path?: string };
- if (typeof normalized.input_path === "string" && normalized.input_path.startsWith("/")) {
- normalized.input_path = normalized.input_path.replace(/^.*tests\/fixtures\//, "tests/fixtures/");
- }
- if (typeof normalized.output_path === "string" && normalized.output_path.startsWith("/")) {
- normalized.output_path = normalized.output_path.replace(/^.*tests\/fixtures\//, "tests/fixtures/");
- }
+ if (typeof normalized.input_path === "string") {
+ normalized.input_path = normalized.input_path
+ .replace(/^.*tests[\\/]+fixtures[\\/]+/, "tests/fixtures/")
+ .replace(/\\/g, "/");
+ }
+ if (typeof normalized.output_path === "string") {
+ normalized.output_path = normalized.output_path
+ .replace(/^.*tests[\\/]+fixtures[\\/]+/, "tests/fixtures/")
+ .replace(/\\/g, "/");
+ }
return normalized as Record<string, unknown>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function normalizeExportVideoPayload(payload: Record<string, unknown>): Record<string, unknown> { | |
| const normalized = structuredClone(payload) as { input_path?: string; output_path?: string }; | |
| if (typeof normalized.input_path === "string" && normalized.input_path.startsWith("/")) { | |
| normalized.input_path = normalized.input_path.replace(/^.*tests\/fixtures\//, "tests/fixtures/"); | |
| } | |
| if (typeof normalized.output_path === "string" && normalized.output_path.startsWith("/")) { | |
| normalized.output_path = normalized.output_path.replace(/^.*tests\/fixtures\//, "tests/fixtures/"); | |
| } | |
| return normalized as Record<string, unknown>; | |
| function normalizeExportVideoPayload(payload: Record<string, unknown>): Record<string, unknown> { | |
| const normalized = structuredClone(payload) as { input_path?: string; output_path?: string }; | |
| if (typeof normalized.input_path === "string") { | |
| normalized.input_path = normalized.input_path | |
| .replace(/^.*tests[\\/]+fixtures[\\/]+/, "tests/fixtures/") | |
| .replace(/\\/g, "/"); | |
| } | |
| if (typeof normalized.output_path === "string") { | |
| normalized.output_path = normalized.output_path | |
| .replace(/^.*tests[\\/]+fixtures[\\/]+/, "tests/fixtures/") | |
| .replace(/\\/g, "/"); | |
| } | |
| return normalized as Record<string, unknown>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/export_video.snapshots.e2e.test.ts` around lines 11 - 19,
normalizeExportVideoPayload currently only normalizes POSIX absolute paths
(starting with "/"), so Windows drive-letter paths like "C:\..." slip through;
update the checks for input_path and output_path in normalizeExportVideoPayload
to also detect Windows absolute paths (e.g., match /^[A-Za-z]:[\\/]/) and use a
replacement that tolerates both forward and backslashes (e.g., replace the
leading path via a regex like /^.*tests[\/\\]fixtures[\/\\]/ to
"tests/fixtures/") so Windows snapshots normalize the same as POSIX.
Addressed/accepted for stacked merge.
1 participant
Summary
export-videoCLI command and domain engine using deterministic recommendation policyVerification
Greptile Summary
This PR adds a complete
export-videocommand that mirrors the existingexport-imagefunctionality but for video files. The implementation uses ffmpeg to transcode videos with deterministic profile-based resizing, supporting both standard crop/scale and white-canvas padding modes. The export preserves audio streams using the-map 0:a?option and normalizes framerates. Comprehensive test coverage includes integration tests and e2e snapshot tests covering reliable/experimental modes, multiple surfaces (reel, feed, story), and white-canvas scenarios.src/domain/export_video.tswith ffmpeg integrationExportVideoInput/Output0:a?)Confidence Score: 5/5
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[CLI: export_video.ts] -->|parseArgs| B[Validate Args] B -->|ExportVideoInput| C[exportVideo] C -->|inspectMedia| D[Get Video Metadata] D -->|width, height, fps, orientation| E[recommend] E -->|RecommendationOutput| F[parseResolution] F -->|width, height| G[buildFilter] G -->|whiteCanvas?| H{White Canvas?} H -->|Yes| I[scale + pad inner + pad outer] H -->|No| J[scale + crop] I --> K[Build ffmpeg Command] J --> K K -->|video + audio mapping| L[Bun.spawnSync ffmpeg] L -->|exitCode = 0?| M{Success?} M -->|Yes| N[Return ExportVideoOutput] M -->|No| O[Throw Error] N -->|json flag?| P{JSON Output?} P -->|Yes| Q[Print JSON] P -->|No| R[Print Human Summary]Last reviewed commit: 510c50f