Bug 1120629 - Cache data directly on MP4Stream rather than relying on the media cache. r=roc, a=sledru
authorBobby Holley <bobbyholley@gmail.com>
Mon, 12 Jan 2015 15:50:09 -0800
changeset 242870 f7bd9ae15c9e
parent 242869 5fba52895751
child 242871 0a648dfd0459
push id4323
push userryanvm@gmail.com
push date2015-01-14 15:46 +0000
treeherdermozilla-beta@19e248751a1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, sledru
bugs1120629
milestone36.0
Bug 1120629 - Cache data directly on MP4Stream rather than relying on the media cache. r=roc, a=sledru
dom/media/fmp4/MP4Reader.cpp
dom/media/fmp4/MP4Stream.cpp
dom/media/fmp4/MP4Stream.h
--- a/dom/media/fmp4/MP4Reader.cpp
+++ b/dom/media/fmp4/MP4Reader.cpp
@@ -2,16 +2,17 @@
 /* vim:set ts=2 sw=2 sts=2 et cindent: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "MP4Reader.h"
 #include "MP4Stream.h"
 #include "MediaResource.h"
+#include "nsPrintfCString.h"
 #include "nsSize.h"
 #include "VideoUtils.h"
 #include "mozilla/dom/HTMLMediaElement.h"
 #include "ImageContainer.h"
 #include "Layers.h"
 #include "SharedThreadPool.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/TimeRanges.h"
@@ -83,42 +84,28 @@ InvokeAndRetry(ThisType* aThisVal, Retur
   AutoPinned<MP4Stream> stream(aStream);
   MP4Stream::ReadRecord prevFailure(-1, 0);
   while (true) {
     ReturnType result = ((*aThisVal).*aMethod)();
     if (result) {
       return result;
     }
     MP4Stream::ReadRecord failure(-1, 0);
-    if (!stream->LastReadFailed(&failure) || failure == prevFailure) {
+    if (NS_WARN_IF(!stream->LastReadFailed(&failure))) {
       return result;
     }
-    prevFailure = failure;
 
-    // Our goal here is to forcibly read the data we want into the cache: since
-    // the stream is pinned, the data is guaranteed to stay in the cache once
-    // it's there, which means that retrying the non-blocking read from inside
-    // the demuxer should succeed.
-    //
-    // But there's one wrinkle: if we read less than an entire cache line and
-    // the data ends up in MediaCacheStream's mPartialBlockBuffer, the data can
-    // be returned by a blocking read but never actually committed to the cache,
-    // and abandoned by a subsequent seek (possibly by another stream accessing
-    // the same underlying resource).
-    //
-    // The way to work around this problem is to round our "priming" read up to the
-    // size of an entire cache block. Note that this may hit EOS for bytes that the
-    // original demuxer read never actually requested. This is OK though because the
-    // call to BlockingReadAt will still return true (just with a less-than-expected
-    // number of actually read bytes, which we ignore).
-    size_t bufferSize = failure.mCount + (MediaCacheStream::BLOCK_SIZE - failure.mCount % MediaCacheStream::BLOCK_SIZE);
-    nsAutoArrayPtr<uint8_t> dummyBuffer(new uint8_t[bufferSize]);
-    MonitorAutoUnlock unlock(*aMonitor);
-    size_t ignored;
-    if (NS_WARN_IF(!stream->BlockingReadAt(failure.mOffset, dummyBuffer, bufferSize, &ignored))) {
+    if (NS_WARN_IF(failure == prevFailure)) {
+      NS_WARNING(nsPrintfCString("Failed reading the same block twice: offset=%lld, count=%lu",
+                                 failure.mOffset, failure.mCount).get());
+      return result;
+    }
+
+    prevFailure = failure;
+    if (NS_WARN_IF(!stream->BlockingReadIntoCache(failure.mOffset, failure.mCount, aMonitor))) {
       return result;
     }
   }
 }
 
 
 MP4Reader::MP4Reader(AbstractMediaDecoder* aDecoder)
   : MediaDecoderReader(aDecoder)
--- a/dom/media/fmp4/MP4Stream.cpp
+++ b/dom/media/fmp4/MP4Stream.cpp
@@ -6,43 +6,52 @@
 
 #include "MP4Stream.h"
 #include "MediaResource.h"
 
 namespace mozilla {
 
 MP4Stream::MP4Stream(MediaResource* aResource)
   : mResource(aResource)
+  , mPinCount(0)
 {
   MOZ_COUNT_CTOR(MP4Stream);
   MOZ_ASSERT(aResource);
 }
 
 MP4Stream::~MP4Stream()
 {
   MOZ_COUNT_DTOR(MP4Stream);
+  MOZ_ASSERT(mPinCount == 0);
 }
 
 bool
-MP4Stream::BlockingReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
-                          size_t* aBytesRead)
+MP4Stream::BlockingReadIntoCache(int64_t aOffset, size_t aCount, Monitor* aToUnlock)
 {
+  MOZ_ASSERT(mPinCount > 0);
+  CacheBlock block(aOffset, aCount);
+
   uint32_t sum = 0;
   uint32_t bytesRead = 0;
   do {
     uint64_t offset = aOffset + sum;
-    char* buffer = reinterpret_cast<char*>(aBuffer) + sum;
+    char* buffer = reinterpret_cast<char*>(block.mBuffer.get()) + sum;
     uint32_t toRead = aCount - sum;
+    MonitorAutoUnlock unlock(*aToUnlock);
     nsresult rv = mResource->ReadAt(offset, buffer, toRead, &bytesRead);
     if (NS_FAILED(rv)) {
       return false;
     }
     sum += bytesRead;
   } while (sum < aCount && bytesRead > 0);
-  *aBytesRead = sum;
+
+  MOZ_ASSERT(block.mCount >= sum);
+  block.mCount = sum;
+
+  mCache.AppendElement(block);
   return true;
 }
 
 // We surreptitiously reimplement the supposedly-blocking ReadAt as a non-
 // blocking CachedReadAt, and record when it fails. This allows MP4Reader
 // to retry the read as an actual blocking read without holding the lock.
 bool
 MP4Stream::ReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
@@ -59,16 +68,24 @@ MP4Stream::ReadAt(int64_t aOffset, void*
 
   return true;
 }
 
 bool
 MP4Stream::CachedReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
                         size_t* aBytesRead)
 {
+  // First, check our local cache.
+  for (size_t i = 0; i < mCache.Length(); ++i) {
+    if (mCache[i].mOffset == aOffset && mCache[i].mCount >= aCount) {
+      memcpy(aBuffer, mCache[i].mBuffer, aCount);
+      *aBytesRead = aCount;
+      return true;
+    }
+  }
 
   nsresult rv = mResource->ReadFromCache(reinterpret_cast<char*>(aBuffer),
                                          aOffset, aCount);
   if (NS_FAILED(rv)) {
     *aBytesRead = 0;
     return false;
   }
   *aBytesRead = aCount;
--- a/dom/media/fmp4/MP4Stream.h
+++ b/dom/media/fmp4/MP4Stream.h
@@ -7,26 +7,27 @@
 #ifndef MP4_STREAM_H_
 #define MP4_STREAM_H_
 
 #include "mp4_demuxer/mp4_demuxer.h"
 
 #include "MediaResource.h"
 
 #include "mozilla/Maybe.h"
+#include "mozilla/Monitor.h"
 
 namespace mozilla {
 
 class Monitor;
 
 class MP4Stream : public mp4_demuxer::Stream {
 public:
   explicit MP4Stream(MediaResource* aResource);
   virtual ~MP4Stream();
-  bool BlockingReadAt(int64_t aOffset, void* aBuffer, size_t aCount, size_t* aBytesRead);
+  bool BlockingReadIntoCache(int64_t aOffset, size_t aCount, Monitor* aToUnlock);
   virtual bool ReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
                       size_t* aBytesRead) MOZ_OVERRIDE;
   virtual bool CachedReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
                             size_t* aBytesRead) MOZ_OVERRIDE;
   virtual bool Length(int64_t* aSize) MOZ_OVERRIDE;
 
   struct ReadRecord {
     ReadRecord(int64_t aOffset, size_t aCount) : mOffset(aOffset), mCount(aCount) {}
@@ -39,19 +40,42 @@ public:
     if (mFailedRead.isSome()) {
       *aOut = mFailedRead.ref();
       return true;
     }
 
     return false;
   }
 
-  void Pin() { mResource->Pin(); }
-  void Unpin() { mResource->Unpin(); }
+  void Pin()
+  {
+    mResource->Pin();
+    ++mPinCount;
+  }
+
+  void Unpin()
+  {
+    mResource->Unpin();
+    MOZ_ASSERT(mPinCount);
+    --mPinCount;
+    if (mPinCount == 0) {
+      mCache.Clear();
+    }
+  }
 
 private:
   nsRefPtr<MediaResource> mResource;
   Maybe<ReadRecord> mFailedRead;
+  uint32_t mPinCount;
+
+  struct CacheBlock {
+    CacheBlock(int64_t aOffset, size_t aCount)
+      : mOffset(aOffset), mCount(aCount), mBuffer(new uint8_t[aCount]) {}
+    int64_t mOffset;
+    size_t mCount;
+    nsAutoArrayPtr<uint8_t> mBuffer;
+  };
+  nsTArray<CacheBlock> mCache;
 };
 
 }
 
 #endif