enzo1982 / freac

The fre:ac audio converter project
https://www.freac.org/
GNU General Public License v2.0
1.42k stars 75 forks source link

Audio scrambled after converting MP3 to MP3.. not a coder. #471

Open the96ears opened 1 year ago

the96ears commented 1 year ago

I'm a user. I had an issue lately with playback in my car (USB flash drive). If I rip music from YouTube, it'll play on my PC, but some files won't play in my car. I have all my music organized and usually in 320 Kbps or VBR/ABR MP3 format. I thought I ripped some files in freac to correct the issue, but when I just tried this with freac 1.1.6 AND then 1.1.7 using LAME, the output files are garbled like in my car. Looks like it double-compressed the file from 3.34 MB to 32.58 KB and the song sounds like high-speed dub/tape noise with a 2 second length. Probably just me being stupid, or my car not supporting something in the MP3 format, but at least the software should report back when trying to convert MP3 to MP3 if it's not supported.

Win 10 Pro Intel(R) Core(TM) i5-10400 CPU @ 2.90GHz 2.90 GHz 8GB RAM shared with onboard framebuffer

enzo1982 commented 1 year ago

You write "the output files are garbled like in my car". Does that mean after converting the files with fre:ac they sound the same as the original files did in your car?

In that case, there probably is something wrong with the files you ripped from YouTube.

Please send one of these files (before converting with fre:ac) to support@freac.org so I can analyze it and figure out what's wrong - either with the file or with the decoder in fre:ac.

the96ears commented 1 year ago

Correct. The files sound fine on the PC after ripping from YouTube (ytmp3.cc), but when copied to a thumb drive and played in the car, it either quickly skips them or play a quick scrambled tape sound and goes to the next track.

When I just tried converting two files that play fine on the PC, fre:ac's output was a new mp3 making that scrambled tape sound on my PC.

enzo1982 commented 1 year ago

Thank you for sending the file! The file is not an MP3, but an MP4 file. The file extension should be changed to .m4a and it can then be opened and converted with fre:ac 1.1.7.

The problem with non-MP3 files renamed to .mp3 is that the MP3 format has a very short synchronization sequence that is often contained in all kinds of other files. An MP3 decoder looks for this synchronization sequence and then tries to decode the file as an MP3 which results in weird sounds.

I'll see if I can add code to detect such cases and reject the files in the MP3 decoder component.

the96ears commented 1 year ago

Awesome! I had a feeling it wasn't the correct file type, but i figured fre:ac would correct the issue, kinda like a filter.. I guess I assumed the software would be able to tell what the file type was. Maybe after your code modification it will!

Thanks a lot!

On Mon., Apr. 10, 2023, 16:44 Robert Kausch, @.***> wrote:

Thank you for sending the file! The file is not an MP3, but an MP4 file. The file extension should be changed to .m4a and it can then be opened and converted with fre:ac 1.1.7.

The problem with non-MP3 files renamed to .mp3 is that the MP3 format has a very short synchronization sequence that is often contained in all kinds of other files. An MP3 decoder looks for this synchronization sequence and then tries to decode the file as an MP3 which results in weird sounds.

I'll see if I can add code to detect such cases and reject the files in the MP3 decoder component.

— Reply to this email directly, view it on GitHub https://github.com/enzo1982/freac/issues/471#issuecomment-1502231922, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7BUCW3ELQPS6G3T2CNHX5LXARPJVANCNFSM6AAAAAAWXQYVKU . You are receiving this because you authored the thread.Message ID: @.***>

Lee-Carre commented 1 year ago

ripping from YouTube (ytmp3.cc), but when copied to a thumb drive and played in the car, it either quickly skips them or play a quick scrambled tape sound and goes to the next track.

The file is not an MP3, but an MP4 file.

Sounds like ytmp3 is doing a bad job of things.

I would recommend, instead, YouTube-DL, which is highly capable, and can certainly be used to extract audio-only, along with tagging based on the video title (eg for artist, track name). It doesn't mangle audio formats, either.

I'm not sure that YouTube still publishes MP3s, but it does publish formats which Fre:AC can convert to MP3 or whatever other codec your car's headunit is able to play. I would suggest fetching one of the Opus-encoded formats (even YT's f251 (if I recall correctly, from memory), which is ~50Kbps, is fine for casual-listening music;; Opus is very efficient).

YouTube-DL is a CLI tool, and since Fre:AC has freaccmd, then it might be better to use a bit of shell scripting to chain the two, for a more automated process to generate the output you want, instead of lots of manual busywork. More effort to set up, but it'll pay off in the long-run by making it quick & easy to fetch more music.

For determining what a media file actually is (such as an M4A masquerading as an MP3), ffprobe (part of FFmpeg) is extremely useful. For those who prefer a GUI, ffprobe is (as far as I can tell) used in the “Media Info” tool, which outputs the same metadata. Though, ffprobe is dead simple & easy; a GUI seems overkill. Filename extensions should reflect the nature of the contents. They do not dictate/determine what the contents is. Mis-naming a file will cause all sorts of problems, just line in this case.

Lee-Carre commented 1 year ago

@enzo1982

I'll see if I can add code to detect such cases and reject the files in the MP3 decoder component.

While I'm all for Postel's Law, in general/principle, I think it unwise to silently hide such mismatches from users. It encourages bad practice. Being familiar with usability/interaction-design, this could well be argued as bad usability (I can elaborate (at length) or provide pointers to reading material, if anyone wishes). Think about why your car has all sorts of dashboard indicators for various problems, instead of being silent on the matter. Better to be told about a problem with the oil, sooner than later, rather than risk engine damage/seizure.

So, either

In either case, definitely detect the problem, but don't necessarily take any further actions.

I'm reminded of advice from a security perspective; never trust user-input. Always sanitise. So, it would seem sensible to perform some kind of container/codec validation on every file, regardless of which encoder is selected. A simple way might be to run the decoder (for the type of codec which the file purports to be) upon it, with all warning modes enabled, to see if the decoder throws any non-info messages. Only if the file passes such scrutiny, then attempt to actually/truly transcode the file.

Related (to the general case of input-sanitisation) seems to be #425. Perhaps a new module, or refactoring of the input/decode module, is warranted.