From 35b36ead151fc16ef136b09afa883b65945ce485 Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Tue, 5 Feb 2013 10:34:23 +1300 Subject: [PATCH] Bug 836927 - Make WMFByteStream bug compatible with WMF's IMFByteStream implementation. r=padenot --- content/media/BufferMediaResource.h | 2 + content/media/MediaDecoder.h | 2 +- content/media/MediaResource.cpp | 12 +- content/media/MediaResource.h | 12 ++ content/media/wmf/WMFByteStream.cpp | 206 ++++++++++++++++------------ content/media/wmf/WMFByteStream.h | 33 ++++- 6 files changed, 172 insertions(+), 95 deletions(-) diff --git a/content/media/BufferMediaResource.h b/content/media/BufferMediaResource.h index f74a974988f9..965a107715af 100644 --- a/content/media/BufferMediaResource.h +++ b/content/media/BufferMediaResource.h @@ -132,6 +132,8 @@ public: return NS_OK; } + bool IsTransportSeekable() MOZ_OVERRIDE { return true; } + private: const uint8_t * mBuffer; uint32_t mLength; diff --git a/content/media/MediaDecoder.h b/content/media/MediaDecoder.h index a6dc370244e5..f545feaadfd3 100644 --- a/content/media/MediaDecoder.h +++ b/content/media/MediaDecoder.h @@ -946,7 +946,7 @@ public: nsCOMPtr mDecoderStateMachine; // Media data resource. - nsAutoPtr mResource; + nsRefPtr mResource; // |ReentrantMonitor| for detecting when the video play state changes. A call // to |Wait| on this monitor will block the thread until the next state diff --git a/content/media/MediaResource.cpp b/content/media/MediaResource.cpp index 5a17045de555..52f0e4e8e831 100644 --- a/content/media/MediaResource.cpp +++ b/content/media/MediaResource.cpp @@ -61,7 +61,8 @@ ChannelMediaResource::ChannelMediaResource(MediaDecoder* aDecoder, mByteRangeDownloads(false), mByteRangeFirstOpen(true), mSeekOffsetMonitor("media.dashseekmonitor"), - mSeekOffset(-1) + mSeekOffset(-1), + mIsTransportSeekable(true) { #ifdef PR_LOGGING if (!gMediaResourceLog) { @@ -308,6 +309,7 @@ ChannelMediaResource::OnStartRequest(nsIRequest* aRequest) { MutexAutoLock lock(mLock); + mIsTransportSeekable = seekable; mChannelStatistics->Start(); } @@ -333,6 +335,13 @@ ChannelMediaResource::OnStartRequest(nsIRequest* aRequest) return NS_OK; } +bool +ChannelMediaResource::IsTransportSeekable() +{ + MutexAutoLock lock(mLock); + return mIsTransportSeekable; +} + nsresult ChannelMediaResource::ParseContentRangeHeader(nsIHttpChannel * aHttpChan, int64_t& aRangeStart, @@ -1301,6 +1310,7 @@ public: return false; } virtual bool IsSuspended() { return false; } + virtual bool IsTransportSeekable() MOZ_OVERRIDE { return true; } nsresult GetCachedRanges(nsTArray& aRanges); diff --git a/content/media/MediaResource.h b/content/media/MediaResource.h index 677d9f338630..ec59625ff7cd 100644 --- a/content/media/MediaResource.h +++ b/content/media/MediaResource.h @@ -193,6 +193,9 @@ inline MediaByteRange::MediaByteRange(TimestampedMediaByteRange& aByteRange) class MediaResource { public: + + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaResource) + virtual ~MediaResource() {} // The following can be called on the main thread only: @@ -323,6 +326,10 @@ public: virtual nsresult ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount) = 0; + // Returns true if the resource can be seeked to unbuffered ranges, i.e. + // for an HTTP network stream this returns true if HTTP1.1 Byte Range + // requests are supported by the connection/server. + virtual bool IsTransportSeekable() = 0; /** * Create a resource, reading data from the channel. Call on main thread only. @@ -494,6 +501,7 @@ public: virtual bool IsDataCachedToEndOfResource(int64_t aOffset); virtual bool IsSuspendedByCache(MediaResource** aActiveResource); virtual bool IsSuspended(); + virtual bool IsTransportSeekable() MOZ_OVERRIDE; class Listener MOZ_FINAL : public nsIStreamListener, public nsIInterfaceRequestor, @@ -599,6 +607,10 @@ protected: // Set to false once first byte range request has been made. bool mByteRangeFirstOpen; + // True if the stream can seek into unbuffered ranged, i.e. if the + // connection supports byte range requests. + bool mIsTransportSeekable; + // For byte range requests, set to the offset requested in |Seek|. // Used in |CacheClientSeek| to find the originally requested byte range. // Read/Write on multiple threads; use |mSeekMonitor|. diff --git a/content/media/wmf/WMFByteStream.cpp b/content/media/wmf/WMFByteStream.cpp index 81a2fa74dd58..3204f62615fb 100644 --- a/content/media/wmf/WMFByteStream.cpp +++ b/content/media/wmf/WMFByteStream.cpp @@ -72,10 +72,18 @@ static nsIThreadPool* sThreadPool = nullptr; // the thread pool. This is read/write on the main thread only. static int32_t sThreadPoolRefCnt = 0; -class ReleaseThreadPoolEvent MOZ_FINAL : public nsRunnable { +class ReleaseWMFByteStreamResourcesEvent MOZ_FINAL : public nsRunnable { public: + ReleaseWMFByteStreamResourcesEvent(already_AddRefed aResource) + : mResource(aResource) {} + virtual ~ReleaseWMFByteStreamResourcesEvent() {} NS_IMETHOD Run() { NS_ASSERTION(NS_IsMainThread(), "Must be on main thread."); + // Explicitly release the MediaResource reference. We *must* do this on + // the main thread, so we must explicitly release it here, we can't rely + // on the destructor to release it, since if this event runs before its + // dispatch call returns the destructor may run on the non-main thread. + mResource = nullptr; NS_ASSERTION(sThreadPoolRefCnt > 0, "sThreadPoolRefCnt Should be non-negative"); sThreadPoolRefCnt--; if (sThreadPoolRefCnt == 0) { @@ -91,11 +99,13 @@ public: } return NS_OK; } + nsRefPtr mResource; }; WMFByteStream::WMFByteStream(MediaResource* aResource) - : mResource(aResource), - mReentrantMonitor("WMFByteStream"), + : mResourceMonitor("WMFByteStream.MediaResource"), + mResource(aResource), + mReentrantMonitor("WMFByteStream.Data"), mOffset(0), mIsShutdown(false) { @@ -114,9 +124,11 @@ WMFByteStream::WMFByteStream(MediaResource* aResource) WMFByteStream::~WMFByteStream() { MOZ_COUNT_DTOR(WMFByteStream); - // The WMFByteStream can be deleted from a WMF work queue thread, so we - // dispatch an event to the main thread to deref the thread pool. - nsCOMPtr event = new ReleaseThreadPoolEvent(); + // The WMFByteStream can be deleted from a thread pool thread, so we + // dispatch an event to the main thread to deref the thread pool and + // deref the MediaResource. + nsCOMPtr event = + new ReleaseWMFByteStreamResourcesEvent(mResource.forget()); NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); } @@ -152,11 +164,8 @@ WMFByteStream::Init() nsresult WMFByteStream::Shutdown() { - NS_ASSERTION(NS_IsMainThread(), "Must be on main thread."); - { - ReentrantMonitorAutoEnter mon(mReentrantMonitor); - mIsShutdown = true; - } + ReentrantMonitorAutoEnter mon(mReentrantMonitor); + mIsShutdown = true; return NS_OK; } @@ -182,9 +191,9 @@ NS_IMPL_THREADSAFE_RELEASE(WMFByteStream) // Stores data regarding an async read opreation. -class AsyncReadRequestState MOZ_FINAL : public IUnknown { +class AsyncReadRequest MOZ_FINAL : public IUnknown { public: - AsyncReadRequestState(int64_t aOffset, BYTE* aBuffer, ULONG aLength) + AsyncReadRequest(int64_t aOffset, BYTE* aBuffer, ULONG aLength) : mOffset(aOffset), mBuffer(aBuffer), mBufferLength(aLength), @@ -206,14 +215,14 @@ public: NS_DECL_OWNINGTHREAD }; -NS_IMPL_THREADSAFE_ADDREF(AsyncReadRequestState) -NS_IMPL_THREADSAFE_RELEASE(AsyncReadRequestState) +NS_IMPL_THREADSAFE_ADDREF(AsyncReadRequest) +NS_IMPL_THREADSAFE_RELEASE(AsyncReadRequest) // IUnknown Methods STDMETHODIMP -AsyncReadRequestState::QueryInterface(REFIID aIId, void **aInterface) +AsyncReadRequest::QueryInterface(REFIID aIId, void **aInterface) { - LOG("WMFByteStream::AsyncReadRequestState::QueryInterface %s", GetGUIDName(aIId).get()); + LOG("AsyncReadRequest::QueryInterface %s", GetGUIDName(aIId).get()); if (aIId == IID_IUnknown) { return DoGetInterface(static_cast(this), aInterface); @@ -223,23 +232,23 @@ AsyncReadRequestState::QueryInterface(REFIID aIId, void **aInterface) return E_NOINTERFACE; } -class PerformReadEvent MOZ_FINAL : public nsRunnable { +class ProcessReadRequestEvent MOZ_FINAL : public nsRunnable { public: - PerformReadEvent(WMFByteStream* aStream, - IMFAsyncResult* aResult, - AsyncReadRequestState* aRequestState) + ProcessReadRequestEvent(WMFByteStream* aStream, + IMFAsyncResult* aResult, + AsyncReadRequest* aRequestState) : mStream(aStream), mResult(aResult), mRequestState(aRequestState) {} NS_IMETHOD Run() { - mStream->PerformRead(mResult, mRequestState); + mStream->ProcessReadRequest(mResult, mRequestState); return NS_OK; } private: RefPtr mStream; RefPtr mResult; - RefPtr mRequestState; + RefPtr mRequestState; }; // IMFByteStream Methods @@ -256,12 +265,12 @@ WMFByteStream::BeginRead(BYTE *aBuffer, LOG("WMFByteStream::BeginRead() mOffset=%lld tell=%lld length=%lu mIsShutdown=%d", mOffset, mResource->Tell(), aLength, mIsShutdown); - if (mIsShutdown) { - return E_FAIL; + if (mIsShutdown || mOffset < 0) { + return E_INVALIDARG; } // Create an object to store our state. - RefPtr requestState = new AsyncReadRequestState(mOffset, aBuffer, aLength); + RefPtr requestState = new AsyncReadRequest(mOffset, aBuffer, aLength); // Create an IMFAsyncResult, this is passed back to the caller as a token to // retrieve the number of bytes read. @@ -273,28 +282,31 @@ WMFByteStream::BeginRead(BYTE *aBuffer, NS_ENSURE_TRUE(SUCCEEDED(hr), hr); // Dispatch an event to perform the read in the thread pool. - nsCOMPtr r = new PerformReadEvent(this, callersResult, requestState); + nsCOMPtr r = new ProcessReadRequestEvent(this, + callersResult, + requestState); nsresult rv = mThreadPool->Dispatch(r, NS_DISPATCH_NORMAL); + if (mResource->GetLength() > -1) { + mOffset = std::min(mOffset + aLength, mResource->GetLength()); + } else { + mOffset += aLength; + } + return NS_SUCCEEDED(rv) ? S_OK : E_FAIL; } -// Note: This is called on one of the thread pool's threads. -void -WMFByteStream::PerformRead(IMFAsyncResult* aResult, AsyncReadRequestState* aRequestState) +nsresult +WMFByteStream::Read(AsyncReadRequest* aRequestState) { + ReentrantMonitorAutoEnter mon(mResourceMonitor); + // Ensure the read head is at the correct offset in the resource. It may not // be if the SourceReader seeked. if (mResource->Tell() != aRequestState->mOffset) { nsresult rv = mResource->Seek(nsISeekableStream::NS_SEEK_SET, aRequestState->mOffset); - if (NS_FAILED(rv)) { - // Let SourceReader know the read failed. - aResult->SetStatus(E_FAIL); - wmf::MFInvokeCallback(aResult); - LOG("WMFByteStream::Invoke() seek to read offset failed, aborting read"); - return; - } + NS_ENSURE_SUCCESS(rv, rv); } NS_ASSERTION(mResource->Tell() == aRequestState->mOffset, "State mismatch!"); @@ -308,16 +320,36 @@ WMFByteStream::PerformRead(IMFAsyncResult* aResult, AsyncReadRequestState* aRequ rv = mResource->Read(reinterpret_cast(buffer), length, reinterpret_cast(&bytesRead)); + NS_ENSURE_SUCCESS(rv, rv); totalBytesRead += bytesRead; - if (NS_FAILED(rv) || bytesRead == 0) { + if (bytesRead == 0) { break; } } + aRequestState->mBytesRead = totalBytesRead; + return NS_OK; +} - // Record the number of bytes read, so the caller can retrieve - // it later. - aRequestState->mBytesRead = NS_SUCCEEDED(rv) ? totalBytesRead : 0; - aResult->SetStatus(S_OK); +// Note: This is called on one of the thread pool's threads. +void +WMFByteStream::ProcessReadRequest(IMFAsyncResult* aResult, + AsyncReadRequest* aRequestState) +{ + if (mResource->GetLength() > -1 && + aRequestState->mOffset > mResource->GetLength()) { + aResult->SetStatus(S_OK); + wmf::MFInvokeCallback(aResult); + LOG("WMFByteStream::Invoke() read offset greater than length, soft-failing read"); + return; + } + + nsresult rv = Read(aRequestState); + if (NS_FAILED(rv)) { + Shutdown(); + aResult->SetStatus(E_ABORT); + } else { + aResult->SetStatus(S_OK); + } LOG("WMFByteStream::Invoke() read %d at %lld finished rv=%x", aRequestState->mBytesRead, aRequestState->mOffset, rv); @@ -357,26 +389,16 @@ WMFByteStream::EndRead(IMFAsyncResult* aResult, ULONG *aBytesRead) if (FAILED(hr) || !unknown) { return E_INVALIDARG; } - AsyncReadRequestState* requestState = - static_cast(unknown.get()); - - // Important: Only advance the read cursor if the caller hasn't seeked - // since it called BeginRead(). If it has seeked, we still must report - // the number of bytes read (in *aBytesRead), but we don't advance the - // read cursor; reads performed after the seek will do that. The SourceReader - // caller seems to keep track of which async read requested which range - // to be read, and does call SetCurrentPosition() before it calls EndRead(). - if (mOffset == requestState->mOffset) { - mOffset += requestState->mBytesRead; - } + AsyncReadRequest* requestState = + static_cast(unknown.get()); // Report result. *aBytesRead = requestState->mBytesRead; - LOG("WMFByteStream::EndRead() offset=%lld *aBytesRead=%u mOffset=%lld hr=0x%x eof=%d", - requestState->mOffset, *aBytesRead, mOffset, hr, (mOffset == mResource->GetLength())); + LOG("WMFByteStream::EndRead() offset=%lld *aBytesRead=%u mOffset=%lld status=0x%x hr=0x%x eof=%d", + requestState->mOffset, *aBytesRead, mOffset, aResult->GetStatus(), hr, IsEOS()); - return S_OK; + return aResult->GetStatus(); } STDMETHODIMP @@ -398,8 +420,12 @@ WMFByteStream::GetCapabilities(DWORD *aCapabilities) { LOG("WMFByteStream::GetCapabilities()"); NS_ENSURE_TRUE(aCapabilities, E_POINTER); + ReentrantMonitorAutoEnter mon(mReentrantMonitor); + bool seekable = mResource->IsTransportSeekable(); *aCapabilities = MFBYTESTREAM_IS_READABLE | - MFBYTESTREAM_IS_SEEKABLE; + MFBYTESTREAM_IS_SEEKABLE | + MFBYTESTREAM_IS_PARTIALLY_DOWNLOADED | + (!seekable ? MFBYTESTREAM_HAS_SLOW_SEEK : 0); return S_OK; } @@ -408,7 +434,14 @@ WMFByteStream::GetCurrentPosition(QWORD *aPosition) { NS_ENSURE_TRUE(aPosition, E_POINTER); ReentrantMonitorAutoEnter mon(mReentrantMonitor); - *aPosition = mOffset; + // Note: Returning the length of stream as position when read + // cursor is < 0 seems to be the behaviour expected by WMF, but + // also note it doesn't seem to expect that the position is an + // unsigned value since if you seek to > length and read WMF + // expects the read to succeed after reading 0 bytes, but if you + // seek to < 0 and read, the read is expected to fails... So + // go figure... + *aPosition = mOffset < 0 ? mResource->GetLength() : mOffset; LOG("WMFByteStream::GetCurrentPosition() %lld", mOffset); return S_OK; } @@ -417,18 +450,26 @@ STDMETHODIMP WMFByteStream::GetLength(QWORD *aLength) { NS_ENSURE_TRUE(aLength, E_POINTER); - int64_t length = mResource->GetLength(); - *aLength = length; - LOG("WMFByteStream::GetLength() %lld", length); + ReentrantMonitorAutoEnter mon(mReentrantMonitor); + *aLength = mResource->GetLength(); + LOG("WMFByteStream::GetLength() %lld", *aLength); return S_OK; } +bool +WMFByteStream::IsEOS() +{ + ReentrantMonitorAutoEnter mon(mReentrantMonitor); + return mResource->GetLength() > -1 && + (mOffset < 0 || + mOffset >= mResource->GetLength()); +} + STDMETHODIMP WMFByteStream::IsEndOfStream(BOOL *aEndOfStream) { NS_ENSURE_TRUE(aEndOfStream, E_POINTER); - ReentrantMonitorAutoEnter mon(mReentrantMonitor); - *aEndOfStream = (mOffset == mResource->GetLength()); + *aEndOfStream = IsEOS(); LOG("WMFByteStream::IsEndOfStream() %d", *aEndOfStream); return S_OK; } @@ -450,16 +491,18 @@ WMFByteStream::Seek(MFBYTESTREAM_SEEK_ORIGIN aSeekOrigin, ReentrantMonitorAutoEnter mon(mReentrantMonitor); - if (mIsShutdown) { - return E_FAIL; - } - + int64_t offset = mOffset; if (aSeekOrigin == msoBegin) { - mOffset = aSeekOffset; + offset = aSeekOffset; } else { - mOffset += aSeekOffset; + offset += aSeekOffset; + } + int64_t length = mResource->GetLength(); + if (length > -1) { + mOffset = std::min(offset, length); + } else { + mOffset = offset; } - if (aCurrentPosition) { *aCurrentPosition = mOffset; } @@ -474,25 +517,12 @@ WMFByteStream::SetCurrentPosition(QWORD aPosition) LOG("WMFByteStream::SetCurrentPosition(%lld)", aPosition); - if (mIsShutdown) { - return E_FAIL; - } - - // Note: WMF calls SetCurrentPosition() sometimes after calling BeginRead() - // but before the read has finished, and thus before it's called EndRead(). - // See comment in EndRead() for more details. - int64_t length = mResource->GetLength(); - if (length >= 0 && aPosition > uint64_t(length)) { - // Despite the fact that the MSDN IMFByteStream::SetCurrentPosition() - // documentation says that we should return E_INVALIDARG when the seek - // position is after the length, if we do that IMFSourceReader::ReadSample() - // fails in some situations. So we just clamp the seek target to - // the EOS, and things seem to just work... - LOG("WMFByteStream::SetCurrentPosition(%lld) clamping position to eos (%lld)", aPosition, length); - aPosition = length; + if (length > -1) { + mOffset = std::min(aPosition, length); + } else { + mOffset = aPosition; } - mOffset = aPosition; return S_OK; } diff --git a/content/media/wmf/WMFByteStream.h b/content/media/wmf/WMFByteStream.h index d9a26d283539..3152434f4059 100644 --- a/content/media/wmf/WMFByteStream.h +++ b/content/media/wmf/WMFByteStream.h @@ -19,13 +19,19 @@ class nsIThreadPool; namespace mozilla { class MediaResource; -class AsyncReadRequestState; +class AsyncReadRequest; // Wraps a MediaResource around an IMFByteStream interface, so that it can // be used by the IMFSourceReader. Each WMFByteStream creates a WMF Work Queue // on which blocking I/O is performed. The SourceReader requests reads // asynchronously using {Begin,End}Read(). The synchronous I/O methods aren't // used by the SourceReader, so they're not implemented on this class. +// +// Note: This implementation attempts to be bug-compatible with Windows Media +// Foundation's implementation of IMFByteStream. The behaviour of WMF's +// IMFByteStream was determined by creating it and testing the edge cases. +// For details see the test code at: +// https://github.com/cpearce/IMFByteStreamBehaviour/ class WMFByteStream MOZ_FINAL : public IMFByteStream { public: @@ -66,17 +72,34 @@ public: STDMETHODIMP Write(const BYTE *, ULONG, ULONG *); // We perform an async read operation in this callback implementation. - void PerformRead(IMFAsyncResult* aResult, AsyncReadRequestState* aRequestState); - + // Processes an async read request, storing the result in aResult, and + // notifying the caller when the read operation is complete. + void ProcessReadRequest(IMFAsyncResult* aResult, + AsyncReadRequest* aRequestState); private: + // Locks the MediaResource and performs the read. This is a helper + // for ProcessReadRequest(). + nsresult Read(AsyncReadRequest* aRequestState); + + // Returns true if the current position of the stream is at end of stream. + bool IsEOS(); + // Reference to the thread pool in which we perform the reads asynchronously. // Note this is pool is shared amongst all active WMFByteStreams. nsCOMPtr mThreadPool; + // Monitor that ensures that multiple concurrent async reads are processed + // in serial on a resource. This prevents concurrent async reads and seeks + // from interleaving, to ensure that reads occur at the offset they're + // supposed to! + ReentrantMonitor mResourceMonitor; + // Resource we're wrapping. Note this object's methods are threadsafe, - // and we only call read/seek on the work queue's thread. - MediaResource* mResource; + // but because multiple reads can be processed concurrently in the thread + // pool we must hold mResourceMonitor whenever we seek+read to ensure that + // another read request's seek+read doesn't interleave. + nsRefPtr mResource; // Protects mOffset, which is accessed by the SourceReaders thread(s), and // on the work queue thread.