Backed out changeset a9ccf315f1f2 (bug 1245463)

dom/media/mediasource/SourceBuffer.cpp

@@ -39,6 +39,27 @@ using media::TimeUnit;
 
 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:
+  RefPtr<SourceBuffer> mSourceBuffer;
+  uint32_t mUpdateID;
+};
+
 void
 SourceBuffer::SetMode(SourceBufferAppendMode aMode, ErrorResult& aRv)
 {
@@ -205,19 +226,10 @@ void
 SourceBuffer::AbortBufferAppend()
 {
   if (mUpdating) {
-    // The spec states: 3.2 Methods Abort.
-    // "Abort the buffer append and stream append loop algorithms if they are running."
-    // but requires the Reset Parser State to finish processing any complete
-    // samples.
-    // We can't abort an asynchronous operation as the result would be
-    // non deterministic. Instead we let the current appendBuffer complete
-    // its job.
-    mAborting = true;
-    // Wait until the current appendBuffer completes.
-    // mAborting will become false once the AppendPromise is resolved.
-    while (mAborting) {
-      NS_ProcessNextEvent(NS_GetCurrentThread(), true);
-    }
+    mPendingAppend.DisconnectIfExists();
+    // TODO: Abort stream append loop algorithms.
+    // cancel any pending buffer append.
+    mContentManager->AbortAppendData();
     AbortUpdating();
   }
 }
@@ -257,13 +269,7 @@ SourceBuffer::RangeRemoval(double aStart, double aEnd)
   mContentManager->RangeRemoval(TimeUnit::FromSeconds(aStart),
                                 TimeUnit::FromSeconds(aEnd))
     ->Then(AbstractThread::MainThread(), __func__,
-           [self] (bool) {
-             if (self->mAborting) {
-               self->mAborting = false;
-               return;
-             }
-             self->StopUpdating();
-           },
+           [self] (bool) { self->StopUpdating(); },
            []() { MOZ_ASSERT(false); });
 }
 
@@ -302,8 +308,8 @@ SourceBuffer::SourceBuffer(MediaSource* aMediaSource, const nsACString& aType)
   : DOMEventTargetHelper(aMediaSource->GetParentObject())
   , mMediaSource(aMediaSource)
   , mUpdating(false)
-  , mAborting(false)
   , mActive(false)
+  , mUpdateID(0)
   , mType(aType)
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -375,6 +381,7 @@ SourceBuffer::StartUpdating()
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mUpdating);
   mUpdating = true;
+  mUpdateID++;
   QueueAsyncSimpleEvent("updatestart");
 }
 
@@ -431,13 +438,23 @@ SourceBuffer::AppendData(const uint8_t* aData, uint32_t aLength, ErrorResult& aR
 
   StartUpdating();
 
-  BufferAppend();
+  nsCOMPtr<nsIRunnable> task = new BufferAppendRunnable(this, mUpdateID);
+  NS_DispatchToMainThread(task);
 }
 
 void
-SourceBuffer::BufferAppend()
+SourceBuffer::BufferAppend(uint32_t aUpdateID)
 {
-  MOZ_ASSERT(mUpdating);
+  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;
+  }
+
   MOZ_ASSERT(mMediaSource);
   MOZ_ASSERT(!mPendingAppend.Exists());
 
