fix: upload rendered video to fal.ai storage instead of base64#84
fix: upload rendered video to fal.ai storage instead of base64#84sweetmantech merged 2 commits intomainfrom
Conversation
The base64 data URL conversion was causing OOM crashes on small-1x (512MB). Now uploads the rendered MP4 to fal.ai storage and returns an HTTPS URL, eliminating the ~2.5x memory inflation from base64. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes replace a data URL-based video output with fal.ai storage uploads. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/renderFinalVideo.ts`:
- Around line 191-205: The function currently returns { videoUrl, mimeType,
sizeBytes } but the type/contract and docstring (RenderFinalVideoOutput) still
reference dataUrl; update the output contract and consumers to match the new
storage URL flow: change RenderFinalVideoOutput to include videoUrl (string)
instead of dataUrl and update its docstring to describe an HTTPS URL, and then
update any callers (e.g., the consumer in src/content/testPipeline.ts that does
result.dataUrl.split(",")[1]) to read result.videoUrl (and stop treating it as a
base64 data URL). Ensure all references to dataUrl are removed or migrated to
videoUrl so types and runtime behavior align.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2fd43df8-7c6a-46a4-95cc-34e993f5dab4
📒 Files selected for processing (3)
src/content/renderFinalVideo.tssrc/tasks/__tests__/createContentTask.test.tssrc/tasks/createContentTask.ts
| // Read the final video and upload to fal.ai storage (avoids base64 OOM) | ||
| const finalBuffer = await readFile(outputPath); | ||
| const mimeType = "video/mp4"; | ||
| const dataUrl = `data:${mimeType};base64,${finalBuffer.toString("base64")}`; | ||
| const sizeBytes = finalBuffer.length; | ||
|
|
||
| logger.log("Final video rendered", { sizeBytes: finalBuffer.length }); | ||
| logger.log("Final video rendered, uploading to fal.ai storage", { sizeBytes }); | ||
|
|
||
| const videoFile = new File([finalBuffer], "final-video.mp4", { type: "video/mp4" }); | ||
| const videoUrl = await fal.storage.upload(videoFile); | ||
|
|
||
| logger.log("Final video uploaded to fal.ai storage", { videoUrl, sizeBytes }); | ||
|
|
||
| return { | ||
| dataUrl, | ||
| mimeType, | ||
| sizeBytes: finalBuffer.length, | ||
| videoUrl, | ||
| mimeType: "video/mp4", | ||
| sizeBytes, |
There was a problem hiding this comment.
Fix the output contract drift (dataUrl vs videoUrl) before merge.
Lines 203-205 return videoUrl, but RenderFinalVideoOutput still defines dataUrl and the docstring still describes data-URL output. This creates a broken typed contract and leaves at least one consumer (src/content/testPipeline.ts:255-267) using result.dataUrl.split(",")[1], which will fail with an HTTPS URL.
Proposed fix
export interface RenderFinalVideoOutput {
- /** Data URL of the final rendered video */
- dataUrl: string;
+ /** HTTPS URL of the final rendered video in fal.ai storage */
+ videoUrl: string;
/** MIME type */
mimeType: string;
/** Size in bytes */
sizeBytes: number;
}
/**
* Renders the final social post video using ffmpeg:
* 1. Downloads the AI-generated video
* 2. Crops 16:9 → 9:16 (portrait for TikTok/Reels)
* 3. Overlays audio clip from the song (unless lipsync mode)
* 4. Overlays caption text (white with black stroke, bottom center)
- * 5. Returns the final video as a data URL
+ * 5. Uploads the final MP4 to fal.ai storage and returns an HTTPS URL
*/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/content/renderFinalVideo.ts` around lines 191 - 205, The function
currently returns { videoUrl, mimeType, sizeBytes } but the type/contract and
docstring (RenderFinalVideoOutput) still reference dataUrl; update the output
contract and consumers to match the new storage URL flow: change
RenderFinalVideoOutput to include videoUrl (string) instead of dataUrl and
update its docstring to describe an HTTPS URL, and then update any callers
(e.g., the consumer in src/content/testPipeline.ts that does
result.dataUrl.split(",")[1]) to read result.videoUrl (and stop treating it as a
base64 data URL). Ensure all references to dataUrl are removed or migrated to
videoUrl so types and runtime behavior align.
Summary
videoSourceUrl) instead of adata:video/mp4;base64,...stringTASK_PROCESS_OOM_KILLEDon small-1x (512MB)Why
The base64 encoding held the raw buffer (~20MB) + base64 string (~27MB) + serialization copy (~27MB) simultaneously, exceeding the 512MB limit. With fal.ai storage upload, peak memory is just the raw buffer (~20MB) which streams out immediately.
Test plan
videoSourceUrlis an HTTPS URL, not base64🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests