Skip to content

fix(engine): accept libx264 preset names with NVENC and QSV#442

Merged
jrusso1020 merged 1 commit intoheygen-com:mainfrom
roiizchak:fix/nvenc-qsv-preset-mapping
Apr 23, 2026
Merged

fix(engine): accept libx264 preset names with NVENC and QSV#442
jrusso1020 merged 1 commit intoheygen-com:mainfrom
roiizchak:fix/nvenc-qsv-preset-mapping

Conversation

@roiizchak
Copy link
Copy Markdown
Contributor

What

Fixes hyperframes render --gpu when the selected quality tier uses a libx264 preset name that NVENC / QSV don't accept. In particular, --quality draft (which maps to ultrafast) now works on NVENC; previously it failed with FFmpeg exited with code -22 and no further diagnostics.

Also surfaces ffmpeg stderr in encoder error messages so the next time a caller hands ffmpeg an arg it rejects, the error string actually says which arg.

Why

ENCODER_PRESETS in chunkEncoder.ts passes the libx264 preset names (ultrafast / medium / slow) straight through to h264_nvenc / hevc_nvenc. NVENC rejects the libx264 vocabulary — it only accepts p1..p7 — so spawn returns AVERROR(EINVAL). On ffmpeg 8.1 that surfaces as exit code -22, and buildEncoderArgs / buildStreamingArgs previously swallowed stderr, so the ffmpeg-level message was invisible:

[h264_nvenc @ 0x...] Error applying encoder options: Invalid argument
Error while opening encoder

So every hyperframes render --gpu --quality draft failed with a bare FFmpeg exited with code -22 and no path forward. QSV has the same problem on a narrower set: it rejects ultrafast, superfast, and placebo.

How

Preset mapping (packages/engine/src/utils/gpuEncoder.ts)

New mapPresetForGpuEncoder(encoder, preset) helper:

  • nvenc: libx264 names → p1..p7. Native pN values pass through. Unknown values fall back to p4 (medium).
  • qsv: ultrafast / superfastveryfast; placeboveryslow. Everything else passes through.
  • videotoolbox / vaapi / null: unchanged (they either ignore -preset or accept the libx264 vocabulary).

buildEncoderArgs (chunkEncoder.ts) and buildStreamingArgs (streamingEncoder.ts) route through the helper before pushing -preset onto the ffmpeg arg vector.

Stderr tail (packages/engine/src/utils/runFfmpeg.ts)

New formatFfmpegError(exitCode, stderr) helper that appends the last 15 non-empty stderr lines to the error message. Adopted in the four call sites that previously produced FFmpeg exited with code N with no context:

  • encodeFramesFromDir (chunkEncoder.ts)
  • muxVideoWithAudio (chunkEncoder.ts)
  • applyFaststart (chunkEncoder.ts)
  • Streaming encoder exit handler (streamingEncoder.ts)

If it turns out this second change feels out of scope, happy to split it into its own PR — it's orthogonal to the preset fix but it's the diagnostic that made the root cause findable in the first place.

Test plan

  • Unit tests added/updated — 46 new tests:
    • utils/gpuEncoder.test.ts (31) — all NVENC / QSV / passthrough mappings + fallback
    • utils/runFfmpeg.test.ts (6) — stderr tail formatting, blank-line handling, null exit code
    • services/chunkEncoder.test.ts (+5) — asserts the correct -preset pN / -preset veryfast lands in the arg vector for each quality tier under NVENC and QSV
    • services/streamingEncoder.test.ts (+4) — same for streaming path
  • Full engine test suite clean: bun run --filter @hyperframes/engine test → 359 pass / 0 fail / 3 skipped. (services/videoFrameExtractor.test.ts fails on my Windows box with a fontconfig error synthesizing the VFR fixture — same failure repros on main at HEAD, unrelated to this PR.)
  • Typecheck clean: bun run --filter @hyperframes/engine typecheck
  • Lint clean: bun run lint
  • Format clean on the 8 files touched: oxfmt --check passes. (The full bun run format:check flags ~610 pre-existing files on main, untouched here.)
  • Manual end-to-end on RTX 4080 + ffmpeg 8.1 NVENC: six renders on a 3-second smoke composition covering --quality draft|standard|high, --video-bitrate 10M, and --crf 25. All rendered in 7–11s; GPU frames spot-checked and visually identical to the CPU baseline.
  • Documentation updated (if applicable) — no doc changes needed; the behavior being fixed matches the existing docs.

