Bug 919572 - Refactor the MP3 frame parser r=cpearce
authorEdwin Flores <eflores@mozilla.com>
Sat, 28 Sep 2013 16:33:32 +1200
changeset 149129 89512636e032
parent 149128 caec8c0c4963
child 149130 5761d3de664c
push id25374
push usercbook@mozilla.com
push dateSun, 29 Sep 2013 09:37:16 +0000
treeherdermozilla-central@8f805d3ef377 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs919572
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 919572 - Refactor the MP3 frame parser r=cpearce
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) {
--- 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.
   };