Bug 1216460 - [1.1] Refactor data types, fix logs and prevent harmful type promotions in SourceBuffer eviction handling. r=jya
authorEugen Sawin <esawin@mozilla.com>
Fri, 18 Mar 2016 22:32:03 +0100
changeset 289890 6495286a5790d50950cb6c2b9afdae0ef1fe4538
parent 289889 8d7ed429bf8537950c26158d9044b6cf1871b9ee
child 289891 9ffa65c64e14209ebd0ce7a44b7254a6c5fc6b9e
push id18337
push usercbook@mozilla.com
push dateWed, 23 Mar 2016 15:30:25 +0000
treeherderfx-team@67ac681f7e53 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1216460
milestone48.0a1
Bug 1216460 - [1.1] Refactor data types, fix logs and prevent harmful type promotions in SourceBuffer eviction handling. r=jya
dom/media/mediasource/SourceBufferContentManager.h
dom/media/mediasource/SourceBufferResource.cpp
dom/media/mediasource/SourceBufferResource.h
dom/media/mediasource/TrackBuffersManager.cpp
dom/media/mediasource/TrackBuffersManager.h
--- a/dom/media/mediasource/SourceBufferContentManager.h
+++ b/dom/media/mediasource/SourceBufferContentManager.h
@@ -65,17 +65,17 @@ public:
   };
 
   // Evicts data up 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 data after the
   // eviction.
   virtual EvictDataResult
   EvictData(media::TimeUnit aPlaybackTime,
-            uint32_t aThreshold,
+            int64_t aThreshold,
             media::TimeUnit* aBufferStartTime) = 0;
 
   // Evicts data up to aTime.
   virtual void EvictBefore(media::TimeUnit aTime) = 0;
 
   // Returns the buffered range currently managed.
   // This may be called on any thread.
   // Buffered must conform to http://w3c.github.io/media-source/index.html#widl-SourceBuffer-buffered
--- a/dom/media/mediasource/SourceBufferResource.cpp
+++ b/dom/media/mediasource/SourceBufferResource.cpp
@@ -129,17 +129,17 @@ SourceBufferResource::ReadFromCache(char
   mOffset = oldOffset; // ReadFromCache isn't supposed to affect the seek position.
   NS_ENSURE_SUCCESS(rv, rv);
 
   // ReadFromCache return failure if not all the data is cached.
   return bytesRead == aCount ? NS_OK : NS_ERROR_FAILURE;
 }
 
 uint32_t
