Skip to content

refactor(producer): extract encodeStage and assembleStage#734

Open
jrusso1020 wants to merge 1 commit into
05-12-refactor_producer_extract_capturehdrstage_hdr___shader-transition_path_from
05-12-refactor_producer_extract_encodestage_and_assemblestage
Open

refactor(producer): extract encodeStage and assembleStage#734
jrusso1020 wants to merge 1 commit into
05-12-refactor_producer_extract_capturehdrstage_hdr___shader-transition_path_from
05-12-refactor_producer_extract_encodestage_and_assemblestage

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 12, 2026

Stacked on #725#726#730#731#733 → this. Rebase once ancestors merge.

What

Phase 1 PR 1.9 of the distributed-render refactor. Two small extractions for the final two stages:

  • encodeStage.ts (Stage 5): handles both branches of the post-capture encode step:
    • png-sequence: rename captured PNGs to frame_NNNNNN.png, copy to outputPath, write audio.aac sidecar if audio is present.
    • encoded output: invokes encodeFramesFromDir or (when enableChunkedEncode is on) encodeFramesChunkedConcat.
  • assembleStage.ts (Stage 6): mux video + audio (muxVideoWithAudio) when hasAudio, otherwise applyFaststart. Skipped entirely for png-sequence (the sequencer gates the call).

Why

These are the final two capture-adjacent stages. After this PR, the only work left in executeRenderJob is the in-sequencer setup (worker calibration, preset selection, etc.) and the eight stage calls. PR 1.10 reduces the sequencer to a thin composition.

How

  • encodeStage.ts exports runEncodeStage(input) → { encodeMs }. Body covers both branches (png copy vs. ffmpeg encode). Sequencer calls it with the same condition gating (isPngSequence ? png-path : encoded-path is inside the stage now).
  • assembleStage.ts exports runAssembleStage(input) → { assembleMs }. Body is the if (hasAudio) ... else ... mux/faststart pair.
  • Sequencer replaces both inline blocks with the stage calls and assigns perfStages.encodeMs / perfStages.assembleMs from the results.
  • Drops the now-orphaned imports from the orchestrator: encodeFramesFromDir, encodeFramesChunkedConcat, muxVideoWithAudio, applyFaststart.

Preserved invariants

  • The updateJobStatus payloads fire from inside the stages at the same code points: "Writing PNG sequence" / "Encoding video" at 75%, "Assembling final video" at 90%.
  • The "png-sequence output requested but no PNGs were captured to ..." error throws verbatim.
  • The png-sequence audio sidecar is only written when hasAudio && existsSync(audioOutputPath).
  • The chunked-concat vs. direct encode selection is identical (enableChunkedEncode flag, same args).
  • "Audio muxing failed: " / "Faststart failed: " throw verbatim on the respective success: false results.

Test plan

  • bunx oxlint + bunx oxfmt --check — clean.
  • bun run --filter @hyperframes/producer typecheck + build — clean.
  • bun test packages/producer/src/services/ — 176 pass, same pre-existing failure.
  • Docker fixtures (5/5 PASS): font-variant-numeric (1.000), many-cuts (0.994), sub-composition-video (0.947), gsap-letters-render-compat (1.000), hdr-regression (1.000). Between them they exercise the encoded mp4 path (encode + assemble both run), the streaming-fusion path (encode is skipped by the sequencer when streaming succeeded), and the HDR path (encode is also skipped because the HDR stage produces videoOnlyPath directly).
  • Full regression matrix on CI via the regression workflow.

🤖 Generated with Claude Code

Move the final two stages of `executeRenderJob` into their own files:

- `services/render/stages/encodeStage.ts` (Stage 5): handles both the
  png-sequence path (rename + copy + audio sidecar) and the encoded path
  (`encodeFramesFromDir` or `encodeFramesChunkedConcat`).
