From 72572eda3e28114b6270ec208bfecaa52e142715 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Mon, 18 May 2009 14:02:20 +1200 Subject: [PATCH] Bug 479863. Add ability to close and reopen suspended streams, and recover from server-side disconnect of suspended streams. r=doublec --HG-- extra : rebase_source : 5ed7825cd919ad3f997f58dafb61c2d4249b60d4 --- content/media/video/public/nsMediaCache.h | 5 +- content/media/video/public/nsMediaStream.h | 22 +++- content/media/video/src/nsMediaCache.cpp | 26 +++-- content/media/video/src/nsMediaStream.cpp | 108 ++++++++++++------ content/media/video/src/nsOggDecoder.cpp | 2 +- content/media/video/src/nsWaveDecoder.cpp | 2 +- content/media/video/test/Makefile.in | 1 + .../video/test/test_closing_connections.html | 61 ++++++++++ 8 files changed, 172 insertions(+), 55 deletions(-) create mode 100644 content/media/video/test/test_closing_connections.html diff --git a/content/media/video/public/nsMediaCache.h b/content/media/video/public/nsMediaCache.h index 32540cd59d7..d94c25b8cc4 100644 --- a/content/media/video/public/nsMediaCache.h +++ b/content/media/video/public/nsMediaCache.h @@ -268,7 +268,8 @@ public: // requested. In particular we might unexpectedly start providing // data at offset 0. This need not be called if the offset is the // offset that the cache requested in - // nsMediaChannelStream::CacheClientSeek. + // nsMediaChannelStream::CacheClientSeek. This can be called at any + // time by the client, not just after a CacheClientSeek. void NotifyDataStarted(PRInt64 aOffset); // Notifies the cache that data has been received. The stream already // knows the offset because data is received in sequence and @@ -320,6 +321,8 @@ public: // because it doesn't know when the decoder was paused, buffering, etc. // Do not pass zero. void SetPlaybackRate(PRUint32 aBytesPerSecond); + // Returns the last set value of SetSeekable. + PRBool IsSeekable(); // These methods must be called on a different thread from the main // thread. They should always be called on the same thread for a given diff --git a/content/media/video/public/nsMediaStream.h b/content/media/video/public/nsMediaStream.h index 188f7c92a5e..1d5da4edfa1 100644 --- a/content/media/video/public/nsMediaStream.h +++ b/content/media/video/public/nsMediaStream.h @@ -158,7 +158,11 @@ public: // return an error. virtual nsresult Close() = 0; // Suspend any downloads that are in progress. - virtual void Suspend() = 0; + // If aCloseImmediately is set, resources should be released immediately + // since we don't expect to resume again any time soon. Otherwise we + // may resume again soon so resources should be held for a little + // while. + virtual void Suspend(PRBool aCloseImmediately) = 0; // Resume any downloads that have been suspended. virtual void Resume() = 0; // Get the current principal for the channel @@ -311,7 +315,7 @@ public: // and no more data from the old load will be notified via // nsMediaCacheStream::NotifyDataReceived/Ended. // This can fail. - nsresult CacheClientSeek(PRInt64 aOffset); + nsresult CacheClientSeek(PRInt64 aOffset, PRBool aResume); // Suspend the current load since data is currently not wanted nsresult CacheClientSuspend(); // Resume the current load since data is wanted again @@ -320,7 +324,7 @@ public: // Main thread virtual nsresult Open(nsIStreamListener** aStreamListener); virtual nsresult Close(); - virtual void Suspend(); + virtual void Suspend(PRBool aCloseImmediately); virtual void Resume(); virtual already_AddRefed GetCurrentPrincipal(); // Return PR_TRUE if the stream has been closed. @@ -372,9 +376,9 @@ protected: PRUint32 aCount); nsresult OnChannelRedirect(nsIChannel* aOld, nsIChannel* aNew, PRUint32 aFlags); - // Opens the channel, using an HTTP byte range request to start at aOffset + // Opens the channel, using an HTTP byte range request to start at mOffset // if possible. Main thread only. - nsresult OpenChannel(nsIStreamListener** aStreamListener, PRInt64 aOffset); + nsresult OpenChannel(nsIStreamListener** aStreamListener); void SetupChannelHeaders(); // Closes the channel. Main thread only. void CloseChannel(); @@ -387,9 +391,15 @@ protected: PRUint32 *aWriteCount); // Main thread access only - PRInt64 mLastSeekOffset; + PRInt64 mOffset; nsRefPtr mListener; PRUint32 mSuspendCount; + // When this flag is set, if we get a network error we should silently + // reopen the stream. + PRPackedBool mReopenOnError; + // When this flag is set, we should not report the next close of the + // channel. + PRPackedBool mIgnoreClose; // Any thread access nsMediaCacheStream mCacheStream; diff --git a/content/media/video/src/nsMediaCache.cpp b/content/media/video/src/nsMediaCache.cpp index 669c116a8f8..3bb4354200d 100644 --- a/content/media/video/src/nsMediaCache.cpp +++ b/content/media/video/src/nsMediaCache.cpp @@ -1031,18 +1031,13 @@ nsMediaCache::Update() // We need to seek now. NS_ASSERTION(stream->mIsSeekable || desiredOffset == 0, "Trying to seek in a non-seekable stream!"); - if (stream->mCacheSuspended) { - LOG(PR_LOG_DEBUG, ("Stream %p Resumed", stream)); - rv = stream->mClient->CacheClientResume(); - stream->mCacheSuspended = PR_FALSE; - } - if (NS_SUCCEEDED(rv)) { - // Round seek offset down to the start of the block - stream->mChannelOffset = (desiredOffset/BLOCK_SIZE)*BLOCK_SIZE; - LOG(PR_LOG_DEBUG, ("Stream %p CacheSeek to %lld", stream, - (long long)stream->mChannelOffset)); - rv = stream->mClient->CacheClientSeek(stream->mChannelOffset); - } + // Round seek offset down to the start of the block + stream->mChannelOffset = (desiredOffset/BLOCK_SIZE)*BLOCK_SIZE; + 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); + stream->mCacheSuspended = PR_FALSE; } else if (enableReading && stream->mCacheSuspended) { LOG(PR_LOG_DEBUG, ("Stream %p Resumed", stream)); rv = stream->mClient->CacheClientResume(); @@ -1534,6 +1529,13 @@ nsMediaCacheStream::SetSeekable(PRBool aIsSeekable) gMediaCache->QueueUpdate(); } +PRBool +nsMediaCacheStream::IsSeekable() +{ + nsAutoMonitor mon(gMediaCache->Monitor()); + return mIsSeekable; +} + void nsMediaCacheStream::Close() { diff --git a/content/media/video/src/nsMediaStream.cpp b/content/media/video/src/nsMediaStream.cpp index 2a21a44928c..87bacd9a0e5 100644 --- a/content/media/video/src/nsMediaStream.cpp +++ b/content/media/video/src/nsMediaStream.cpp @@ -65,10 +65,11 @@ using mozilla::TimeStamp; nsMediaChannelStream::nsMediaChannelStream(nsMediaDecoder* aDecoder, nsIChannel* aChannel, nsIURI* aURI) : nsMediaStream(aDecoder, aChannel, aURI), - mLastSeekOffset(0), mSuspendCount(0), + mOffset(0), mSuspendCount(0), + mReopenOnError(PR_FALSE), mIgnoreClose(PR_FALSE), mCacheStream(this), mLock(nsAutoLock::NewLock("media.channel.stream")), - mCacheSuspendCount(0) + mCacheSuspendCount(0) { } @@ -166,7 +167,7 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest) ranges); PRBool acceptsRanges = ranges.EqualsLiteral("bytes"); - if (mLastSeekOffset == 0) { + if (mOffset == 0) { // Look for duration headers from known Ogg content systems. In the case // of multiple options for obtaining the duration the order of precedence is; // 1) The Media resource metadata if possible (done by the decoder itself). @@ -190,12 +191,13 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest) PRUint32 responseStatus = 0; hc->GetResponseStatus(&responseStatus); - if (mLastSeekOffset > 0 && responseStatus == HTTP_OK_CODE) { + if (mOffset > 0 && responseStatus == HTTP_OK_CODE) { // If we get an OK response but we were seeking, we have to assume // that seeking doesn't work. We also need to tell the cache that // it's getting data for the start of the stream. mCacheStream.NotifyDataStarted(0); - } else if (mLastSeekOffset == 0 && + mOffset = 0; + } else if (mOffset == 0 && (responseStatus == HTTP_OK_CODE || responseStatus == HTTP_PARTIAL_RESPONSE_CODE)) { // We weren't seeking and got a valid response status, @@ -206,6 +208,8 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest) mCacheStream.NotifyDataLength(cl); } } + // XXX we probably should examine the Content-Range header in case + // the server gave us a range which is not quite what we asked for // If we get an HTTP_OK_CODE response to our byte range request, // and the server isn't sending Accept-Ranges:bytes then we don't @@ -230,6 +234,8 @@ nsMediaChannelStream::OnStartRequest(nsIRequest* aRequest) mChannelStatistics.Start(TimeStamp::Now()); } + mReopenOnError = PR_FALSE; + mIgnoreClose = PR_FALSE; if (mSuspendCount > 0) { // Re-suspend the channel if it needs to be suspended mChannel->Suspend(); @@ -246,18 +252,30 @@ nsresult nsMediaChannelStream::OnStopRequest(nsIRequest* aRequest, nsresult aStatus) { NS_ASSERTION(mChannel.get() == aRequest, "Wrong channel!"); + NS_ASSERTION(mSuspendCount == 0, + "How can OnStopRequest fire while we're suspended?"); { nsAutoLock lock(mLock); mChannelStatistics.Stop(TimeStamp::Now()); } - mCacheStream.NotifyDataEnded(aStatus); - if (mDecoder) { - mDecoder->NotifyDownloadEnded(aStatus); + if (NS_FAILED(aStatus) && aStatus != NS_ERROR_PARSED_DATA_CACHED && + mReopenOnError) { + nsresult rv = CacheClientSeek(mOffset, PR_FALSE); + if (NS_SUCCEEDED(rv)) + return rv; + // If the reopen/reseek fails, just fall through and treat this + // error as fatal. } - NS_ASSERTION(mSuspendCount == 0, - "How can OnStopRequest fire while we're suspended?"); + + if (!mIgnoreClose) { + mCacheStream.NotifyDataEnded(aStatus); + if (mDecoder) { + mDecoder->NotifyDownloadEnded(aStatus); + } + } + return NS_OK; } @@ -284,6 +302,8 @@ nsMediaChannelStream::CopySegmentToCache(nsIInputStream *aInStream, PRUint32 *aWriteCount) { CopySegmentClosure* closure = static_cast(aClosure); + // Keep track of where we're up to + closure->mStream->mOffset += aCount; closure->mStream->mCacheStream.NotifyDataReceived(aCount, aFromSegment, closure->mPrincipal); *aWriteCount = aCount; @@ -336,11 +356,11 @@ nsresult nsMediaChannelStream::Open(nsIStreamListener **aStreamListener) nsresult rv = mCacheStream.Init(); if (NS_FAILED(rv)) return rv; - return OpenChannel(aStreamListener, 0); + NS_ASSERTION(mOffset == 0, "Who set mOffset already?"); + return OpenChannel(aStreamListener); } -nsresult nsMediaChannelStream::OpenChannel(nsIStreamListener** aStreamListener, - PRInt64 aOffset) +nsresult nsMediaChannelStream::OpenChannel(nsIStreamListener** aStreamListener) { NS_ASSERTION(NS_IsMainThread(), "Only call on main thread"); NS_ENSURE_TRUE(mChannel, NS_ERROR_NULL_POINTER); @@ -350,8 +370,6 @@ nsresult nsMediaChannelStream::OpenChannel(nsIStreamListener** aStreamListener, *aStreamListener = nsnull; } - mLastSeekOffset = aOffset; - mListener = new Listener(this); NS_ENSURE_TRUE(mListener, NS_ERROR_OUT_OF_MEMORY); @@ -402,11 +420,11 @@ void nsMediaChannelStream::SetupChannelHeaders() nsCOMPtr hc = do_QueryInterface(mChannel); if (hc) { nsCAutoString rangeString("bytes="); - rangeString.AppendInt(mLastSeekOffset); + rangeString.AppendInt(mOffset); rangeString.Append("-"); hc->SetRequestHeader(NS_LITERAL_CSTRING("Range"), rangeString, PR_FALSE); } else { - NS_ASSERTION(mLastSeekOffset == 0, "Don't know how to seek on this channel type"); + NS_ASSERTION(mOffset == 0, "Don't know how to seek on this channel type"); } } @@ -479,38 +497,53 @@ PRInt64 nsMediaChannelStream::Tell() return mCacheStream.Tell(); } -void nsMediaChannelStream::Suspend() +void nsMediaChannelStream::Suspend(PRBool aCloseImmediately) { NS_ASSERTION(NS_IsMainThread(), "Don't call on main thread"); - if (mSuspendCount == 0 && mChannel) { - { - nsAutoLock lock(mLock); - mChannelStatistics.Stop(TimeStamp::Now()); + if (mChannel) { + if (aCloseImmediately && mCacheStream.IsSeekable()) { + // Kill off our channel right now, but don't tell anyone about it. + mIgnoreClose = PR_TRUE; + CloseChannel(); + } else if (mSuspendCount == 0) { + { + nsAutoLock lock(mLock); + mChannelStatistics.Stop(TimeStamp::Now()); + } + mChannel->Suspend(); } - mChannel->Suspend(); } + ++mSuspendCount; } void nsMediaChannelStream::Resume() { NS_ASSERTION(NS_IsMainThread(), "Don't call on main thread"); + NS_ASSERTION(mSuspendCount > 0, "Too many resumes!"); --mSuspendCount; - if (mSuspendCount == 0 && mChannel) { - { - nsAutoLock lock(mLock); - mChannelStatistics.Start(TimeStamp::Now()); + if (mSuspendCount == 0) { + if (mChannel) { + // Just wake up our existing channel + { + nsAutoLock lock(mLock); + mChannelStatistics.Start(TimeStamp::Now()); + } + // if an error occurs after Resume, assume it's because the server + // timed out the connection and we should reopen it. + mReopenOnError = PR_TRUE; + mChannel->Resume(); + } else { + // Need to recreate the channel + CacheClientSeek(mOffset, PR_FALSE); } - mChannel->Resume(); - // XXX need to do something fancier here because we often won't - // be able to resume cleanly } } nsresult -nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset) +nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset, PRBool aResume) { NS_ASSERTION(NS_IsMainThread(), "Don't call on main thread"); @@ -528,6 +561,12 @@ nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset) nsCOMPtr loadGroup = element->GetDocumentLoadGroup(); NS_ENSURE_TRUE(loadGroup, NS_ERROR_NULL_POINTER); + if (aResume) { + NS_ASSERTION(mSuspendCount > 0, "Too many resumes!"); + // No need to mess with the channel, since we're making a new one + --mSuspendCount; + } + nsresult rv = NS_NewChannel(getter_AddRefs(mChannel), mURI, @@ -537,7 +576,8 @@ nsMediaChannelStream::CacheClientSeek(PRInt64 aOffset) loadFlags); if (NS_FAILED(rv)) return rv; - return OpenChannel(nsnull, aOffset); + mOffset = aOffset; + return OpenChannel(nsnull); } class SuspendedStatusChanged : public nsRunnable @@ -569,7 +609,7 @@ nsMediaChannelStream::CacheClientSuspend() nsAutoLock lock(mLock); ++mCacheSuspendCount; } - Suspend(); + Suspend(PR_FALSE); // We have to spawn an event here since we're being called back from // a sensitive place in nsMediaCache, which doesn't want us to reenter @@ -676,7 +716,7 @@ public: // Main thread virtual nsresult Open(nsIStreamListener** aStreamListener); virtual nsresult Close(); - virtual void Suspend() {} + virtual void Suspend(PRBool aCloseImmediately) {} virtual void Resume() {} virtual already_AddRefed GetCurrentPrincipal(); diff --git a/content/media/video/src/nsOggDecoder.cpp b/content/media/video/src/nsOggDecoder.cpp index 397e57c273d..1131e292a84 100644 --- a/content/media/video/src/nsOggDecoder.cpp +++ b/content/media/video/src/nsOggDecoder.cpp @@ -2232,7 +2232,7 @@ PRBool nsOggDecoder::GetSeekable() void nsOggDecoder::Suspend() { if (mReader) { - mReader->Stream()->Suspend(); + mReader->Stream()->Suspend(PR_TRUE); } } diff --git a/content/media/video/src/nsWaveDecoder.cpp b/content/media/video/src/nsWaveDecoder.cpp index c563f0601cf..a5ecabb7759 100644 --- a/content/media/video/src/nsWaveDecoder.cpp +++ b/content/media/video/src/nsWaveDecoder.cpp @@ -1616,7 +1616,7 @@ void nsWaveDecoder::Suspend() { if (mStream) { - mStream->Suspend(); + mStream->Suspend(PR_TRUE); } } diff --git a/content/media/video/test/Makefile.in b/content/media/video/test/Makefile.in index d16c63d21b4..8a7d8ccc2d0 100644 --- a/content/media/video/test/Makefile.in +++ b/content/media/video/test/Makefile.in @@ -71,6 +71,7 @@ _TEST_FILES += \ test_bug468190.html \ test_bug482461.html \ test_can_play_type_ogg.html \ + test_closing_connections.html \ test_contentDuration1.html \ test_contentDuration2.html \ test_contentDuration3.html \ diff --git a/content/media/video/test/test_closing_connections.html b/content/media/video/test/test_closing_connections.html new file mode 100644 index 00000000000..e1165f9151f --- /dev/null +++ b/content/media/video/test/test_closing_connections.html @@ -0,0 +1,61 @@ + + + + + Test for Bug 479863 --- loading many connections + + + + + + +Mozilla Bug 479863 +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+
+ +