NVENC rejects the libx264 preset vocabulary (ultrafast / medium / slow /
...) with AVERROR(EINVAL) ("Error applying encoder options: Invalid
argument"), which surfaces as a bare `FFmpeg exited with code -22` from
spawn(). Because ENCODER_PRESETS passes these names straight through to
h264_nvenc / hevc_nvenc, every `--gpu` render using the `draft` tier
failed; `standard` (medium) and `high` (slow) only worked coincidentally
on ffmpeg builds that happened to accept those aliases. QSV has the same
problem on a narrower set (ultrafast / superfast / placebo).

Add `mapPresetForGpuEncoder` in utils/gpuEncoder.ts that translates the
libx264 vocabulary to each encoder's native names:

- nvenc: libx264 -> p1..p7 (already-native pN values pass through);
  unknown values fall back to p4 (medium)
- qsv:   ultrafast / superfast -> veryfast; placebo -> veryslow;
  everything else passes through
- videotoolbox / vaapi / null: unchanged

Both buildEncoderArgs (chunkEncoder.ts) and buildStreamingArgs
(streamingEncoder.ts) now route through the helper before pushing
`-preset` to the ffmpeg arg vector.

To make the next encoder-options failure diagnosable without re-running
ffmpeg by hand, \`formatFfmpegError\` in utils/runFfmpeg.ts now appends
the last 15 non-empty stderr lines to the error string. The four call
sites that previously swallowed stderr (encodeFramesFromDir,
muxVideoWithAudio, applyFaststart, and the streaming encoder exit
handler) have been updated.

Tested end-to-end on an RTX 4080 with ffmpeg 8.1 NVENC across
\`--quality draft|standard|high\` plus \`--video-bitrate\` and \`--crf\`
overrides; the 6 renders were visually equivalent to the CPU baseline.
@jrusso1020 jrusso1020 self-requested a review April 23, 2026 15:13
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve

Tight, surgical fix for a real user-visible bug. Root cause is correctly identified (NVENC rejects libx264 preset vocabulary — p1..p7 only — and QSV rejects a narrower set of three), the fix sits at the right boundary (translate once, before the arg vector), and the blast radius is GPU-path-only with CPU completely untouched.

What's right

  • Translation at the correct layer. mapPresetForGpuEncoder runs at the -preset arg push site in both buildEncoderArgs (chunkEncoder.ts:118) and buildStreamingArgs (streamingEncoder.ts:200), not inside the encoder spawn. That keeps the internal preset model in libx264 vocabulary and translates once at the edge — exactly the right seam.
  • NVENC mapping is conventional. ultrafast → p1 / medium → p4 / slow → p5 / veryslow → p7 is the analog FFmpeg/Nvidia have established, and the pN passthrough via /^p[1-7]$/ ensures callers who already speak NVENC-native don't get clobbered. Unknown-preset fallback to p4 is the right conservative default — silent "best guess" instead of propagating a value that'll fail spawn.
  • QSV mapping is appropriately minimal. Only rewrites the three documented-unsupported names (ultrafast, superfast, placebo); everything else passes through. Avoids building an exhaustive table that would need maintenance as QSV evolves.
  • Stderr tail helper is the diagnostic that would have made this findable in the first place. formatFfmpegError(exitCode, stderr, 15) filters blank lines so real signal isn't hidden, caps the line count, and handles the null-exit-code spawn-failure case distinctly from a numeric exit. Adopted consistently at all four sites that previously produced bare FFmpeg exited with code N (encodeFramesFromDir, muxVideoWithAudio, applyFaststart, streaming exit handler). Not scope creep — it's the companion diagnostic that should have shipped alongside the runFfmpeg abstraction originally. Happy to see it in the same PR.
  • Test coverage is excellent. 31 parametrized preset-mapping cases (gpuEncoder.test.ts), both NVENC and QSV integration assertions on buildEncoderArgs / buildStreamingArgs confirming the correct -preset pN / -preset veryfast lands in the actual arg array, and edge cases on stderr tail (blank-line strip, line cap, null exit code, empty stderr).

Minor

  • hevc_nvenc also uses p1..p7 and the fix correctly applies to both H.264 and H.265 NVENC paths — but the integration tests only cover codec: "h264". NVENC's preset vocabulary is codec-agnostic so the mapping is correct for both, but one codec: "h265" case in chunkEncoder.test.ts would lock in "both codecs share the mapping" against a future refactor. Non-blocking.
  • Stderr tail default is 15 lines. Fine for the "Error applying encoder options" class of failures; for denser signals (HDR color-space, VFR fallback) callers can bump via the third arg. No action needed.

Risk

Trivial. Blast radius = GPU path only; CPU path untouched. pN passthrough + QSV-supported passthrough means existing working configs are unaffected. Revert is a one-line-per-site change. No schema / config / migration surface.

CI hasn't run on this fork PR yet (only the WIP marketplace check has ticked through) — worth confirming regression-shards, Tests, and Render on windows-latest pass once a maintainer CI-approves authorizes them to run before merging.


Review by hyperframes

@jrusso1020 jrusso1020 merged commit 3b8de7a into heygen-com:main Apr 23, 2026
27 checks passed
jrusso1020 pushed a commit that referenced this pull request Apr 23, 2026
hevc_nvenc uses the same p1..p7 preset vocabulary as h264_nvenc, so the
mapping in `mapPresetForGpuEncoder` applies to both codecs. The initial
regression suite only covered `codec: "h264"`, which left a gap: a
future refactor that split the H.264 and H.265 NVENC paths could
silently regress one codec without any test catching it.

Add three-case loops (ultrafast → p1, medium → p4, veryslow → p7) under
`codec: "h265"` to both `buildEncoderArgs` and `buildStreamingArgs`
test blocks. Each case also asserts that `-c:v hevc_nvenc` is selected
so the test fails loudly if the codec plumbing is broken, not just the
preset translation.

Follow-up to #442 per review comment from @jrusso1020.

Co-authored-by: roi32 <75878108+roi32@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants