Skip to content

fix: cleanup FFmpeg temp files after export#514

Open
divyanshi-adhikari wants to merge 9 commits into
magic-peach:mainfrom
divyanshi-adhikari:fix/ffmpeg-temp-cleanup
Open

fix: cleanup FFmpeg temp files after export#514
divyanshi-adhikari wants to merge 9 commits into
magic-peach:mainfrom
divyanshi-adhikari:fix/ffmpeg-temp-cleanup

Conversation

@divyanshi-adhikari
Copy link
Copy Markdown

Fix: Cleanup FFmpeg temporary files after export

Problem

While exporting videos using FFmpeg WASM, temporary input/output files were being created inside the virtual filesystem but were not always properly deleted after the export process.

This caused:

  • Accumulation of temporary files across multiple exports
  • Increased memory usage over time
  • Potential risk of file conflicts in repeated exports

What was changed

  • Ensured input file is always deleted after export using finally block
  • Added safe cleanup for output files
  • Added fallback cleanup for failed export scenarios
  • Improved reliability of FFmpeg virtual filesystem management

How it was tested

  • Ran multiple export cycles consecutively
  • Verified no leftover files remain in FFmpeg virtual filesystem
  • Checked memory usage in Chrome DevTools Performance Monitor
  • Confirmed export works correctly in both success and fallback cases

Issue reference

Fixes #11

@vercel
Copy link
Copy Markdown

vercel Bot commented May 16, 2026

@divyanshi-adhikari is attempting to deploy a commit to the magic-peach1's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 16, 2026

✅ PR Format Check Passed — @divyanshi-adhikari

Basic format checks passed. A maintainer will review your code changes.

This does not mean the PR is approved — it just means the format is correct.

@github-actions
Copy link
Copy Markdown
Contributor

👋 Thanks for your PR, @divyanshi-adhikari!

Welcome to Reframe — a browser-based video editor built for everyone 🎬

What happens next

  1. 🤖 Automated checks — build & TypeScript typecheck will run automatically
  2. Vercel preview — a preview deployment will be created (requires maintainer authorization for fork PRs)
  3. 👀 Code review — a maintainer will review your changes
  4. 🚀 Merge — once approved, your PR will be merged!

