Bug 1055904 - Improve MSE eviction calculation - r=jya

Fixes a bug in the SourceBufferResource eviction code where it was
using the mOffset of the resource as the min bound for what to evict.
This offset is almost always zero though due to ReadFromCache being
used which never updates the offset. This prevented eviction from
happening in most cases.

Moves the code to remove old decoders so that it does this during
the same loop as that which remove data from existing decoders.
This more aggressively prunes old decoders and is more likely to
keep data in the current playing decoder around for seeking, etc.

Prevent removing any decoder that the MediaSourceReader is
currently using for playback to prevent RemoveDecoder crashes.

Add a threshold to subtract from the current time when working out
the time bound to evict before to make it less likely to evict
current data that is needed for current playback.

Remove all data from evicted decoders in the initial iteration then
iterate after to remove empty decoders to put the RemoveDecoder
logic in one place.

Iterate decoders in order that they were added rather than sorted
by time so the logic that removes entire decoders can do it only
to those old decoders that existed before the existing one was
created.

Keeps track of the time that was evicted from the current decoder
and uses that as the time to EvictBefore for all decoders in the
track buffer when doing MediaSource::NotifyEvict.

--HG--
extra : rebase_source : f7b4fe263a8041b3882585caea389742b2a1a9b3

dom/media/MediaResource.h

@@ -133,7 +133,7 @@ public:
   MediaByteRange(int64_t aStart, int64_t aEnd)
     : mStart(aStart), mEnd(aEnd)
   {
-    NS_ASSERTION(mStart < mEnd, "Range should end after start!");
+    NS_ASSERTION(mStart <= mEnd, "Range should end after start!");
   }
 
   explicit MediaByteRange(TimestampedMediaByteRange& aByteRange);

dom/media/mediasource/MediaSourceDecoder.cpp

@@ -268,4 +268,10 @@ MediaSourceDecoder::SetCDMProxy(CDMProxy* aProxy)
 }
 #endif
 
+bool
+MediaSourceDecoder::IsActiveReader(MediaDecoderReader* aReader)
+{
+  return mReader->IsActiveReader(aReader);
+}
+
 } // namespace mozilla

dom/media/mediasource/MediaSourceDecoder.h

@@ -74,6 +74,10 @@ public:
 
   MediaSourceReader* GetReader() { return mReader; }
 
+  // Returns true if aReader is a currently active audio or video
+  // reader in this decoders MediaSourceReader.
+  bool IsActiveReader(MediaDecoderReader* aReader);
+
 private:
   // The owning MediaSource holds a strong reference to this decoder, and
   // calls Attach/DetachMediaSource on this decoder to set and clear

dom/media/mediasource/MediaSourceReader.cpp

@@ -908,4 +908,11 @@ MediaSourceReader::SetCDMProxy(CDMProxy* aProxy)
 }
 #endif
 
+bool
+MediaSourceReader::IsActiveReader(MediaDecoderReader* aReader)
+{
+  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+  return aReader == mVideoReader.get() || aReader == mAudioReader.get();
+}
+
 } // namespace mozilla

dom/media/mediasource/MediaSourceReader.h

@@ -138,6 +138,9 @@ public:
            (!mVideoReader || mVideoReader->IsAsync());
   }
 
+  // Returns true if aReader is a currently active audio or video
+  bool IsActiveReader(MediaDecoderReader* aReader);
+
 private:
   // Switch the current audio/video reader to the reader that
   // contains aTarget (or up to aError after target). Both

dom/media/mediasource/ResourceQueue.h

