failure to return error code for bogus header
Brought to you by:
sobukus
Menu
▾
▴
#113 failure to return error code for bogus header
Hiya,
We use some bogus 'mp3' files to test error handling in libraries we use and in our own code. Prior versions of mpg123 (1.7.0...) did not notice the problem, but returned 0 for the samplerate from mpg123_getformat(). Now, in version 1.9.0, error messages get logged to stderr, and mpg123_getformat() and mpg123_info() report the same, non-zero samplerate, but MPG123_OK is returned from all functions, and the information from mpg123_info() isn't self consistent.
Once an error has been noticed with a file-based handle, mpg123 should never return MPG123_OK from functions that are handed the bogus handle.
Here's the stderr messages:
Note: Illegal Audio-MPEG-Header 0x00060009 at offset 0xda3.
Note: Trying to resync...
Note: Skipped 129 bytes in input.
The file in question is too big to attach to this defect report. You can find it here:
http://hltcoe.hodain.net/problem.mp3
-Hugh
Related
No time yet to deep-investigate, but there is room for interpretation on how some garbage in between is to be handled.
mpg123's stance is that when the resync process suceeds, MPG123_OK is justified.
But, I guess I need to have a closer look to determine if there is any success ... did you actually put a "good" frame in there or only distorted information?
Heh, the thing that brought this to my attention is that the mpg123 behavior vis-a-vis the reported samplerate changed from 1.7 to 1.9..., so now our tests fail for one of these versions.
I didn't understand the resync behavior.... Having the API expose clear, and perhaps parametrically controlled, semantics for what it does with bogus headers would be a nice thing. At the very least, it would let me use mpg123 to discover which files have bogus headers/frames.
I don't have the provenance of this file; it won't play in QuickTime which I believe is what brought it to my attention -- heh heh, which gets back to the point that it would be nice to be able to use mpg123 to dissect it enough to answer your question about a "good" frame or distorted info!
-Hugh
Well, the default of mpg123 has always been to try to work with damaged files ("damage" could always mean some funky additions like strange tags, album art, RIFF headers...). Again, I need to find some time (probably not during the week) to investigate the file closer.
What I can see now is that mplayer seems to play it (mplayer uses some derived mpg123 code for that)... and I suspect the addition of free format support....
OK, I got suspicious when mplayer played the file... as plain uncompressed pcm!
What you have there is a RIFF wave file with a rather exotic MPEG header prepended. MPlayer for some reason does what mpg123 does in attempt to find valid MPEG data, in reverse: It skips the first bytes before the RIFF header.
Per default, mpg123 tries hard to find MPEG data, in fact, also by skipping a leading RIFF header (since that apparently happened in the past, too). OK, so now we got the reversed case. If you want to check for error-free MPEG files, I suggest using MPG123_NO_RESYNC (the --no-resync command line flag).
All clear now?
When I change our use of the library to set the parameter flag MPG123_NO_RESYNC, the message that's logged to stderr changes, but the mpg123 functions still all return MPG123_OK. Here's the new error message that goes to stderr:
Note: Illegal Audio-MPEG-Header 0x00060009 at offset 0xda3.
[parse.c:658] error: not attempting to resync...
I believe it is a defect. But if not, what can I do in order to have the mpg123 library let my library know that the file is bogus? (Parsing stderr is not an option.)
Hm, OK. You get MPG123_DONE, right? Seems like we need to define another flag that should treat any stream error as hard error. Currently, we have things in mind like a trailing BMP album art (seen that!) that should not trigger failure of the decoder. Mpg123 is coded to be tolerant and giving...
But I see, that for stream checking a strict mode might be good, that one should also include missing bit reservoir parts, anything that's wrong... right?
Just wanna note that I stumbled over this again. We didn't really come to a conclusion... I got another idea: Just store a stream error counter in the handle. As mpg123 is forgiving, it could still try hard to find valid data and ignore junk, resulting in MPG123_OK, but one could query afterwards if there were errors in the stream that got glossed over.
Would that work for you? (over two years later...)
Wow. 8 years later. Just too busy with other stuff:-/ A strict mode still sounds sensible. Or rather … API to extract information about parsing and decoding errors. Your checker would call that API during/after decoding to know if errors occured. A running count of parser and decoder errors. And/or a counter of bytes that contained no valid data/had decoding issues.
I'd consider a patch implementing this, of course. So far, there is no strong demand for this detail.
Wow. Four jobs later, and busy too! ;-) This issue is no longer relevant to what I’m working on.
Regards,
-Hugh
Related
Bugs:
#113Maybe diagnostics will be added at some point.