From de61e94cba5bb6ffde2d0d11e0d2a79e105b6842 Mon Sep 17 00:00:00 2001 From: Jonas Rohde Date: Fri, 1 May 2026 17:22:26 +0200 Subject: [PATCH] refactor: route media subprocesses through adapter --- docs/repo_refresh_audit.md | 10 +++++++ src/domain/export_image.ts | 38 +++++++++++------------- src/domain/export_video.ts | 52 +++++++++++++++------------------ src/domain/media_inspector.ts | 28 ++++++++---------- src/domain/media_process.ts | 27 +++++++++++++++++ src/domain/objective_metrics.ts | 40 ++++++++++++------------- 6 files changed, 108 insertions(+), 87 deletions(-) create mode 100644 src/domain/media_process.ts diff --git a/docs/repo_refresh_audit.md b/docs/repo_refresh_audit.md index be5cbdb..e6c8a9e 100644 --- a/docs/repo_refresh_audit.md +++ b/docs/repo_refresh_audit.md @@ -154,6 +154,16 @@ Rationale: The smallest useful fix is to centralize value extraction and validat Consequence: Commands still own their own required-argument checks and range checks. A deeper parser abstraction can wait until there is a stronger reason to collapse command-specific parsing further. +### 2026-05-01: media subprocess adapter + +Context: The audit identified direct `Bun.spawnSync` calls for ffmpeg/ffprobe in production domain modules. + +Decision: Add `src/domain/media_process.ts` with `runFfmpeg` and `runFfprobe`, then route export, inspection, and objective-metric subprocess calls through it. + +Rationale: This isolates process execution without changing ffmpeg argument construction or domain behavior. It is the smallest useful boundary for future missing-binary handling and subprocess tests. + +Consequence: Fixture-generation scripts still call `Bun.spawnSync` directly because they are test tooling, not product domain behavior. + ## Sources - [Instagram Help Center](https://www.facebook.com/help/1631821640426723/) diff --git a/src/domain/export_image.ts b/src/domain/export_image.ts index 566bdd3..b370da2 100644 --- a/src/domain/export_image.ts +++ b/src/domain/export_image.ts @@ -2,6 +2,7 @@ import { mkdirSync } from "node:fs"; import { dirname, resolve } from "node:path"; import type { ExportImageInput, ExportImageOutput } from "../types/contracts"; import { loadExportProfiles, selectImageExportProfile } from "./export_profiles"; +import { runFfmpeg } from "./media_process"; import { inspectMedia } from "./media_inspector"; import { recommend } from "./recommend"; import { parseResolution } from "./rules"; @@ -71,29 +72,24 @@ export function exportImage(input: ExportImageInput): ExportImageOutput { const outputPath = resolve(input.out); mkdirSync(dirname(outputPath), { recursive: true }); - const proc = Bun.spawnSync({ - cmd: [ - "ffmpeg", - "-y", - "-hide_banner", - "-loglevel", - "error", - "-i", - inputPath, - "-vf", - filter, - "-frames:v", - "1", - "-q:v", - String(quality), - outputPath, - ], - stdout: "pipe", - stderr: "pipe", - }); + const proc = runFfmpeg([ + "-y", + "-hide_banner", + "-loglevel", + "error", + "-i", + inputPath, + "-vf", + filter, + "-frames:v", + "1", + "-q:v", + String(quality), + outputPath, + ]); if (proc.exitCode !== 0) { - throw new Error(`ffmpeg export failed: ${proc.stderr.toString().trim()}`); + throw new Error(`ffmpeg export failed: ${proc.stderr.trim()}`); } return { diff --git a/src/domain/export_video.ts b/src/domain/export_video.ts index 71c68b7..15dd05d 100644 --- a/src/domain/export_video.ts +++ b/src/domain/export_video.ts @@ -2,6 +2,7 @@ import { mkdirSync } from "node:fs"; import { dirname, resolve } from "node:path"; import type { ExportVideoInput, ExportVideoOutput } from "../types/contracts"; import { loadExportProfiles, selectVideoExportProfile } from "./export_profiles"; +import { runFfmpeg } from "./media_process"; import { inspectMedia } from "./media_inspector"; import { recommend } from "./recommend"; import { parseResolution } from "./rules"; @@ -72,36 +73,31 @@ export function exportVideo(input: ExportVideoInput): ExportVideoOutput { mkdirSync(dirname(outputPath), { recursive: true }); const fps = media.fps > 0 ? media.fps : 30; - const proc = Bun.spawnSync({ - cmd: [ - "ffmpeg", - "-y", - "-hide_banner", - "-loglevel", - "error", - "-i", - inputPath, - "-vf", - filter, - "-r", - String(fps), - "-c:v", - exportProfile.ffmpeg_video_codec, - "-crf", - String(crf), - "-pix_fmt", - exportProfile.pix_fmt, - "-movflags", - exportProfile.movflags, - ...(exportProfile.strip_audio ? ["-an"] : []), - outputPath, - ], - stdout: "pipe", - stderr: "pipe", - }); + const proc = runFfmpeg([ + "-y", + "-hide_banner", + "-loglevel", + "error", + "-i", + inputPath, + "-vf", + filter, + "-r", + String(fps), + "-c:v", + exportProfile.ffmpeg_video_codec, + "-crf", + String(crf), + "-pix_fmt", + exportProfile.pix_fmt, + "-movflags", + exportProfile.movflags, + ...(exportProfile.strip_audio ? ["-an"] : []), + outputPath, + ]); if (proc.exitCode !== 0) { - throw new Error(`ffmpeg video export failed: ${proc.stderr.toString().trim()}`); + throw new Error(`ffmpeg video export failed: ${proc.stderr.trim()}`); } const outputMeta = inspectMedia(outputPath); diff --git a/src/domain/media_inspector.ts b/src/domain/media_inspector.ts index 4855a33..89de6db 100644 --- a/src/domain/media_inspector.ts +++ b/src/domain/media_inspector.ts @@ -1,6 +1,7 @@ import { resolve } from "node:path"; import { readFileSync } from "node:fs"; import type { MediaInspection, Orientation } from "../types/contracts"; +import { runFfprobe } from "./media_process"; function nextToken(source: Uint8Array, state: { index: number }): string | null { while (state.index < source.length) { @@ -259,28 +260,23 @@ function readVideoMetadata(path: string): { audioSampleFormat: string | null; audioBitrateKbps: number | null; } { - const proc = Bun.spawnSync({ - cmd: [ - "ffprobe", - "-v", - "error", - "-show_entries", - "stream=codec_type,codec_name,width,height,avg_frame_rate,r_frame_rate,channels,channel_layout,sample_fmt,sample_rate,bit_rate:format=duration,bit_rate", - "-of", - "json", - path, - ], - stdout: "pipe", - stderr: "pipe", - }); + const proc = runFfprobe([ + "-v", + "error", + "-show_entries", + "stream=codec_type,codec_name,width,height,avg_frame_rate,r_frame_rate,channels,channel_layout,sample_fmt,sample_rate,bit_rate:format=duration,bit_rate", + "-of", + "json", + path, + ]); if (proc.exitCode !== 0) { - throw new Error(`ffprobe failed for ${path}: ${proc.stderr.toString().trim()}`); + throw new Error(`ffprobe failed for ${path}: ${proc.stderr.trim()}`); } let parsed: unknown; try { - parsed = JSON.parse(proc.stdout.toString()); + parsed = JSON.parse(proc.stdout); } catch { throw new Error(`ffprobe returned invalid JSON for ${path}`); } diff --git a/src/domain/media_process.ts b/src/domain/media_process.ts new file mode 100644 index 0000000..52aacdb --- /dev/null +++ b/src/domain/media_process.ts @@ -0,0 +1,27 @@ +export type MediaProcessResult = { + exitCode: number; + stdout: string; + stderr: string; +}; + +function runMediaProcess(cmd: string[]): MediaProcessResult { + const proc = Bun.spawnSync({ + cmd, + stdout: "pipe", + stderr: "pipe", + }); + + return { + exitCode: proc.exitCode, + stdout: proc.stdout.toString(), + stderr: proc.stderr.toString(), + }; +} + +export function runFfmpeg(args: string[]): MediaProcessResult { + return runMediaProcess(["ffmpeg", ...args]); +} + +export function runFfprobe(args: string[]): MediaProcessResult { + return runMediaProcess(["ffprobe", ...args]); +} diff --git a/src/domain/objective_metrics.ts b/src/domain/objective_metrics.ts index 4fe9d5d..10f6686 100644 --- a/src/domain/objective_metrics.ts +++ b/src/domain/objective_metrics.ts @@ -1,4 +1,5 @@ import { resolve } from "node:path"; +import { runFfmpeg } from "./media_process"; export type ObjectiveMetrics = { psnrDb: number | null; @@ -40,36 +41,31 @@ export function computeObjectiveMetrics(inputPath: string, outputPath: string): const inFile = resolve(inputPath); const outFile = resolve(outputPath); - const proc = Bun.spawnSync({ - cmd: [ - "ffmpeg", - "-hide_banner", - "-loglevel", - "info", - "-i", - inFile, - "-i", - outFile, - "-filter_complex", - "[0:v][1:v]scale2ref=flags=bicubic[dist][ref];[dist]split[dist_psnr][dist_ssim];[ref]split[ref_psnr][ref_ssim];[dist_psnr][ref_psnr]psnr;[dist_ssim][ref_ssim]ssim", - "-an", - "-f", - "null", - "-", - ], - stdout: "pipe", - stderr: "pipe", - }); + const proc = runFfmpeg([ + "-hide_banner", + "-loglevel", + "info", + "-i", + inFile, + "-i", + outFile, + "-filter_complex", + "[0:v][1:v]scale2ref=flags=bicubic[dist][ref];[dist]split[dist_psnr][dist_ssim];[ref]split[ref_psnr][ref_ssim];[dist_psnr][ref_psnr]psnr;[dist_ssim][ref_ssim]ssim", + "-an", + "-f", + "null", + "-", + ]); if (proc.exitCode !== 0) { return { psnrDb: null, ssim: null, - note: `objective metrics unavailable: ${proc.stderr.toString().trim()}`, + note: `objective metrics unavailable: ${proc.stderr.trim()}`, }; } - const stderr = proc.stderr.toString(); + const stderr = proc.stderr; const psnrDb = parsePsnrAverage(stderr); const ssim = parseSsimAll(stderr);