Handle false-positive MP3 frame resulting in wrong samplerate value

Changes

@@ -1,5 +1,9 @@
 Revision history for Audio::Scan
 
+0.86
+        - MP3: In a bare MP3 stream, use samplerate from last frame instead of the
+          first, because the first can be a false positive.
+
 0.85    2010-09-08 23:40:00
         - Changed audio_md5 to use bytes from middle of file instead of the start,
           to reduce chance of false matches due to padding or silence.  Added new

MANIFEST

@@ -100,6 +100,7 @@ t/mac/apev1.ape
 t/mac/apev2.ape
 t/mp3/ape-no-v1.mp3
 t/mp3/ape-v1.mp3
+t/mp3/bad-first-samplerate.mp3
 t/mp3/bug5686/latin/good.mp3
 t/mp3/bug5686/utf/invalid.mp3
 t/mp3/gentoo-bug-210564.mp3

TODO

@@ -27,6 +27,7 @@ Support iTunes iTunSMPB comment for gapless info, translate into encoder delay/p
 Support iTunNORM, translate into replay gain values
   This would be useful
 ID3v1 comment + COMM are not merged but stacked
+ID3v1 genre + ID3v2 genre of same numeric code should be merged
 
 FLAC
 ----

lib/Audio/Scan.pm

@@ -2,7 +2,7 @@ package Audio::Scan;
 
 use strict;
 
-our $VERSION = '0.85';
+our $VERSION = '0.86';
 
 require XSLoader;
 XSLoader::load('Audio::Scan', $VERSION);

src/id3.c

@@ -273,12 +273,12 @@ _id3_parse_v2(id3info *id3)
 
     // We don't care about the value of the extended flags or CRC, so just read the size and skip it
     ehsize = buffer_get_int(id3->buf);
+    DEBUG_TRACE("  Skipping extended header, size %d\n", ehsize);
+
     if ( !_check_buf(id3->infile, id3->buf, ehsize, ID3_BLOCK_SIZE) ) {
       ret = 0;
       goto out;
     }
-
-    DEBUG_TRACE("  Skipping extended header, size %d\n", ehsize);
     buffer_consume(id3->buf, ehsize);
 
     id3->size_remain -= ehsize + 4;

src/mp3.c

@@ -230,6 +230,7 @@ static short _mp3_get_average_bitrate(mp3info *mp3, uint32_t offset, uint32_t au
   int done = 0;
   int wrap_skip = 0;
   int prev_bitrate = 0;
+  int last_samplerate = 0;
   bool vbr = FALSE;
 
   unsigned char *bptr;
@@ -273,6 +274,10 @@ static short _mp3_get_average_bitrate(mp3info *mp3, uint32_t offset, uint32_t au
         frame_count++;
         bitrate_total += frame.bitrate_kbps;
 
+        // The first frame may be incorrectly detected with the wrong samplerate,
+        // so use the last samplerate instead of the first
+        last_samplerate = frame.samplerate;
+
         if ( !vbr ) {
           // If we see the bitrate changing, we have a VBR file, and read
           // the entire file.  Otherwise, if we see 20 frames with the same
@@ -291,7 +296,7 @@ static short _mp3_get_average_bitrate(mp3info *mp3, uint32_t offset, uint32_t au
           }
         }
 
-        //DEBUG_TRACE("  Frame %d: %dkbps\n", frame_count, frame.bitrate_kbps);
+        //DEBUG_TRACE("  Frame %d: %dkbps, %dkHz\n", frame_count, frame.bitrate_kbps, frame.samplerate);
 
         if (frame.frame_size > buffer_len(mp3->buf)) {
           // Partial frame in buffer
@@ -315,6 +320,8 @@ static short _mp3_get_average_bitrate(mp3info *mp3, uint32_t offset, uint32_t au
   if (!frame_count) return -1;
 
   DEBUG_TRACE("Average of %d frames: %dkbps\n", frame_count, bitrate_total / frame_count);
+
+  my_hv_store( mp3->info, "samplerate", newSVuv(last_samplerate) );
 
   return bitrate_total / frame_count;
 }
@@ -729,7 +736,10 @@ _mp3_parse(PerlIO *infile, char *file, HV *info)
   my_hv_store( info, "audio_size", newSVuv(mp3->audio_size) );
   my_hv_store( info, "audio_offset", newSVuv(mp3->audio_offset) );
   my_hv_store( info, "bitrate", newSVuv( mp3->bitrate * 1000 ) );
-  my_hv_store( info, "samplerate", newSVuv( frame.samplerate ) );
+
+  // Average bitrate code may have already set the samplerate
+  if ( !my_hv_exists( info, "samplerate" ) )
+    my_hv_store( info, "samplerate", newSVuv( frame.samplerate ) );
 
   if (mp3->xing_frame->xing_tag || mp3->xing_frame->info_tag) {
     if (mp3->xing_frame->xing_frames) {

t/mp3.t

@@ -2,7 +2,7 @@ use strict;
 
 use File::Spec::Functions;
 use FindBin ();
-use Test::More tests => 378;
+use Test::More tests => 379;
 
 use Audio::Scan;
 
@@ -1259,6 +1259,14 @@ eval {
     is( $tags->{TPE1}, 'Herbert Eimert', 'v2.2 multiple null strings TP1 value ok' );
 }
 
+# Bad first samplerate (stream from Radio Paradise)
+{
+    my $s = Audio::Scan->scan( _f('bad-first-samplerate.mp3') );
+    my $info = $s->{info};
+
+    is( $info->{samplerate}, 44100, 'Bad first samplerate detected as 44100 ok' );
+}
+
 sub _f {    
     return catfile( $FindBin::Bin, 'mp3', shift );
 }