[FFmpeg-devel] [PATCH] fix parsing of broken mp3 streams

[Zdenek Kabelac]

2009/4/20 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Apr 20, 2009 at 09:37:25PM +0200, Zdenek Kabelac wrote:
>> 2009/4/19 Michael Niedermayer <michaelni at gmx.at>:
>> > On Sun, Apr 19, 2009 at 11:18:06PM +0200, Zdenek Kabelac wrote:
>> >> Hi
>> >>
>> >> Here is a small patch that fixes of running out-of-buffer in parsing
>> >> broken mp3 data stream.
>> >> This solution is rather a hotfix - better solution would be to check
>> >> at least one or two next mp3
>> >> frames in sequence whether they are part of the same audio stream or
>> >> some random junk
>> >> which has 0xfffx header inside. With this patch ugly noise could be
>> >> sometimes noticed.
>> >>
>> >> Also questionable is whether it should return -1 if no header is found
>> >> or rather return skipped
>> >> bytes and out_size = 0 - as then usually such packet is rescaned
>> >> multiple times with
>> >> one-byte step forward...
>> >>
>> >> Zdenek
>> >>
>> >> - Fix buffer overrun
>> >> - Properly return parsed bytes together with skipped bytes
>> >
>> > please provide a sample so we can confirm the bugfix, the patch
>> > looks mostly correct though
>> >
>>
>> I've upload just one mp3 dumped stream upload.ffmpeg.org as
>> junk_at_mp3stream ?directory - together with short text and two patch
>
>> - I'm attaching patch for api-example.c ?to easily compare results.
>
> i dont care what a modified tool does
> is there a problem that is reproduceable with ffmpeg or ffplay that
> your patch fixes?

Patch is fixing mp3 decoder to skip only broken junk inside passed
data  while decoding as much mp3 frames as possible and avoid buffer
over reading - don't ask me which tools are muxing avi streams with
junk in packets - obviously it some kind of re-synchronization from
splinting huge avi streams into small chunks....

You could check for your self is to compare the result of extracted
wav size via api-example and then do
the same with ffmpeg -i junk.mp3  o.wav - you might observe small
difference 4027436 != 4018220
To do my homework and complete the list: mplayer -ao pcm:file=wav
junk.mp3 - creates 4022830 - but IMHO it decodes some broken packets
at the begining)

(btw the patch for api-example should be probably commited into svn as well...)
Usually such badly muxed sample streams are way to small to notice
significant desynchronization.

I've not yet made the patch for ffmpeg & ffplay - these would be more
complicated as I'd like to resolve also playback of VBR files with
these tool - they are unfortunately not correctly handled - and
requires some time and thinking about the correct solution.

Also I don't like my current proposed patch either - as it still
requires some logic from the user of decoder - but it's better then
current state.

Zdenek