@@ -105,19 +105,36 @@ public:
   // Tries to evict at least aSizeToEvict from the queue up until
   // aOffset. Returns amount evicted.
   uint32_t Evict(uint64_t aOffset, uint32_t aSizeToEvict) {
+    SBR_DEBUG("ResourceQueue(%p)::Evict(aOffset=%llu, aSizeToEvict=%u)",
+              this, aOffset, aSizeToEvict);
+    return EvictBefore(std::min(aOffset, (uint64_t)aSizeToEvict));
+  }
+
+  uint32_t EvictBefore(uint64_t aOffset) {
+    SBR_DEBUG("ResourceQueue(%p)::EvictBefore(%llu)", this, aOffset);
     uint32_t evicted = 0;
     while (ResourceItem* item = ResourceAt(0)) {
-      if (item->mData.Length() + mOffset > aOffset) {
+      SBR_DEBUG("ResourceQueue(%p)::EvictBefore item=%p length=%d offset=%llu",
+                this, item, item->mData.Length(), mOffset);
+      if (item->mData.Length() + mOffset >= aOffset) {
         break;
       }
       mOffset += item->mData.Length();
       evicted += item->mData.Length();
-      SBR_DEBUGV("ResourceQueue(%p)::Evict(%llu, %u) removed chunk length=%u",
-                 this, aOffset, aSizeToEvict, item->mData.Length());
       delete PopFront();
-      if (aSizeToEvict && evicted >= aSizeToEvict) {
-        break;
-      }
+    }
+    return evicted;
+  }
+
+  uint32_t EvictAll() {
+    SBR_DEBUG("ResourceQueue(%p)::EvictAll()", this);
+    uint32_t evicted = 0;
+    while (ResourceItem* item = ResourceAt(0)) {
+      SBR_DEBUG("ResourceQueue(%p)::EvictAll item=%p length=%d offset=%llu",
+                this, item, item->mData.Length(), mOffset);
+      mOffset += item->mData.Length();
+      evicted += item->mData.Length();
+      delete PopFront();
     }
     return evicted;
   }

dom/media/mediasource/SourceBuffer.cpp

@@ -387,14 +387,17 @@ SourceBuffer::PrepareAppend(ErrorResult& aRv)
   // TODO: Make the eviction threshold smaller for audio-only streams.
   // TODO: Drive evictions off memory pressure notifications.
   // TODO: Consider a global eviction threshold  rather than per TrackBuffer.
-  bool evicted = mTrackBuffer->EvictData(mEvictionThreshold);
+  double newBufferStartTime = 0.0;
+  bool evicted =
+    mTrackBuffer->EvictData(mMediaSource->GetDecoder()->GetCurrentTime(),
+                            mEvictionThreshold, &newBufferStartTime);
   if (evicted) {
     MSE_DEBUG("SourceBuffer(%p)::AppendData Evict; current buffered start=%f",
               this, GetBufferedStart());
 
     // We notify that we've evicted from the time range 0 through to
     // the current start point.
-    mMediaSource->NotifyEvicted(0.0, GetBufferedStart());
+    mMediaSource->NotifyEvicted(0.0, newBufferStartTime);
   }
 
   // TODO: Test buffer full flag.

dom/media/mediasource/SourceBufferResource.cpp

@@ -172,11 +172,12 @@ SourceBufferResource::ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCo
 }
 
 uint32_t
-SourceBufferResource::EvictData(uint32_t aThreshold)
+SourceBufferResource::EvictData(uint64_t aPlaybackOffset, uint32_t aThreshold)
 {
-  SBR_DEBUG("SourceBufferResource(%p)::EvictData(aThreshold=%u)", this, aThreshold);
+  SBR_DEBUG("SourceBufferResource(%p)::EvictData(aPlaybackOffset=%llu,"
+            "aThreshold=%u)", this, aPlaybackOffset, aThreshold);
   ReentrantMonitorAutoEnter mon(mMonitor);
-  return mInputBuffer.Evict(mOffset, aThreshold);
+  return mInputBuffer.Evict(aPlaybackOffset, aThreshold);
 }
 
 void
@@ -186,10 +187,18 @@ SourceBufferResource::EvictBefore(uint64_t aOffset)
   ReentrantMonitorAutoEnter mon(mMonitor);
   // If aOffset is past the current playback offset we don't evict.
   if (aOffset < mOffset) {
-    mInputBuffer.Evict(aOffset, 0);
+    mInputBuffer.EvictBefore(aOffset);
   }
 }
 
