feat: add ffmpeg encoding profiles for 'fast' export - #48
Open
PlkMarudny wants to merge 5 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds configurable ffmpeg encoding profiles with JSON-backed validation, browser and project selection, REST and MCP discovery, and profile-specific single-pass video encoding. ChangesEncoding profile support
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 4
🤖 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 `@encode-profiles.js`:
- Around line 134-156: Update validateMux to validate movflags against the
profile’s permitted pattern/character set before it reaches buildMuxArgs,
correcting invalid values and calling warn for both invalid types and invalid
strings. When format and extension conflict, correct one value to the other’s
supported container/extension mapping and report the correction instead of only
warning, keeping exportContainer and exportOutputExtension aligned. Preserve
explicit clearing and fallback behavior for valid inputs.
- Around line 318-336: Update buildVideoEncodeArgs so the v.x264opts argument is
appended only when v.codec is "libx264", while retaining the existing preset,
CRF, and other codec-specific options for both libx264 and libx265.
In `@mcp-server.js`:
- Around line 323-328: Update encodeProfile validation in the setProject
handling loop to require a non-empty string, rejecting arrays, empty strings,
and other non-string values before persistence or dispatch; pass the original
validated value directly to resolveProfile instead of coercing with String().
Apply the same non-empty-string validation to the profile argument in the
related profile-listing path so an empty profile is not treated as omitted and
does not return the full list.
- Around line 175-183: Update the export-profile reporting logic around
loadEncodeProfiles so an explicitly set project encodeProfile with no matching
cfg.profiles entry reports the invalid profile value and its fallback to
cfg.default separately. Preserve the existing server-default message when no
project profile is configured, and keep valid profile reporting unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cab1edfc-9160-4e48-8c5a-175bbaf24416
📒 Files selected for processing (8)
CLAUDE.mdapp.jsencode-profiles.jsencoding-profiles.jsonindex.htmlmcp-server.jsserver.jsstyle.css
Comment on lines
+134
to
+156
| function validateMux(m, fallback, warn) { | ||
| const given = m && typeof m === "object" ? m : {}; | ||
| const out = { ...fallback, ...given }; | ||
| // an explicit null/"" clears an inherited value (e.g. drop +faststart) | ||
| if ("movflags" in given && (given.movflags === null || given.movflags === "")) delete out.movflags; | ||
| else if (out.movflags != null && typeof out.movflags !== "string") out.movflags = fallback.movflags; | ||
| if (out.format) { | ||
| const fmt = MUX_FORMAT_ALIAS[String(out.format).toLowerCase()]; | ||
| if (fmt) out.format = fmt; | ||
| else { delete out.format; warn(`mux.format "${given.format}" unknown — using mp4`); } | ||
| } | ||
| if (out.extension) { | ||
| const raw = String(out.extension).toLowerCase(); | ||
| const ext = raw.startsWith(".") ? raw : `.${raw}`; | ||
| if (OUT_EXT.has(ext)) out.extension = ext; | ||
| else { delete out.extension; warn(`mux.extension "${given.extension}" unsupported`); } | ||
| } | ||
| // a bare extension implies the container; keeps -f and the filename in step | ||
| if (!out.format && out.extension) out.format = EXT_FORMAT[out.extension]; | ||
| if (out.format && out.extension && EXT_FORMAT[out.extension] !== out.format) | ||
| warn(`mux.extension ${out.extension} does not match mux.format ${out.format}`); | ||
| return out; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
validateMux leaves movflags unvalidated and only warns (doesn't correct) on a format/extension mismatch.
Three gaps relative to the profile-validation contract:
movflags(a string) is never checked against any allowed pattern/character set, unlike every other field in this file (codec, preset, pixFmt, tune, profile, vf, x264opts all go through an allow-list or regex). Any string set in the JSON flows straight into-movflagsinbuildMuxArgs.- Line 139's type-mismatch correction (
out.movflags = fallback.movflags) silently falls back without callingwarn(), unlike every other correction in this file. - Lines 153-154 only
warn()whenmux.extensionandmux.formatdisagree but don't correct it —exportContainerprefersformatwhileexportOutputExtensionprefersextension, so a profile with e.g.format:"mov", extension:".mkv"would mux as MOV but write the file with a.mkvextension.
As per coding guidelines, **encoding-profiles.json**: "Invalid encoding-profile values must be corrected and reported rather than silently applied."
🛡️ Proposed fix sketch
function validateMux(m, fallback, warn) {
const given = m && typeof m === "object" ? m : {};
const out = { ...fallback, ...given };
if ("movflags" in given && (given.movflags === null || given.movflags === "")) delete out.movflags;
- else if (out.movflags != null && typeof out.movflags !== "string") out.movflags = fallback.movflags;
+ else if (out.movflags != null && typeof out.movflags !== "string") {
+ warn(`mux.movflags "${out.movflags}" ignored — must be a string`);
+ out.movflags = fallback.movflags;
+ } else if (out.movflags != null && !SAFE_FFMPEG_STR.test(out.movflags)) {
+ warn(`mux.movflags "${out.movflags}" contains disallowed characters — using ${fallback.movflags}`);
+ out.movflags = fallback.movflags;
+ }
...
- if (out.format && out.extension && EXT_FORMAT[out.extension] !== out.format)
- warn(`mux.extension ${out.extension} does not match mux.format ${out.format}`);
+ if (out.format && out.extension && EXT_FORMAT[out.extension] !== out.format) {
+ warn(`mux.extension ${out.extension} does not match mux.format ${out.format} — using ${out.format}`);
+ out.extension = FORMAT_EXT[out.format];
+ }🤖 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 `@encode-profiles.js` around lines 134 - 156, Update validateMux to validate
movflags against the profile’s permitted pattern/character set before it reaches
buildMuxArgs, correcting invalid values and calling warn for both invalid types
and invalid strings. When format and extension conflict, correct one value to
the other’s supported container/extension mapping and report the correction
instead of only warning, keeping exportContainer and exportOutputExtension
aligned. Preserve explicit clearing and fallback behavior for valid inputs.
Source: Coding guidelines
Comment on lines
+323
to
+328
| const allowed = ["name", "width", "height", "fps", "background", "markers", "disabledTracks", "encodeProfile"]; | ||
| for (const [k, v] of Object.entries(op.set || {})) { | ||
| if (!allowed.includes(k)) throw new Error(`setProject: '${k}' not settable (allowed: ${allowed.join(", ")})`); | ||
| if (k === "encodeProfile" && v != null) { | ||
| resolveProfile(String(v)); // validate id exists | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject non-string and empty profile ids before persisting or dispatching.
String(v) is not strict validation: encodeProfile: "" resolves to the default and is stored, while ["delivery"] resolves successfully but stores an array. Separately, an empty profile argument is treated as if it were omitted and returns the full list. Enforce a non-empty string and pass that exact value to resolveProfile.
Suggested fix
for (const [k, v] of Object.entries(op.set || {})) {
if (!allowed.includes(k)) throw new Error(`setProject: '${k}' not settable (allowed: ${allowed.join(", ")})`);
if (k === "encodeProfile" && v != null) {
- resolveProfile(String(v)); // validate id exists
+ if (typeof v !== "string" || !v)
+ throw new Error("setProject: encodeProfile must be a non-empty profile id");
+ resolveProfile(v);
} case "fablecut_encode_profiles": {
- if (args.profile) {
- const p = resolveProfile(String(args.profile));
+ if (Object.prototype.hasOwnProperty.call(args || {}, "profile")) {
+ if (typeof args.profile !== "string" || !args.profile)
+ throw new Error("profile must be a non-empty profile id");
+ const p = resolveProfile(args.profile);Also applies to: 461-464
🤖 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 `@mcp-server.js` around lines 323 - 328, Update encodeProfile validation in the
setProject handling loop to require a non-empty string, rejecting arrays, empty
strings, and other non-string values before persistence or dispatch; pass the
original validated value directly to resolveProfile instead of coercing with
String(). Apply the same non-empty-string validation to the profile argument in
the related profile-listing path so an empty profile is not treated as omitted
and does not return the full list.
added 4 commits
July 29, 2026 10:37
Ignore all the validation of parameters and the baroque processing - if someone is adding a ffmpeg profile, he knows what he is doing
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@CLAUDE.md`:
- Around line 570-572: Label the Markdown code fence containing the ffmpeg
command as shell by changing the opening fence to ```sh, while leaving the
command content unchanged.
- Around line 601-602: Update the MCP documentation entry for fablecut_status to
describe it as showing the MCP-visible project/default profile, not the
effective profile. Keep the fablecut_encode_profiles descriptions unchanged.
In `@encode-profiles.js`:
- Around line 80-82: Update the profile lookup logic in the export flow and the
profiles configuration handling to verify that IDs such as defaultId and pid are
own properties of the profiles object before using their values. Preserve normal
normalized-profile behavior, but treat inherited-property matches as missing so
profile.args is never read from an unassociated entry.
In `@encoding-profiles.json`:
- Around line 42-49: Update both the broadcast1080i50 and prores422 encoding
profiles to explicitly include “-movflags”, “+faststart” immediately before
“-shortest” in their args arrays, preserving the existing required flags and
ordering otherwise.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b1cb70c-c287-4da6-a2d0-afe339d09991
📒 Files selected for processing (6)
CLAUDE.mdapp.jsencode-profiles.jsencoding-profiles.jsonmcp-server.jsserver.js
🚧 Files skipped from review as they are similar to previous changes (3)
- mcp-server.js
- app.js
- server.js
Comment on lines
+570
to
+572
| ``` | ||
| ffmpeg -y -f image2pipe -framerate <fps> -i - [-i audio.wav] <args…> exports/<name><extension> | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the command fence as shell.
The fence violates MD040. Use ```sh for the ffmpeg command example.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 570-570: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@CLAUDE.md` around lines 570 - 572, Label the Markdown code fence containing
the ffmpeg command as shell by changing the opening fence to ```sh, while
leaving the command content unchanged.
Source: Linters/SAST tools
Comment on lines
+601
to
+602
| **MCP:** `fablecut_encode_profiles` lists profiles; `{detail:true}` includes each `args` | ||
| array; `{profile:"hq"}` returns one profile. `fablecut_status` shows the effective profile. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not describe fablecut_status as showing the effective profile.
The status implementation only reads project.encodeProfile or the file default; it cannot see the higher-priority browser dialog or localStorage choice documented at lines 596-599. Describe it as the MCP-visible project/default profile instead.
🤖 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 `@CLAUDE.md` around lines 601 - 602, Update the MCP documentation entry for
fablecut_status to describe it as showing the MCP-visible project/default
profile, not the effective profile. Keep the fablecut_encode_profiles
descriptions unchanged.
Comment on lines
+80
to
+82
| const profiles = {}; | ||
| if (file?.profiles && typeof file.profiles === "object") | ||
| for (const [id, raw] of Object.entries(file.profiles)) profiles[id] = normalizeProfile(id, raw); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== available files =="
git ls-files | rg '(^|/)encode-profiles\.js$|profiles|default|normalizeProfile' || true
echo
echo "== file outline/size =="
if [ -f encode-profiles.js ]; then
wc -l encode-profiles.js
ast-grep outline encode-profiles.js || true
fi
echo
echo "== relevant references =="
rg -n "profiles|normalizeProfile|default|constructor|__proto__|toString" encode-profiles.js 2>/dev/null || true
echo
echo "== relevant code section =="
if [ -f encode-profiles.js ]; then
sed -n '1,180p' encode-profiles.js | cat -n
fiRepository: ronak-create/FableCut
Length of output: 11046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
# Behavioral probe modeled after in-memory plain-object profile lookup.
def current_profiles(file):
profiles = {}
if file.get("profiles") and isinstance(file["profiles"], dict):
for id_, raw in file["profiles"].items():
profiles[id_] = {"args": "normalized"}
return profiles
cases = [
{"profiles": {"default": "toString"}},
{"profiles": {"default": "constructor"}},
{"profiles": {"default": "__proto__", "__proto__": "not a profile"}},
{"profiles": {"default": "valueOf"}},
]
for file in cases:
import copy
p = copy.deepcopy(file)
p["profiles"]["default"] = p["profiles"]["default"] # no change; probe lookup only.
pro = current_profiles(file)
default = pro.get("default")
try:
print("case=", file, "default_lookup_type=", type(default).__name__, "default_lookup_is_function=", callable(default), "default_lookup_args=", getattr(default, "__name__", None))
except Exception as exc:
print("case=", file, "default_lookup_type=ERROR", str(exc))
print()
print("JSON parser behavior for constructor/toString/__proto__:")
samples = [
'{"profiles": {"default": "constructor"}}',
'{"profiles": {"default": "toString"}}',
'{"profiles": {"default": "__proto__"}}',
'{"profiles": {"default": "valueOf"}}',
'{"profiles": {"default": "valueOf", "valueOf": "not a profile"}}',
]
for raw in samples:
data = json.loads(raw)
p = data.get("profiles")
print(raw, ">", p.get("default"), "type", type(p.get("default")).__name__, "hasOwnProperty default", ({}).__class__.__name__ if "default" in p else "missing")
print(" default in p:", "default" in p)
PYRepository: ronak-create/FableCut
Length of output: 1337
Keep default IDs safe from inherited object properties.
profiles[defaultId] and cfg.profiles[pid] use plain-object lookup, so a user-supplied profile ID such as toString, constructor, hasOwnProperty, or valueOf can satisfy the lookup without an associated normalized profile. This can pass missing/incorrect profile metadata through export, since profile.args is assumed to exist.
Proposed fix
- const profiles = {};
+ const profiles = Object.create(null);📝 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.
Suggested change
| const profiles = {}; | |
| if (file?.profiles && typeof file.profiles === "object") | |
| for (const [id, raw] of Object.entries(file.profiles)) profiles[id] = normalizeProfile(id, raw); | |
| const profiles = Object.create(null); | |
| if (file?.profiles && typeof file.profiles === "object") | |
| for (const [id, raw] of Object.entries(file.profiles)) profiles[id] = normalizeProfile(id, raw); |
🤖 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 `@encode-profiles.js` around lines 80 - 82, Update the profile lookup logic in
the export flow and the profiles configuration handling to verify that IDs such
as defaultId and pid are own properties of the profiles object before using
their values. Preserve normal normalized-profile behavior, but treat
inherited-property matches as missing so profile.args is never read from an
unassociated entry.
Comment on lines
+42
to
+49
| "args": [ | ||
| "-c:v", "libx264", "-preset", "veryfast", "-crf", "18", "-pix_fmt", "yuv420p", | ||
| "-g", "10", "-maxrate", "60M", "-bufsize", "70M", | ||
| "-x264opts", "open_gop=0:interlaced=1:me=hex:subme=2:cabac=1:no-deblock:analyse=all:8x8dct=1:bframes=2:weightp=0:sliced-threads:level=41", | ||
| "-vf", "scale=1920:1080:interl=-1,fps=50,tinterlace=interleave_top:bypass_il", | ||
| "-c:a", "aac", "-b:a", "384k", "-ar", "48000", "-strict", "-2", | ||
| "-shortest" | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Configure +faststart for both MOV profiles.
broadcast1080i50 and prores422 omit -movflags +faststart, unlike the MP4 profiles. Add it explicitly before -shortest so these exported MOV files retain the configured playback-optimized layout. As per coding guidelines, encoding profiles must explicitly configure required flags such as +faststart, -shortest, and pixel format.
Also applies to: 56-59
🤖 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 `@encoding-profiles.json` around lines 42 - 49, Update both the
broadcast1080i50 and prores422 encoding profiles to explicitly include
“-movflags”, “+faststart” immediately before “-shortest” in their args arrays,
preserving the existing required flags and ordering otherwise.
Source: Coding guidelines
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add encoding presets for ffmpeg
Add ffmpeg export profiles that live in
encoding-profiles.json. During 'fast' export a profile can be set. The existing profile is called Delivery · H.264 balanced, three other profiles are added.Type of change
How was it verified?
node --check server.js && node --check app.js && node --check mcp-server.jspassesCLAUDE.md/README.mdif the schema, props, or API changedChecklist
Summary by CodeRabbit
New Features
Documentation