[MPlayer-dev-eng] mp3 recognize bugfix for MPlayer-1.0pre7::libmpdemux/demux_audio.c

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Tue Sep 6 15:12:05 CEST 2005


Hi,
On Tue, Sep 06, 2005 at 01:50:25PM +0200, Stefan Bambach wrote:
> You remove each list item, when you call it "invalid". Invalid your way
> means e.g. "next_frame_pos < st_pos". That can always happen, because of
> random mp3 data is sometimes a possible mp3 header. 

Yes, that exactly was the bug that was fixed since pre7. I think you
missed the important change (attached).

> So I collect and count all elements, that are in a row in decoding means
> (nextframe = pos+framesize) and delete nothing and break, if a list
> elements reaches the counter.

Your algorithm is just a simpler version of mine (I think), it traverses
the whole list each time instead of keeping it sorted - can be an
performance hit if you have a file with lots of 0xff bytes.

> Question:
> Why checking layer, channels, freq, ... to get correct header sequence ?
> What about VBR mp3s ? Some parameters can change each frame !

Yes, but channels, layer type and frequency can not change! At least
MPlayers decoder would break in that case anyway. Bitrate may change
though!
Not checking layer might cause MPlayer to use the wrong decoder based on
a first broken packet - and at least for samplerate we have that problem
with avi files where the first MP3 header specifies the wrong one.

Greetings,
Reimar Döffinger
-------------- next part --------------
Index: demux_audio.c
===================================================================
RCS file: /cvsroot/mplayer/main/libmpdemux/demux_audio.c,v
retrieving revision 1.32
retrieving revision 1.35
diff -u -r1.32 -r1.35
--- demux_audio.c	17 Apr 2005 09:42:51 -0000	1.32
+++ demux_audio.c	18 Apr 2005 20:51:34 -0000	1.35
@@ -101,6 +101,9 @@
     }
   }
   if (!in_list) { // does not belong into an existing chain, insert
+    // find right position to insert to keep sorting
+    while (*list && (*list)->next_frame_pos <= st_pos + mp3_flen)
+      list = &((*list)->next);
     tmp = malloc(sizeof(mp3_hdr_t));
     tmp->frame_pos = st_pos;
     tmp->next_frame_pos = st_pos + mp3_flen;


More information about the MPlayer-dev-eng mailing list