Bug 1218157: Only ever read from cached data in NotifyDataArrived. r=cpearce a=sylvestre FIREFOX_BETA_42_END
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 28 Oct 2015 11:28:21 +1100
changeset 291325 0ec8472a93ac0c7ef0e98ebb91ac780bde12d5a5
parent 291324 d03bf9e14763ebbb5df611e2bd121815e6b71870
child 291326 68048d18799823f3aea8a1b5107a0539c28db7a6
child 298309 18008d8082545af8aba10cc8665c924125a99b76
push id945
push userryanvm@gmail.com
push dateThu, 29 Oct 2015 15:01:47 +0000
treeherdermozilla-release@4661413507b3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, sylvestre
bugs1218157
milestone42.0
Bug 1218157: Only ever read from cached data in NotifyDataArrived. r=cpearce a=sylvestre The logic of queuing NotifyDataArrived and read data there was fundamentally flawed as we would continually perform reads from the same MediaResource at two different ends. This would cause repetitive seeks and data being removed from the media cache. Worse, a read in NotifyDataArrived would cause another NotifyDataArrived to be scheduled. As range-request are extremely slow, it would result in stutters and constant interruptions. CLOSED TREE
dom/media/apple/AppleMP3Reader.cpp
dom/media/directshow/DirectShowReader.cpp
dom/media/gstreamer/GStreamerReader.cpp
dom/media/omx/MediaCodecReader.cpp
dom/media/omx/MediaOmxReader.cpp
dom/media/webm/WebMReader.cpp
--- a/dom/media/apple/AppleMP3Reader.cpp
+++ b/dom/media/apple/AppleMP3Reader.cpp
@@ -516,29 +516,40 @@ AppleMP3Reader::Seek(int64_t aTime, int6
 void
 AppleMP3Reader::NotifyDataArrivedInternal(uint32_t aLength, int64_t aOffset)
 {
   MOZ_ASSERT(OnTaskQueue());
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  AutoPinned<MediaResource> resource(mResource.GetResource());
+  nsTArray<MediaByteRange> byteRanges;
+  nsresult rv = resource->GetCachedRanges(byteRanges);
+
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  IntervalSet<int64_t> intervals;
+  for (auto& range : byteRanges) {
+    intervals += mFilter.NotifyDataArrived(range.Length(), range.mStart);
+  }
   for (const auto& interval : intervals) {
     nsRefPtr<MediaByteBuffer> bytes =
-      mResource.MediaReadAt(interval.mStart, interval.Length());
+      resource->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/directshow/DirectShowReader.cpp
+++ b/dom/media/directshow/DirectShowReader.cpp
@@ -403,28 +403,38 @@ DirectShowReader::SeekInternal(int64_t a
 void
 DirectShowReader::NotifyDataArrivedInternal(uint32_t aLength, int64_t aOffset)
 {
   MOZ_ASSERT(OnTaskQueue());
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
+  nsTArray<MediaByteRange> byteRanges;
+  nsresult rv = resource->GetCachedRanges(byteRanges);
+
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  IntervalSet<int64_t> intervals;
+  for (auto& range : byteRanges) {
+    intervals += mFilter.NotifyDataArrived(range.Length(), range.mStart);
+  }
   for (const auto& interval : intervals) {
     nsRefPtr<MediaByteBuffer> bytes =
-      mDecoder->GetResource()->MediaReadAt(interval.mStart, interval.Length());
+      resource->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/gstreamer/GStreamerReader.cpp
+++ b/dom/media/gstreamer/GStreamerReader.cpp
@@ -1287,32 +1287,42 @@ void GStreamerReader::NotifyDataArrivedI
   MOZ_ASSERT(OnTaskQueue());
   if (HasVideo()) {
     return;
   }
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  AutoPinned<MediaResource> resource(mResource.GetResource());
+  nsTArray<MediaByteRange> byteRanges;
+  nsresult rv = resource->GetCachedRanges(byteRanges);
+
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  IntervalSet<int64_t> intervals;
+  for (auto& range : byteRanges) {
+    intervals += mFilter.NotifyDataArrived(range.Length(), range.mStart);
+  }
   for (const auto& interval : intervals) {
     nsRefPtr<MediaByteBuffer> bytes =
-      mResource.MediaReadAt(interval.mStart, interval.Length());
+      resource->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/omx/MediaCodecReader.cpp
+++ b/dom/media/omx/MediaCodecReader.cpp
@@ -521,20 +521,31 @@ MediaCodecReader::HasVideo()
 {
   return mInfo.HasVideo();
 }
 
 void
 MediaCodecReader::NotifyDataArrivedInternal(uint32_t aLength,
                                             int64_t aOffset)
 {
-  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
+  nsTArray<MediaByteRange> byteRanges;
+  nsresult rv = resource->GetCachedRanges(byteRanges);
+
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  IntervalSet<int64_t> intervals;
+  for (auto& range : byteRanges) {
+    intervals += mFilter.NotifyDataArrived(range.Length(), range.mStart);
+  }
   for (const auto& interval : intervals) {
     nsRefPtr<MediaByteBuffer> bytes =
-      mDecoder->GetResource()->MediaReadAt(interval.mStart, interval.Length());
+      resource->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;
--- a/dom/media/omx/MediaOmxReader.cpp
+++ b/dom/media/omx/MediaOmxReader.cpp
@@ -468,31 +468,41 @@ void MediaOmxReader::NotifyDataArrivedIn
   }
   if (HasVideo()) {
     return;
   }
   if (!mMP3FrameParser.NeedsData()) {
     return;
   }
 
-  IntervalSet<int64_t> intervals = mFilter.NotifyDataArrived(aLength, aOffset);
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
+  nsTArray<MediaByteRange> byteRanges;
+  nsresult rv = resource->GetCachedRanges(byteRanges);
+
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  IntervalSet<int64_t> intervals;
+  for (auto& range : byteRanges) {
+    intervals += mFilter.NotifyDataArrived(range.Length(), range.mStart);
+  }
   for (const auto& interval : intervals) {
     nsRefPtr<MediaByteBuffer> bytes =
-      mDecoder->GetResource()->MediaReadAt(interval.mStart, interval.Length());
+      resource->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/webm/WebMReader.cpp
+++ b/dom/media/webm/WebMReader.cpp
@@ -824,20 +824,30 @@ media::TimeIntervals WebMReader::GetBuff
   }
 
   return buffered;
 }
 
 void WebMReader::NotifyDataArrivedInternal(uint32_t aLength, int64_t aOffset)
 {
   MOZ_ASSERT(OnTaskQueue());
-  nsRefPtr<MediaByteBuffer> bytes =
-    mDecoder->GetResource()->MediaReadAt(aOffset, aLength);
-  NS_ENSURE_TRUE_VOID(bytes);
-  mBufferedState->NotifyDataArrived(bytes->Elements(), aLength, aOffset);
+  AutoPinned<MediaResource> resource(mResource.GetResource());
+  nsTArray<MediaByteRange> byteRanges;
+  nsresult rv = resource->GetCachedRanges(byteRanges);
+
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  for (auto& range : byteRanges) {
+    nsRefPtr<MediaByteBuffer> bytes =
+      resource->MediaReadAt(range.mStart, range.Length());
+    NS_ENSURE_TRUE_VOID(bytes);
+    mBufferedState->NotifyDataArrived(bytes->Elements(), aLength, aOffset);
+  }
 }
 
 int64_t WebMReader::GetEvictionOffset(double aTime)
 {
   int64_t offset;
   if (!mBufferedState->GetOffsetForTime(aTime * NS_PER_S, &offset)) {
     return -1;
   }