Bug 919572 - Refactor the MP3 frame parser. r=cpearce, a=koi+
authorEdwin Flores <eflores@mozilla.com>
Sun, 27 Oct 2013 17:26:14 -0400
changeset 155956 df153717f1ff
parent 155955 561e09c7b506
child 155957 43bce1bb6978
push id4514
push userryanvm@gmail.com
push dateSun, 27 Oct 2013 21:26:40 +0000
treeherdermozilla-aurora@df153717f1ff [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, koi
bugs919572
milestone26.0a2
Bug 919572 - Refactor the MP3 frame parser. r=cpearce, a=koi+
content/media/MP3FrameParser.cpp
content/media/MP3FrameParser.h
--- a/content/media/MP3FrameParser.cpp
+++ b/content/media/MP3FrameParser.cpp
@@ -4,462 +4,367 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <algorithm>
 #include "nsMemory.h"
 #include "MP3FrameParser.h"
 #include "VideoUtils.h"
 
+
 namespace mozilla {
 
-// An ID3Buffer contains data of an ID3v2 header. The supplied buffer must
-// point to an ID3 header and at least the size of ID_HEADER_LENGTH. Run the
-// Parse method to read in the header's values.
+/*
+ * Following code taken from http://www.hydrogenaudio.org/forums/index.php?showtopic=85125
+ * with permission from the author, Nick Wallette <sirnickity@gmail.com>.
+ */
 
-class ID3Buffer
-{
-public:
+/* BEGIN shameless copy and paste */
 
-  enum {
-    ID3_HEADER_LENGTH = 10
-  };
+// MPEG versions - use [version]
+const uint8_t mpeg_versions[4] = { 25, 0, 2, 1 };
+
+// Layers - use [layer]
+const uint8_t mpeg_layers[4] = { 0, 3, 2, 1 };
 
-  ID3Buffer(const uint8_t* aBuffer, uint32_t aLength)
-  : mBuffer(aBuffer),
-    mLength(aLength),
-    mSize(0)
-  {
-    MOZ_ASSERT(mBuffer || !mLength);
+// Bitrates - use [version][layer][bitrate]
+const uint16_t mpeg_bitrates[4][4][16] = {
+  { // Version 2.5
+    { 0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0, 0 }, // Reserved
+    { 0,   8,  16,  24,  32,  40,  48,  56,  64,  80,  96, 112, 128, 144, 160, 0 }, // Layer 3
+    { 0,   8,  16,  24,  32,  40,  48,  56,  64,  80,  96, 112, 128, 144, 160, 0 }, // Layer 2
+    { 0,  32,  48,  56,  64,  80,  96, 112, 128, 144, 160, 176, 192, 224, 256, 0 }  // Layer 1
+  },
+  { // Reserved
+    { 0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0, 0 }, // Invalid
+    { 0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0, 0 }, // Invalid
+    { 0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0, 0 }, // Invalid
+    { 0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0, 0 }  // Invalid
+  },
+  { // Version 2
+    { 0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0, 0 }, // Reserved
+    { 0,   8,  16,  24,  32,  40,  48,  56,  64,  80,  96, 112, 128, 144, 160, 0 }, // Layer 3
+    { 0,   8,  16,  24,  32,  40,  48,  56,  64,  80,  96, 112, 128, 144, 160, 0 }, // Layer 2
+    { 0,  32,  48,  56,  64,  80,  96, 112, 128, 144, 160, 176, 192, 224, 256, 0 }  // Layer 1
+  },
+  { // Version 1
+    { 0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0, 0 }, // Reserved
+    { 0,  32,  40,  48,  56,  64,  80,  96, 112, 128, 160, 192, 224, 256, 320, 0 }, // Layer 3
+    { 0,  32,  48,  56,  64,  80,  96, 112, 128, 160, 192, 224, 256, 320, 384, 0 }, // Layer 2
+    { 0,  32,  64,  96, 128, 160, 192, 224, 256, 288, 320, 352, 384, 416, 448, 0 }, // Layer 1
   }
+};
 
-  nsresult Parse();
-
-  int64_t Length() const {
-    return ID3_HEADER_LENGTH + mSize;
-  }
-
-private:
-  const uint8_t* mBuffer;
-  uint32_t       mLength;
-  uint32_t       mSize;
+// Sample rates - use [version][srate]
+const uint16_t mpeg_srates[4][4] = {
+    { 11025, 12000,  8000, 0 }, // MPEG 2.5
+    {     0,     0,     0, 0 }, // Reserved
+    { 22050, 24000, 16000, 0 }, // MPEG 2
+    { 44100, 48000, 32000, 0 }  // MPEG 1
 };
 
-nsresult ID3Buffer::Parse()
-{
-  NS_ENSURE_TRUE(mBuffer && mLength >= ID3_HEADER_LENGTH, NS_ERROR_INVALID_ARG);
+// Samples per frame - use [version][layer]
+const uint16_t mpeg_frame_samples[4][4] = {
+//    Rsvd     3     2     1  < Layer  v Version
+    {    0,  576, 1152,  384 }, //       2.5
+    {    0,    0,    0,    0 }, //       Reserved
+    {    0,  576, 1152,  384 }, //       2
+    {    0, 1152, 1152,  384 }  //       1
+};
+
+// Slot size (MPEG unit of measurement) - use [layer]
+const uint8_t mpeg_slot_size[4] = { 0, 1, 1, 4 }; // Rsvd, 3, 2, 1
 
-  if ((mBuffer[0] != 'I') ||
-      (mBuffer[1] != 'D') ||
-      (mBuffer[2] != '3') ||
-      (mBuffer[6] & 0x80) ||
-      (mBuffer[7] & 0x80) ||
-      (mBuffer[8] & 0x80) ||
-      (mBuffer[9] & 0x80)) {
-    return NS_ERROR_INVALID_ARG;
-  }
+uint16_t
+MP3Frame::CalculateLength()
+{
+  // Lookup real values of these fields
+  uint32_t  bitrate   = mpeg_bitrates[mVersion][mLayer][mBitrate] * 1000;
+  uint32_t  samprate  = mpeg_srates[mVersion][mSampleRate];
+  uint16_t  samples   = mpeg_frame_samples[mVersion][mLayer];
+  uint8_t   slot_size = mpeg_slot_size[mLayer];
 
-  mSize = ((static_cast<uint32_t>(mBuffer[6])<<21) |
-           (static_cast<uint32_t>(mBuffer[7])<<14) |
-           (static_cast<uint32_t>(mBuffer[8])<<7)  |
-            static_cast<uint32_t>(mBuffer[9]));
+  // In-between calculations
+  float     bps       = (float)samples / 8.0;
+  float     fsize     = ( (bps * (float)bitrate) / (float)samprate )
+    + ( (mPad) ? slot_size : 0 );
 
-  return NS_OK;
+  // Frame sizes are truncated integers
+  return (uint16_t)fsize;
 }
 
-// The MP3Buffer contains MP3 frame data. The supplied buffer must point
-// to a frame header. Call the method Parse to extract information from
-// the MP3 frame headers in the supplied buffer.
+/* END shameless copy and paste */
+
+
+/** MP3Parser methods **/
 
-class MP3Buffer
+MP3Parser::MP3Parser()
+  : mCurrentChar(0)
+{ }
+
+void
+MP3Parser::Reset()
 {
-public:
-
-  enum {
-    MP3_HEADER_LENGTH   = 4,
-    MP3_FRAMESIZE_CONST = 144000,
-    MP3_DURATION_CONST  = 8000
-  };
+  mCurrentChar = 0;
+}
 
-  MP3Buffer(const uint8_t* aBuffer, uint32_t aLength)
-  : mBuffer(aBuffer),
-    mLength(aLength),
-    mDurationUs(0),
-    mNumFrames(0),
-    mBitRateSum(0),
-    mSampleRate(0),
-    mFrameSizeSum(0)
-  {
-    MOZ_ASSERT(mBuffer || !mLength);
-  }
+uint16_t
+MP3Parser::ParseFrameLength(uint8_t ch)
+{
+  mData.mRaw[mCurrentChar] = ch;
 
-  nsresult Parse();
+  MP3Frame &frame = mData.mFrame;
 
-  int64_t GetDuration() const {
-    return mDurationUs;
-  }
-
-  int64_t GetNumberOfFrames() const {
-    return mNumFrames;
-  }
+  // Validate MP3 header as we read. We can't mistake the start of an MP3 frame
+  // for the middle of another frame due to the sync byte at the beginning
+  // of the frame.
 
-  int64_t GetBitRateSum() const {
-    return mBitRateSum;
-  }
-
-  int16_t GetSampleRate() const {
-    return mSampleRate;
-  }
-
-  int64_t GetFrameSizeSum() const {
-    return mFrameSizeSum;
+  // The only valid position for an all-high byte is the sync byte at the
+  // beginning of the frame.
+  if (ch == 0xff) {
+    mCurrentChar = 0;
   }
 
-private:
-
-  enum MP3FrameHeaderField {
-    MP3_HDR_FIELD_SYNC,
-    MP3_HDR_FIELD_VERSION,
-    MP3_HDR_FIELD_LAYER,
-    MP3_HDR_FIELD_BITRATE,
-    MP3_HDR_FIELD_SAMPLERATE,
-    MP3_HDR_FIELD_PADDING,
-    MP3_HDR_FIELDS // Must be last enumerator value
-  };
-
-  enum {
-    MP3_HDR_CONST_FRAMESYNC = 0x7ff,
-    MP3_HDR_CONST_VERSION   = 3,
-    MP3_HDR_CONST_LAYER     = 1
-  };
-
-  static uint32_t ExtractBits(uint32_t aValue, uint32_t aOffset,
-                              uint32_t aBits);
-  static uint32_t ExtractFrameHeaderField(uint32_t aHeader,
-                                          enum MP3FrameHeaderField aField);
-  static uint32_t ExtractFrameHeader(const uint8_t* aBuffer);
-  static nsresult DecodeFrameHeader(const uint8_t* aBuffer,
-                                    uint32_t* aFrameSize,
-                                    uint32_t* aBitRate,
-                                    uint16_t* aSampleRate,
-                                    uint64_t* aDuration);
+  // Make sure the current byte is valid in context. If not, reset the parser.
+  if (mCurrentChar == 2) {
+    if (frame.mBitrate == 0x0f) {
+      goto fail;
+    }
+  } else if (mCurrentChar == 1) {
+    if (frame.mSync2 != 0x07
+        || frame.mVersion == 0x01
+        || frame.mLayer == 0x00) {
+      goto fail;
+    }
+  }
 
-  static const uint16_t sBitRate[16];
-  static const uint16_t sSampleRate[4];
-
-  const uint8_t* mBuffer;
-  uint32_t       mLength;
+  // The only valid character at the beginning of the header is 0xff. Fail if
+  // it's different.
+  if (mCurrentChar == 0 && frame.mSync1 != 0xff) {
+    // Couldn't find the sync byte. Fail.
+    return 0;
+  }
 
-  // The duration of this parsers data in milliseconds.
-  int64_t mDurationUs;
-
-  // The number of frames in the range.
-  int64_t mNumFrames;
-
-  // The sum of all frame's bit rates.
-  int64_t mBitRateSum;
+  mCurrentChar++;
+  MOZ_ASSERT(mCurrentChar <= sizeof(MP3Frame));
 
-  // The number of audio samples per second
-  int16_t mSampleRate;
-
-  // The sum of all frame's sizes in byte.
-  int32_t mFrameSizeSum;
-};
+  // Don't have a full header yet.
+  if (mCurrentChar < sizeof(MP3Frame)) {
+    return 0;
+  }
 
-const uint16_t MP3Buffer::sBitRate[16] = {
-  0, 32, 40, 48, 56, 64, 80, 96, 112, 128, 160, 192, 224, 256, 320, 0
-};
+  // Woo, valid header. Return the length.
+  mCurrentChar = 0;
+  return frame.CalculateLength();
 
-const uint16_t MP3Buffer::sSampleRate[4] = {
-  44100, 48000, 32000, 0
-};
-
-uint32_t MP3Buffer::ExtractBits(uint32_t aValue, uint32_t aOffset, uint32_t aBits)
-{
-  return (aValue >> aOffset) & ((0x1ul << aBits) - 1);
+fail:
+  Reset();
+  return 0;
 }
 
-uint32_t MP3Buffer::ExtractFrameHeaderField(uint32_t aHeader, enum MP3FrameHeaderField aField)
+uint32_t
+MP3Parser::GetSampleRate()
 {
-  static const uint8_t sField[MP3_HDR_FIELDS][2] = {
-    {21, 11}, {19, 2}, {17, 2}, {12, 4}, {10, 2}, {9, 1}
-  };
-
-  MOZ_ASSERT(aField < MP3_HDR_FIELDS);
-  return ExtractBits(aHeader, sField[aField][0], sField[aField][1]);
+  MP3Frame &frame = mData.mFrame;
+  return mpeg_srates[frame.mVersion][frame.mSampleRate];
 }
 
-uint32_t MP3Buffer::ExtractFrameHeader(const uint8_t* aBuffer)
-{
-  MOZ_ASSERT(aBuffer);
+
+/** ID3Parser methods **/
 
-  uint32_t header = (static_cast<uint32_t>(aBuffer[0])<<24) |
-                    (static_cast<uint32_t>(aBuffer[1])<<16) |
-                    (static_cast<uint32_t>(aBuffer[2])<<8)  |
-                     static_cast<uint32_t>(aBuffer[3]);
+const char sID3Head[3] = { 'I', 'D', '3' };
+const uint32_t ID3_HEADER_LENGTH = 10;
 
-  uint32_t frameSync = ExtractFrameHeaderField(header, MP3_HDR_FIELD_SYNC);
-  uint32_t version = ExtractFrameHeaderField(header, MP3_HDR_FIELD_VERSION);
-  uint32_t layer = ExtractFrameHeaderField(header, MP3_HDR_FIELD_LAYER);
-  uint32_t bitRate = sBitRate[ExtractFrameHeaderField(header, MP3_HDR_FIELD_BITRATE)];
-  uint32_t sampleRate = sSampleRate[ExtractFrameHeaderField(header, MP3_HDR_FIELD_SAMPLERATE)];
+ID3Parser::ID3Parser()
+  : mCurrentChar(0)
+  , mHeaderLength(0)
+{ }
 
-  // branch-less implementation of
-  //
-  //  if (fields-are-valid)
-  //    return header;
-  //  else
-  //    return 0;
-  //
-  return (frameSync == uint32_t(MP3_HDR_CONST_FRAMESYNC)) *
-         (version == uint32_t(MP3_HDR_CONST_VERSION)) *
-         (layer == uint32_t(MP3_HDR_CONST_LAYER)) * !!bitRate * !!sampleRate * header;
+void
+ID3Parser::Reset()
+{
+  mCurrentChar = mHeaderLength = 0;
 }
 
-nsresult MP3Buffer::DecodeFrameHeader(const uint8_t* aBuffer,
-                                      uint32_t* aFrameSize,
-                                      uint32_t* aBitRate,
-                                      uint16_t* aSampleRate,
-                                      uint64_t* aDuration)
+bool
+ID3Parser::ParseChar(char ch)
 {
-  uint32_t header = ExtractFrameHeader(aBuffer);
-
-  if (!header) {
-    return NS_ERROR_INVALID_ARG;
+  // First three bytes of an ID3v2 header must match the string "ID3".
+  if (mCurrentChar < sizeof(sID3Head) / sizeof(*sID3Head)
+      && ch != sID3Head[mCurrentChar]) {
+    goto fail;
   }
 
-  uint32_t bitRate = sBitRate[ExtractFrameHeaderField(header, MP3_HDR_FIELD_BITRATE)];
-  uint32_t sampleRate = sSampleRate[ExtractFrameHeaderField(header, MP3_HDR_FIELD_SAMPLERATE)];
-
-  uint32_t padding = ExtractFrameHeaderField(header, MP3_HDR_FIELD_PADDING);
-  uint32_t frameSize = (uint64_t(MP3_FRAMESIZE_CONST) * bitRate) / sampleRate + padding;
-
-  MOZ_ASSERT(aBitRate);
-  *aBitRate = bitRate;
+  // The last four bytes of the header is a 28-bit unsigned integer with the
+  // high bit of each byte unset.
+  if (mCurrentChar >= 6 && mCurrentChar < ID3_HEADER_LENGTH) {
+    if (ch & 0x80) {
+      goto fail;
+    } else {
+      mHeaderLength <<= 7;
+      mHeaderLength |= ch;
+    }
+  }
 
-  MOZ_ASSERT(aFrameSize);
-  *aFrameSize = frameSize;
+  mCurrentChar++;
+
+  return IsParsed();
 
-  MOZ_ASSERT(aDuration);
-  *aDuration = (uint64_t(MP3_DURATION_CONST) * frameSize) / bitRate;
-
-  MOZ_ASSERT(aSampleRate);
-  *aSampleRate = sampleRate;
-
-  return NS_OK;
+fail:
+  Reset();
+  return false;
 }
 
-nsresult MP3Buffer::Parse()
+bool
+ID3Parser::IsParsed() const
 {
-  // We walk over the newly arrived data and sum up the
-  // bit rates, sizes, durations, etc. of the contained
-  // MP3 frames.
-
-  const uint8_t* buffer = mBuffer;
-  uint32_t length = mLength;
-
-  while (length >= MP3_HEADER_LENGTH) {
-
-    uint32_t frameSize;
-    uint32_t bitRate;
-    uint16_t sampleRate;
-    uint64_t duration;
+  return mCurrentChar >= ID3_HEADER_LENGTH;
+}
 
-    nsresult rv = DecodeFrameHeader(buffer, &frameSize, &bitRate,
-                                    &sampleRate, &duration);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-
-    mBitRateSum += bitRate;
-    mDurationUs += duration;
-    ++mNumFrames;
-
-    mFrameSizeSum += frameSize;
+uint32_t
+ID3Parser::GetHeaderLength() const
+{
+  MOZ_ASSERT(IsParsed(),
+             "Queried length of ID3 header before parsing finished.");
+  return mHeaderLength;
+}
 
-    mSampleRate = sampleRate;
 
-    if (frameSize <= length) {
-      length -= frameSize;
-    } else {
-      length = 0;
-    }
-
-    buffer += frameSize;
-  }
-
-  return NS_OK;
-}
+/** MP3FrameParser methods **/
 
 // Some MP3's have large ID3v2 tags, up to 150KB, so we allow lots of
 // skipped bytes to be read, just in case, before we give up and assume
 // we're not parsing an MP3 stream.
 static const uint32_t MAX_SKIPPED_BYTES = 200 * 1024;
 
 // The number of audio samples per MP3 frame. This is constant over all MP3
 // streams. With this constant, the stream's sample rate, and an estimated
 // number of frames in the stream, we can estimate the stream's duration
 // fairly accurately.
 static const uint32_t SAMPLES_PER_FRAME = 1152;
 
+enum {
+  MP3_HEADER_LENGTH   = 4,
+};
+
 MP3FrameParser::MP3FrameParser(int64_t aLength)
-: mBufferLength(0),
-  mLock("MP3FrameParser.mLock"),
-  mDurationUs(0),
-  mBitRateSum(0),
+: mLock("MP3FrameParser.mLock"),
   mTotalFrameSize(0),
   mNumFrames(0),
   mOffset(0),
   mLength(aLength),
   mMP3Offset(-1),
-  mSkippedBytes(0),
   mSampleRate(0),
   mIsMP3(MAYBE_MP3)
 { }
 
 nsresult MP3FrameParser::ParseBuffer(const uint8_t* aBuffer,
                                      uint32_t aLength,
                                      int64_t aStreamOffset,
                                      uint32_t* aOutBytesRead)
 {
   // Iterate forwards over the buffer, looking for ID3 tag, or MP3
   // Frame headers.
-  uint32_t bufferOffset = 0;
-  uint32_t headersParsed = 0;
-  while (bufferOffset < aLength) {
-    const uint8_t* buffer = aBuffer + bufferOffset;
-    const uint32_t length = aLength - bufferOffset;
-    if (mMP3Offset == -1) {
-      // We've not found any MP3 frames yet, there may still be ID3 tags in
-      // the stream, so test for them.
-      if (length < ID3Buffer::ID3_HEADER_LENGTH) {
-        // We don't have enough data to get a complete ID3 header, bail.
-        break;
-      }
-      ID3Buffer id3Buffer(buffer, length);
-      if (NS_SUCCEEDED(id3Buffer.Parse())) {
-        bufferOffset += id3Buffer.Length();
-        // Try to parse the next chunk.
-        headersParsed++;
-        continue;
+
+  const uint8_t *buffer = aBuffer;
+  const uint8_t *bufferEnd = aBuffer + aLength;
+
+  // If we haven't found any MP3 frame data yet, there might be ID3 headers
+  // we can skip over.
+  if (mMP3Offset < 0) {
+    for (const uint8_t *ch = buffer; ch < bufferEnd; ch++) {
+      if (mID3Parser.ParseChar(*ch)) {
+        // Found an ID3 header. We don't care about the body of the header, so
+        // just skip past.
+        buffer = ch + mID3Parser.GetHeaderLength() - (ID3_HEADER_LENGTH - 1);
+        ch = buffer;
+
+        // Yes, this is an MP3!
+        mIsMP3 = DEFINITELY_MP3;
+
+        mID3Parser.Reset();
       }
     }
-    if (length < MP3Buffer::MP3_HEADER_LENGTH) {
-      // We don't have enough data to get a complete MP3 frame header, bail.
-      break;
-    }
-    MP3Buffer mp3Buffer(buffer, length);
-    if (NS_SUCCEEDED(mp3Buffer.Parse())) {
-      headersParsed++;
-      if (mMP3Offset == -1) {
-        mMP3Offset = aStreamOffset + bufferOffset;
+  }
+
+  while (buffer < bufferEnd) {
+    uint16_t frameLen = mMP3Parser.ParseFrameLength(*buffer);
+
+    if (frameLen) {
+
+      if (mMP3Offset < 0) {
+        // Found our first frame: mark this stream as MP3 and let the decoder
+        // know where in the stream the MP3 data starts.
+        mIsMP3 = DEFINITELY_MP3;
+        // We're at the last byte of an MP3Frame, so MP3 data started
+        // sizeof - 1 bytes ago.
+        mMP3Offset = aStreamOffset
+          + (buffer - aBuffer)
+          - (sizeof(MP3Frame) - 1);
       }
-      mDurationUs += mp3Buffer.GetDuration();
-      mBitRateSum += mp3Buffer.GetBitRateSum();
-      mTotalFrameSize += mp3Buffer.GetFrameSizeSum();
-      mSampleRate = mp3Buffer.GetSampleRate();
-      mNumFrames += mp3Buffer.GetNumberOfFrames();
-      bufferOffset += mp3Buffer.GetFrameSizeSum();
+
+      mSampleRate = mMP3Parser.GetSampleRate();
+      mTotalFrameSize += frameLen;
+      mNumFrames++;
+
+      buffer += frameLen - sizeof(MP3Frame);
     } else {
-      // No ID3 or MP3 frame header here. Try the next byte.
-      ++bufferOffset;
+      buffer++;
     }
   }
-  if (headersParsed == 0) {
-    if (mIsMP3 == MAYBE_MP3) {
-      mSkippedBytes += aLength;
-      if (mSkippedBytes > MAX_SKIPPED_BYTES) {
-        mIsMP3 = NOT_MP3;
-        return NS_ERROR_FAILURE;
-      }
-    }
-  } else {
-    mIsMP3 = DEFINITELY_MP3;
-    mSkippedBytes = 0;
-  }
-  *aOutBytesRead = bufferOffset;
+
+  *aOutBytesRead = buffer - aBuffer;
   return NS_OK;
 }
 
 void MP3FrameParser::Parse(const char* aBuffer, uint32_t aLength, int64_t aOffset)
 {
   MutexAutoLock mon(mLock);
 
   const uint8_t* buffer = reinterpret_cast<const uint8_t*>(aBuffer);
-  const int64_t lastChunkEnd = mOffset + mBufferLength;
-  if (aOffset + aLength <= lastChunkEnd) {
-    // We already processed this fragment.
-    return;
-  } else if (aOffset < lastChunkEnd) {
-    // mOffset is within the new fragment, shorten range.
-    aLength -= lastChunkEnd - aOffset;
-    buffer += lastChunkEnd - aOffset;
-    aOffset = lastChunkEnd;
-  } else if (aOffset > lastChunkEnd) {
-    // Fragment comes after current position, store difference.
-    mOffset += aOffset - lastChunkEnd;
-    mSkippedBytes = 0;
+  int32_t length = aLength;
+  int64_t offset = aOffset;
+
+  // Got some data we have seen already. Skip forward to what we need.
+  if (aOffset < mOffset) {
+    buffer += mOffset - aOffset;
+    length -= mOffset - aOffset;
+    offset = mOffset;
+
+    if (length <= 0) {
+      return;
+    }
   }
 
-  if (mBufferLength > 0) {
-    // We have some data which was left over from the last buffer we received.
-    // Append to it, so that we have enough data to parse a complete header, and
-    // try to parse it.
-    uint32_t copyLength = std::min<size_t>(NS_ARRAY_LENGTH(mBuffer)-mBufferLength, aLength);
-    memcpy(mBuffer+mBufferLength, buffer, copyLength*sizeof(*mBuffer));
-    // Caculate the offset of the data in the start of the buffer.
-    int64_t streamOffset = mOffset - mBufferLength;
-    uint32_t bufferLength = mBufferLength + copyLength;
-    uint32_t bytesRead = 0;
-    if (NS_FAILED(ParseBuffer(mBuffer,
-                              bufferLength,
-                              streamOffset,
-                              &bytesRead))) {
-      return;
+  // If there is a discontinuity in the input stream, reset the state of the
+  // parsers so we don't get any partial headers.
+  if (mOffset < aOffset) {
+    if (!mID3Parser.IsParsed()) {
+      // Only reset this if it hasn't finished yet.
+      mID3Parser.Reset();
     }
-    MOZ_ASSERT(bytesRead >= mBufferLength, "Parse should leave original buffer");
-
-    // Adjust the incoming buffer pointer/length so that it reflects that we may have
-    // consumed data from buffer.
-    uint32_t adjust = bytesRead - mBufferLength;
-    mBufferLength = 0;
-    if (adjust >= aLength) {
-      // The frame or tag found in the buffer finishes outside the range.
-      // Just set the offset to the end of that tag/frame, and return.
-      mOffset = streamOffset + bytesRead;
-      if (mOffset > mLength) {
-        mLength = mOffset;
-      }
-      return;
-    }
-    aOffset += adjust;
-    MOZ_ASSERT(aLength >= adjust);
-    aLength -= adjust;
+    mMP3Parser.Reset();
   }
 
   uint32_t bytesRead = 0;
   if (NS_FAILED(ParseBuffer(buffer,
-                            aLength,
-                            aOffset,
+                            length,
+                            offset,
                             &bytesRead))) {
     return;
   }
-  mOffset += bytesRead;
+
+  MOZ_ASSERT(length <= (int)bytesRead, "All bytes should have been consumed");
 
-  if (bytesRead < aLength) {
-    // We have some data left over. Store trailing bytes in temporary buffer
-    // to be parsed next time we receive more data.
-    uint32_t trailing = aLength - bytesRead;
-    MOZ_ASSERT(trailing < (NS_ARRAY_LENGTH(mBuffer)*sizeof(mBuffer[0])));
-    memcpy(mBuffer, buffer+(aLength-trailing), trailing);
-    mBufferLength = trailing;
-  }
+  // Update next data offset
+  mOffset = offset + bytesRead;
 
-  if (mOffset > mLength) {
-    mLength = mOffset;
+  // If we've parsed lots of data and we still have nothing, just give up.
+  if (!mID3Parser.IsParsed() && !mNumFrames && mOffset > MAX_SKIPPED_BYTES) {
+    mIsMP3 = NOT_MP3;
   }
 }
 
 int64_t MP3FrameParser::GetDuration()
 {
   MutexAutoLock mon(mLock);
 
   if (!mNumFrames || !mSampleRate) {
--- a/content/media/MP3FrameParser.h
+++ b/content/media/MP3FrameParser.h
@@ -4,16 +4,73 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <stdint.h>
 #include "mozilla/Mutex.h"
 
 namespace mozilla {
 
+// Simple parser to tell whether we've found an ID3 header and how long it is,
+// so that we can skip it.
+// XXX maybe actually parse this stuff?
+class ID3Parser
+{
+public:
+  ID3Parser();
+
+  void Reset();
+  bool ParseChar(char ch);
+  bool IsParsed() const;
+  uint32_t GetHeaderLength() const;
+
+private:
+  uint32_t mCurrentChar;
+  uint32_t mHeaderLength;
+};
+
+struct MP3Frame {
+  uint16_t mSync1 : 8;      // Always all set
+  uint16_t mProtected : 1;  // Ignored
+  uint16_t mLayer : 2;
+  uint16_t mVersion : 2;
+  uint16_t mSync2 : 3;      // Always all set
+  uint16_t mPrivate : 1;    // Ignored
+  uint16_t mPad : 1;
+  uint16_t mSampleRate : 2; // Index into mpeg_srates above
+  uint16_t mBitrate : 4;    // Index into mpeg_bitrates above
+
+  uint16_t CalculateLength();
+};
+
+// Buffering parser for MP3 frames.
+class MP3Parser
+{
+public:
+  MP3Parser();
+
+  // Forget all data the parser has seen so far.
+  void Reset();
+
+  // Parse the given byte. If we have found a frame header, return the length of
+  // the frame.
+  uint16_t ParseFrameLength(uint8_t ch);
+
+  // Get the sample rate from the current header.
+  uint32_t GetSampleRate();
+
+private:
+  uint32_t mCurrentChar;
+  union {
+    uint8_t mRaw[3];
+    MP3Frame mFrame;
+  } mData;
+};
+
+
 // A description of the MP3 format and its extensions is available at
 //
 //  http://www.codeproject.com/Articles/8295/MPEG-Audio-Frame-Header
 //
 // The data in MP3 streams is split into small frames, with each frame
 // containing a fixed number of samples. The duration of a frame depends
 // on the frame's bit rate and sample rate. Both values can vary among
 // frames, so it is necessary to examine each individual frame of an MP3
@@ -63,47 +120,42 @@ private:
   // greater than aLength if the headers in the buffer indicate that
   // the frame or ID3 tag extends outside of aBuffer. Returns failure
   // if too many non-MP3 bytes are parsed.
   nsresult ParseBuffer(const uint8_t* aBuffer,
                        uint32_t aLength,
                        int64_t aStreamOffset,
                        uint32_t* aOutBytesRead);
 
-  // mBuffer must be at least 19 bytes long, in case the last byte in the
-  // buffer is the first byte in a 10 byte long ID3 tag header.
-  uint8_t  mBuffer[32];
-  uint32_t mBufferLength;
-
   // A low-contention lock for protecting the parser results
   Mutex mLock;
 
+  // ID3 header parser. Keeps state between reads in case the header falls
+  // in between.
+  ID3Parser mID3Parser;
+
+  // MP3 frame header parser.
+  MP3Parser mMP3Parser;
+
   // All fields below are protected by mLock
-  uint64_t mDurationUs;
-  uint64_t mBitRateSum;
   uint64_t mTotalFrameSize;
   uint64_t mNumFrames;
 
   // Offset of the last data parsed. This is the end offset of the last data
   // block parsed, so it's the start offset we expect to get on the next
   // call to Parse().
   int64_t  mOffset;
 
   // Total length of the stream in bytes.
   int64_t  mLength;
 
   // Offset of first MP3 frame in the bitstream. Has value -1 until the
   // first MP3 frame is found.
   int64_t mMP3Offset;
 
-  // Count of bytes that have been parsed but skipped over because we couldn't
-  // find a sync pattern or an ID3 header. If this gets too high, we assume
-  // the stream either isn't MP3, or is corrupt.
-  uint32_t mSkippedBytes;
-
   // Number of audio samples per second. Fixed through the whole file.
   uint16_t mSampleRate;
 
   enum eIsMP3 {
     MAYBE_MP3, // We're giving the stream the benefit of the doubt...
     DEFINITELY_MP3, // We've hit at least one ID3 tag or MP3 frame.
     NOT_MP3 // Not found any evidence of the stream being MP3.
   };