Bug 1005366 - Tighten up handling of SourceBuffer decoder resets. r=cajbir
authorMatthew Gregan <kinetik@flim.org>
Thu, 08 May 2014 14:22:34 +1200
changeset 182081 3c695f32262b66ad19bd211aeb8568c12a4cfbdf
parent 182080 82ad7813f515acfdc0ebbaaaa0a04f406f2cf6d0
child 182082 b766e7d111b90d1502b06fa289b21ad449d23cd0
push id43202
push usermgregan@mozilla.com
push dateThu, 08 May 2014 02:23:17 +0000
treeherdermozilla-inbound@3c695f32262b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscajbir
bugs1005366
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1005366 - Tighten up handling of SourceBuffer decoder resets. r=cajbir
content/media/mediasource/SourceBuffer.cpp
content/media/mediasource/SourceBuffer.h
content/media/mediasource/test/crashtests/1005366.html
content/media/mediasource/test/crashtests/926665.html
content/media/mediasource/test/crashtests/crashtests.list
content/media/test/crashtests/926665.html
content/media/test/crashtests/crashtests.list
--- a/content/media/mediasource/SourceBuffer.cpp
+++ b/content/media/mediasource/SourceBuffer.cpp
@@ -275,20 +275,17 @@ SourceBuffer::Abort(ErrorResult& aRv)
     // TODO: Abort segment parser loop, buffer append, and stream append loop algorithms.
     AbortUpdating();
   }
   // TODO: Run reset parser algorithm.
   mAppendWindowStart = 0;
   mAppendWindowEnd = PositiveInfinity<double>();
 
   MSE_DEBUG("%p Abort: Discarding decoder.", this);
