Repository navigation
fix(web): make the MP4 to MP3 converter write MP3 - #3427
patrickballard wants to merge 2 commits into
Conversation
The MP4 to MP3 tool asked Remotion for a WAV container and downloaded the result as .mp3. Convert with mediabunny, which the web app already uses, to an MP3 output encoded by its LAME extension, since browsers can't encode MP3 through WebCodecs. A video without an audio track now gets an error instead of an empty file.
|
@patrickballard is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| parserControllerRef.current = { | ||
| abort: () => { | ||
| void conversion.cancel(); | ||
| }, |
There was a problem hiding this comment.
Switching formats during MP3 conversion calls conversion.cancel(), which rejects the running conversion with ConversionCanceledError. The shared catch only checks MediaParser.hasBeenAborted, so users see “Conversion failed” and the page sends media_format_conversion_failed for work they intentionally stopped. Recognize ConversionCanceledError or translate it to the existing cancellation error before rethrowing.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/components/tools/MediaFormatConverter.tsx
Line: 436-439
Comment:
**Cancellation shows a failure**
Switching formats during MP3 conversion calls `conversion.cancel()`, which rejects the running conversion with `ConversionCanceledError`. The shared catch only checks `MediaParser.hasBeenAborted`, so users see “Conversion failed” and the page sends `media_format_conversion_failed` for work they intentionally stopped. Recognize `ConversionCanceledError` or translate it to the existing cancellation error before rethrowing.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in adf49e1. Switching formats mid-conversion now shows "Conversion was cancelled" and no longer tracks a failure. A cancel mid-write rejects with mediabunny's "Output has been canceled." rather than ConversionCanceledError (seen in Chromium and in the test), so the converter rethrows any error after a cancel as ConversionCanceledError and the catch recognises it by name, as the Loom import does. A new test switches to GIF during a real conversion.
Switching formats during an MP3 conversion cancels it, but the converter only recognised Remotion's abort error, so it showed "Conversion failed: Output has been canceled." and tracked a failed conversion. A cancelled mediabunny conversion can reject with the output's own error rather than ConversionCanceledError, so rethrow any error after a cancel as ConversionCanceledError and treat that as a cancellation.
Closes #3426.
extractAudioFromVideonow converts with mediabunny, whichapps/webalready depends on for Loom imports, toMp3OutputFormat. No browser encodes MP3 through WebCodecs, so it registers@mediabunny/mp3-encoder, mediabunny's LAME extension, which encodes in a worker. Both load only when someone converts to MP3, as Remotion already does in this component.tone-test.mp3tone-test.mp3A video without audio used to finish with "Conversion complete!" and a 44-byte WAV; it now shows "Conversion failed: This video has no audio track." If the video has audio the browser can't decode (AC-3 in Chrome, say), it shows "Your browser can't decode this video's audio."
Switching formats while converting cancels the conversion and shows "Conversion was cancelled", as Remotion's path did.
New dependency:
@mediabunny/mp3-encoder1.62.1 (MPL-2.0, like mediabunny; peermediabunny ^1.0.0, and the lockfile keeps mediabunny at 1.49.0). Its chunk is about 312 KB with the LAME WASM inlined, loaded only on conversion. The bitrate is mediabunny's default (QUALITY_HIGH, 320 kbps for 48 kHz stereo).Tests
media-format-converter.test.ts(already in CI's web test list), running real mediabunny and LAME (in aworker_threadsworker under Node):converter-audio.mp4(H.264 plus 0.25 s of PCM audio, which mediabunny decodes without WebCodecs), comes out as anaudio/mpegMP3 of about 0.25 s, downloaded asclip.mp3;gif-6s.mp4, which has no audio, shows the no-audio error and saves nothing;The first two fail without the change (
audio/wav, and "Conversion complete!").<audio>element and decodes to 3.0 s with its 440 Hz tone intact; cap.so'spresenter.mp4, which has no audio, gets the no-audio error.next build, Turbopack) that imports both packages the same way. Its output has one copy each of mediabunny and the encoder, the encoder's Node-onlyworker_threadsfallback resolves to an empty module, and the built page converted the same 3-second MP4 to the same 120,168-byte MP3.bun install --frozen-lockfile: no changes.bun run typecheck: no errors. Biome: the test file is clean, and the component's ten existing lint errors are unchanged, none on the changed lines.The PR appears safe to merge, with a non-blocking fix needed for how canceled MP3 conversions are reported.
Findings
Fix with agent prompt
Summary
Replaces WAV output in the MP4-to-MP3 tool with actual MP3 encoding.
Reviews (1) · Last reviewed commit: "fix(web): make the MP4 to MP3 converter ..." · Reviewed by Greptile