Skip to content

fix(producer): wire --crf and --video-bitrate CLI overrides into encoders#372

Open
vanceingalls wants to merge 2 commits intomainfrom
vance/cli-encode-overrides
Open

fix(producer): wire --crf and --video-bitrate CLI overrides into encoders#372
vanceingalls wants to merge 2 commits intomainfrom
vance/cli-encode-overrides

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Apr 21, 2026

Summary

Re-wire the --crf and --video-bitrate CLI flags through the three encoder spawn sites in renderOrchestrator.ts. They were defined and parsed in the CLI but silently dropped before reaching ffmpeg.

Why

Chunk 10 of plans/hdr-followups.md. PR #292 originally wired these through with a baseEncoderOpts object using effectiveQuality/effectiveBitrate; PR #268 rewrote the encode paths and reverted to preset.quality only, accidentally dropping the override. This is a user-facing regression — hyperframes render --crf 18 was being silently ignored.

What changed

  • At the three encoder spawn sites (HDR streaming, SDR streaming, disk-based encode), quality defaults to preset.quality but is overridden by job.config.crf when set, and bitrate is set from job.config.videoBitrate. Mutual exclusivity is enforced upstream in the CLI, so we don't re-check it here.
  • Fix the contradictory note in docs/packages/cli.mdx that claimed CRF/bitrate were now driven only by --quality. The flags table now lists --crf and --video-bitrate consistent with docs/guides/rendering.mdx.

Test plan

  • hyperframes render --crf 18 ... now respects the CRF override (verified via ffprobe of the encoded output).
  • hyperframes render --hdr ... still works (no behavior change at the default path).
  • hyperframes render --help shows all flags consistent with the docs.

Stack

Chunk 10 of plans/hdr-followups.md. Independent of all other chunks.

Copy link
Copy Markdown
Collaborator Author

vanceingalls commented Apr 21, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vanceingalls vanceingalls changed the base branch from vance/resource-management to graphite-base/372 April 21, 2026 20:43
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from bc48483 to 57d6459 Compare April 21, 2026 20:48
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 57d6459 to 9b1f281 Compare April 21, 2026 20:54
@vanceingalls vanceingalls marked this pull request as ready for review April 21, 2026 20:57
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 9b1f281 to 9199f7c Compare April 22, 2026 01:16
@graphite-app graphite-app Bot changed the base branch from graphite-base/372 to vance/resource-management April 22, 2026 22:47
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 9934545 to 8418f43 Compare April 22, 2026 23:26
@vanceingalls vanceingalls force-pushed the vance/resource-management branch from 1060004 to dd57087 Compare April 22, 2026 23:59
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 8418f43 to 0922d9a Compare April 23, 2026 00:00
vanceingalls added a commit that referenced this pull request Apr 23, 2026
Programmatic callers can construct RenderConfig directly and bypass the
CLI's mutual-exclusivity guard for --crf vs --video-bitrate. Add an
orchestrator-level check that logs a warning and explicitly nulls out
videoBitrate when crf is also set, so the encoder gets unambiguous
inputs and downstream users aren't confused by a quietly-different
bitrate than they passed in.

Addresses non-blocking review feedback on PR #372.
@vanceingalls vanceingalls force-pushed the vance/resource-management branch from dd57087 to 9490d71 Compare April 23, 2026 00:04
vanceingalls added a commit that referenced this pull request Apr 23, 2026
Programmatic callers can construct RenderConfig directly and bypass the
CLI's mutual-exclusivity guard for --crf vs --video-bitrate. Add an
orchestrator-level check that logs a warning and explicitly nulls out
videoBitrate when crf is also set, so the encoder gets unambiguous
inputs and downstream users aren't confused by a quietly-different
bitrate than they passed in.

Addresses non-blocking review feedback on PR #372.
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 0922d9a to c1e9467 Compare April 23, 2026 00:05
@vanceingalls vanceingalls force-pushed the vance/resource-management branch from 9490d71 to fd205f3 Compare April 23, 2026 00:45
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from c1e9467 to ebef5d5 Compare April 23, 2026 00:45
@vanceingalls vanceingalls force-pushed the vance/resource-management branch from fd205f3 to 658ed49 Compare April 23, 2026 01:58
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from ebef5d5 to 3643a05 Compare April 23, 2026 01:58
@vanceingalls vanceingalls force-pushed the vance/resource-management branch from 658ed49 to 4fd6596 Compare April 23, 2026 02:52
vanceingalls added a commit that referenced this pull request Apr 23, 2026
Programmatic callers can construct RenderConfig directly and bypass the
CLI's mutual-exclusivity guard for --crf vs --video-bitrate. Add an
orchestrator-level check that logs a warning and explicitly nulls out
videoBitrate when crf is also set, so the encoder gets unambiguous
inputs and downstream users aren't confused by a quietly-different
bitrate than they passed in.