+uint32_t
+SourceBufferResource::EvictAll()
+{
+  SBR_DEBUG("SourceBufferResource(%p)::EvictAll()", this);
+  ReentrantMonitorAutoEnter mon(mMonitor);
+  return mInputBuffer.EvictAll();
+}
+
 void
 SourceBufferResource::AppendData(const uint8_t* aData, uint32_t aLength)
 {

dom/media/mediasource/SourceBufferResource.h

@@ -115,11 +115,14 @@ public:
   void Ended();
   // Remove data from resource if it holds more than the threshold
   // number of bytes. Returns amount evicted.
-  uint32_t EvictData(uint32_t aThreshold);
+  uint32_t EvictData(uint64_t aPlaybackOffset, uint32_t aThreshold);
 
   // Remove data from resource before the given offset.
   void EvictBefore(uint64_t aOffset);
 
+  // Remove all data from the resource
+  uint32_t EvictAll();
+
   // Returns the amount of data currently retained by this resource.
   int64_t GetSize() {
     ReentrantMonitorAutoEnter mon(mMonitor);

dom/media/mediasource/TrackBuffer.cpp

@@ -33,6 +33,11 @@ extern PRLogModuleInfo* GetMediaSourceAPILog();
 #define MSE_API(...)
 #endif
 
+// Time in seconds to substract from the current time when deciding the
+// time point to evict data before in a decoder. This is used to help
+// precent evicting the current playback point.
+#define MSE_EVICT_THRESHOLD_TIME 2.0
+
 namespace mozilla {
 
 TrackBuffer::TrackBuffer(MediaSourceDecoder* aParentDecoder, const nsACString& aType)
@@ -240,11 +245,17 @@ public:
 };
 
 bool
-TrackBuffer::EvictData(uint32_t aThreshold)
+TrackBuffer::EvictData(double aPlaybackTime,
+                       uint32_t aThreshold,
+                       double* aBufferStartTime)
 {
   MOZ_ASSERT(NS_IsMainThread());
   ReentrantMonitorAutoEnter mon(mParentDecoder->GetReentrantMonitor());
 
+  if (!mCurrentDecoder) {
+    return false;
+  }
+
   int64_t totalSize = 0;
   for (uint32_t i = 0; i < mDecoders.Length(); ++i) {
     totalSize += mDecoders[i]->GetResource()->GetSize();
@@ -255,46 +266,85 @@ TrackBuffer::EvictData(uint32_t aThreshold)
     return false;
   }
 
-  // Get a list of initialized decoders, sorted by their start times.
+  // Get a list of initialized decoders
   nsTArray<SourceBufferDecoder*> decoders;
   decoders.AppendElements(mInitializedDecoders);
-  decoders.Sort(DecoderSorter());
 
   // First try to evict data before the current play position, starting
   // with the earliest time.
   uint32_t i = 0;
-  for (; i < decoders.Length(); ++i) {
-    MSE_DEBUG("TrackBuffer(%p)::EvictData decoder=%u threshold=%u toEvict=%lld",
-              this, i, aThreshold, toEvict);
-    toEvict -= decoders[i]->GetResource()->EvictData(toEvict);
-    if (!decoders[i]->GetResource()->GetSize() &&
-        decoders[i] != mCurrentDecoder) {
-      RemoveDecoder(decoders[i]);
+  bool pastCurrentDecoder = true;
+  for (; i < decoders.Length() && toEvict > 0; ++i) {
+    nsRefPtr<dom::TimeRanges> buffered = new dom::TimeRanges();
+    decoders[i]->GetBuffered(buffered);
+    bool onCurrent = decoders[i] == mCurrentDecoder;
+    if (onCurrent) {
+      pastCurrentDecoder = false;
     }
-    if (toEvict <= 0 || decoders[i] == mCurrentDecoder) {
-      break;
-    }
-  }
 
-  // If we still need to evict more, then try to evict entire decoders,
-  // starting from the end.
-  if (toEvict > 0) {
-    uint32_t end = i;
-    MOZ_ASSERT(decoders[end] == mCurrentDecoder);
+    MSE_DEBUG("TrackBuffer(%p)::EvictData decoder=%u/%u threshold=%u "
+              "toEvict=%lld current=%s pastCurrent=%s",
+              this, i, decoders.Length(), aThreshold, toEvict,
+              onCurrent ? "true" : "false",
+              pastCurrentDecoder ? "true" : "false");
 
-    for (i = decoders.Length() - 1; i > end; --i) {
-      MSE_DEBUG("TrackBuffer(%p)::EvictData removing entire decoder=%u from end toEvict=%lld",
-                this, i, toEvict);
-      // TODO: We could implement forward-eviction within a decoder and
-      // be able to evict within the current decoder.
-      toEvict -= decoders[i]->GetResource()->GetSize();
-      RemoveDecoder(decoders[i]);
-      if (toEvict <= 0) {
-        break;
+    if (pastCurrentDecoder
+        && !mParentDecoder->IsActiveReader(decoders[i]->GetReader())) {
+      // Remove data from older decoders than the current one.
+      // Don't remove data if it is currently active.
+      MSE_DEBUG("TrackBuffer(%p)::EvictData evicting all before start "
+                "bufferedStart=%f bufferedEnd=%f aPlaybackTime=%f size=%lld",
+                this, buffered->GetStartTime(), buffered->GetEndTime(),
+                aPlaybackTime, decoders[i]->GetResource()->GetSize());
+      toEvict -= decoders[i]->GetResource()->EvictAll();
+    } else {
+      // To ensure we don't evict data past the current playback position
+      // we apply a threshold of a few seconds back and evict data up to
+      // that point.
+      if (aPlaybackTime > MSE_EVICT_THRESHOLD_TIME) {
+        double time = aPlaybackTime - MSE_EVICT_THRESHOLD_TIME;
+        int64_t playbackOffset = decoders[i]->ConvertToByteOffset(time);
+        MSE_DEBUG("TrackBuffer(%p)::EvictData evicting some bufferedEnd=%f"
+                  "aPlaybackTime=%f time=%f, playbackOffset=%lld size=%lld",
+                  this, buffered->GetEndTime(), aPlaybackTime, time,
+                  playbackOffset, decoders[i]->GetResource()->GetSize());
+        if (playbackOffset > 0) {
+          toEvict -= decoders[i]->GetResource()->EvictData(playbackOffset,
+                                                           toEvict);
+        }
       }
     }
   }
-  return toEvict < (totalSize - aThreshold);
+
+  // Remove decoders that have no data in them
+  for (i = 0; i < decoders.Length(); ++i) {
+    nsRefPtr<dom::TimeRanges> buffered = new dom::TimeRanges();
+    decoders[i]->GetBuffered(buffered);
+    MSE_DEBUG("TrackBuffer(%p):EvictData maybe remove empty decoders=%d "
+              "size=%lld start=%f end=%f",
+              this, i, decoders[i]->GetResource()->GetSize(),
+              buffered->GetStartTime(), buffered->GetEndTime());
+    if (decoders[i] == mCurrentDecoder
+        || mParentDecoder->IsActiveReader(decoders[i]->GetReader())) {
+      continue;
+    }
+
+    if (decoders[i]->GetResource()->GetSize() == 0 ||
+        buffered->GetStartTime() < 0.0 ||
+        buffered->GetEndTime() < 0.0) {
+      MSE_DEBUG("TrackBuffer(%p):EvictData remove empty decoders=%d", this, i);
+      RemoveDecoder(decoders[i]);
+    }
+  }
+
+  bool evicted = toEvict < (totalSize - aThreshold);
+  if (evicted) {
+    nsRefPtr<TimeRanges> ranges = new TimeRanges();
+    mCurrentDecoder->GetBuffered(ranges);
+    *aBufferStartTime = std::max(0.0, ranges->GetStartTime());
+  }
+
+  return evicted;
 }
 
 void
@@ -307,10 +357,6 @@ TrackBuffer::EvictBefore(double aTime)
     if (endOffset > 0) {
       MSE_DEBUG("TrackBuffer(%p)::EvictBefore decoder=%u offset=%lld", this, i, endOffset);
       mInitializedDecoders[i]->GetResource()->EvictBefore(endOffset);
-      if (!mInitializedDecoders[i]->GetResource()->GetSize() &&
-          mInitializedDecoders[i] != mCurrentDecoder) {
-        RemoveDecoder(mInitializedDecoders[i]);
-      }
     }
   }
 }
@@ -646,6 +692,9 @@ TrackBuffer::RemoveDecoder(SourceBufferDecoder* aDecoder)
 
   {
     ReentrantMonitorAutoEnter mon(mParentDecoder->GetReentrantMonitor());
+    // There should be no other references to the decoder. Assert that
+    // we aren't using it in the MediaSourceReader.
+    MOZ_ASSERT(!mParentDecoder->IsActiveReader(aDecoder->GetReader()));
     mInitializedDecoders.RemoveElement(aDecoder);
     mDecoders.RemoveElement(aDecoder);
 

dom/media/mediasource/TrackBuffer.h

@@ -40,7 +40,17 @@ public:
   // NotifyDataArrived on the decoder to keep buffered range computation up
   // to date.  Returns false if the append failed.
   bool AppendData(const uint8_t* aData, uint32_t aLength, int64_t aTimestampOffset /* microseconds */);
-  bool EvictData(uint32_t aThreshold);
+
+  // Evicts data held in the current decoders SourceBufferResource from the
+  // start of the buffer through to aPlaybackTime. aThreshold is used to
+  // bound the data being evicted. It will not evict more than aThreshold
+  // bytes. aBufferStartTime contains the new start time of the current
+  // decoders buffered data after the eviction. Returns true if data was
+  // evicted.
+  bool EvictData(double aPlaybackTime, uint32_t aThreshold, double* aBufferStartTime);
+
+  // Evicts data held in all the decoders SourceBufferResource from the start
+  // of the buffer through to aTime.
   void EvictBefore(double aTime);
 
   // Returns the highest end time of all of the buffered ranges in the