[MPlayer-dev-eng] Re: [PATCH] don't skip first mp3 frame

[Michael Behrisch]

On Mon, May 31, 2004 at 04:22:57PM -0400, D Richard Felker III wrote:
> > 
> > it's CF for win32, but i thought 'patch' strips these
> > automatically, hmm, oh well, anyways here it is, again, hopefully
> > it's ok
> 
> Was this patch ever applied? IIRC at the time I refused to commit it
> myself because I couldn't do thorough tests, but it looks right.
> Anyway, no one seems to be stepping forward to test, so I'm going to
> commit this soon and let our users do the testing after the next
> release... :))

I would rather have You apply my patch, which I sent several times
to Albeu (even to his private mail) but he did not answer.
It checks better for valid mp3. It does clean up the double removal
of the first frame (in demux_audio and mp3lib) and if you wish
I can modify the patch such that it also removes the first
frame skipping in mp3lib.

Yours,
 Michael

-------------- next part --------------
--- libmpdemux/demux_audio.old	2003-10-09 10:02:45.000000000 +0200
+++ libmpdemux/demux_audio.c	2004-04-12 21:19:46.000000000 +0200
@@ -21,6 +21,7 @@
 
 
 #define HDR_SIZE 4
+#define MIN_VALID_HEADERS 6
 
 typedef struct da_priv {
   int frmt;
@@ -37,7 +38,8 @@
   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 frmt = 0, n = 0, pos = 0, bytes_to_read = HDR_SIZE;
+  int len, mp3_freq, mp3_chans;
   da_priv_t* priv;
 #ifdef MP_DEBUG
   assert(demuxer != NULL);
@@ -46,49 +48,59 @@
   
   s = demuxer->stream;
 
-  while(n < 5 && ! s->eof) {
-    st_pos = stream_tell(s);
-    step = 1;
-    if(pos < HDR_SIZE) {
-      stream_read(s,&hdr[pos],HDR_SIZE-pos);
-      pos = HDR_SIZE;
-    }
+  while(n < 3000 && ! s->eof) {
+    stream_read(s,&hdr[HDR_SIZE-bytes_to_read],bytes_to_read);
+    pos = stream_tell(s) - HDR_SIZE;
+    bytes_to_read = 1;
 
     if( hdr[0] == 'R' && hdr[1] == 'I' && hdr[2] == 'F' && hdr[3] == 'F' ) {
       stream_skip(s,4);
-      if(s->eof)
-	break;
-      stream_read(s,hdr,4);
-      if(s->eof)
-	break;
+      if(s->eof) break;
+      stream_read(s,hdr,HDR_SIZE);
+      if(s->eof) break;
       if(hdr[0] != 'W' || hdr[1] != 'A' || hdr[2] != 'V'  || hdr[3] != 'E' )
 	stream_skip(s,-8);
       else
       // We found wav header. Now we can have 'fmt ' or a mp3 header
       // empty the buffer
-	step = 4;
+        bytes_to_read = HDR_SIZE;
     } else if( hdr[0] == 'I' && hdr[1] == 'D' && hdr[2] == '3' && (hdr[3] >= 2)) {
-      int len;
       stream_skip(s,2);
-      stream_read(s,hdr,4);
+      if(s->eof) break;
+      stream_read(s,hdr,HDR_SIZE);
+      if(s->eof) break;
       len = (hdr[0]<<21) | (hdr[1]<<14) | (hdr[2]<<7) | hdr[3];
       stream_skip(s,len);
-      step = 4;
+      bytes_to_read = HDR_SIZE;
     } else if( hdr[0] == 'f' && hdr[1] == 'm' && hdr[2] == 't' && hdr[3] == ' ' ) {
       frmt = WAV;
       break;      
-    } else if((n = mp_get_mp3_header(hdr,&mp3_chans,&mp3_freq)) > 0) {
+    } else if((len = mp_get_mp3_header(hdr,&mp3_chans,&mp3_freq)) > 20) {
+      int i, mp3_freq2, mp3_chans2;
+      for(i = 1; i < MIN_VALID_HEADERS ; i++) {
+        stream_skip(s,len-HDR_SIZE);
+        if(s->eof) break;
+        stream_read(s,hdr,HDR_SIZE);
+        if(s->eof) break;
+        len = mp_get_mp3_header(hdr,&mp3_chans2,&mp3_freq2);
+        if(len < 21 || mp3_chans != mp3_chans2 || mp3_freq != mp3_freq2) break;
+      }
+      if(i == MIN_VALID_HEADERS) {
       frmt = MP3;
       break;
+      }
+      if(s->eof || !s->end_pos) break;
+      stream_seek(s, pos+1);
+      bytes_to_read = HDR_SIZE;
     } else if( hdr[0] == 'f' && hdr[1] == 'L' && hdr[2] == 'a' && hdr[3] == 'C' ) {
       frmt = fLaC;
       stream_skip(s,-4);
       break;
     }
     // Add here some other audio format detection
-    if(step < HDR_SIZE)
-      memmove(hdr,&hdr[step],HDR_SIZE-step);
-    pos -= step;
+    if(bytes_to_read < HDR_SIZE)
+      memmove(hdr,&hdr[bytes_to_read],HDR_SIZE-bytes_to_read);
+    n++;
   }
 
   if(!frmt)
@@ -99,7 +111,7 @@
   switch(frmt) {
   case MP3:
     sh_audio->format = 0x55;
-    demuxer->movi_start = st_pos-HDR_SIZE+n;
+    demuxer->movi_start = pos;
     sh_audio->audio.dwSampleSize= 0;
     sh_audio->audio.dwScale = 1152;
     sh_audio->audio.dwRate = mp3_freq;
@@ -110,6 +122,7 @@
     sh_audio->wf->nBlockAlign = 1152;
     sh_audio->wf->wBitsPerSample = 16;
     sh_audio->wf->cbSize = 0;    
+    //following loop can be removed after testing new detection
     for(n = 0; n < 5 ; n++) {
       pos = mp_decode_mp3_header(hdr);
       if(pos < 0)