Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 144 additions & 0 deletions apps/web/src/app/api/convert-video/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { type NextRequest, NextResponse } from "next/server";
import { writeFile, readFile, rm, mkdtemp } from "fs/promises";
import { spawn } from "child_process";
import path from "path";
import os from "os";

const GENERIC_CONVERSION_ERROR = "Video conversion failed.";

function sanitizeContentDispositionName({
rawName,
}: {
rawName: string;
}): string {
// Strip characters that can break the Content-Disposition header (quotes,
// backslashes, CR/LF, control bytes) and replace non-ASCII so the
// "filename=" parameter stays unambiguous across clients.
const stripped = rawName
// eslint-disable-next-line no-control-regex
.replace(/[\\"\r\n\x00-\x1f]/g, "_")
.replace(/[^\x20-\x7e]/g, "_")
.slice(0, 200);
return stripped.length > 0 ? stripped : "video";
}

export async function POST(request: NextRequest) {
let workDir: string | null = null;

try {
const formData = await request.formData();
const file = formData.get("file") as File | null;

if (!file || !(file instanceof File)) {
return NextResponse.json(
{ error: "No file provided" },
{ status: 400 },
);
}

workDir = await mkdtemp(path.join(os.tmpdir(), "opencut-convert-"));
const ext = path.extname(file.name) || ".mp4";
const inputPath = path.join(workDir, `input${ext}`);
const outputPath = path.join(workDir, "output.mp4");

const buffer = Buffer.from(await file.arrayBuffer());
await writeFile(inputPath, buffer);
Comment on lines +29 to +45

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add an explicit upload size limit before buffering.

await file.arrayBuffer() loads the entire payload into memory. A single oversized upload can exhaust memory and degrade availability. Reject large files early with 413.

🔧 Proposed fix
 const GENERIC_CONVERSION_ERROR = "Video conversion failed.";
+const MAX_UPLOAD_BYTES = 250 * 1024 * 1024; // adjust to product limit

 export async function POST(request: NextRequest) {
@@
 		if (!file || !(file instanceof File)) {
@@
 		}
+
+		if (file.size > MAX_UPLOAD_BYTES) {
+			return NextResponse.json(
+				{ error: "File too large" },
+				{ status: 413 },
+			);
+		}
 
 		workDir = await mkdtemp(path.join(os.tmpdir(), "opencut-convert-"));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/app/api/convert-video/route.ts` around lines 29 - 45, The route
currently calls await file.arrayBuffer() which can OOM for large uploads; add an
explicit upload size check before buffering: define a MAX_UPLOAD_SIZE constant
(e.g., in route.ts), check file.size (or fallback to
request.headers.get('content-length')) and if it exceeds the limit return
NextResponse.json({ error: "File too large" }, { status: 413 }); only proceed to
Buffer.from(await file.arrayBuffer()) and writeFile(inputPath, ...) when the
size is within limits. Target symbols: request.formData, the File object from
formData.get("file"), file.arrayBuffer(), and the inputPath/outputPath handling.


await runFfmpeg({
inputPath,
outputPath,
});

const convertedBuffer = await readFile(outputPath);

const safeName = sanitizeContentDispositionName({
rawName: path.basename(file.name, ext),
});

return new Response(new Uint8Array(convertedBuffer), {
status: 200,
headers: {
"Content-Type": "video/mp4",
"Content-Disposition": `attachment; filename="${safeName}.mp4"`,
},
});
} catch (error) {
console.error("Video conversion error:", error);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use the project logger instead of console.error.

Switch this to your structured logger to satisfy linting and keep observability consistent.

As per coding guidelines, "Don't use console".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/app/api/convert-video/route.ts` at line 66, Replace the raw
console.error call in the convert-video route (the console.error("Video
conversion error:", error) line) with the project's structured logger (e.g.,
logger.error or processLogger.error) and pass the same message plus the error
object as metadata; import the logger used across the project (or reuse the
existing processLogger) at the top of route.ts, remove the console usage, and
keep the message "Video conversion error" while ensuring the error is supplied
as the second argument to the logger call so structured fields are preserved.

return NextResponse.json(
{ error: GENERIC_CONVERSION_ERROR },
{ status: 500 },
);
} finally {
if (workDir) {
await rm(workDir, { recursive: true, force: true }).catch(() => {});
}
}
}

function runFfmpeg({
inputPath,
outputPath,
}: {
inputPath: string;
outputPath: string;
}): Promise<void> {
return new Promise((resolve, reject) => {
const proc = spawn("ffmpeg", [
"-i",
inputPath,
"-c:v",
"libx264",
"-profile:v",
"baseline",
"-level",
"3.1",
"-preset",
"veryfast",
"-crf",
"23",
"-pix_fmt",
"yuv420p",
"-g",
"60",
"-keyint_min",
"60",
"-sc_threshold",
"0",
"-movflags",
"+faststart",
"-tag:v",
"avc1",
"-c:a",
"aac",
"-ac",
"2",
"-ar",
"48000",
"-b:a",
"128k",
"-y",
outputPath,
]);

let stderr = "";
proc.stderr.on("data", (data) => {
stderr += data.toString();
});

proc.on("close", (code) => {
if (code === 0) {
resolve();
} else {
reject(
new Error(
`ffmpeg exited with code ${code}. stderr: ${stderr.slice(-500)}`,
),
);
}
});

proc.on("error", (err) => {
reject(err);
});
});
Comment on lines +85 to +143

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

fd -t f "route.ts" --path "*convert-video*"

Repository: OpenCut-app/OpenCut

Length of output: 297


🏁 Script executed:

cat -n apps/web/src/app/api/convert-video/route.ts | head -160

Repository: OpenCut-app/OpenCut

Length of output: 4170


Enforce a timeout/abort path for ffmpeg execution.

If ffmpeg hangs, this request will stall indefinitely with no cleanup. Add a hard timeout that kills the process and rejects.

🔧 Proposed fix
 function runFfmpeg({
 	inputPath,
 	outputPath,
 }: {
 	inputPath: string;
 	outputPath: string;
 }): Promise<void> {
 	return new Promise((resolve, reject) => {
+		const FFMPEG_TIMEOUT_MS = 120_000;
 		const proc = spawn("ffmpeg", [
@@
 			outputPath,
 		]);
+		const timeout = setTimeout(() => {
+			proc.kill("SIGKILL");
+			reject(new Error(`ffmpeg timed out after ${FFMPEG_TIMEOUT_MS}ms`));
+		}, FFMPEG_TIMEOUT_MS);
 
 		let stderr = "";
 		proc.stderr.on("data", (data) => {
 			stderr += data.toString();
 		});
 
 		proc.on("close", (code) => {
+			clearTimeout(timeout);
 			if (code === 0) {
 				resolve();
 			} else {
@@
 		});
 
 		proc.on("error", (err) => {
+			clearTimeout(timeout);
 			reject(err);
 		});
 	});
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return new Promise((resolve, reject) => {
const proc = spawn("ffmpeg", [
"-i",
inputPath,
"-c:v",
"libx264",
"-profile:v",
"baseline",
"-level",
"3.1",
"-preset",
"veryfast",
"-crf",
"23",
"-pix_fmt",
"yuv420p",
"-g",
"60",
"-keyint_min",
"60",
"-sc_threshold",
"0",
"-movflags",
"+faststart",
"-tag:v",
"avc1",
"-c:a",
"aac",
"-ac",
"2",
"-ar",
"48000",
"-b:a",
"128k",
"-y",
outputPath,
]);
let stderr = "";
proc.stderr.on("data", (data) => {
stderr += data.toString();
});
proc.on("close", (code) => {
if (code === 0) {
resolve();
} else {
reject(
new Error(
`ffmpeg exited with code ${code}. stderr: ${stderr.slice(-500)}`,
),
);
}
});
proc.on("error", (err) => {
reject(err);
});
});
return new Promise((resolve, reject) => {
const FFMPEG_TIMEOUT_MS = 120_000;
const proc = spawn("ffmpeg", [
"-i",
inputPath,
"-c:v",
"libx264",
"-profile:v",
"baseline",
"-level",
"3.1",
"-preset",
"veryfast",
"-crf",
"23",
"-pix_fmt",
"yuv420p",
"-g",
"60",
"-keyint_min",
"60",
"-sc_threshold",
"0",
"-movflags",
"+faststart",
"-tag:v",
"avc1",
"-c:a",
"aac",
"-ac",
"2",
"-ar",
"48000",
"-b:a",
"128k",
"-y",
outputPath,
]);
const timeout = setTimeout(() => {
proc.kill("SIGKILL");
reject(new Error(`ffmpeg timed out after ${FFMPEG_TIMEOUT_MS}ms`));
}, FFMPEG_TIMEOUT_MS);
let stderr = "";
proc.stderr.on("data", (data) => {
stderr += data.toString();
});
proc.on("close", (code) => {
clearTimeout(timeout);
if (code === 0) {
resolve();
} else {
reject(
new Error(
`ffmpeg exited with code ${code}. stderr: ${stderr.slice(-500)}`,
),
);
}
});
proc.on("error", (err) => {
clearTimeout(timeout);
reject(err);
});
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/app/api/convert-video/route.ts` around lines 85 - 143, The
ffmpeg spawn Promise (the block that creates proc via spawn("ffmpeg", ...))
lacks a timeout/abort path so a hung ffmpeg will stall forever; implement a hard
timeout by starting a timer (e.g. setTimeout) after spawning proc that, when
fired, kills proc (proc.kill('SIGKILL') or similar) and rejects the Promise with
a descriptive Error, and ensure you clear that timer in both proc.on("close")
and proc.on("error") handlers to avoid leaks; keep using the existing stderr
capture and include the timeout case in the rejection message so callers can
distinguish timeouts from normal ffmpeg failures.

}
95 changes: 90 additions & 5 deletions apps/web/src/core/managers/audio-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import {
Input,
type WrappedAudioBuffer,
} from "mediabunny";
import {
type AudioBufferSinkLike,
FallbackAudioBufferSink,
} from "@/services/audio-cache/fallback-audio-buffer-sink";

export class AudioManager {
private audioContext: AudioContext | null = null;
Expand All @@ -30,7 +34,7 @@ export class AudioManager {
private lookaheadSeconds = 2;
private scheduleIntervalMs = 500;
private clips: AudioClipSource[] = [];
private sinks = new Map<string, AudioBufferSink>();
private sinks = new Map<string, AudioBufferSinkLike>();
private inputs = new Map<string, Input>();
private activeClipIds = new Set<string>();
private clipIterators = new Map<
Expand Down Expand Up @@ -267,7 +271,42 @@ export class AudioManager {

const iterator = sink.buffers(sourceStartTime);
this.clipIterators.set(clip.id, iterator);
let consecutiveDroppedBufferCount = 0;

try {
await this.consumeClipIterator({
clip,
clipEnd,
iterator,
audioContext,
sessionId,
initialDroppedCount: 0,
});
} catch (error) {
console.warn(
`[AudioManager] clip iterator failed for ${clip.id}; aborting playback for this clip.`,
error,
);
Comment on lines +285 to +288

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use the project logger instead of console.warn in new fallback paths.

Please route these warnings through the existing logging/telemetry abstraction to keep lint/compliance and runtime logging behavior consistent.

As per coding guidelines, "Don't use console".

Also applies to: 655-657, 755-757

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/core/managers/audio-manager.ts` around lines 285 - 288, Replace
the console.warn calls in AudioManager’s clip iterator fallback paths with the
project logger: change the console.warn lines (the one shown and the other two
occurrences around the clip iterator/fallback logic) to use the AudioManager
logger (e.g., this.logger.warn(...) or logger.warn(...) consistent with how
other methods in AudioManager use the logger) and pass the error object into the
logger call; if a logger instance/import is not present in the file, import or
initialize the same logger used by this class (e.g., getLogger('AudioManager')
or the existing logger symbol) so all three fallback warning sites use the
project logging abstraction instead of console.

this.clipIterators.delete(clip.id);
this.activeClipIds.delete(clip.id);
}
}

private async consumeClipIterator({
clip,
clipEnd,
iterator,
audioContext,
sessionId,
initialDroppedCount,
}: {
clip: AudioClipSource;
clipEnd: number;
iterator: AsyncGenerator<WrappedAudioBuffer, void, unknown>;
audioContext: AudioContext;
sessionId: number;
initialDroppedCount: number;
}): Promise<void> {
let consecutiveDroppedBufferCount = initialDroppedCount;

for await (const { buffer, timestamp } of iterator) {
if (!this.editor.playback.getIsPlaying()) return;
Expand Down Expand Up @@ -605,7 +644,18 @@ export class AudioManager {
return null;
}

const sink = new AudioBufferSink(audioTrack);
const canDecode = await safeCanDecode({ audioTrack });
const sink: AudioBufferSinkLike = canDecode
? new AudioBufferSink(audioTrack)
: await FallbackAudioBufferSink.create({
file: clip.file,
audioContext,
});
if (!canDecode) {
console.warn(
"[AudioManager] WebCodecs unavailable; decoding clip audio via decodeAudioData.",
);
}
const chunks: AudioBuffer[] = [];
let totalSamples = 0;

Expand Down Expand Up @@ -673,12 +723,13 @@ export class AudioManager {
clip,
}: {
clip: AudioClipSource;
}): Promise<AudioBufferSink | null> {
}): Promise<AudioBufferSinkLike | null> {
const existingSink = this.sinks.get(clip.sourceKey);
if (existingSink) return existingSink;

let input: Input | null = null;
try {
const input = new Input({
input = new Input({
source: new BlobSource(clip.file),
formats: ALL_FORMATS,
});
Expand All @@ -688,13 +739,47 @@ export class AudioManager {
return null;
}

const canDecode = await safeCanDecode({ audioTrack });
if (!canDecode) {
input.dispose();
input = null;
const audioContext = this.ensureAudioContext();
if (!audioContext) {
return null;
}
const fallback = await FallbackAudioBufferSink.create({
file: clip.file,
audioContext,
});
this.sinks.set(clip.sourceKey, fallback);
console.warn(
"[AudioManager] WebCodecs unavailable; using decodeAudioData fallback for audio clip.",
);
return fallback;
}

const sink = new AudioBufferSink(audioTrack);
this.inputs.set(clip.sourceKey, input);
this.sinks.set(clip.sourceKey, sink);
return sink;
} catch (error) {
if (input) {
input.dispose();
}
console.warn("Failed to initialize audio sink:", error);
return null;
}
}
}

async function safeCanDecode({
audioTrack,
}: {
audioTrack: { canDecode: () => Promise<boolean> };
}): Promise<boolean> {
try {
return await audioTrack.canDecode();
} catch {
return false;
}
}
22 changes: 21 additions & 1 deletion apps/web/src/media/audio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import { canTrackHaveAudio } from "@/timeline";
import { mediaSupportsAudio } from "@/media/media-utils";
import { getSourceTimeAtClipTime, renderRetimedBuffer } from "@/retime";
import { Input, ALL_FORMATS, BlobSource, AudioBufferSink } from "mediabunny";
import {
type AudioBufferSinkLike,
FallbackAudioBufferSink,
} from "@/services/audio-cache/fallback-audio-buffer-sink";
import { TICKS_PER_SECOND } from "@/wasm";
import {
computeRmsBuckets,
Expand Down Expand Up @@ -271,7 +275,23 @@ async function resolveAudioBufferForAsset({
const audioTrack = await input.getPrimaryAudioTrack();
if (!audioTrack) return null;

const sink = new AudioBufferSink(audioTrack);
let audioTrackCanDecode = false;
try {
audioTrackCanDecode = await audioTrack.canDecode();
} catch {
audioTrackCanDecode = false;
}
const sink: AudioBufferSinkLike = audioTrackCanDecode
? new AudioBufferSink(audioTrack)
: await FallbackAudioBufferSink.create({
file: asset.file,
audioContext,
});
if (!audioTrackCanDecode) {
console.warn(
"[audio] WebCodecs unavailable; decoding asset via decodeAudioData fallback.",
);
Comment on lines +291 to +293

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Replace new console.warn fallback log with the shared logger.

Use the same centralized logger path used elsewhere instead of direct console output.

As per coding guidelines, "Don't use console".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/media/audio.ts` around lines 291 - 293, Replace the direct
console.warn call in apps/web/src/media/audio.ts with the project’s centralized
logger (use the same logger symbol used elsewhere, e.g., logger or
processLogger) by importing or referencing that shared logger and calling
logger.warn("[audio] WebCodecs unavailable; decoding asset via decodeAudioData
fallback."); ensure no direct console usage remains in that module and keep the
message text identical when switching to the shared logger.

}
const targetSampleRate = audioContext.sampleRate;

const chunks: AudioBuffer[] = [];
Expand Down
22 changes: 22 additions & 0 deletions apps/web/src/media/ffmpeg-convert.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export async function convertVideoToH264({
file,
}: {
file: File;
}): Promise<File> {
const formData = new FormData();
formData.append("file", file);

const response = await fetch("/api/convert-video", {
method: "POST",
body: formData,
});

if (!response.ok) {
const errorData = await response.json().catch(() => ({}));
throw new Error(errorData.error || `Conversion failed: ${response.status}`);
}

const blob = await response.blob();
const baseName = file.name.replace(/\.[^.]+$/, "");
return new File([blob], `${baseName}.mp4`, { type: "video/mp4" });
}
Loading