Bug 1119456 - Work around the fact that media cache does not quite guarantee the property we want. r=roc
authorBobby Holley <bobbyholley@gmail.com>
Sun, 11 Jan 2015 13:24:26 -0800
changeset 223161 3ffd66a3f1bd9d2bca1acf5c6086c85dec1119c2
parent 223160 19b8c420b13a31d1d1e12cc642493be2e82059de
child 223162 a0ec121ce6bffb7aff20ae78be3fa7136ec0560b
push id28082
push usercbook@mozilla.com
push dateMon, 12 Jan 2015 10:44:52 +0000
treeherdermozilla-central@643589c3ef94 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1119456
milestone37.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 1119456 - Work around the fact that media cache does not quite guarantee the property we want. r=roc
dom/media/fmp4/MP4Reader.cpp
--- a/dom/media/fmp4/MP4Reader.cpp
+++ b/dom/media/fmp4/MP4Reader.cpp
@@ -87,20 +87,38 @@ InvokeAndRetry(ThisType* aThisVal, Retur
     if (result) {
       return result;
     }
     MP4Stream::ReadRecord failure(-1, 0);
     if (!stream->LastReadFailed(&failure) || failure == prevFailure) {
       return result;
     }
     prevFailure = failure;
-    nsAutoArrayPtr<uint8_t> dummyBuffer(new uint8_t[failure.mCount]);
+
+    // 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;
-    MonitorAutoUnlock unlock(*aMonitor);
-    if (!stream->BlockingReadAt(failure.mOffset, dummyBuffer, failure.mCount, &ignored)) {
+    if (NS_WARN_IF(!stream->BlockingReadAt(failure.mOffset, dummyBuffer, bufferSize, &ignored))) {
       return result;
     }
   }
 }
 
 
 MP4Reader::MP4Reader(AbstractMediaDecoder* aDecoder)
   : MediaDecoderReader(aDecoder)