-SourceBufferResource::EvictData(uint64_t aPlaybackOffset, uint32_t aThreshold,
+SourceBufferResource::EvictData(uint64_t aPlaybackOffset, int64_t aThreshold,
                                 ErrorResult& aRv)
 {
   SBR_DEBUG("EvictData(aPlaybackOffset=%llu,"
             "aThreshold=%u)", aPlaybackOffset, aThreshold);
   ReentrantMonitorAutoEnter mon(mMonitor);
   uint32_t result = mInputBuffer.Evict(aPlaybackOffset, aThreshold, aRv);
   if (result > 0) {
     // Wake up any waiting threads in case a ReadInternal call
--- a/dom/media/mediasource/SourceBufferResource.h
+++ b/dom/media/mediasource/SourceBufferResource.h
@@ -108,17 +108,17 @@ public:
   void Ended();
   bool IsEnded()
   {
     ReentrantMonitorAutoEnter mon(mMonitor);
     return mEnded;
   }
   // Remove data from resource if it holds more than the threshold
   // number of bytes. Returns amount evicted.
-  uint32_t EvictData(uint64_t aPlaybackOffset, uint32_t aThreshold,
+  uint32_t EvictData(uint64_t aPlaybackOffset, int64_t aThreshold,
                      ErrorResult& aRv);
 
   // Remove data from resource before the given offset.
   void EvictBefore(uint64_t aOffset, ErrorResult& aRv);
 
   // Remove all data from the resource
   uint32_t EvictAll();
 
--- a/dom/media/mediasource/TrackBuffersManager.cpp
+++ b/dom/media/mediasource/TrackBuffersManager.cpp
@@ -193,17 +193,17 @@ TrackBuffersManager::RangeRemoval(TimeUn
 
   return InvokeAsync(GetTaskQueue(), this, __func__,
                      &TrackBuffersManager::CodedFrameRemovalWithPromise,
                      TimeInterval(aStart, aEnd));
 }
 
 TrackBuffersManager::EvictDataResult
 TrackBuffersManager::EvictData(TimeUnit aPlaybackTime,
-                               uint32_t aThreshold,
+                               int64_t aThresholdReduct,
                                TimeUnit* aBufferStartTime)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MSE_DEBUG("");
 
   int64_t toEvict = GetSize() - aThreshold;
   if (toEvict <= 0) {
     return EvictDataResult::NO_DATA_EVICTED;
@@ -348,56 +348,55 @@ TrackBuffersManager::CompleteResetParser
   SetAppendState(AppendState::WAITING_FOR_SEGMENT);
 
   // Reject our promise immediately.
   mAppendPromise.RejectIfExists(NS_ERROR_ABORT, __func__);
 }
 
 void
 TrackBuffersManager::DoEvictData(const TimeUnit& aPlaybackTime,
-                                 uint32_t aSizeToEvict)
+                                 int64_t aSizeToEvict)
 {
   MOZ_ASSERT(OnTaskQueue());
 
   // Video is what takes the most space, only evict there if we have video.
   const auto& track = HasVideo() ? mVideoTracks : mAudioTracks;
   const auto& buffer = track.mBuffers.LastElement();
   // Remove any data we've already played, or before the next sample to be
   // demuxed whichever is lowest.
   TimeUnit lowerLimit = std::min(track.mNextSampleTime, aPlaybackTime);
   uint32_t lastKeyFrameIndex = 0;
   int64_t toEvict = aSizeToEvict;
-  uint32_t partialEvict = 0;
+  int64_t partialEvict = 0;
   for (uint32_t i = 0; i < buffer.Length(); i++) {
     const auto& frame = buffer[i];
     if (frame->mKeyframe) {
       lastKeyFrameIndex = i;
       toEvict -= partialEvict;
       if (toEvict < 0) {
         break;
       }
       partialEvict = 0;
     }
     if (frame->mTime >= lowerLimit.ToMicroseconds()) {
       break;
     }
     partialEvict += frame->ComputedSizeOfIncludingThis();
   }
 
-  int64_t finalSize = mSizeSourceBuffer - aSizeToEvict;
-
   if (lastKeyFrameIndex > 0) {
-    MSE_DEBUG("Step1. Evicting %u bytes prior currentTime",
+    MSE_DEBUG("Step1. Evicting %lld bytes prior currentTime",
               aSizeToEvict - toEvict);
     CodedFrameRemoval(
       TimeInterval(TimeUnit::FromMicroseconds(0),
                    TimeUnit::FromMicroseconds(buffer[lastKeyFrameIndex]->mTime - 1)));
   }
 
-  if (mSizeSourceBuffer <= finalSize) {
+  const int64_t finalSize = mSizeSourceBuffer - aSizeToEvict;
+  if (mSizeSourceBuffer <= finalSize || !buffer.Length()) {
     return;
   }
 
   toEvict = mSizeSourceBuffer - finalSize;
 
   // Still some to remove. Remove data starting from the end, up to 30s ahead
   // of the later of the playback time or the next sample to be demuxed.
   // 30s is a value chosen as it appears to work with YouTube.
@@ -410,18 +409,18 @@ TrackBuffersManager::DoEvictData(const T
       // We've reached a frame that shouldn't be evicted -> Evict after it -> i+1.
       // Or the previous loop reached the eviction threshold -> Evict from it -> i+1.
       evictedFramesStartIndex = i + 1;
       break;
     }
     toEvict -= frame->ComputedSizeOfIncludingThis();
   }
   if (evictedFramesStartIndex < buffer.Length()) {
-    MSE_DEBUG("Step2. Evicting %u bytes from trailing data",
-              mSizeSourceBuffer - finalSize);
+    MSE_DEBUG("Step2. Evicting %lld bytes from trailing data",
+              mSizeSourceBuffer - finalSize - toEvict);
     CodedFrameRemoval(
       TimeInterval(TimeUnit::FromMicroseconds(buffer[evictedFramesStartIndex]->mTime),
                    TimeUnit::FromInfinity()));
   }
 }
 
 RefPtr<TrackBuffersManager::RangeRemovalPromise>
 TrackBuffersManager::CodedFrameRemovalWithPromise(TimeInterval aInterval)
@@ -1625,17 +1624,17 @@ TrackBuffersManager::InsertFrames(TrackB
 
 void
 TrackBuffersManager::RemoveFrames(const TimeIntervals& aIntervals,
                                   TrackData& aTrackData,
                                   uint32_t aStartIndex)
 {
   TrackBuffer& data = aTrackData.mBuffers.LastElement();
   Maybe<uint32_t> firstRemovedIndex;
-  uint32_t lastRemovedIndex;
+  uint32_t lastRemovedIndex = 0;
 
   // We loop from aStartIndex to avoid removing frames that we inserted earlier
   // and part of the current coded frame group. This is allows to handle step
   // 14 of the coded frame processing algorithm without having to check the value
   // of highest end timestamp:
   // "Remove existing coded frames in track buffer:
   //  If highest end timestamp for track buffer is not set:
   //   Remove all coded frames from track buffer that have a presentation timestamp greater than or equal to presentation timestamp and less than frame end timestamp.
--- a/dom/media/mediasource/TrackBuffersManager.h
+++ b/dom/media/mediasource/TrackBuffersManager.h
@@ -51,17 +51,17 @@ public:
 
   void ResetParserState() override;
 
   RefPtr<RangeRemovalPromise> RangeRemoval(media::TimeUnit aStart,
                                              media::TimeUnit aEnd) override;
 
   EvictDataResult
   EvictData(media::TimeUnit aPlaybackTime,
-            uint32_t aThreshold,
+            int64_t aThresholdReduct,
             media::TimeUnit* aBufferStartTime) override;
 
   void EvictBefore(media::TimeUnit aTime) override;
 
   media::TimeIntervals Buffered() override;
 
   int64_t GetSize() override;
 
@@ -195,17 +195,17 @@ private:
   void DoDemuxAudio();
   void OnAudioDemuxCompleted(RefPtr<MediaTrackDemuxer::SamplesHolder> aSamples);
   void OnAudioDemuxFailed(DemuxerFailureReason aFailure)
   {
     mAudioTracks.mDemuxRequest.Complete();
     OnDemuxFailed(TrackType::kAudioTrack, aFailure);
   }
 
-  void DoEvictData(const media::TimeUnit& aPlaybackTime, uint32_t aThreshold);
+  void DoEvictData(const media::TimeUnit& aPlaybackTime, int64_t aThreshold);
 
   struct TrackData {
     TrackData()
       : mNumTracks(0)
       , mNeedRandomAccessPoint(true)
       , mSizeBuffer(0)
     {}
     uint32_t mNumTracks;