7 files changed, 65 insertions, 170 deletions

diff --git a/dom/media/mediasource/MediaSourceDecoder.cpp b/dom/media/mediasource/MediaSourceDecoder.cpp
index 98fc77990..db3cf3f44 100644
--- a/dom/media/mediasource/MediaSourceDecoder.cpp
+++ b/dom/media/mediasource/MediaSourceDecoder.cpp
@@ -299,6 +299,12 @@ MediaDecoderOwner::NextFrameStatus
 MediaSourceDecoder::NextFrameBufferedStatus()
 {
   MOZ_ASSERT(NS_IsMainThread());
+
+  if (!mMediaSource ||
+      mMediaSource->ReadyState() == dom::MediaSourceReadyState::Closed) {
+    return MediaDecoderOwner::NEXT_FRAME_UNAVAILABLE;
+  }
+
   // Next frame hasn't been decoded yet.
   // Use the buffered range to consider if we have the next frame available.
   TimeUnit currentPosition = TimeUnit::FromMicroseconds(CurrentPosition());
diff --git a/dom/media/mediasource/MediaSourceDemuxer.cpp b/dom/media/mediasource/MediaSourceDemuxer.cpp
index ec9323db9..768308e5d 100644
--- a/dom/media/mediasource/MediaSourceDemuxer.cpp
+++ b/dom/media/mediasource/MediaSourceDemuxer.cpp
@@ -19,7 +19,7 @@ using media::TimeIntervals;
 
 MediaSourceDemuxer::MediaSourceDemuxer()
   : mTaskQueue(new MediaTaskQueue(GetMediaThreadPool(MediaThreadType::PLAYBACK),
-                                  /* aSupportsTailDispatch = */ true))
+                                  /* aSupportsTailDispatch = */ false))
   , mMonitor("MediaSourceDemuxer")
 {
   MOZ_ASSERT(NS_IsMainThread());
diff --git a/dom/media/mediasource/SourceBuffer.cpp b/dom/media/mediasource/SourceBuffer.cpp
index 30b98c0b4..5c063db03 100644
--- a/dom/media/mediasource/SourceBuffer.cpp
+++ b/dom/media/mediasource/SourceBuffer.cpp
@@ -43,27 +43,6 @@ namespace mozilla {
 
 namespace dom {
 
-class BufferAppendRunnable : public nsRunnable {
-public:
-  BufferAppendRunnable(SourceBuffer* aSourceBuffer,
-                       uint32_t aUpdateID)
-  : mSourceBuffer(aSourceBuffer)
-  , mUpdateID(aUpdateID)
-  {
-  }
-
-  NS_IMETHOD Run() override final {
-
-    mSourceBuffer->BufferAppend(mUpdateID);
-
-    return NS_OK;
-  }
-
-private:
-  nsRefPtr<SourceBuffer> mSourceBuffer;
-  uint32_t mUpdateID;
-};
-
 void
 SourceBuffer::SetMode(SourceBufferAppendMode aMode, ErrorResult& aRv)
 {
@@ -219,10 +198,13 @@ void
 SourceBuffer::AbortBufferAppend()
 {
   if (mUpdating) {
-    mPendingAppend.DisconnectIfExists();
-    // TODO: Abort stream append loop algorithms.
-    // cancel any pending buffer append.
-    mContentManager->AbortAppendData();
+    if (mPendingAppend.Exists()) {
+      mPendingAppend.Disconnect();
+      mContentManager->AbortAppendData();
+      // Some data may have been added by the Segment Parser Loop.
+      // Check if we need to update the duration.
+      CheckEndTime();
+    }
     AbortUpdating();
   }
 }
@@ -303,7 +285,6 @@ SourceBuffer::SourceBuffer(MediaSource* aMediaSource, const nsACString& aType)
   , mMediaSource(aMediaSource)
   , mUpdating(false)
   , mActive(false)
-  , mUpdateID(0)
   , mReportedOffset(0)
   , mType(aType)
 {
@@ -380,7 +361,6 @@ SourceBuffer::StartUpdating()
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mUpdating);
   mUpdating = true;
-  mUpdateID++;
   QueueAsyncSimpleEvent("updatestart");
 }
 
@@ -389,12 +369,8 @@ SourceBuffer::StopUpdating()
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (!mUpdating) {
-    // The buffer append algorithm has been interrupted by abort().
-    //
-    // If the sequence appendBuffer(), abort(), appendBuffer() occurs before
-    // the first StopUpdating() runnable runs, then a second StopUpdating()
-    // runnable will be scheduled, but still only one (the first) will queue
-    // events.
+    // The buffer append or range removal algorithm  has been interrupted by
+    // abort().
     return;
   }
   mUpdating = false;
@@ -406,7 +382,6 @@ void
 SourceBuffer::AbortUpdating()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(mUpdating);
   mUpdating = false;
   QueueAsyncSimpleEvent("abort");
   QueueAsyncSimpleEvent("updateend");
@@ -440,23 +415,14 @@ SourceBuffer::AppendData(const uint8_t* aData, uint32_t aLength, ErrorResult& aR
   MOZ_ASSERT(mIsUsingFormatReader ||
              mAttributes->GetAppendMode() == SourceBufferAppendMode::Segments,
              "We don't handle timestampOffset for sequence mode yet");
-  nsRefPtr<nsIRunnable> task = new BufferAppendRunnable(this, mUpdateID);
-  NS_DispatchToMainThread(task);
+
+  BufferAppend();
 }
 
 void
-SourceBuffer::BufferAppend(uint32_t aUpdateID)
-{
-  if (!mUpdating || aUpdateID != mUpdateID) {
-    // The buffer append algorithm has been interrupted by abort().
-    //
-    // If the sequence appendBuffer(), abort(), appendBuffer() occurs before
-    // the first StopUpdating() runnable runs, then a second StopUpdating()
-    // runnable will be scheduled, but still only one (the first) will queue
-    // events.
-    return;
-  }
-
+SourceBuffer::BufferAppend()
+{
+  MOZ_ASSERT(mUpdating);
   MOZ_ASSERT(mMediaSource);
   MOZ_ASSERT(!mPendingAppend.Exists());
 
@@ -469,11 +435,8 @@ SourceBuffer::BufferAppend(uint32_t aUpdateID)
 void
 SourceBuffer::AppendDataCompletedWithSuccess(bool aHasActiveTracks)
 {
+  MOZ_ASSERT(mUpdating);
   mPendingAppend.Complete();
-  if (!mUpdating) {
-    // The buffer append algorithm has been interrupted by abort().
-    return;
-  }
 
   if (aHasActiveTracks) {
     if (!mActive) {
@@ -502,7 +465,9 @@ SourceBuffer::AppendDataCompletedWithSuccess(bool aHasActiveTracks)
 void
 SourceBuffer::AppendDataErrored(nsresult aError)
 {
+  MOZ_ASSERT(mUpdating);
   mPendingAppend.Complete();
+
   switch (aError) {
     case NS_ERROR_ABORT:
       // Nothing further to do as the trackbuffer has been shutdown.
@@ -518,10 +483,7 @@ void
 SourceBuffer::AppendError(bool aDecoderError)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  if (!mUpdating) {
-    // The buffer append algorithm has been interrupted by abort().
-    return;
-  }
+
   mContentManager->ResetParserState();
 
   mUpdating = false;
diff --git a/dom/media/mediasource/SourceBuffer.h b/dom/media/mediasource/SourceBuffer.h
index fe69489c2..f856d72a2 100644
--- a/dom/media/mediasource/SourceBuffer.h
+++ b/dom/media/mediasource/SourceBuffer.h
@@ -247,7 +247,7 @@ private:
 
   // Shared implementation of AppendBuffer overloads.
   void AppendData(const uint8_t* aData, uint32_t aLength, ErrorResult& aRv);
-  void BufferAppend(uint32_t aAppendID);
+  void BufferAppend();
 
   // Implement the "Append Error Algorithm".
   // Will call endOfStream() with "decode" error if aDecodeError is true.
@@ -276,10 +276,6 @@ private:
 
   mozilla::Atomic<bool> mActive;
 
-  // Each time mUpdating is set to true, mUpdateID will be incremented.
-  // This allows for a queued AppendData task to identify if it was earlier
-  // aborted and another AppendData queued.
-  uint32_t mUpdateID;
   int64_t mReportedOffset;
 
   MediaPromiseRequestHolder<SourceBufferContentManager::AppendPromise> mPendingAppend;
diff --git a/dom/media/mediasource/SourceBufferContentManager.h b/dom/media/mediasource/SourceBufferContentManager.h
index 66a2ca483..f7ac18594 100644
--- a/dom/media/mediasource/SourceBufferContentManager.h
+++ b/dom/media/mediasource/SourceBufferContentManager.h
@@ -107,10 +107,6 @@ public:
   virtual void RestartGroupStartTimestamp() {}
   virtual TimeUnit GroupEndTimestamp() = 0;
 
-#if defined(DEBUG)
-  virtual void Dump(const char* aPath) { }
-#endif
-
 protected:
   virtual ~SourceBufferContentManager() { }
 };
diff --git a/dom/media/mediasource/TrackBuffersManager.cpp b/dom/media/mediasource/TrackBuffersManager.cpp
index e939442d3..fb57a4506 100644
--- a/dom/media/mediasource/TrackBuffersManager.cpp
+++ b/dom/media/mediasource/TrackBuffersManager.cpp
@@ -55,15 +55,14 @@ TrackBuffersManager::TrackBuffersManager(dom::SourceBufferAttributes* aAttribute
   , mType(aType)
   , mParser(ContainerParser::CreateForMIMEType(aType))
   , mProcessedInput(0)
-  , mAppendRunning(false)
   , mTaskQueue(aParentDecoder->GetDemuxer()->GetTaskQueue())
   , mSourceBufferAttributes(aAttributes)
   , mParentDecoder(new nsMainThreadPtrHolder<MediaSourceDecoder>(aParentDecoder, false /* strict */))
-  , mAbort(false)
   , mEvictionThreshold(Preferences::GetUint("media.mediasource.eviction_threshold",
                                             100 * (1 << 20)))
   , mEvictionOccurred(false)
   , mMonitor("TrackBuffersManager")
+  , mAppendRunning(false)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Must be instantiated on the main thread");
 }
@@ -94,7 +93,6 @@ TrackBuffersManager::AppendIncomingBuffer(IncomingBuffer aData)
 {
   MOZ_ASSERT(OnTaskQueue());
   mIncomingBuffers.AppendElement(aData);
-  mAbort = false;
 }
 
 nsRefPtr<TrackBuffersManager::AppendPromise>
@@ -103,49 +101,42 @@ TrackBuffersManager::BufferAppend()
   MOZ_ASSERT(NS_IsMainThread());
   MSE_DEBUG("");
 
+  mAppendRunning = true;
   return ProxyMediaCall(GetTaskQueue(), this,
                         __func__, &TrackBuffersManager::InitSegmentParserLoop);
 }
 
-// Abort any pending AppendData.
-// We don't really care about really aborting our inner loop as by spec the
-// process is happening asynchronously, as such where and when we would abort is
-// non-deterministic. The SourceBuffer also makes sure BufferAppend
-// isn't called should the appendBuffer be immediately aborted.
-// We do however want to ensure that no new task will be dispatched on our task
-// queue and only let the current one finish its job. For this we set mAbort
-// to true.
+// The MSE spec requires that we abort the current SegmentParserLoop
+// which is then followed by a call to ResetParserState.
+// However due to our asynchronous design this causes inherent difficulities.
+// As the spec behaviour is non deterministic anyway, we instead wait until the
+// current AppendData has completed its run.
 void
 TrackBuffersManager::AbortAppendData()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MSE_DEBUG("");
 
-  mAbort = true;
+  MonitorAutoLock mon(mMonitor);
+  while (mAppendRunning) {
+    mon.Wait();
+  }
 }
 
 void
 TrackBuffersManager::ResetParserState()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(!mAppendRunning, "AbortAppendData must have been called");
+  MOZ_RELEASE_ASSERT(!mAppendRunning, "Append is running, abort must have been called");
   MSE_DEBUG("");
 
   // 1. If the append state equals PARSING_MEDIA_SEGMENT and the input buffer contains some complete coded frames, then run the coded frame processing algorithm until all of these complete coded frames have been processed.
-  if (mAppendState == AppendState::PARSING_MEDIA_SEGMENT) {
-    nsCOMPtr<nsIRunnable> task =
-      NS_NewRunnableMethod(this, &TrackBuffersManager::FinishCodedFrameProcessing);
-    GetTaskQueue()->Dispatch(task.forget());
-  } else {
-    nsCOMPtr<nsIRunnable> task =
-      NS_NewRunnableMethod(this, &TrackBuffersManager::CompleteResetParserState);
-    GetTaskQueue()->Dispatch(task.forget());
-  }
+  // SourceBuffer.abort() has ensured that all complete coded frames have been
+  // processed. As such, we don't need to check for the value of mAppendState.
+  nsCOMPtr<nsIRunnable> task =
+    NS_NewRunnableMethod(this, &TrackBuffersManager::CompleteResetParserState);
+  GetTaskQueue()->Dispatch(task.forget());
 
-  // Our ResetParserState is really asynchronous, the current task has been
-  // interrupted and will complete shortly (or has already completed).
-  // We must however present to the main thread a stable, reset state.
-  // So we run the following operation now in the main thread.
   // 7. Set append state to WAITING_FOR_SEGMENT.
   SetAppendState(AppendState::WAITING_FOR_SEGMENT);
 }
@@ -154,6 +145,7 @@ nsRefPtr<TrackBuffersManager::RangeRemovalPromise>
 TrackBuffersManager::RangeRemoval(TimeUnit aStart, TimeUnit aEnd)
 {
   MOZ_ASSERT(NS_IsMainThread());
+  MOZ_RELEASE_ASSERT(!mAppendRunning, "Append is running");
   MSE_DEBUG("From %.2f to %.2f", aStart.ToSeconds(), aEnd.ToSeconds());
 
   mEnded = false;
@@ -264,20 +256,6 @@ TrackBuffersManager::Detach()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MSE_DEBUG("");
-
-  // Abort pending operations if any.
-  AbortAppendData();
-
-  nsRefPtr<TrackBuffersManager> self = this;
-  nsCOMPtr<nsIRunnable> task =
-    NS_NewRunnableFunction([self] () {
-      // Clear our sourcebuffer
-      self->CodedFrameRemoval(TimeInterval(TimeUnit::FromSeconds(0),
-                                           TimeUnit::FromInfinity()));
-      self->mProcessingPromise.RejectIfExists(NS_ERROR_ABORT, __func__);
-      self->mAppendPromise.RejectIfExists(NS_ERROR_ABORT, __func__);
-    });
-  GetTaskQueue()->Dispatch(task.forget());
 }
 
 #if defined(DEBUG)
@@ -289,28 +267,9 @@ TrackBuffersManager::Dump(const char* aPath)
 #endif
 
 void
-TrackBuffersManager::FinishCodedFrameProcessing()
-{
-  MOZ_ASSERT(OnTaskQueue());
-
-  if (mProcessingRequest.Exists()) {
-    NS_WARNING("Processing request pending");
-    mProcessingRequest.Disconnect();
-  }
-  // The spec requires us to complete parsing synchronously any outstanding
-  // frames in the current media segment. This can't be implemented in a way
-  // that makes sense.
-  // As such we simply completely ignore the result of any pending input buffer.
-  // TODO: Link to W3C bug.
-
-  CompleteResetParserState();
-}
-
-void
 TrackBuffersManager::CompleteResetParserState()
 {
   MOZ_ASSERT(OnTaskQueue());
-  MOZ_ASSERT(!mAppendRunning);
   MSE_DEBUG("");
 
   for (auto& track : GetTracksList()) {
@@ -446,7 +405,6 @@ bool
 TrackBuffersManager::CodedFrameRemoval(TimeInterval aInterval)
 {
   MOZ_ASSERT(OnTaskQueue());
-  MOZ_ASSERT(!mAppendRunning, "Logic error: Append in progress");
   MSE_DEBUG("From %.2fs to %.2f",
             aInterval.mStart.ToSeconds(), aInterval.mEnd.ToSeconds());
 
@@ -548,8 +506,9 @@ nsRefPtr<TrackBuffersManager::AppendPromise>
 TrackBuffersManager::InitSegmentParserLoop()
 {
   MOZ_ASSERT(OnTaskQueue());
+  MOZ_RELEASE_ASSERT(mAppendPromise.IsEmpty());
+  MSE_DEBUG("");
 
-  MOZ_ASSERT(mAppendPromise.IsEmpty() && !mAppendRunning);
   nsRefPtr<AppendPromise> p = mAppendPromise.Ensure(__func__);
 
   AppendIncomingBuffers();
@@ -583,6 +542,7 @@ void
 TrackBuffersManager::SegmentParserLoop()
 {
   MOZ_ASSERT(OnTaskQueue());
+
   while (true) {
     // 1. If the input buffer is empty, then jump to the need more data step below.
     if (!mInputBuffer || mInputBuffer->IsEmpty()) {
@@ -656,7 +616,7 @@ TrackBuffersManager::SegmentParserLoop()
           ->Then(GetTaskQueue(), __func__,
                  [self] (bool aNeedMoreData) {
                    self->mProcessingRequest.Complete();
-                   if (aNeedMoreData || self->mAbort) {
+                   if (aNeedMoreData) {
                      self->NeedMoreData();
                    } else {
                      self->ScheduleSegmentParserLoop();
@@ -675,10 +635,13 @@ void
 TrackBuffersManager::NeedMoreData()
 {
   MSE_DEBUG("");
-  if (!mAbort) {
-    RestoreCachedVariables();
-  }
+  RestoreCachedVariables();
   mAppendRunning = false;
+  {
+    // Wake-up any pending Abort()
+    MonitorAutoLock mon(mMonitor);
+    mon.NotifyAll();
+  }
   mAppendPromise.ResolveIfExists(mActiveTrack, __func__);
 }
 
@@ -687,6 +650,11 @@ TrackBuffersManager::RejectAppend(nsresult aRejectValue, const char* aName)
 {
   MSE_DEBUG("rv=%d", aRejectValue);
   mAppendRunning = false;
+  {
+    // Wake-up any pending Abort()
+    MonitorAutoLock mon(mMonitor);
+    mon.NotifyAll();
+  }
   mAppendPromise.RejectIfExists(aRejectValue, aName);
 }
 
@@ -788,14 +756,8 @@ void
 TrackBuffersManager::OnDemuxerInitDone(nsresult)
 {
   MOZ_ASSERT(OnTaskQueue());
-  MSE_DEBUG("mAbort:%d", static_cast<bool>(mAbort));
   mDemuxerInitRequest.Complete();
 
-  if (mAbort) {
-    RejectAppend(NS_ERROR_ABORT, __func__);
-    return;
-  }
-
   MediaInfo info;
 
   uint32_t numVideos = mInputDemuxer->GetNumberTracks(TrackInfo::kVideoTrack);
@@ -1034,9 +996,8 @@ TrackBuffersManager::OnDemuxFailed(TrackType aTrack,
                                    DemuxerFailureReason aFailure)
 {
   MOZ_ASSERT(OnTaskQueue());
-  MSE_DEBUG("Failed to demux %s, failure:%d mAbort:%d",
-            aTrack == TrackType::kVideoTrack ? "video" : "audio",
-            aFailure, static_cast<bool>(mAbort));
+  MSE_DEBUG("Failed to demux %s, failure:%d",
+            aTrack == TrackType::kVideoTrack ? "video" : "audio", aFailure);
   switch (aFailure) {
     case DemuxerFailureReason::END_OF_STREAM:
     case DemuxerFailureReason::WAITING_FOR_DATA:
@@ -1063,15 +1024,10 @@ void
 TrackBuffersManager::DoDemuxVideo()
 {
   MOZ_ASSERT(OnTaskQueue());
-  MSE_DEBUG("mAbort:%d", static_cast<bool>(mAbort));
   if (!HasVideo()) {
     DoDemuxAudio();
     return;
   }
-  if (mAbort) {
-    RejectProcessing(NS_ERROR_ABORT, __func__);
-    return;
-  }
   mVideoTracks.mDemuxRequest.Begin(mVideoTracks.mDemuxer->GetSamples(-1)
                              ->Then(GetTaskQueue(), __func__, this,
                                     &TrackBuffersManager::OnVideoDemuxCompleted,
@@ -1092,11 +1048,6 @@ void
 TrackBuffersManager::DoDemuxAudio()
 {
   MOZ_ASSERT(OnTaskQueue());
-  MSE_DEBUG("mAbort:%d", static_cast<bool>(mAbort));
-  if (mAbort) {
-    RejectProcessing(NS_ERROR_ABORT, __func__);
-    return;
-  }
   if (!HasAudio()) {
     CompleteCodedFrameProcessing();
     return;
@@ -1121,7 +1072,6 @@ void
 TrackBuffersManager::CompleteCodedFrameProcessing()
 {
   MOZ_ASSERT(OnTaskQueue());
-  MSE_DEBUG("mAbort:%d", static_cast<bool>(mAbort));
 
   // 1. For each coded frame in the media segment run the following steps:
   // Coded Frame Processing steps 1.1 to 1.21.
@@ -1189,22 +1139,12 @@ TrackBuffersManager::CompleteCodedFrameProcessing()
 void
 TrackBuffersManager::RejectProcessing(nsresult aRejectValue, const char* aName)
 {
-  if (mAbort) {
-    // mAppendPromise will be resolved immediately upon mProcessingPromise
-    // completing.
-    mAppendRunning = false;
-  }
   mProcessingPromise.RejectIfExists(aRejectValue, __func__);
 }
 
 void
 TrackBuffersManager::ResolveProcessing(bool aResolveValue, const char* aName)
 {
-  if (mAbort) {
-    // mAppendPromise will be resolved immediately upon mProcessingPromise
-    // completing.
-    mAppendRunning = false;
-  }
   mProcessingPromise.ResolveIfExists(aResolveValue, __func__);
 }
 
diff --git a/dom/media/mediasource/TrackBuffersManager.h b/dom/media/mediasource/TrackBuffersManager.h
index 714e81403..2def2b02a 100644
--- a/dom/media/mediasource/TrackBuffersManager.h
+++ b/dom/media/mediasource/TrackBuffersManager.h
@@ -120,9 +120,7 @@ private:
   // media segment have been processed.
   nsRefPtr<CodedFrameProcessingPromise> CodedFrameProcessing();
   void CompleteCodedFrameProcessing();
-  // Called by ResetParserState. Complete parsing the input buffer for the
-  // current media segment.
-  void FinishCodedFrameProcessing();
+  // Called by ResetParserState.
   void CompleteResetParserState();
   nsRefPtr<RangeRemovalPromise>
     CodedFrameRemovalWithPromise(TimeInterval aInterval);
@@ -293,10 +291,6 @@ private:
   MediaPromiseHolder<CodedFrameProcessingPromise> mProcessingPromise;
 
   MediaPromiseHolder<AppendPromise> mAppendPromise;
-  // Set to true while SegmentParserLoop is running. This is used for diagnostic
-  // purposes only. We can't rely on mAppendPromise to be empty as it is only
-  // cleared in a follow up task.
-  bool mAppendRunning;
 
   // Trackbuffers definition.
   nsTArray<TrackData*> GetTracksList();
@@ -332,8 +326,6 @@ private:
   nsRefPtr<dom::SourceBufferAttributes> mSourceBufferAttributes;
   nsMainThreadPtrHandle<MediaSourceDecoder> mParentDecoder;
 
-  // Set to true if abort is called.
-  Atomic<bool> mAbort;
   // Set to true if mediasource state changed to ended.
   Atomic<bool> mEnded;
 
@@ -343,7 +335,10 @@ private:
   Atomic<bool> mEvictionOccurred;
 
   // Monitor to protect following objects accessed across multiple threads.
+  // mMonitor is also notified if the value of mAppendRunning becomes false.
   mutable Monitor mMonitor;
+  // Set to true while a BufferAppend is running or is pending.
+  Atomic<bool> mAppendRunning;
   // Stable audio and video track time ranges.
   TimeIntervals mVideoBufferedRanges;
   TimeIntervals mAudioBufferedRanges;