From: Tsuda Kageyu <tsuda.kageyu@gmail.com>
Date: Tue, 22 Dec 2015 11:11:26 +0000 (+0900)
Subject: A bit more tolerant check for the MPEG frame length.
X-Git-Tag: v1.11beta~15^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4064b34effda6dfe5064b6c48ed6945904e79287;p=taglib

A bit more tolerant check for the MPEG frame length.
---

diff --git a/taglib/mpeg/mpegheader.cpp b/taglib/mpeg/mpegheader.cpp
index 4e4e4e37..87329614 100644
--- a/taglib/mpeg/mpegheader.cpp
+++ b/taglib/mpeg/mpegheader.cpp
@@ -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.
diff --git a/taglib/mpeg/mpegheader.h b/taglib/mpeg/mpegheader.h
index cd77d37a..024aa112 100644
--- a/taglib/mpeg/mpegheader.h
+++ b/taglib/mpeg/mpegheader.h
@@ -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;
diff --git a/taglib/mpeg/mpegproperties.cpp b/taglib/mpeg/mpegproperties.cpp
index b9ae8675..6e7bb823 100644
--- a/taglib/mpeg/mpegproperties.cpp
+++ b/taglib/mpeg/mpegproperties.cpp
@@ -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);
   }
diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp
index 455fde2c..b9f463a4 100644
--- a/tests/test_mpeg.cpp
+++ b/tests/test_mpeg.cpp
@@ -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());
   }