Bug 1208953: [mp3] Don't parse data we've already parsed. r=cpearce
authorJean-Yves Avenard <jyavenard@mozilla.com>
Tue, 29 Sep 2015 17:29:44 +1000
changeset 265133 7fc9c161460291a9e71cffe9eb1997990fc162f9
parent 265132 6fad4ce51fad711b31ed0b8733b8e87811ec3b96
child 265134 42657128cf5602b4844b5913722b94ab4e236588
push id65856
push userjyavenard@mozilla.com
push dateWed, 30 Sep 2015 06:08:21 +0000
treeherdermozilla-inbound@7fc9c1614602 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1208953
milestone44.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 1208953: [mp3] Don't parse data we've already parsed. r=cpearce NotifyDataArrived may be called again due to reads performed in NotifyDataArrived ; causing stall and serious slowdowns.
dom/media/MP3FrameParser.h
dom/media/apple/AppleMP3Reader.cpp
dom/media/apple/AppleMP3Reader.h
dom/media/directshow/DirectShowReader.cpp
dom/media/directshow/DirectShowReader.h
dom/media/gstreamer/GStreamerReader.cpp
dom/media/gstreamer/GStreamerReader.h
dom/media/omx/MediaCodecReader.cpp
dom/media/omx/MediaCodecReader.h
dom/media/omx/MediaOmxReader.cpp
dom/media/omx/MediaOmxReader.h
--- a/dom/media/MP3FrameParser.h
+++ b/dom/media/MP3FrameParser.h
@@ -6,16 +6,17 @@
 
 #ifndef MP3FrameParser_h
 #define MP3FrameParser_h
 
 #include <stdint.h>
 
 #include "mozilla/Mutex.h"
 #include "nsString.h"
+#include "Intervals.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
 {
@@ -208,11 +209,29 @@ private:
     DEFINITELY_MP3, // We've hit at least one ID3 tag or MP3 frame.
     NOT_MP3 // Not found any evidence of the stream being MP3.
   };
 
   eIsMP3 mIsMP3;
 
 };
 
+class NotifyDataArrivedFilter {
+public:
+  media::IntervalSet<int64_t> NotifyDataArrived(uint32_t aLength, int64_t aOffset) {
+    media::Interval<int64_t> interval(aOffset, aOffset + aLength);
+    media::IntervalSet<int64_t> newIntervals(interval);
+    newIntervals -= mIntervals;
+    mIntervals += interval;
+    return newIntervals;
+  }
+
+  const media::IntervalSet<int64_t>& GetIntervals() {
+    return mIntervals;
+  }
+
+private:
+  media::IntervalSet<int64_t> mIntervals;
+};
+
 } // namespace mozilla
 
 #endif
--- a/dom/media/apple/AppleMP3Reader.cpp
+++ b/dom/media/apple/AppleMP3Reader.cpp
@@ -516,26 +516,29 @@ AppleMP3Reader::Seek(int64_t aTime, int6
 void
 AppleMP3Reader::NotifyDataArrivedInternal(uint32_t aLength, int64_t aOffset)
 {
   MOZ_ASSERT(OnTaskQueue());
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  nsRefPtr<MediaByteBuffer> bytes =
-    mDecoder->GetResource()->MediaReadAt(aOffset, aLength);
-  NS_ENSURE_TRUE_VOID(bytes);
-  mMP3FrameParser.Parse(bytes->Elements(), aLength, aOffset);
-  if (!mMP3FrameParser.IsMP3()) {
-    return;
-  }
+  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  for (const auto& interval : intervals) {
+    nsRefPtr<MediaByteBuffer> bytes =
+      mResource.MediaReadAt(interval.mStart, interval.Length());
+    NS_ENSURE_TRUE_VOID(bytes);
+    mMP3FrameParser.Parse(bytes->Elements(), interval.Length(), interval.mStart);
+    if (!mMP3FrameParser.IsMP3()) {
+      return;
+    }
 
-  uint64_t duration = mMP3FrameParser.GetDuration();
-  if (duration != mDuration) {
-    LOGD("Updating media duration to %lluus\n", duration);
-    MOZ_ASSERT(mDecoder);
-    mDuration = duration;
-    mDecoder->DispatchUpdateEstimatedMediaDuration(duration);
+    uint64_t duration = mMP3FrameParser.GetDuration();
+    if (duration != mDuration) {
+      LOGD("Updating media duration to %lluus\n", duration);
+      MOZ_ASSERT(mDecoder);
+      mDuration = duration;
+      mDecoder->DispatchUpdateEstimatedMediaDuration(duration);
+    }
   }
 }
 
 } // namespace mozilla
--- a/dom/media/apple/AppleMP3Reader.h
+++ b/dom/media/apple/AppleMP3Reader.h
@@ -77,13 +77,14 @@ private:
   uint64_t mDuration;
 
   AudioFileStreamID mAudioFileStream;
   AudioConverterRef mAudioConverter;
 
   MP3FrameParser mMP3FrameParser;
 
   MediaResourceIndex mResource;
+  NotifyDataArrivedFilter mFilter;
 };
 
 } // namespace mozilla
 
 #endif // __AppleMP3Reader_h__
