Bug 1218157: Only ever read from cached data in NotifyDataArrived. r=cpearce a=sylvestre
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 28 Oct 2015 11:28:21 +1100
changeset 291322 2216fd8a49fe
parent 291321 5aecc34603af
child 291326 68048d187998
push id944
push userjyavenard@mozilla.com
push date2015-10-29 09:55 +0000
treeherdermozilla-release@2216fd8a49fe [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;
   }