Addresses non-blocking review feedback on PR #372.
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 3643a05 to 7468d97 Compare April 23, 2026 02:53
@vanceingalls vanceingalls force-pushed the vance/resource-management branch from 4fd6596 to 0947ad8 Compare April 23, 2026 03:15
vanceingalls added a commit that referenced this pull request Apr 23, 2026
Programmatic callers can construct RenderConfig directly and bypass the
CLI's mutual-exclusivity guard for --crf vs --video-bitrate. Add an
orchestrator-level check that logs a warning and explicitly nulls out
videoBitrate when crf is also set, so the encoder gets unambiguous
inputs and downstream users aren't confused by a quietly-different
bitrate than they passed in.

Addresses non-blocking review feedback on PR #372.
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 7468d97 to 3196e2c Compare April 23, 2026 03:16
@vanceingalls vanceingalls force-pushed the vance/resource-management branch 2 times, most recently from b7b30c6 to 504e005 Compare April 23, 2026 03:37
vanceingalls added a commit that referenced this pull request Apr 23, 2026
Programmatic callers can construct RenderConfig directly and bypass the
CLI's mutual-exclusivity guard for --crf vs --video-bitrate. Add an
orchestrator-level check that logs a warning and explicitly nulls out
videoBitrate when crf is also set, so the encoder gets unambiguous
inputs and downstream users aren't confused by a quietly-different
bitrate than they passed in.

Addresses non-blocking review feedback on PR #372.
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 3196e2c to 34926b3 Compare April 23, 2026 03:37
@vanceingalls vanceingalls changed the base branch from vance/resource-management to graphite-base/372 April 23, 2026 04:45
…ders

The CLI flags `--crf` and `--video-bitrate` were defined and parsed in
`packages/cli/src/commands/render.ts`, validated for mutual exclusivity,
and threaded into `RenderConfig.crf`/`RenderConfig.videoBitrate`, but
the values were silently dropped at the encoder spawn sites in
`renderOrchestrator.ts`. PR #292 originally wired these through with a
`baseEncoderOpts` object using `effectiveQuality`/`effectiveBitrate`;
PR #268 rewrote the encode paths and reverted to `preset.quality` only.

This change re-introduces the override at the three encoder spawn
sites:

  1. HDR streaming encoder (rgb48le path)
  2. SDR streaming encoder (jpeg/png path)
  3. Disk-based encode (encodeFramesFromDir / encodeFramesChunkedConcat)

At each site, `quality` defaults to `preset.quality` but is overridden
by `job.config.crf` when set, and `bitrate` is set from
`job.config.videoBitrate`. Mutual exclusivity is enforced upstream in
the CLI, so we do not need to re-check it here.

Also fixes the contradictory note in `docs/packages/cli.mdx` that
claimed CRF/bitrate were now driven only by `--quality`. The flags
table now lists `--crf` and `--video-bitrate` consistent with
`docs/guides/rendering.mdx`.
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 34926b3 to 86e4952 Compare April 23, 2026 04:46
vanceingalls added a commit that referenced this pull request Apr 23, 2026
Programmatic callers can construct RenderConfig directly and bypass the
CLI's mutual-exclusivity guard for --crf vs --video-bitrate. Add an
orchestrator-level check that logs a warning and explicitly nulls out
videoBitrate when crf is also set, so the encoder gets unambiguous
inputs and downstream users aren't confused by a quietly-different
bitrate than they passed in.

Addresses non-blocking review feedback on PR #372.
@graphite-app graphite-app Bot changed the base branch from graphite-base/372 to main April 23, 2026 04:46
Programmatic callers can construct RenderConfig directly and bypass the
CLI's mutual-exclusivity guard for --crf vs --video-bitrate. Add an
orchestrator-level check that logs a warning and explicitly nulls out
videoBitrate when crf is also set, so the encoder gets unambiguous
inputs and downstream users aren't confused by a quietly-different
bitrate than they passed in.

Addresses non-blocking review feedback on PR #372.
@vanceingalls vanceingalls force-pushed the vance/cli-encode-overrides branch from 86e4952 to e7bae09 Compare April 23, 2026 04:46
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.

2 participants