--- a/dom/media/directshow/DirectShowReader.cpp
+++ b/dom/media/directshow/DirectShowReader.cpp
@@ -403,24 +403,28 @@ DirectShowReader::SeekInternal(int64_t a
 void
 DirectShowReader::NotifyDataArrivedInternal(uint32_t aLength, int64_t aOffset)
 {
   MOZ_ASSERT(OnTaskQueue());
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  nsRefPtr<MediaByteBuffer> bytes = mDecoder->GetResource()->MediaReadAt(aOffset, aLength);
-  NS_ENSURE_TRUE_VOID(bytes);
-  mMP3FrameParser.Parse(bytes->Elements(), aLength, aOffset);
-  if (!mMP3FrameParser.IsMP3()) {
-    return;
-  }
+  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  for (const auto& interval : intervals) {
+    nsRefPtr<MediaByteBuffer> bytes =
+      mDecoder->GetResource()->MediaReadAt(interval.mStart, interval.Length());
+    NS_ENSURE_TRUE_VOID(bytes);
+    mMP3FrameParser.Parse(bytes->Elements(), interval.Length(), interval.mStart);
+    if (!mMP3FrameParser.IsMP3()) {
+      return;
+    }
 
-  int64_t duration = mMP3FrameParser.GetDuration();
-  if (duration != mDuration) {
-    MOZ_ASSERT(mDecoder);
-    mDuration = duration;
-    mDecoder->DispatchUpdateEstimatedMediaDuration(mDuration);
+    int64_t duration = mMP3FrameParser.GetDuration();
+    if (duration != mDuration) {
+      MOZ_ASSERT(mDecoder);
+      mDuration = duration;
+      mDecoder->DispatchUpdateEstimatedMediaDuration(mDuration);
+    }
   }
 }
 
 } // namespace mozilla
--- a/dom/media/directshow/DirectShowReader.h
+++ b/dom/media/directshow/DirectShowReader.h
@@ -105,13 +105,15 @@ private:
   // Samples per second in the audio stream.
   uint32_t mAudioRate;
 
   // Number of bytes per sample. Can be either 1 or 2.
   uint32_t mBytesPerSample;
 
   // Duration of the stream, in microseconds.
   int64_t mDuration;
+
+  NotifyDataArrivedFilter mFilter;
 };
 
 } // namespace mozilla
 
 #endif
