Bug 836927 - Make WMFByteStream bug compatible with WMF's IMFByteStream implementation. r=padenot
authorChris Pearce <cpearce@mozilla.com>
Tue, 05 Feb 2013 10:34:23 +1300
changeset 130678 b1f54c7df31742c8ccc7324d558974e99a2c7ad8
parent 130677 8d404fcb334b0249fa611a362f97997d8e03ab63
child 130679 fcb1d5ec2d6000167aca115b4c5c8400c7204ee4
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs836927
milestone21.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 836927 - Make WMFByteStream bug compatible with WMF's IMFByteStream implementation. r=padenot
content/media/BufferMediaResource.h
content/media/MediaDecoder.h
content/media/MediaResource.cpp
content/media/MediaResource.h
content/media/wmf/WMFByteStream.cpp
content/media/wmf/WMFByteStream.h
--- a/content/media/BufferMediaResource.h
+++ b/content/media/BufferMediaResource.h
@@ -127,16 +127,18 @@ public:
   }
 
   virtual nsresult GetCachedRanges(nsTArray<MediaByteRange>& aRanges)
   {
     aRanges.AppendElement(MediaByteRange(0, mLength));
     return NS_OK;
   }
 
+  bool IsTransportSeekable() MOZ_OVERRIDE { return true; }
+
 private:
   const uint8_t * mBuffer;
   uint32_t mLength;
   uint32_t mOffset;
   nsCOMPtr<nsIPrincipal> mPrincipal;
 };
 
 }
--- a/content/media/MediaDecoder.h
+++ b/content/media/MediaDecoder.h
@@ -941,17 +941,17 @@ public:
   // The state machine object for handling the decoding. It is safe to
   // call methods of this object from other threads. Its internal data
   // is synchronised on a monitor. The lifetime of this object is
   // after mPlayState is LOADING and before mPlayState is SHUTDOWN. It
   // is safe to access it during this period.
   nsCOMPtr<MediaDecoderStateMachine> mDecoderStateMachine;
 
   // Media data resource.
-  nsAutoPtr<MediaResource> mResource;
+  nsRefPtr<MediaResource> 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
   // change.
   // Using a wrapper class to restrict direct access to the |ReentrantMonitor|
   // object. Subclasses may override |MediaDecoder|::|GetReentrantMonitor|
   // e.g. |DASHRepDecoder|::|GetReentrantMonitor| returns the monitor in the
   // main |DASHDecoder| object. In this case, directly accessing the
--- a/content/media/MediaResource.cpp
+++ b/content/media/MediaResource.cpp
@@ -56,17 +56,18 @@ ChannelMediaResource::ChannelMediaResour
     mReopenOnError(false), mIgnoreClose(false),
     mCacheStream(this),
     mLock("ChannelMediaResource.mLock"),
     mIgnoreResume(false),
     mSeekingForMetadata(false),
     mByteRangeDownloads(false),
     mByteRangeFirstOpen(true),
     mSeekOffsetMonitor("media.dashseekmonitor"),
-    mSeekOffset(-1)
+    mSeekOffset(-1),
+    mIsTransportSeekable(true)
 {
 #ifdef PR_LOGGING
   if (!gMediaResourceLog) {
     gMediaResourceLog = PR_NewLogModule("MediaResource");
   }
 #endif
 }
 
@@ -303,16 +304,17 @@ ChannelMediaResource::OnStartRequest(nsI
       mDecoder->SetInfinite(false);
     }
   }
   mDecoder->SetTransportSeekable(seekable);
   mCacheStream.SetTransportSeekable(seekable);
 
   {
     MutexAutoLock lock(mLock);
+    mIsTransportSeekable = seekable;
     mChannelStatistics->Start();
   }
 
   mReopenOnError = false;
   // If we are seeking to get metadata, because we are playing an OGG file,
   // ignore if the channel gets closed without us suspending it explicitly. We
   // don't want to tell the element that the download has finished whereas we
   // just happended to have reached the end of the media while seeking.
@@ -328,16 +330,23 @@ ChannelMediaResource::OnStartRequest(nsI
 
   // Fires an initial progress event and sets up the stall counter so stall events
   // fire if no download occurs within the required time frame.
   mDecoder->Progress(false);
 
   return NS_OK;
 }
 
