Bug 687972. Set mCacheSuspended/mChannelEnded flags on all streams *before* calling CacheClientSeek/Resume/Suspend on any stream, because nsMediaChannelStream::CacheClientSuspend will call nsBuiltinDecoder::NotifySuspendedStatusChanged which will call nsMediaChannelStream::IsSuspendedByCache which will call nsMediaCacheStream::AreAllStreamsForResourceSuspended which relies on mCacheSuspended/mChannelEnded being set correctly for all streams for that resource. r=doublec
authorRobert O'Callahan <robert@ocallahan.org>
Fri, 02 Dec 2011 17:43:42 +1300
changeset 82776 ffa6204d08892eeb0e84725ba7b520bffd3b9af1
parent 82775 13185042410dc19cdce3166d67f84da8e1ed522a
child 82777 43adf69a4a7c892f283d735b3b4a2cb7a42ff0ae
push id519
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 00:38:35 +0000
treeherdermozilla-beta@788ea1ef610b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdoublec
bugs687972
milestone11.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 687972. Set mCacheSuspended/mChannelEnded flags on all streams *before* calling CacheClientSeek/Resume/Suspend on any stream, because nsMediaChannelStream::CacheClientSuspend will call nsBuiltinDecoder::NotifySuspendedStatusChanged which will call nsMediaChannelStream::IsSuspendedByCache which will call nsMediaCacheStream::AreAllStreamsForResourceSuspended which relies on mCacheSuspended/mChannelEnded being set correctly for all streams for that resource. r=doublec
content/media/nsMediaCache.cpp
--- a/content/media/nsMediaCache.cpp
+++ b/content/media/nsMediaCache.cpp
@@ -1072,17 +1072,17 @@ nsMediaCache::PredictNextUseForIncomingD
   }
   if (bytesAhead <= 0)
     return TimeDuration(0);
   PRInt64 millisecondsAhead = bytesAhead*1000/aStream->mPlaybackBytesPerSecond;
   return TimeDuration::FromMilliseconds(
       NS_MIN<PRInt64>(millisecondsAhead, PR_INT32_MAX));
 }
 
-enum StreamAction { NONE, SEEK, RESUME, SUSPEND };
+enum StreamAction { NONE, SEEK, SEEK_AND_RESUME, RESUME, SUSPEND };
 
 void
 nsMediaCache::Update()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   // The action to use for each stream. We store these so we can make
   // decisions while holding the cache lock but implement those decisions
@@ -1317,66 +1317,83 @@ nsMediaCache::Update()
       if (stream->mChannelOffset != desiredOffset && enableReading) {
         // We need to seek now.
         NS_ASSERTION(stream->mIsSeekable || desiredOffset == 0,
                      "Trying to seek in a non-seekable stream!");
         // Round seek offset down to the start of the block. This is essential
         // because we don't want to think we have part of a block already
         // in mPartialBlockBuffer.
         stream->mChannelOffset = (desiredOffset/BLOCK_SIZE)*BLOCK_SIZE;
-        actions[i] = SEEK;
+        actions[i] = stream->mCacheSuspended ? SEEK_AND_RESUME : SEEK;
       } else if (enableReading && stream->mCacheSuspended) {
         actions[i] = RESUME;
       } else if (!enableReading && !stream->mCacheSuspended) {
         actions[i] = SUSPEND;
       }
     }
 #ifdef DEBUG
     mInUpdate = false;
 #endif
   }
 
   // Update the channel state without holding our cache lock. While we're
   // doing this, decoder threads may be running and seeking, reading or changing
   // other cache state. That's OK, they'll trigger new Update events and we'll
   // get back here and revise our decisions. The important thing here is that
   // performing these actions only depends on mChannelOffset and
-  // mCacheSuspended, which can only be written by the main thread (i.e., this
+  // the action, which can only be written by the main thread (i.e., this
   // thread), so we don't have races here.
+
+  // First, update the mCacheSuspended/mCacheEnded flags so that they're all correct
+  // when we fire our CacheClient commands below. Those commands can rely on these flags
+  // being set correctly for all streams.
   for (PRUint32 i = 0; i < mStreams.Length(); ++i) {
     nsMediaCacheStream* stream = mStreams[i];
-    nsresult rv = NS_OK;
     switch (actions[i]) {
     case SEEK:
-      LOG(PR_LOG_DEBUG, ("Stream %p CacheSeek to %lld (resume=%d)", stream,
-           (long long)stream->mChannelOffset, stream->mCacheSuspended));
-      rv = stream->mClient->CacheClientSeek(stream->mChannelOffset,
-                                            stream->mCacheSuspended);
+	case SEEK_AND_RESUME:
       stream->mCacheSuspended = false;
       stream->mChannelEnded = false;
       break;
+    case RESUME:
+      stream->mCacheSuspended = false;
+      break;
+    case SUSPEND:
+      stream->mCacheSuspended = true;
+      break;
+    default:
+      break;
+    }
+    stream->mHasHadUpdate = true;
+  }
 
+  for (PRUint32 i = 0; i < mStreams.Length(); ++i) {
+    nsMediaCacheStream* stream = mStreams[i];
+    nsresult rv;
+    switch (actions[i]) {
+    case SEEK:
+	case SEEK_AND_RESUME:
+      LOG(PR_LOG_DEBUG, ("Stream %p CacheSeek to %lld (resume=%d)", stream,
+           (long long)stream->mChannelOffset, actions[i] == SEEK_AND_RESUME));
+      rv = stream->mClient->CacheClientSeek(stream->mChannelOffset,
+                                            actions[i] == SEEK_AND_RESUME);
+      break;
     case RESUME:
       LOG(PR_LOG_DEBUG, ("Stream %p Resumed", stream));
       rv = stream->mClient->CacheClientResume();
-      stream->mCacheSuspended = false;
       break;
-
     case SUSPEND:
       LOG(PR_LOG_DEBUG, ("Stream %p Suspended", stream));
       rv = stream->mClient->CacheClientSuspend();
-      stream->mCacheSuspended = true;
       break;
-
     default:
+      rv = NS_OK;
       break;
     }
 
-    stream->mHasHadUpdate = true;
-
     if (NS_FAILED(rv)) {
       // Close the streams that failed due to error. This will cause all
       // client Read and Seek operations on those streams to fail. Blocked
       // Reads will also be woken up.
       ReentrantMonitorAutoEnter mon(mReentrantMonitor);
       stream->CloseInternal(mon);
     }
   }