--- a/dom/media/gstreamer/GStreamerReader.cpp
+++ b/dom/media/gstreamer/GStreamerReader.cpp
@@ -1287,29 +1287,32 @@ void GStreamerReader::NotifyDataArrivedI
   MOZ_ASSERT(OnTaskQueue());
   if (HasVideo()) {
     return;
   }
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  nsRefPtr<MediaByteBuffer> bytes =
-    mResource.MediaReadAt(aOffset, aLength);
-  NS_ENSURE_TRUE_VOID(bytes);
-  mMP3FrameParser.Parse(bytes->Elements(), aLength, aOffset);
-  if (!mMP3FrameParser.IsMP3()) {
-    return;
-  }
+  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  for (const auto& interval : intervals) {
+    nsRefPtr<MediaByteBuffer> bytes =
+      mResource.MediaReadAt(interval.mStart, interval.Length());
+    NS_ENSURE_TRUE_VOID(bytes);
+    mMP3FrameParser.Parse(bytes->Elements(), interval.Length(), interval.mStart);
+    if (!mMP3FrameParser.IsMP3()) {
+      return;
+    }
 
-  int64_t duration = mMP3FrameParser.GetDuration();
-  if (duration != mLastParserDuration && mUseParserDuration) {
-    MOZ_ASSERT(mDecoder);
-    mLastParserDuration = duration;
-    mDecoder->DispatchUpdateEstimatedMediaDuration(mLastParserDuration);
+    int64_t duration = mMP3FrameParser.GetDuration();
+    if (duration != mLastParserDuration && mUseParserDuration) {
+      MOZ_ASSERT(mDecoder);
+      mLastParserDuration = duration;
+      mDecoder->DispatchUpdateEstimatedMediaDuration(mLastParserDuration);
+    }
   }
 }
 
 #if GST_VERSION_MAJOR >= 1
 GstCaps* GStreamerReader::BuildAudioSinkCaps()
 {
   GstCaps* caps = gst_caps_from_string("audio/x-raw, channels={1,2}");
   const char* format;
--- a/dom/media/gstreamer/GStreamerReader.h
+++ b/dom/media/gstreamer/GStreamerReader.h
@@ -260,13 +260,14 @@ private:
   bool mReachedVideoEos;
 #if GST_VERSION_MAJOR >= 1
   bool mConfigureAlignment;
 #endif
   int fpsNum;
   int fpsDen;
 
   MediaResourceIndex mResource;
+  NotifyDataArrivedFilter mFilter;
 };
 
 } // namespace mozilla
 
 #endif