@@ -451,6 +468,10 @@ void
 SourceBuffer::AppendDataCompletedWithSuccess(bool aHasActiveTracks)
 {
   mPendingAppend.Complete();
+  if (!mUpdating) {
+    // The buffer append algorithm has been interrupted by abort().
+    return;
+  }
 
   if (aHasActiveTracks) {
     if (!mActive) {
@@ -467,11 +488,6 @@ SourceBuffer::AppendDataCompletedWithSuccess(bool aHasActiveTracks)
 
   CheckEndTime();
 
-  if (mAborting) {
-    mAborting = false;
-    return;
-  }
-
   StopUpdating();
 }
 
@@ -479,12 +495,6 @@ void
 SourceBuffer::AppendDataErrored(nsresult aError)
 {
   mPendingAppend.Complete();
-
-  if (mAborting) {
-    mAborting = false;
-    return;
-  }
-
   switch (aError) {
     case NS_ERROR_ABORT:
       // Nothing further to do as the trackbuffer has been shutdown.
@@ -500,7 +510,10 @@ 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;

dom/media/mediasource/SourceBuffer.h

@@ -238,7 +238,7 @@ private:
 
   // Shared implementation of AppendBuffer overloads.
   void AppendData(const uint8_t* aData, uint32_t aLength, ErrorResult& aRv);
-  void BufferAppend();
+  void BufferAppend(uint32_t aAppendID);
 
   // Implement the "Append Error Algorithm".
   // Will call endOfStream() with "decode" error if aDecodeError is true.
@@ -263,10 +263,14 @@ private:
   RefPtr<SourceBufferAttributes> mAttributes;
 
   bool mUpdating;
-  bool mAborting;
 
   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;
+
   MozPromiseRequestHolder<SourceBufferContentManager::AppendPromise> mPendingAppend;
   const nsCString mType;
 

dom/media/mediasource/SourceBufferContentManager.h

@@ -43,6 +43,9 @@ public:
   // http://w3c.github.io/media-source/index.html#sourcebuffer-buffer-append
   virtual RefPtr<AppendPromise> BufferAppend() = 0;
 
+  // Abort any pending AppendData.
+  virtual void AbortAppendData() = 0;
+
   // Run MSE Reset Parser State Algorithm.
   // 3.5.2 Reset Parser State
   // http://w3c.github.io/media-source/#sourcebuffer-reset-parser-state
@@ -105,6 +108,10 @@ public:
   virtual void RestartGroupStartTimestamp() {}
   virtual media::TimeUnit GroupEndTimestamp() = 0;
 
+#if defined(DEBUG)
+  virtual void Dump(const char* aPath) { }
+#endif
+
 protected:
   virtual ~SourceBufferContentManager() { }
 };

dom/media/mediasource/TrackBuffersManager.cpp

@@ -101,6 +101,7 @@ TrackBuffersManager::TrackBuffersManager(dom::SourceBufferAttributes* aAttribute
   , mSourceBufferAttributes(aAttributes)
   , mParentDecoder(new nsMainThreadPtrHolder<MediaSourceDecoder>(aParentDecoder, false /* strict */))
   , mMediaSourceDuration(mTaskQueue, Maybe<double>(), "TrackBuffersManager::mMediaSourceDuration (Mirror)")
+  , mAbort(false)
   , mEvictionThreshold(Preferences::GetUint("media.mediasource.eviction_threshold",
                                             100 * (1 << 20)))
   , mEvictionOccurred(false)
@@ -141,6 +142,7 @@ TrackBuffersManager::AppendIncomingBuffer(IncomingBuffer aData)
 {
   MOZ_ASSERT(OnTaskQueue());
   mIncomingBuffers.AppendElement(aData);
+  mAbort = false;
 }
 
 RefPtr<TrackBuffersManager::AppendPromise>
@@ -153,6 +155,23 @@ TrackBuffersManager::BufferAppend()
                      __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.
+void
+TrackBuffersManager::AbortAppendData()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MSE_DEBUG("");
+
+  mAbort = true;
+}
+
 void
 TrackBuffersManager::ResetParserState()
 {
@@ -291,6 +310,9 @@ TrackBuffersManager::Detach()
   MOZ_ASSERT(NS_IsMainThread());
   MSE_DEBUG("");
 
+  // Abort pending operations if any.
+  AbortAppendData();
+
   RefPtr<TrackBuffersManager> self = this;
   nsCOMPtr<nsIRunnable> task =
     NS_NewRunnableFunction([self] () {
@@ -711,7 +733,7 @@ TrackBuffersManager::SegmentParserLoop()
           ->Then(GetTaskQueue(), __func__,
                  [self] (bool aNeedMoreData) {
                    self->mProcessingRequest.Complete();
-                   if (aNeedMoreData) {
+                   if (aNeedMoreData || self->mAbort) {
                      self->NeedMoreData();
                    } else {
                      self->ScheduleSegmentParserLoop();
@@ -730,7 +752,9 @@ void
 TrackBuffersManager::NeedMoreData()
 {
   MSE_DEBUG("");
-  RestoreCachedVariables();
+  if (!mAbort) {
+    RestoreCachedVariables();
+  }
   mAppendRunning = false;
   mAppendPromise.ResolveIfExists(mActiveTrack, __func__);
 }
@@ -816,7 +840,12 @@ void
 TrackBuffersManager::OnDemuxerResetDone(nsresult)
 {
   MOZ_ASSERT(OnTaskQueue());
+  MSE_DEBUG("mAbort:%d", static_cast<bool>(mAbort));
   mDemuxerInitRequest.Complete();
+  if (mAbort) {
+    RejectAppend(NS_ERROR_ABORT, __func__);
+    return;
+  }
   // mInputDemuxer shouldn't have been destroyed while a demuxer init/reset
   // request was being processed. See bug 1239983.
   MOZ_DIAGNOSTIC_ASSERT(mInputDemuxer);
@@ -888,8 +917,13 @@ 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;
+  }
   // mInputDemuxer shouldn't have been destroyed while a demuxer init/reset
   // request was being processed. See bug 1239983.
   MOZ_DIAGNOSTIC_ASSERT(mInputDemuxer);
@@ -1149,8 +1183,9 @@ TrackBuffersManager::OnDemuxFailed(TrackType aTrack,
                                    DemuxerFailureReason aFailure)
 {
   MOZ_ASSERT(OnTaskQueue());
-  MSE_DEBUG("Failed to demux %s, failure:%d",
-            aTrack == TrackType::kVideoTrack ? "video" : "audio", aFailure);
+  MSE_DEBUG("Failed to demux %s, failure:%d mAbort:%d",
+            aTrack == TrackType::kVideoTrack ? "video" : "audio",
+            aFailure, static_cast<bool>(mAbort));
   switch (aFailure) {
     case DemuxerFailureReason::END_OF_STREAM:
     case DemuxerFailureReason::WAITING_FOR_DATA:
@@ -1177,10 +1212,15 @@ 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,
@@ -1201,10 +1241,15 @@ void
 TrackBuffersManager::DoDemuxAudio()
 {
   MOZ_ASSERT(OnTaskQueue());
+  MSE_DEBUG("mAbort:%d", static_cast<bool>(mAbort));
   if (!HasAudio()) {
     CompleteCodedFrameProcessing();
     return;
   }
+  if (mAbort) {
+    RejectProcessing(NS_ERROR_ABORT, __func__);
+    return;
+  }
   mAudioTracks.mDemuxRequest.Begin(mAudioTracks.mDemuxer->GetSamples(-1)
                              ->Then(GetTaskQueue(), __func__, this,
                                     &TrackBuffersManager::OnAudioDemuxCompleted,
@@ -1225,6 +1270,7 @@ 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.
@@ -1295,12 +1341,22 @@ 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__);
 }
 

dom/media/mediasource/TrackBuffersManager.h

@@ -47,6 +47,8 @@ public:
 
   RefPtr<AppendPromise> BufferAppend() override;
 
+  void AbortAppendData() override;
+
   void ResetParserState() override;
 
   RefPtr<RangeRemovalPromise> RangeRemoval(media::TimeUnit aStart,
@@ -309,7 +311,8 @@ private:
 
   MozPromiseHolder<AppendPromise> mAppendPromise;
   // Set to true while SegmentParserLoop is running. This is used for diagnostic
-  // purposes only.
+  // 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.
@@ -349,6 +352,8 @@ private:
   // MediaSource duration mirrored from MediaDecoder on the main thread..
   Mirror<Maybe<double>> mMediaSourceDuration;
 
+  // Set to true if abort was called.
+  Atomic<bool> mAbort;
   // Set to true if mediasource state changed to ended.
   Atomic<bool> mEnded;