lordmulder / LameXP

Audio Encoder Front-End
http://lordmulder.github.io/LameXP
Other
184 stars 18 forks source link

Alert for "Frankenstein Streams" #30

Closed molkemon closed 8 years ago

molkemon commented 8 years ago

So I have no idea what a Frankenstein stream might be, but I'm currently in the process of converting my entire music library to aac and yeah yeah, lossy to lossy is bad, but I really don't care. The problem is that some (thankfully not a whole lot) of my mp3 files don't seem to decode correctly with mpg123, and while they play fine on any player known to man, they simply won't decode and the resulting aac stream will either be a 2 second file, or even worse, a few seconds of the song and then full stop.

Even worse yet, LameXP reports such a conversion as great success and there is no indication that something went wrong alltogether.

Interestingly, recoding to mp3 works without issue, and if I then try to recode that output mp3, it works aswell. Obviously, this is not a desireable solution.

So please: As soon as mpg123 mentions a Frankenstein monster, please pop an error instead of success. Also I'm fairly sure that if lamexp would use ffmpeg instead of mpg123 for decoding, this would not happen.

lordmulder commented 8 years ago

MP3 files do not use a proper "container" format. Instead, they are simply a sequence of "raw" MP3 frame - which is also the reason why seeking in a MP3 frame is delicate. Now, while each MP3 frame in the file (stream) starts with its own header, there are certain header flags (parameters) that are allowed to change between frames; and there are some flags that must not change within a stream. If a parameter of the latter kind changes anyway, this is what mpg123 calls a "Frankenstein" stream – a stream that apparently was spliced together from different (incompatible) parts. Such streams are invalid MP3 files!

In most cases when you get the "Frankenstein stream" error, however, it actually is not a stream spliced together from different (incompatible) parts, but just a "corrupted" stream/file with invalid data in the frame header! This invalid (corrupted) header data is detected as an illegal parameter change by mpg123.

Anyway, an illegal stream is an illegal stream. And "the file plays fine in my player" is not an argument here. As long as the stream is 100% standard-compliant, the behavior of the decoder is well-defined. But, as soon as there are any errors in the stream, the behavior of the decoder is simply undefined. Of course, most decoders try to recover from decoding errors. Sometimes this may work, sometimes not.


I suggest you check your stream with MPEG Audio Info, just to be sure: MPEGAudioInfo.2011-04-24.zip

In addition to that, you could also try the MP3val application: http://mp3val.sourceforge.net/

molkemon commented 8 years ago

Yes, I do realize that the stream is invalid, and I shouldn,t use it, even though it is possible to do so by reconverting to mp3 first. What I am asking though is that LameXP throws an error when it encounters an invalid stream. Right now, it says it completed succesfully.

lordmulder commented 8 years ago

Well, you need to be aware that not LameXP itself, but the mpg123 decoder prints out that "Frankenstein stream" warning. LameXP will check the exit code returned by the mpg123 process, of course! And, as long as mpg123 reports a zero exit code, LameXP has to assume that the file was decoded correctly.

Apparently, the mpg123 decoder does not abort with non-zero exit code immediately, when it encounters a decoding glitch. Instead, it treats the decoding glitch as warning rather than a fatal error. Then it tries to go on. In many cases it may be able to recover 99% of the corrupted file – sometimes only 2 seconds.

I don't know whether mpg123 has an "abort with fatal error, as soon as a decoding glitch is encountered" switch. And, if there was such a switch, we probably wouldn't want to use that by default ...

lordmulder commented 8 years ago

As there seems to be no indication that anything is wrong on LameXP's side, I'm gonna close this one.