[MPlayer-dev-eng] Re: [Patch] Trying harder to detect MP3

[Michael Behrisch]

On Sun, Dec 28, 2003 at 02:29:35PM +0100, Alban Bedel wrote:
> Hi Michael Behrisch,
> 
> on Sat, 27 Dec 2003 21:14:32 +0100 you wrote:
> 
> > Hello,
> > 
> > this one incorporates code from mp3check in order
> > to recognize mp3's with (at most 100 bytes of) junk at start.
> > It works only with files (seekable streams).
> 
> The main idea is ok to me, but it's not implemented at the right place.
> The function is cut into 2 parts. The first try to findout the file
> type. The second one setup the demuxer stream, etc.
> In the case of mp3 it's not so clear bcs the second part also perform some
> extra test to ensure that the file is really an mp3.
> So imho your extra check should go into the first part where it currently
> only set frmt to MP3.

i thought it is very similar to the check, which already is there,
looking for 5 consecutive valid headers, that's why I put it there

I did it a little rework and resent the patch in the appendix
Maybe it's clearer then why it simply replaces the check which
was there. Also demuxer->movi_start is ok now.

> IIRC ad_mp3lib skip the first frame too now, so it may not be needed anymore.
> Anyway i highly recommend that you checks your patch with LOTS of mp3. (And
> with LOTS i really mean at least some 1000's including old stuff ;)

I don't have thousands but I checked with some, and it works fine for me.

Michael
-------------- next part --------------
--- libmpdemux.old/demux_audio.c	2003-10-09 10:02:45.000000000 +0200
+++ libmpdemux/demux_audio.c	2004-01-24 18:14:08.000000000 +0100
@@ -21,6 +21,8 @@
 
 
 #define HDR_SIZE 4
+#define MIN_VALID_HEADERS 5
+#define MAX_JUNK_START 100
 
 typedef struct da_priv {
   int frmt;
@@ -37,7 +39,7 @@
   stream_t *s;
   sh_audio_t* sh_audio;
   uint8_t hdr[HDR_SIZE];
-  int st_pos = 0,frmt = 0, n = 0, pos = 0, step, mp3_freq,mp3_chans;
+  int st_pos = 0,frmt = 0, n = 0, i, pos = 0, step, mp3_freq,mp3_chans, mp3_freq2,mp3_chans2;
   da_priv_t* priv;
 #ifdef MP_DEBUG
   assert(demuxer != NULL);
@@ -98,8 +100,23 @@
 
   switch(frmt) {
   case MP3:
+    for(i=1; i < MAX_JUNK_START; i++) {
+      for(n = 0; n < MIN_VALID_HEADERS ; n++) {
+        pos = mp_get_mp3_header(hdr,&mp3_chans2,&mp3_freq2);
+        if(pos < 21 || mp3_chans != mp3_chans2 || mp3_freq != mp3_freq2) break;
+        stream_skip(s,pos-HDR_SIZE);
+        if(s->eof) return 0;
+        stream_read(s,hdr,HDR_SIZE);
+        if(s->eof) return 0;
+      }
+      if(n == MIN_VALID_HEADERS) break;
+      stream_seek(s, st_pos+i);
+      stream_read(s,hdr,HDR_SIZE);
+      mp_get_mp3_header(hdr,&mp3_chans,&mp3_freq);
+    }
+    if(i==MAX_JUNK_START) return 0;
     sh_audio->format = 0x55;
-    demuxer->movi_start = st_pos-HDR_SIZE+n;
+    demuxer->movi_start = st_pos+i;
     sh_audio->audio.dwSampleSize= 0;
     sh_audio->audio.dwScale = 1152;
     sh_audio->audio.dwRate = mp3_freq;
@@ -110,17 +127,6 @@
     sh_audio->wf->nBlockAlign = 1152;
     sh_audio->wf->wBitsPerSample = 16;
     sh_audio->wf->cbSize = 0;    
-    for(n = 0; n < 5 ; n++) {
-      pos = mp_decode_mp3_header(hdr);
-      if(pos < 0)
-	return 0;
-      stream_skip(s,pos-4);
-      if(s->eof)
-	return 0;
-      stream_read(s,hdr,4);
-      if(s->eof)
-	return 0;
-    }
     if(s->end_pos) {
       char tag[4];
       stream_seek(s,s->end_pos-128);