- `services/render/stages/assembleStage.ts` (Stage 6): runs
  `muxVideoWithAudio` when `hasAudio`, otherwise `applyFaststart`.
  Skipped for png-sequence (sequencer gates the call).

Both stages are mechanical extractions of small, self-contained blocks.
The sequencer's call sites preserve the same conditions and the same
`perfStages.encodeMs` / `perfStages.assembleMs` assignments.

Hard constraints preserved verbatim:
- The `updateJobStatus` payloads ("Writing PNG sequence" / "Encoding
  video" at 75%; "Assembling final video" at 90%) fire from inside the
  stages at the same code points.
- The png-sequence "no PNGs were captured" error throws verbatim.
- The png-sequence audio sidecar is only written when
  `hasAudio && existsSync(audioOutputPath)`.
- `enableChunkedEncode` selects `encodeFramesChunkedConcat` vs.
  `encodeFramesFromDir` with the same args.
- The mux + faststart error messages (`Audio muxing failed: ...`,
  `Faststart failed: ...`) throw verbatim on `success: false`.

Removes the now-orphaned imports from the orchestrator:
`encodeFramesFromDir`, `encodeFramesChunkedConcat`, `muxVideoWithAudio`,
`applyFaststart`.

Verified inside `Dockerfile.test`:
- font-variant-numeric (1.000), many-cuts (0.994),
  sub-composition-video (0.947), gsap-letters-render-compat (1.000),
  hdr-regression (1.000) — 5/5 PASS, audio correlations identical to
  prior PRs in the stack. Exercises encoded mp4 + HDR (encode + assemble
  both run) and the streaming-fusion path (encode skipped by sequencer).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls 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.

Bundles two stages (encodeStage, assembleStage) into one PR. Both are byte-clean extractions, each independent.

encodeStage (Stage 5):

  • Two branches — png-sequence (copy + rename + audio sidecar) and encoded (mp4 / webm / mov via encodeFramesFromDir or encodeFramesChunkedConcat). Both lifted verbatim.
  • The "png-sequence output requested but no PNGs were captured" throw preserved; the enableChunkedEncode branch selector preserved; the Encoding failed: <err> throw preserved.
  • updateJobStatus(... "encoding", "Writing PNG sequence" / "Encoding video", 75, ...) payload fires at the same point — inside the stage, at 75%.
  • Skip when streamingHandled === true — gated correctly on the sequencer side.

assembleStage (Stage 6):

  • hasAudio ? muxVideoWithAudio : applyFaststart branch preserved verbatim with the same throw messages (Audio muxing failed: <err> / Faststart failed: <err>).
  • Stage is correctly skipped for png-sequence in the sequencer — same if (!isPngSequence) guard as pre-extraction.
  • updateJobStatus(... "assembling", "Assembling final video", 90, ...) payload preserved at 90% from inside the stage.

The bundling is justified. Both stages are post-capture, both are skip-aware (encodeStage skipped by streaming fusion; assembleStage skipped by png-sequence), neither has internal state worth a third file. Splitting them across two PRs would be churn for no review benefit. The stage interfaces are independent — assembleStage doesn't depend on anything encodeStage returns beyond what was already in the sequencer's scope.

Nits:

  • encodeStage.ts:53,69,103fps: Fps and useGpu: boolean | undefined are passed in, but #735 immediately drops them in favor of reading job.config.fps and job.config.useGpu directly. Worth folding into this PR instead of a separate cleanup.
  • Same runtime cycle pattern as the other stages (imports updateJobStatus from the orchestrator). #737 resolves it.

Praise: assembleStage is the shortest stage in the stack — 70 lines including the JSDoc — and reads end-to-end. The pattern of preserving every updateJobStatus call site inside the stage at the same progress percentage keeps perfStages and the on-screen progress bar in lock-step across the migration. Easy to verify.

— Vai

Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

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

Clean mechanical extraction — no behavior changes, no introduced bugs. Verified imports, error handling, and cleanup invariants are preserved. LGTM. — Magi

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