]> granicus.if.org Git - taglib/commitdiff
A bit more tolerant check for the MPEG frame length.
authorTsuda Kageyu <tsuda.kageyu@gmail.com>
Tue, 22 Dec 2015 11:11:26 +0000 (20:11 +0900)
committerTsuda Kageyu <tsuda.kageyu@gmail.com>
Tue, 22 Dec 2015 11:39:58 +0000 (20:39 +0900)
taglib/mpeg/mpegheader.cpp
taglib/mpeg/mpegheader.h
taglib/mpeg/mpegproperties.cpp
tests/test_mpeg.cpp

index 4e4e4e37ad2a51e090c3fde1bca0cc8b8336d76a..8732961458c2d484e877b8b2aba74ff81d0e2c85 100644 (file)
@@ -74,10 +74,10 @@ MPEG::Header::Header(const ByteVector &data) :
   debug("MPEG::Header::Header() - This constructor is no longer used.");
 }
 
-MPEG::Header::Header(File *file, long offset) :
+MPEG::Header::Header(File *file, long offset, bool checkLength) :
   d(new HeaderPrivate())
 {
-  parse(file, offset);
+  parse(file, offset, checkLength);
 }
 
 MPEG::Header::Header(const Header &h) :
@@ -169,7 +169,7 @@ MPEG::Header &MPEG::Header::operator=(const Header &h)
 // private members
 ////////////////////////////////////////////////////////////////////////////////
 
-void MPEG::Header::parse(File *file, long offset)
+void MPEG::Header::parse(File *file, long offset, bool checkLength)
 {
   file->seek(offset);
   const ByteVector data = file->readBlock(4);
@@ -301,12 +301,26 @@ void MPEG::Header::parse(File *file, long offset)
   // Check if the frame length has been calculated correctly, or the next frame
   // synch bytes are right next to the end of this frame.
 
-  file->seek(offset + d->frameLength);
-  const ByteVector nextSynch = file->readBlock(2);
+  // We read some extra bytes to be a bit tolerant.
 
-  if(nextSynch.size() < 2 || !firstSyncByte(nextSynch[0]) || !secondSynchByte(nextSynch[1])) {
-    debug("MPEG::Header::parse() -- Calculated frame length did not match the actual length.");
-    return;
+  if(checkLength) {
+
+    bool nextFrameFound = false;
+
+    file->seek(offset + d->frameLength);
+    const ByteVector nextSynch = file->readBlock(4);
+
+    for(int i = 0; i < static_cast<int>(nextSynch.size()) - 1; ++i) {
+      if(firstSyncByte(nextSynch[0]) && secondSynchByte(nextSynch[1])) {
+        nextFrameFound = true;
+        break;
+      }
+    }
+
+    if(!nextFrameFound) {
+      debug("MPEG::Header::parse() -- Calculated frame length did not match the actual length.");
+      return;
+    }
   }
 
   // Now that we're done parsing, set this to be a valid frame.
index cd77d37a203f0bfc5ee691462eef066420cc5898..024aa112e23416e27105bc4667834fd091c1e615 100644 (file)
@@ -56,8 +56,12 @@ namespace TagLib {
 
       /*!
        * Parses an MPEG header based on \a file and \a offset.
+       *
+       * \note If \a checkLength is true, this requires the next MPEG frame to
+       * check if the frame length is parsed and calculated correctly.  So it's
+       * suitable for seeking for the first valid frame.
        */
-      Header(File *file, long offset);
+      Header(File *file, long offset, bool checkLength = true);
 
       /*!
        * Does a shallow copy of \a h.
@@ -163,7 +167,7 @@ namespace TagLib {
       Header &operator=(const Header &h);
 
     private:
-      void parse(File *file, long offset);
+      void parse(File *file, long offset, bool checkLength);
 
       class HeaderPrivate;
       HeaderPrivate *d;
index b9ae8675e6ce18f8c3dd14f815781a4143231d20..6e7bb8232be15e95e3e73831de1ddb759704627b 100644 (file)
@@ -207,16 +207,13 @@ void MPEG::Properties::read(File *file)
 
     // Look for the last MPEG audio frame to calculate the stream length.
 
-    // This actually finds the second last valid frame, since MPEG::Header requires
-    // the next frame header to check if the frame length is calculated correctly.
-
     long lastFrameOffset = file->lastFrameOffset();
     if(lastFrameOffset < 0) {
       debug("MPEG::Properties::read() -- Could not find an MPEG frame in the stream.");
       return;
     }
 
-    Header lastHeader(file, lastFrameOffset);
+    Header lastHeader(file, lastFrameOffset, false);
 
     while(!lastHeader.isValid()) {
       lastFrameOffset = file->previousFrameOffset(lastFrameOffset);
@@ -225,10 +222,10 @@ void MPEG::Properties::read(File *file)
         return;
       }
 
-      lastHeader = Header(file, lastFrameOffset);
+      lastHeader = Header(file, lastFrameOffset, false);
     }
 
-    const long streamLength = lastFrameOffset - firstFrameOffset + lastHeader.frameLength() * 2;
+    const long streamLength = lastFrameOffset - firstFrameOffset + lastHeader.frameLength();
     if(streamLength > 0)
       d->length = static_cast<int>(streamLength * 8.0 / d->bitrate + 0.5);
   }
index 455fde2ce8f419a3c0c6b8ecd72535aed228db74..b9f463a4d43c929040c4f83cd37d27efd60bcbf1 100644 (file)
@@ -93,18 +93,15 @@ public:
     CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
     CPPUNIT_ASSERT(!f.audioProperties()->xingHeader());
 
-    // This actually finds the second last valid frame, since MPEG::Header requires
-    // the next frame header to check if the frame length is calculated correctly.
-
     long last = f.lastFrameOffset();
-    MPEG::Header lastHeader(&f, last);
+    MPEG::Header lastHeader(&f, last, false);
 
     while(!lastHeader.isValid()) {
       last = f.previousFrameOffset(last);
-      lastHeader = MPEG::Header(&f, last);
+      lastHeader = MPEG::Header(&f, last, false);
     }
 
-    CPPUNIT_ASSERT_EQUAL(28004L, last);
+    CPPUNIT_ASSERT_EQUAL(28213L, last);
     CPPUNIT_ASSERT_EQUAL(209, lastHeader.frameLength());
   }