-  if (mDecoder) {
-    mDecoder->GetResource()->Ended();
-    mDecoder = nullptr;
-  }
+  DiscardDecoder();
 }
 
 void
 SourceBuffer::Remove(double aStart, double aEnd, ErrorResult& aRv)
 {
   MSE_DEBUG("%p Remove(Start=%f End=%f)", this, aStart, aEnd);
   if (!IsAttached() || mUpdating ||
       mMediaSource->ReadyState() != MediaSourceReadyState::Open) {
@@ -304,17 +301,17 @@ SourceBuffer::Remove(double aStart, doub
   /// TODO: Run coded frame removal algorithm asynchronously (would call StopUpdating()).
   StopUpdating();
 }
 
 void
 SourceBuffer::Detach()
 {
   Ended();
-  mDecoder = nullptr;
+  DiscardDecoder();
   mMediaSource = nullptr;
 }
 
 void
 SourceBuffer::Ended()
 {
   if (mDecoder) {
     mDecoder->GetResource()->Ended();
@@ -325,36 +322,34 @@ SourceBuffer::SourceBuffer(MediaSource* 
   : DOMEventTargetHelper(aMediaSource->GetParentObject())
   , mMediaSource(aMediaSource)
   , mType(aType)
   , mAppendWindowStart(0)
   , mAppendWindowEnd(PositiveInfinity<double>())
   , mTimestampOffset(0)
   , mAppendMode(SourceBufferAppendMode::Segments)
   , mUpdating(false)
-  , mDecoderInit(false)
+  , mDecoderInitialized(false)
 {
   MOZ_ASSERT(aMediaSource);
   mParser = ContainerParser::CreateForMIMEType(aType);
   MSE_DEBUG("%p SourceBuffer: Creating initial decoder.", this);
   InitNewDecoder();
 }
 
 already_AddRefed<SourceBuffer>
 SourceBuffer::Create(MediaSource* aMediaSource, const nsACString& aType)
 {
   nsRefPtr<SourceBuffer> sourceBuffer = new SourceBuffer(aMediaSource, aType);
   return sourceBuffer.forget();
 }
 
 SourceBuffer::~SourceBuffer()
 {
-  if (mDecoder) {
-    mDecoder->GetResource()->Ended();
-  }
+  DiscardDecoder();
 }
 
 MediaSource*
 SourceBuffer::GetParentObject() const
 {
   return mMediaSource;
 }
 
@@ -377,26 +372,38 @@ SourceBuffer::QueueAsyncSimpleEvent(cons
   MSE_DEBUG("%p Queuing event %s to SourceBuffer", this, aName);
   nsCOMPtr<nsIRunnable> event = new AsyncEventRunner<SourceBuffer>(this, aName);
   NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
 }
 
 bool
 SourceBuffer::InitNewDecoder()
 {
+  MOZ_ASSERT(!mDecoder);
   MediaSourceDecoder* parentDecoder = mMediaSource->GetDecoder();
   nsRefPtr<SubBufferDecoder> decoder = parentDecoder->CreateSubDecoder(mType);
   if (!decoder) {
     return false;
   }
   mDecoder = decoder;
+  mDecoderInitialized = false;
   return true;
 }
 
 void
+SourceBuffer::DiscardDecoder()
+{
+  if (mDecoder) {
+    mDecoder->GetResource()->Ended();
+  }
+  mDecoder = nullptr;
+  mDecoderInitialized = false;
+}
+
+void
 SourceBuffer::StartUpdating()
 {
   MOZ_ASSERT(!mUpdating);
   mUpdating = true;
   QueueAsyncSimpleEvent("updatestart");
 }
 
 void
@@ -427,28 +434,38 @@ SourceBuffer::AppendData(const uint8_t* 
   }
   if (mMediaSource->ReadyState() == MediaSourceReadyState::Ended) {
     mMediaSource->SetReadyState(MediaSourceReadyState::Open);
   }
   // TODO: Run coded frame eviction algorithm.
   // TODO: Test buffer full flag.
   StartUpdating();
   // TODO: Run buffer append algorithm asynchronously (would call StopUpdating()).
-  if (!mDecoder || mParser->IsInitSegmentPresent(aData, aLength)) {
-    if (!mDecoder || mDecoderInit) {
-      MSE_DEBUG("%p AppendBuffer: New initialization segment, creating decoder.", this);
-      mDecoder->GetResource()->Ended();
+  if (mParser->IsInitSegmentPresent(aData, aLength)) {
+    MSE_DEBUG("%p AppendBuffer: New initialization segment.", this);
+    if (mDecoderInitialized) {
+      // Existing decoder has been used, time for a new one.
+      DiscardDecoder();
+    }
 
-      if (!InitNewDecoder()) {
-        aRv.Throw(NS_ERROR_FAILURE); // XXX: Review error handling.
-        return;
-      }
+    // If we've got a decoder here, it's not initialized, so we can use it
+    // rather than creating a new one.
+    if (!mDecoder && !InitNewDecoder()) {
+      aRv.Throw(NS_ERROR_FAILURE); // XXX: Review error handling.
+      return;
     }
     MSE_DEBUG("%p AppendBuffer: Decoder marked as initialized.", this);
-    mDecoderInit = true;
+    mDecoderInitialized = true;
+  } else if (!mDecoderInitialized) {
+    MSE_DEBUG("%p AppendBuffer: Non-initialization segment appended during initialization.");
+    Optional<MediaSourceEndOfStreamError> decodeError(MediaSourceEndOfStreamError::Decode);
+    ErrorResult dummy;
+    mMediaSource->EndOfStream(decodeError, dummy);
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
   }
   // XXX: For future reference: NDA call must run on the main thread.
   mDecoder->NotifyDataArrived(reinterpret_cast<const char*>(aData),
                               aLength,
                               mDecoder->GetResource()->GetLength());
   mDecoder->GetResource()->AppendData(aData, aLength);
 
   // Eviction uses a byte threshold. If the buffer is greater than the
--- a/content/media/mediasource/SourceBuffer.h
+++ b/content/media/mediasource/SourceBuffer.h
@@ -114,19 +114,23 @@ public:
 
 private:
   SourceBuffer(MediaSource* aMediaSource, const nsACString& aType);
 
   friend class AsyncEventRunner<SourceBuffer>;
   void DispatchSimpleEvent(const char* aName);
   void QueueAsyncSimpleEvent(const char* aName);
 
-  // Create a new decoder for mType, add it to mDecoders and update mCurrentDecoder.
+  // Create a new decoder for mType, and store the result in mDecoder.
+  // Returns true if mDecoder was set.
   bool InitNewDecoder();
 
+  // Set mDecoder to null and reset mDecoderInitialized.
+  void DiscardDecoder();
+
   // Update mUpdating and fire the appropriate events.
   void StartUpdating();
   void StopUpdating();
   void AbortUpdating();
 
   // Shared implementation of AppendBuffer overloads.
   void AppendData(const uint8_t* aData, uint32_t aLength, ErrorResult& aRv);
 
@@ -145,15 +149,15 @@ private:
   double mAppendWindowStart;
   double mAppendWindowEnd;
 
   double mTimestampOffset;
 
   SourceBufferAppendMode mAppendMode;
   bool mUpdating;
 
-  bool mDecoderInit;
+  bool mDecoderInitialized;
 };
 
 } // namespace dom
 
 } // namespace mozilla
 #endif /* mozilla_dom_SourceBuffer_h_ */
new file mode 100644
--- /dev/null
+++ b/content/media/mediasource/test/crashtests/1005366.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="UTF-8">
+<script>
+
+/*
+user_pref("media.mediasource.enabled", true);
+*/
+
+function boom()
+{
+    var source = new window.MediaSource();
+    var videoElement = document.createElementNS('http://www.w3.org/1999/xhtml', 'video');
+    videoElement.src = URL.createObjectURL(source);
+
+    setTimeout(function() {
+        var buf = source.addSourceBuffer("video/webm");
+        buf.abort();
+        buf.appendBuffer(new Float32Array(203));
+    }, 0);
+}
+
+</script>
+</head>
+<body onload="boom();"></body>
+</html>
rename from content/media/test/crashtests/926665.html
rename to content/media/mediasource/test/crashtests/926665.html
new file mode 100644
--- /dev/null
+++ b/content/media/mediasource/test/crashtests/crashtests.list
@@ -0,0 +1,2 @@
+test-pref(media.mediasource.enabled,true) load 926665.html
+test-pref(media.mediasource.enabled,true) load 1005366.html
--- a/content/media/test/crashtests/crashtests.list
+++ b/content/media/test/crashtests/crashtests.list
@@ -64,9 +64,9 @@ load 933151.html
 load 933156.html
 load 952756.html
 load 986901.html
 load buffer-source-ended-1.html
 load offline-buffer-source-ended-1.html
 skip-if(B2G) HTTP load media-element-source-seek-1.html # intermittent B2G timeouts, bug 994351
 skip-if(B2G) load oscillator-ended-1.html # intermittent B2G timeouts, bug 920338
 skip-if(B2G) load oscillator-ended-2.html # intermittent B2G timeouts, bug 920338
-test-pref(media.mediasource.enabled,true) load 926665.html
+include ../../mediasource/test/crashtests/crashtests.list