Remux WAVE_FORMAT_MPEGLAYER3 WAV files to MP3 during import#6526
Remux WAVE_FORMAT_MPEGLAYER3 WAV files to MP3 during import#6526snejus merged 13 commits intobeetbox:masterfrom
Conversation
WAV files containing an MP3 stream (WAVE_FORMAT_MPEGLAYER3, wFormatTag=0x0055) were silently imported with incorrect metadata and format reported as WAVE instead of MP3. When a WAV file fails to open due to FileTypeError, check if it is a MPEGLAYER3 WAV. If it is, use ffmpeg to remux it to a proper MP3 file, removing the WAV container. The original WAV file is deleted after successful remuxing. Adds a regression test and test fixture. Fixes beetbox#6455
6540242 to
7a20252
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6526 +/- ##
=======================================
Coverage 70.99% 70.99%
=======================================
Files 150 150
Lines 19124 19150 +26
Branches 3078 3082 +4
=======================================
+ Hits 13577 13596 +19
- Misses 4896 4900 +4
- Partials 651 654 +3
🚀 New features to boost your workflow:
|
|
I'm not sure we want to be automatically converting files on import by default - could this be achieved by a plugin? Or is there a way for us to add support for these MP3-in-WAV files? |
|
Thanks for the feedback! I understand the concern about automatically modifying files during import. I did explore the native approach earlier. Mutagen opens MPEGLAYER3 WAV files as WAVE objects and can't read or write their tags correctly. However, I found that the raw MP3 stream can be extracted from the data chunk and read correctly by mutagen.mp3.MP3. So in theory, mediafile could be extended to extract and re-open the stream as MP3, but writing tags back to the WAV container would be more complex. |
|
@elainec2024 note I've just released mediafile v0.16.1 with your adjustment, so update |
An alternative would be to add a configuration under import:
remux_mp3_in_wav_to_mp3: yeswhere users have the ability to opt-out if they don't want this to happen. On the other hand, it seems like users do expect this to happen: #6455 (comment) |
|
Happy to have some form of |
|
I don't think so. We will need to have handling similar to the logic in |
bd13a2d to
623eb04
Compare
|
I've addressed the first two suggestions of bumping the mediafile dependency and adding a config option under the import section. For the third suggestion, I can replace ffmpeg with pure Python byte extraction if you'd prefer. I confirmed that the MP3 stream can be extracted directly from the WAV data chunk and read correctly by mutagen.mp3.MP3, so it's feasible without any external dependencies. Happy to implement that if that's the direction you want to go! |
This is very cool! Sounds like such solution may be less verbose, too? |
snejus
left a comment
There was a problem hiding this comment.
Will review the ffmpeg -> mutagen adjustment once it's finished!
3bf5c6c to
480e67c
Compare
|
The docs formatting check is consistently failing on CI but I'm unable to reproduce it locally — running poe format-docs --check and docstrfmt directly both pass with no changes needed. The CI only reports "could be reformatted" without showing the specific diff. Could you help identify what I need to change? |
Just checked out your branch locally and ran Are you using the same version of Make sure to |
|
Got it! Running on a fresh clone reproduced it. Thank you! |
Follow-up to #6526. This change moves the `remux_mpeglayer3_wav` call to before `library.Item.from_path` in `read_item`, so beets no longer depends on mediafile raising `FileTypeError` for MPEGLAYER3 WAV files. This is related to beetbox/mediafile#107.
Description
Fixes #6455
WAV files can contain MP3 audio streams (WAVE_FORMAT_MPEGLAYER3, wFormatTag=0x0055), but beets has no way to tag them correctly and would silently import them with wrong duration, bitrate, and format metadata.
This PR fixes the issue by automatically detecting such files during import and using ffmpeg to remux them into proper .mp3 files. The original WAV is deleted after successful remuxing, and the MP3 is imported normally with correct metadata.
This fix depends on beetbox/mediafile#105 which raises a clear FileTypeError for these files, allowing beets to detect and handle them.
To Do
Documentation