--- a/dom/media/omx/MediaCodecReader.cpp
+++ b/dom/media/omx/MediaCodecReader.cpp
@@ -521,37 +521,38 @@ MediaCodecReader::HasVideo()
 {
   return mInfo.HasVideo();
 }
 
 void
 MediaCodecReader::NotifyDataArrivedInternal(uint32_t aLength,
                                             int64_t aOffset)
 {
-  nsRefPtr<MediaByteBuffer> bytes =
-    mDecoder->GetResource()->MediaReadAt(aOffset, aLength);
-  NS_ENSURE_TRUE_VOID(bytes);
-
-  MonitorAutoLock monLock(mParserMonitor);
-  if (mNextParserPosition == mParsedDataLength &&
-      mNextParserPosition >= aOffset &&
-      mNextParserPosition <= aOffset + aLength) {
-    // No pending parsing runnable currently. And available data are adjacent to
-    // parsed data.
-    int64_t shift = mNextParserPosition - aOffset;
-    const char* buffer = reinterpret_cast<const char*>(bytes->Elements()) + shift;
-    uint32_t length = aLength - shift;
-    int64_t offset = mNextParserPosition;
-    if (length > 0) {
-      MonitorAutoUnlock monUnlock(mParserMonitor);
-      ParseDataSegment(buffer, length, offset);
+  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  for (const auto& interval : intervals) {
+    nsRefPtr<MediaByteBuffer> bytes =
+      mDecoder->GetResource()->MediaReadAt(interval.mStart, interval.Length());
+    MonitorAutoLock monLock(mParserMonitor);
+    if (mNextParserPosition == mParsedDataLength &&
+        mNextParserPosition >= interval.mStart &&
+        mNextParserPosition <= interval.mEnd) {
+      // No pending parsing runnable currently. And available data are adjacent to
+      // parsed data.
+      int64_t shift = mNextParserPosition - interval.mStart;
+      const char* buffer = reinterpret_cast<const char*>(bytes->Elements()) + shift;
+      uint32_t length = interval.Length() - shift;
+      int64_t offset = mNextParserPosition;
+      if (length > 0) {
+        MonitorAutoUnlock monUnlock(mParserMonitor);
+        ParseDataSegment(buffer, length, offset);
+      }
+      mParseDataFromCache = false;
+      mParsedDataLength = offset + length;
+      mNextParserPosition = mParsedDataLength;
     }
-    mParseDataFromCache = false;
-    mParsedDataLength = offset + length;
-    mNextParserPosition = mParsedDataLength;
   }
 }
 
 int64_t
 MediaCodecReader::ProcessCachedData(int64_t aOffset,
                                     nsRefPtr<SignalObject> aSignal)
 {
   // We read data in chunks of 32 KiB. We can reduce this
--- a/dom/media/omx/MediaCodecReader.h
+++ b/dom/media/omx/MediaCodecReader.h
@@ -22,16 +22,18 @@
 #include "I420ColorConverterHelper.h"
 #include "MediaCodecProxy.h"
 #include "MediaOmxCommonReader.h"
 #include "mozilla/layers/FenceUtils.h"
 #if MOZ_WIDGET_GONK && ANDROID_VERSION >= 17
 #include <ui/Fence.h>
 #endif
 
+#include "MP3FrameParser.h"
+
 namespace android {
 struct ALooper;
 struct AMessage;
 
 class MOZ_EXPORT MediaExtractor;
 class MOZ_EXPORT MetaData;
 class MOZ_EXPORT MediaBuffer;
 struct MOZ_EXPORT MediaSource;
@@ -431,13 +433,15 @@ private:
   struct ReleaseItem {
     ReleaseItem(size_t aIndex, const FenceHandle& aFence)
     : mReleaseIndex(aIndex)
     , mReleaseFence(aFence) {}
     size_t mReleaseIndex;
     FenceHandle mReleaseFence;
   };
   nsTArray<ReleaseItem> mPendingReleaseItems;
+
+  NotifyDataArrivedFilter mFilter;
 };
 
 } // namespace mozilla
 
 #endif // MEDIA_CODEC_READER_H
--- a/dom/media/omx/MediaOmxReader.cpp
+++ b/dom/media/omx/MediaOmxReader.cpp
@@ -468,28 +468,31 @@ void MediaOmxReader::NotifyDataArrivedIn
   }
   if (HasVideo()) {
     return;
   }
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  nsRefPtr<MediaByteBuffer> bytes =
-    mDecoder->GetResource()->MediaReadAt(aOffset, aLength);
-  NS_ENSURE_TRUE_VOID(bytes);
-  mMP3FrameParser.Parse(bytes->Elements(), aLength, aOffset);
-  if (!mMP3FrameParser.IsMP3()) {
-    return;
-  }
+  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  for (const auto& interval : intervals) {
+    nsRefPtr<MediaByteBuffer> bytes =
+      mDecoder->GetResource()->MediaReadAt(interval.mStart, interval.Length());
+    NS_ENSURE_TRUE_VOID(bytes);
+    mMP3FrameParser.Parse(bytes->Elements(), interval.Length(), interval.mStart);
+    if (!mMP3FrameParser.IsMP3()) {
+      return;
+    }
 
-  int64_t duration = mMP3FrameParser.GetDuration();
-  if (duration != mLastParserDuration) {
-    mLastParserDuration = duration;
-    decoder->DispatchUpdateEstimatedMediaDuration(mLastParserDuration);
+    int64_t duration = mMP3FrameParser.GetDuration();
+    if (duration != mLastParserDuration) {
+      mLastParserDuration = duration;
+      decoder->DispatchUpdateEstimatedMediaDuration(mLastParserDuration);
+    }
   }
 }
 
 bool MediaOmxReader::DecodeAudioData()
 {
   MOZ_ASSERT(OnTaskQueue());
   EnsureActive();
 
--- a/dom/media/omx/MediaOmxReader.h
+++ b/dom/media/omx/MediaOmxReader.h
@@ -123,13 +123,15 @@ private:
   bool IsShutdown() {
     MutexAutoLock lock(mShutdownMutex);
     return mIsShutdown;
   }
 
   int64_t ProcessCachedData(int64_t aOffset);
 
   already_AddRefed<AbstractMediaDecoder> SafeGetDecoder();
+
+  NotifyDataArrivedFilter mFilter;
 };
 
 } // namespace mozilla
 
 #endif