Quick checklist

  • PR title follows Conventional Commits (e.g. feat: add dark mode)
  • Linked the issue this PR closes (e.g. Closes #123)
  • Tested the changes locally (bun run dev)
  • Build passes (bun run build)

Useful links

Happy coding! 🎉

@github-actions github-actions Bot added level:intermediate Intermediate level - 35 pts type:bug Bug fix type:performance Performance type:refactor Code refactor labels May 16, 2026
@magic-peach magic-peach added the gssoc'26 GirlScript Summer of Code 2026 label May 17, 2026
@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! Cleaning up FFmpeg temp files is a great idea, but the implementation has a significant regression.

Issues:

  1. Removes multi-format support — The original code supported webm, mkv, and mp4 export formats via getOutputConfig(). Your PR removes this entirely and hardcodes output to output.mp4 only, breaking WebM and MKV export.

  2. Removes abort signal from fetchFile — The line await ffmpeg.writeFile(inputName, await fetchFile(file)) drops the { signal } parameter, which means the file write can't be cancelled anymore.

  3. Simplified filenames can cause collisions — Using input. and output.mp4 as fixed names means concurrent exports (or rapid re-exports) could clobber each other. The original sessionId-based names prevented this.

What to do:

  • Keep the original getOutputConfig() and session-based filename approach
  • Only add cleanup logic (e.g., ffmpeg.deleteFile() in the finally block) without changing the existing logic
  • Keep the { signal } parameter in the fetchFile call

Thanks for working on this — just needs a more surgical fix!

@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! Thank you for working on FFmpeg temp file cleanup — that's a real issue. However, this PR has significant regressions:

  1. Removes multi-format export support: The original code handles MP4, WebM, and MKV output formats. Your version only outputs MP4 (hardcoded output.mp4). This breaks the format selector UI.

  2. Removes abort signal from file write: await ffmpeg.writeFile(inputName, await fetchFile(file)) drops the { signal } parameter — this means cancelling an export won't interrupt file writing, making cancellation unreliable.

  3. Removes session-based filenames: The session ID in filenames prevents collisions if multiple exports run. Hardcoding input.ext / output.mp4 could cause issues.

The cleanup intent is valid — if you want to add ffmpeg.deleteFile() calls after export, please do so without removing the multi-format support, abort signals, or session IDs.

@divyanshi-adhikari divyanshi-adhikari force-pushed the fix/ffmpeg-temp-cleanup branch from ab9a19c to 7dd6c76 Compare May 17, 2026 07:48
@github-actions github-actions Bot added the level:advanced Advanced level - 55 pts label May 17, 2026
@divyanshi-adhikari
Copy link
Copy Markdown
Author

Thanks for the review. I’ve updated the PR as per your feedback and reverted the unintended changes.

The multi-format support, session-based filenames, and abort signal handling are all preserved now, and I’ve only added the FFmpeg cleanup logic in the finally block.

@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! The build is now passing on this PR, but it has merge conflicts with main that need to be resolved. Please rebase your branch:

git fetch upstream
git rebase upstream/main
# resolve any conflicts
git push --force-with-lease origin <your-branch>

Once the conflicts are resolved and CI passes on the updated branch, this will be reviewed for merge. Thanks!

@magic-peach magic-peach removed level:intermediate Intermediate level - 35 pts type:refactor Code refactor labels May 17, 2026
@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! Cleaning up FFmpeg temp files is a good memory management improvement. This PR has merge conflicts with main. Please rebase:

git fetch origin
git rebase origin/main
git push --force-with-lease

@github-actions github-actions Bot added level:intermediate Intermediate level - 35 pts type:refactor Code refactor labels May 17, 2026
@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! This PR needs a rebase on the latest main to trigger fresh CI checks. Please run:

git fetch origin
git rebase origin/main
git push --force-with-lease

Once CI (build/lint/typecheck) passes, we'll review and merge!

@magic-peach magic-peach removed the level:advanced Advanced level - 55 pts label May 18, 2026
@github-actions github-actions Bot added the level:advanced Advanced level - 55 pts label May 18, 2026
@magic-peach magic-peach removed the level:advanced Advanced level - 55 pts label May 18, 2026
@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! This PR looks interesting (FFmpeg temp file cleanup). The build/lint/typecheck CI hasn't run yet. Please rebase onto the latest main to trigger fresh checks:

git fetch upstream && git rebase upstream/main && git push --force-with-lease

Once CI passes, we'll review and merge!

@github-actions github-actions Bot added the level:advanced Advanced level - 55 pts label May 18, 2026
@github-actions github-actions Bot added the level:beginner Beginner level - 20 pts label May 18, 2026
@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! 👋

We've added a new requirement for all PRs: a screen recording showing your changes working on your local machine must be attached before a PR can be merged.

Please add a recording to this PR that shows:

  1. bun run dev running at http://localhost:3000
  2. The full working flow of your change (demonstrate the feature/fix end-to-end)
  3. Any tests passing — if your change touches logic with tests, show bun run lint and bunx tsc --noEmit completing without errors in the terminal

How to record:

  • macOS: Cmd + Shift + 5 → Record Selected Portion, or QuickTime Player
  • Windows: Win + G → Xbox Game Bar → Capture
  • Linux: OBS Studio, GNOME Screenshot, or kazam
  • Any OS: Loom (free, easy to share)

Once you have the recording, drag the file directly into a comment on this PR, or paste a Loom link. This is now a hard requirement — see CONTRIBUTING.md for full details.

Thanks for contributing to Reframe! 🎬

@divyanshi-adhikari
Copy link
Copy Markdown
Author

Hi @magic-peach
Here is the screen recording demonstrating the changes working locally:

ffmpeg.ts.-.reframe.-.Visual.Studio.Code.2026-05-19.14-32-00.online-video-cutter.com.mp4

@divyanshi-adhikari
Copy link
Copy Markdown
Author

Hi @magic-peach I hope you are doing well.

Sorry if I’m bothering you. I just wanted to kindly ask if you could give me a brief idea of when this PR might be merged. I would be really grateful.

Thank you.

@divyanshi-adhikari
Copy link
Copy Markdown
Author

Can you please add gssoc:approved label to it.

@magic-peach magic-peach removed level:beginner Beginner level - 20 pts level:advanced Advanced level - 55 pts labels May 23, 2026
@magic-peach
Copy link
Copy Markdown
Owner

Hey @divyanshi-adhikari! Cleaning up FFmpeg temp files after export is an important fix to prevent OPFS/memory leaks. This PR has merge conflicts with main that need to be resolved:

git fetch origin
git rebase origin/main
# resolve any conflicts
git push --force-with-lease

Once rebased and CI (build/lint/typecheck) passes, this will be reviewed for merging. Thanks!

@divyanshi-adhikari
Copy link
Copy Markdown
Author

Hi @magic-peach I have resolved the conflict. And also, can you please add gssoc:approved tag instead of gssoc'26 I be really grateful.

Thank you!

@magic-peach
Copy link
Copy Markdown
Owner

@divyanshi-adhikari Cleaning up FFmpeg temp files with a finally block is an important fix to prevent memory leaks on repeated exports! Please resolve the merge conflicts with main:

git fetch origin
git rebase origin/main
git push --force-with-lease

Regarding the gssoc:approved label — that can only be added by a human maintainer after reviewing the contribution. Once CI passes and conflicts are resolved, we'll flag this for maintainer review to get it approved.

Once CI (build/lint/typecheck) passes, this will be ready for merge!

@divyanshi-adhikari divyanshi-adhikari force-pushed the fix/ffmpeg-temp-cleanup branch 2 times, most recently from 366cc89 to 74ce116 Compare May 24, 2026 11:06
@divyanshi-adhikari
Copy link
Copy Markdown
Author

Hi @magic-peach, I hope you are doing great.

I have solved the conflicts. Please check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc'26 GirlScript Summer of Code 2026 level:intermediate Intermediate level - 35 pts type:bug Bug fix type:performance Performance type:refactor Code refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] FFmpeg virtual filesystem accumulates temp files between exports

2 participants