+bool
+ChannelMediaResource::IsTransportSeekable()
+{
+  MutexAutoLock lock(mLock);
+  return mIsTransportSeekable;
+}
+
 nsresult
 ChannelMediaResource::ParseContentRangeHeader(nsIHttpChannel * aHttpChan,
                                               int64_t& aRangeStart,
                                               int64_t& aRangeEnd,
                                               int64_t& aRangeTotal)
 {
   NS_ENSURE_ARG(aHttpChan);
 
@@ -1296,16 +1305,17 @@ public:
   virtual bool    IsSuspendedByCache(MediaResource** aActiveResource)
   {
     if (aActiveResource) {
       *aActiveResource = nullptr;
     }
     return false;
   }
   virtual bool    IsSuspended() { return false; }
+  virtual bool    IsTransportSeekable() MOZ_OVERRIDE { return true; }
 
   nsresult GetCachedRanges(nsTArray<MediaByteRange>& aRanges);
 
 private:
   // Ensures mSize is initialized, if it can be.
   // mLock must be held when this is called, and mInput must be non-null.
   void EnsureSizeInitialized();
 
--- a/content/media/MediaResource.h
+++ b/content/media/MediaResource.h
@@ -188,16 +188,19 @@ inline MediaByteRange::MediaByteRange(Ti
  * handle any URI for which Necko supports AsyncOpen.
  * The 'file:' protocol can be implemented efficiently with direct random
  * access, so the FileMediaResource implementation class bypasses the cache.
  * MediaResource::Create automatically chooses the best implementation class.
  */
 class MediaResource
 {
 public:
+
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaResource)
+
   virtual ~MediaResource() {}
 
   // The following can be called on the main thread only:
   // Get the URI
   virtual nsIURI* URI() const { return nullptr; }
   // Close the resource, stop any listeners, channels, etc.
   // Cancels any currently blocking Read request and forces that request to
   // return an error.
@@ -318,16 +321,20 @@ public:
   // Reads only data which is cached in the media cache. If you try to read
   // any data which overlaps uncached data, or if aCount bytes otherwise can't
   // be read, this function will return failure. This function be called from
   // any thread, and it is the only read operation which is safe to call on
   // the main thread, since it's guaranteed to be non blocking.
   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.
    * The caller must follow up by calling resource->Open().
    */
   static MediaResource* Create(MediaDecoder* aDecoder, nsIChannel* aChannel);
 
   /**
@@ -489,16 +496,17 @@ public:
   virtual void    Unpin();
   virtual double  GetDownloadRate(bool* aIsReliable);
   virtual int64_t GetLength();
   virtual int64_t GetNextCachedData(int64_t aOffset);
   virtual int64_t GetCachedDataEnd(int64_t aOffset);
   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,
                              public nsIChannelEventSink
   {
   public:
     Listener(ChannelMediaResource* aResource) : mResource(aResource) {}
     ~Listener() {}
@@ -594,16 +602,20 @@ protected:
   MediaByteRange mByteRange;
 
   // True if resource was opened with a byte rage request.
   bool mByteRangeDownloads;
 
   // 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|.
   ReentrantMonitor mSeekOffsetMonitor;
   int64_t mSeekOffset;
 };
 
 } // namespace mozilla
--- a/content/media/wmf/WMFByteStream.cpp
+++ b/content/media/wmf/WMFByteStream.cpp
@@ -67,40 +67,50 @@ ThreadPoolListener::OnThreadShuttingDown
 // Thread pool on which read requests are processed.
 // This is created and destroyed on the main thread only.
 static nsIThreadPool* sThreadPool = nullptr;
 
 // Counter of the number of WMFByteStreams that are instantiated and that need
 // 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<MediaResource> 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) {
       NS_ASSERTION(sThreadPool != nullptr, "Should have thread pool ref if sThreadPoolRefCnt==0.");
       // Note: store ref to thread pool, then clear global ref, then
       // Shutdown() using the stored ref. Events can run during the Shutdown()
       // call, so if we release after calling Shutdown(), another event may
       // have incremented the refcnt in the meantime, and have a dangling
       // pointer to the now destroyed threadpool!
       nsCOMPtr<nsIThreadPool> pool = sThreadPool;
       NS_IF_RELEASE(sThreadPool);
       pool->Shutdown();
     }
     return NS_OK;
   }
+  nsRefPtr<MediaResource> mResource;
 };
 
 WMFByteStream::WMFByteStream(MediaResource* aResource)
-  : mResource(aResource),
-    mReentrantMonitor("WMFByteStream"),
+  : mResourceMonitor("WMFByteStream.MediaResource"),
+    mResource(aResource),
+    mReentrantMonitor("WMFByteStream.Data"),
     mOffset(0),
     mIsShutdown(false)
 {
   NS_ASSERTION(NS_IsMainThread(), "Must be on main thread.");
   NS_ASSERTION(mResource, "Must have a valid media resource");
 
 #ifdef PR_LOGGING
   if (!gWMFByteStreamLog) {
@@ -109,19 +119,21 @@ WMFByteStream::WMFByteStream(MediaResour
 #endif
 
   MOZ_COUNT_CTOR(WMFByteStream);
 }
 
 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<nsIRunnable> 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<nsIRunnable> event =
+    new ReleaseWMFByteStreamResourcesEvent(mResource.forget());
   NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
 }
 
 nsresult
 WMFByteStream::Init()
 {
   NS_ASSERTION(NS_IsMainThread(), "Must be on main thread.");
 
@@ -147,21 +159,18 @@ WMFByteStream::Init()
   mThreadPool = sThreadPool;
 
   return NS_OK;
 }
 
 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;
 }
 
 // IUnknown Methods
 STDMETHODIMP
 WMFByteStream::QueryInterface(REFIID aIId, void **aInterface)
 {
   LOG("WMFByteStream::QueryInterface %s", GetGUIDName(aIId).get());
@@ -177,19 +186,19 @@ WMFByteStream::QueryInterface(REFIID aII
   return E_NOINTERFACE;
 }
 
 NS_IMPL_THREADSAFE_ADDREF(WMFByteStream)
 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),
       mBytesRead(0)
   {}
 
   // IUnknown Methods
   STDMETHODIMP QueryInterface(REFIID aRIID, LPVOID *aOutObject);
@@ -201,128 +210,151 @@ public:
   ULONG mBufferLength;
   ULONG mBytesRead;
 
   // IUnknown ref counting.
   nsAutoRefCnt mRefCnt;
   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<IUnknown*>(this), aInterface);
   }
 
   *aInterface = NULL;
   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<WMFByteStream> mStream;
   RefPtr<IMFAsyncResult> mResult;
-  RefPtr<AsyncReadRequestState> mRequestState;
+  RefPtr<AsyncReadRequest> mRequestState;
 };
 
 // IMFByteStream Methods
 STDMETHODIMP
 WMFByteStream::BeginRead(BYTE *aBuffer,
                          ULONG aLength,
                          IMFAsyncCallback *aCallback,
                          IUnknown *aCallerState)
 {
   NS_ENSURE_TRUE(aBuffer, E_POINTER);
   NS_ENSURE_TRUE(aCallback, E_POINTER);
 
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
   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<AsyncReadRequestState> requestState = new AsyncReadRequestState(mOffset, aBuffer, aLength);
+  RefPtr<AsyncReadRequest> 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.
   RefPtr<IMFAsyncResult> callersResult;
   HRESULT hr = wmf::MFCreateAsyncResult(requestState,
                                         aCallback,
                                         aCallerState,
                                         byRef(callersResult));
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
   // Dispatch an event to perform the read in the thread pool.
-  nsCOMPtr<nsIRunnable> r = new PerformReadEvent(this, callersResult, requestState);
+  nsCOMPtr<nsIRunnable> r = new ProcessReadRequestEvent(this,
+                                                        callersResult,
+                                                        requestState);
   nsresult rv = mThreadPool->Dispatch(r, NS_DISPATCH_NORMAL);
 
+  if (mResource->GetLength() > -1) {
+    mOffset = std::min<int64_t>(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!");
 
   // Read in a loop to ensure we fill the buffer, when possible.
   ULONG totalBytesRead = 0;
   nsresult rv = NS_OK;
   while (totalBytesRead < aRequestState->mBufferLength) {
     BYTE* buffer = aRequestState->mBuffer + totalBytesRead;
     ULONG bytesRead = 0;
     ULONG length = aRequestState->mBufferLength - totalBytesRead;
     rv = mResource->Read(reinterpret_cast<char*>(buffer),
                          length,
                          reinterpret_cast<uint32_t*>(&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);
 
   // Let caller know read is complete.
   DebugOnly<HRESULT> hr = wmf::MFInvokeCallback(aResult);
   NS_ASSERTION(SUCCEEDED(hr), "Failed to invoke callback!");
 }
@@ -352,36 +384,26 @@ WMFByteStream::EndRead(IMFAsyncResult* a
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
   // Extract our state object.
   RefPtr<IUnknown> unknown;
   HRESULT hr = aResult->GetObject(byRef(unknown));
   if (FAILED(hr) || !unknown) {
     return E_INVALIDARG;
   }
-  AsyncReadRequestState* requestState =
-    static_cast<AsyncReadRequestState*>(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<AsyncReadRequest*>(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
 WMFByteStream::EndWrite(IMFAsyncResult *, ULONG *)
 {
   LOG("WMFByteStream::EndWrite()");
   return E_NOTIMPL;
 }
@@ -393,47 +415,66 @@ WMFByteStream::Flush()
   return S_OK;
 }
 
 STDMETHODIMP
 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;
 }
 
 STDMETHODIMP
 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;
 }
 
 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;
 }
 
 STDMETHODIMP
 WMFByteStream::Read(BYTE*, ULONG, ULONG*)
 {
   LOG("WMFByteStream::Read()");
@@ -445,60 +486,49 @@ WMFByteStream::Seek(MFBYTESTREAM_SEEK_OR
                     LONGLONG aSeekOffset,
                     DWORD aSeekFlags,
                     QWORD *aCurrentPosition)
 {
   LOG("WMFByteStream::Seek(%d, %lld)", aSeekOrigin, aSeekOffset);
 
   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<int64_t>(offset, length);
+  } else {
+    mOffset = offset;
+  }
   if (aCurrentPosition) {
     *aCurrentPosition = mOffset;
   }
 
   return S_OK;
 }
 
 STDMETHODIMP
 WMFByteStream::SetCurrentPosition(QWORD aPosition)
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
   LOG("WMFByteStream::SetCurrentPosition(%lld)",
       aPosition);
 
-  if (mIsShutdown) {
-    return E_FAIL;
+  int64_t length = mResource->GetLength();
+  if (length > -1) {
+    mOffset = std::min<int64_t>(aPosition, length);
+  } else {
+    mOffset = aPosition;
   }
 
-  // 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;
-  }
-  mOffset = aPosition;
-
   return S_OK;
 }
 
 STDMETHODIMP
 WMFByteStream::SetLength(QWORD)
 {
   LOG("WMFByteStream::SetLength()");
   return E_NOTIMPL;
--- a/content/media/wmf/WMFByteStream.h
+++ b/content/media/wmf/WMFByteStream.h
@@ -14,23 +14,29 @@
 #include "mozilla/Attributes.h"
 #include "nsAutoPtr.h"
 
 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:
   WMFByteStream(MediaResource* aResource);
   ~WMFByteStream();
 
   nsresult Init();
   nsresult Shutdown();
@@ -61,27 +67,44 @@ public:
                     LONGLONG aSeekOffset,
                     DWORD aSeekFlags,
                     QWORD *aCurrentPosition);
   STDMETHODIMP SetCurrentPosition(QWORD aPosition);
   STDMETHODIMP SetLength(QWORD);
   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:
 
-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<nsIThreadPool> 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<MediaResource> mResource;
 
   // Protects mOffset, which is accessed by the SourceReaders thread(s), and
   // on the work queue thread.
   ReentrantMonitor mReentrantMonitor;
 
   // Current offset of the logical read cursor. We maintain this separately
   // from the media resource's offset since a partially complete read (in Invoke())
   // would leave the resource's offset at a value unexpected by the caller,