Bug 1296453 - [MSE] P2. Clean up SourceBufferResource. r=gerald, a=lizzard
authorJean-Yves Avenard <jyavenard@mozilla.com>
Tue, 23 Aug 2016 17:38:46 +1200
changeset 342448 4c552e7562362b5736687587d1e4375e9cdff6b3
parent 342447 cbe685bc5bc45cf9aef4f19dcca24dacc4ad4bb8
child 342449 f4870f71d0b587e78c1179c503b112cf89dccd3e
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald, lizzard
bugs1296453, 1190238
milestone49.0
Bug 1296453 - [MSE] P2. Clean up SourceBufferResource. r=gerald, a=lizzard The code was designed around the need for a MediaResource::Read which is no longer possible since bug 1190238 MozReview-Commit-ID: 7s76YWg93jS
dom/media/mediasource/SourceBufferResource.cpp
dom/media/mediasource/SourceBufferResource.h
--- a/dom/media/mediasource/SourceBufferResource.cpp
+++ b/dom/media/mediasource/SourceBufferResource.cpp
@@ -31,107 +31,80 @@ SourceBufferResource::Close()
   SBR_DEBUG("Close");
   //MOZ_ASSERT(!mClosed);
   mClosed = true;
   mon.NotifyAll();
   return NS_OK;
 }
 
 nsresult
-SourceBufferResource::ReadInternal(char* aBuffer, uint32_t aCount, uint32_t* aBytes, bool aMayBlock)
-{
-  mMonitor.AssertCurrentThreadIn();
-  MOZ_ASSERT_IF(!aMayBlock, aBytes);
-
-  // Cache the offset for the read in case mOffset changes while waiting on the
-  // monitor below. It's basically impossible to implement these API semantics
-  // sanely. :-(
-  uint64_t readOffset = mOffset;
-
-  while (aMayBlock &&
-         !mEnded &&
-         readOffset + aCount > static_cast<uint64_t>(GetLength())) {
-    SBR_DEBUGV("waiting for data");
-    mMonitor.Wait();
-    // The callers of this function should have checked this, but it's
-    // possible that we had an eviction while waiting on the monitor.
-    if (readOffset < mInputBuffer.GetOffset()) {
-      return NS_ERROR_FAILURE;
-    }
-  }
-
-  uint32_t available = GetLength() - readOffset;
-  uint32_t count = std::min(aCount, available);
-  SBR_DEBUGV("readOffset=%llu GetLength()=%u available=%u count=%u mEnded=%d",
-             readOffset, GetLength(), available, count, mEnded);
-  if (available == 0) {
-    SBR_DEBUGV("reached EOF");
-    *aBytes = 0;
-    return NS_OK;
-  }
-
-  mInputBuffer.CopyData(readOffset, count, aBuffer);
-  *aBytes = count;
-
-  // From IRC:
-  // <@cpearce>bholley: *this* is why there should only every be a ReadAt() and
-  // no Read() on a Stream abstraction! there's no good answer, they all suck.
-  mOffset = readOffset + count;
-
-  return NS_OK;
-}
-
-nsresult
 SourceBufferResource::ReadAt(int64_t aOffset, char* aBuffer, uint32_t aCount, uint32_t* aBytes)
 {
   SBR_DEBUG("ReadAt(aOffset=%lld, aBuffer=%p, aCount=%u, aBytes=%p)",
             aOffset, aBytes, aCount, aBytes);
   ReentrantMonitorAutoEnter mon(mMonitor);
   return ReadAtInternal(aOffset, aBuffer, aCount, aBytes, /* aMayBlock = */ true);
 }
 
 nsresult
 SourceBufferResource::ReadAtInternal(int64_t aOffset, char* aBuffer, uint32_t aCount, uint32_t* aBytes,
                                      bool aMayBlock)
 {
   mMonitor.AssertCurrentThreadIn();
-  nsresult rv = SeekInternal(aOffset);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
 
-  return ReadInternal(aBuffer, aCount, aBytes, aMayBlock);
-}
-
-nsresult
-SourceBufferResource::SeekInternal(int64_t aOffset)
-{
-  mMonitor.AssertCurrentThreadIn();
+  MOZ_ASSERT_IF(!aMayBlock, aBytes);
 
   if (mClosed ||
       aOffset < 0 ||
       uint64_t(aOffset) < mInputBuffer.GetOffset() ||
       aOffset > GetLength()) {
     return NS_ERROR_FAILURE;
   }
 
-  mOffset = aOffset;
+  while (aMayBlock &&
+         !mEnded &&
+         aOffset + aCount > GetLength()) {
+    SBR_DEBUGV("waiting for data");
+    mMonitor.Wait();
+    // The callers of this function should have checked this, but it's
+    // possible that we had an eviction while waiting on the monitor.
+    if (uint64_t(aOffset) < mInputBuffer.GetOffset()) {
+      return NS_ERROR_FAILURE;
+    }
+  }
+
+  uint32_t available = GetLength() - aOffset;
+  uint32_t count = std::min(aCount, available);
+
+  // Keep the position of the last read to have Tell() approximately give us
+  // the position we're up to in the stream.
+  mOffset = aOffset + count;
+
+  SBR_DEBUGV("offset=%llu GetLength()=%u available=%u count=%u mEnded=%d",
+             aOffset, GetLength(), available, count, mEnded);
+  if (available == 0) {
+    SBR_DEBUGV("reached EOF");
+    *aBytes = 0;
+    return NS_OK;
+  }
+
+  mInputBuffer.CopyData(aOffset, count, aBuffer);
+  *aBytes = count;
+
   return NS_OK;
 }
 
 nsresult
 SourceBufferResource::ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount)
 {
   SBR_DEBUG("ReadFromCache(aBuffer=%p, aOffset=%lld, aCount=%u)",
             aBuffer, aOffset, aCount);
   ReentrantMonitorAutoEnter mon(mMonitor);
   uint32_t bytesRead;
-  int64_t oldOffset = mOffset;
   nsresult rv = ReadAtInternal(aOffset, aBuffer, aCount, &bytesRead, /* aMayBlock = */ false);
-  mOffset = oldOffset; // ReadFromCache isn't supposed to affect the seek position.
   NS_ENSURE_SUCCESS(rv, rv);
 
   // ReadFromCache return failure if not all the data is cached.
   return bytesRead == aCount ? NS_OK : NS_ERROR_FAILURE;
 }
 
 uint32_t
 SourceBufferResource::EvictData(uint64_t aPlaybackOffset, int64_t aThreshold,
--- a/dom/media/mediasource/SourceBufferResource.h
+++ b/dom/media/mediasource/SourceBufferResource.h
@@ -131,18 +131,16 @@ public:
 #if defined(DEBUG)
   void Dump(const char* aPath) {
     mInputBuffer.Dump(aPath);
   }
 #endif
 
 private:
   virtual ~SourceBufferResource();
-  nsresult SeekInternal(int64_t aOffset);
-  nsresult ReadInternal(char* aBuffer, uint32_t aCount, uint32_t* aBytes, bool aMayBlock);
   nsresult ReadAtInternal(int64_t aOffset, char* aBuffer, uint32_t aCount, uint32_t* aBytes, bool aMayBlock);
 
   const nsCString mType;
 
   // Provides synchronization between SourceBuffers and InputAdapters.
   // Protects all of the member variables below.  Read() will await a
   // Notify() (from Seek, AppendData, Ended, or Close) when insufficient
   // data is available in mData.