From 83d11ac5957b03d251a40a0543a2bc8f78b37ada Mon Sep 17 00:00:00 2001 From: James Date: Fri, 27 Mar 2026 03:40:35 +0000 Subject: [PATCH] refactor: simplify review fixes for WebM PR - Use static import for copyFileSync (was unnecessary dynamic import) - Shallow-copy config before mutating forceScreenshot (prevents caller-provided config from being permanently modified) - Consolidate isWebm/isWebmRender/outputFormat into single early declaration in renderOrchestrator - Fix debug output extension for WebM (was hardcoded .mp4) - Log unexpected audio extraction errors instead of silently swallowing Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/engine/src/services/chunkEncoder.ts | 4 +--- packages/producer/src/regression-harness.ts | 8 ++++++-- .../src/services/renderOrchestrator.ts | 19 +++++++++---------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/engine/src/services/chunkEncoder.ts b/packages/engine/src/services/chunkEncoder.ts index fc2f95203..34286efd7 100644 --- a/packages/engine/src/services/chunkEncoder.ts +++ b/packages/engine/src/services/chunkEncoder.ts @@ -6,7 +6,7 @@ */ import { spawn } from "child_process"; -import { existsSync, mkdirSync, readdirSync, statSync, writeFileSync } from "fs"; +import { copyFileSync, existsSync, mkdirSync, readdirSync, statSync, writeFileSync } from "fs"; import { join, dirname } from "path"; import { DEFAULT_CONFIG, type EngineConfig } from "../config.js"; import { type GpuEncoder, getCachedGpuEncoder, getGpuEncoderName } from "../utils/gpuEncoder.js"; @@ -414,8 +414,6 @@ export async function applyFaststart( ): Promise { // faststart is MP4-only (moves moov atom to file start for streaming) if (outputPath.endsWith(".webm")) { - // For WebM, just copy the file as-is - const { copyFileSync } = await import("fs"); if (inputPath !== outputPath) copyFileSync(inputPath, outputPath); return { success: true, outputPath, durationMs: 0 }; } diff --git a/packages/producer/src/regression-harness.ts b/packages/producer/src/regression-harness.ts index 6f30440ec..9161e2c34 100644 --- a/packages/producer/src/regression-harness.ts +++ b/packages/producer/src/regression-harness.ts @@ -333,8 +333,12 @@ function extractMonoPcm16(videoPath: string): Int16Array { return new Int16Array(0); } return new Int16Array(stdout.buffer, stdout.byteOffset, Math.floor(stdout.byteLength / 2)); - } catch { - // No audio stream in the video (e.g., WebM without audio) + } catch (err) { + // No audio stream (e.g., WebM without audio) — log but don't fail + const msg = err instanceof Error ? err.message : String(err); + if (!msg.includes("does not contain any stream")) { + logPretty(`Audio extraction warning: ${msg.slice(0, 200)}`, "⚠️"); + } return new Int16Array(0); } } diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index 693d2e2dd..b278d7033 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -300,9 +300,11 @@ export async function executeRenderJob( let restoreLogger: (() => void) | null = null; const perfStages: Record = {}; const perfOutputPath = join(workDir, "perf-summary.json"); - const cfg = job.config.producerConfig ?? resolveConfig(); + const cfg = { ...(job.config.producerConfig ?? resolveConfig()) }; + const outputFormat = (job.config.format ?? "mp4") as "mp4" | "webm"; + const isWebm = outputFormat === "webm"; // WebM/transparency requires screenshot mode — beginFrame doesn't support alpha channel - if (job.config.format === "webm") { + if (isWebm) { cfg.forceScreenshot = true; } const enableChunkedEncode = cfg.enableChunkedEncode; @@ -363,7 +365,6 @@ export async function executeRenderJob( }); assertNotAborted(); - const isWebm = job.config.format === "webm"; const captureOpts: CaptureOptions = { width, height, @@ -600,19 +601,17 @@ export async function executeRenderJob( const framesDir = join(workDir, "captured-frames"); if (!existsSync(framesDir)) mkdirSync(framesDir, { recursive: true }); - const outputFormat = job.config.format ?? "mp4"; - const isWebmRender = outputFormat === "webm"; const captureOptions: CaptureOptions = { width, height, fps: job.config.fps, - format: isWebmRender ? "png" : "jpeg", - quality: isWebmRender ? undefined : job.config.quality === "draft" ? 80 : 95, + format: isWebm ? "png" : "jpeg", + quality: isWebm ? undefined : job.config.quality === "draft" ? 80 : 95, }; const workerCount = calculateOptimalWorkers(job.totalFrames!, job.config.workers, cfg); - const videoExt = isWebmRender ? ".webm" : ".mp4"; + const videoExt = isWebm ? ".webm" : ".mp4"; const videoOnlyPath = join(workDir, `video-only${videoExt}`); const preset = getEncoderPreset(job.config.quality, outputFormat); @@ -846,7 +845,7 @@ export async function executeRenderJob( const stage5Start = Date.now(); updateJobStatus(job, "encoding", "Encoding video", 75, onProgress); - const frameExt = isWebmRender ? "png" : "jpg"; + const frameExt = isWebm ? "png" : "jpg"; const framePattern = `frame_%06d.${frameExt}`; const encoderOpts = { fps: job.config.fps, @@ -962,7 +961,7 @@ export async function executeRenderJob( if (job.config.debug) { // Copy output MP4 into debug dir for easy access if (existsSync(outputPath)) { - const debugOutput = join(workDir, "output.mp4"); + const debugOutput = join(workDir, isWebm ? "output.webm" : "output.mp4"); copyFileSync(outputPath, debugOutput); } } else {