Bug 1119456 - Make MP4Demuxer's blocking reads non-blocking and hoist blocking into callers with a hacky retry strategy. r=k17e
authorBobby Holley <bobbyholley@gmail.com>
Sun, 11 Jan 2015 13:24:26 -0800
changeset 223160 19b8c420b13a31d1d1e12cc642493be2e82059de
parent 223159 7e4003516dd5f56463945c8e06515f28abefa83c
child 223161 3ffd66a3f1bd9d2bca1acf5c6086c85dec1119c2
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)
reviewersk17e
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 - Make MP4Demuxer's blocking reads non-blocking and hoist blocking into callers with a hacky retry strategy. r=k17e
dom/media/fmp4/MP4Reader.cpp
dom/media/fmp4/MP4Reader.h
dom/media/fmp4/MP4Stream.cpp
dom/media/fmp4/MP4Stream.h
dom/media/gtest/TestMP4Demuxer.cpp
media/libstagefright/binding/Index.cpp
media/libstagefright/binding/mp4_demuxer.cpp
--- a/dom/media/fmp4/MP4Reader.cpp
+++ b/dom/media/fmp4/MP4Reader.cpp
@@ -58,16 +58,55 @@ TrackTypeToStr(TrackType aTrack)
   case kVideo:
     return "Video";
   default:
     return "Unknown";
   }
 }
 #endif
 
+// MP4Demuxer wants to do various blocking reads, which cause deadlocks while
+// mDemuxerMonitor is held. This stuff should really be redesigned, but we don't
+// have time for that right now. So in order to get proper synchronization while
+// keeping behavior as similar as possible, we do the following nasty hack:
+//
+// The demuxer has a Stream object with APIs to do both blocking and non-blocking
+// reads. When it does a blocking read, MP4Stream actually redirects it to a non-
+// blocking read, but records the parameters of the read on the MP4Stream itself.
+// This means that, when the read failure bubbles up to MP4Reader.cpp, we can
+// detect whether we in fact just needed to block, and do that while releasing the
+// monitor. We distinguish these fake failures from bonafide EOS by tracking the
+// previous failed read as well. If we ever do a blocking read on the same segment
+// twice, we know we've hit EOS.
+template<typename ThisType, typename ReturnType>
+ReturnType
+InvokeAndRetry(ThisType* aThisVal, ReturnType(ThisType::*aMethod)(), MP4Stream* aStream, Monitor* aMonitor)
+{
+  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) {
+      return result;
+    }
+    prevFailure = failure;
+    nsAutoArrayPtr<uint8_t> dummyBuffer(new uint8_t[failure.mCount]);
+    size_t ignored;
+    MonitorAutoUnlock unlock(*aMonitor);
+    if (!stream->BlockingReadAt(failure.mOffset, dummyBuffer, failure.mCount, &ignored)) {
+      return result;
+    }
+  }
+}
+
+
 MP4Reader::MP4Reader(AbstractMediaDecoder* aDecoder)
   : MediaDecoderReader(aDecoder)
   , mAudio(MediaData::AUDIO_DATA, Preferences::GetUint("media.mp4-audio-decode-ahead", 2))
   , mVideo(MediaData::VIDEO_DATA, Preferences::GetUint("media.mp4-video-decode-ahead", 2))
   , mLastReportedNumDecodedFrames(0)
   , mLayersBackendType(layers::LayersBackend::LAYERS_NONE)
   , mDemuxerInitialized(false)
   , mIsEncrypted(false)
@@ -152,17 +191,18 @@ MP4Reader::InitLayersBackendType()
 
 static bool sIsEMEEnabled = false;
 
 nsresult
 MP4Reader::Init(MediaDecoderReader* aCloneDonor)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Must be on main thread.");
   PlatformDecoderModule::Init();
-  mDemuxer = new MP4Demuxer(new MP4Stream(mDecoder->GetResource(), &mDemuxerMonitor), GetDecoder()->GetTimestampOffset(), &mDemuxerMonitor);
+  mStream = new MP4Stream(mDecoder->GetResource());
+  mTimestampOffset = GetDecoder()->GetTimestampOffset();
 
   InitLayersBackendType();
 
   mAudio.mTaskQueue = new MediaTaskQueue(GetMediaDecodeThreadPool());
   NS_ENSURE_TRUE(mAudio.mTaskQueue, NS_ERROR_FAILURE);
 
   mVideo.mTaskQueue = new MediaTaskQueue(GetMediaDecodeThreadPool());
   NS_ENSURE_TRUE(mVideo.mTaskQueue, NS_ERROR_FAILURE);
@@ -284,23 +324,30 @@ MP4Reader::IsSupportedVideoMimeType(cons
 void
 MP4Reader::PreReadMetadata()
 {
   if (mPlatform) {
     RequestCodecResource();
   }
 }
 
+bool
+MP4Reader::InitDemuxer()
+{
+  mDemuxer = new MP4Demuxer(mStream, mTimestampOffset, &mDemuxerMonitor);
+  return mDemuxer->Init();
+}
+
 nsresult
 MP4Reader::ReadMetadata(MediaInfo* aInfo,
                         MetadataTags** aTags)
 {
   if (!mDemuxerInitialized) {
     MonitorAutoLock mon(mDemuxerMonitor);
-    bool ok = mDemuxer->Init();
+    bool ok = InvokeAndRetry(this, &MP4Reader::InitDemuxer, mStream, &mDemuxerMonitor);
     NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
     mIndexReady = true;
 
     // To decode, we need valid video and a place to put it.
     mInfo.mVideo.mHasVideo = mVideo.mActive = mDemuxer->HasValidVideo() &&
                                               mDecoder->GetImageContainer();
 
     mInfo.mAudio.mHasAudio = mAudio.mActive = mDemuxer->HasValidAudio();
@@ -644,23 +691,22 @@ MP4Reader::PopSample(TrackType aTrack)
 }
 
 MP4Sample*
 MP4Reader::PopSampleLocked(TrackType aTrack)
 {
   mDemuxerMonitor.AssertCurrentThreadOwns();
   switch (aTrack) {
     case kAudio:
-      return mDemuxer->DemuxAudioSample();
-
+      return InvokeAndRetry(mDemuxer.get(), &MP4Demuxer::DemuxAudioSample, mStream, &mDemuxerMonitor);
     case kVideo:
       if (mQueuedVideoSample) {
         return mQueuedVideoSample.forget();
       }
-      return mDemuxer->DemuxVideoSample();
+      return InvokeAndRetry(mDemuxer.get(), &MP4Demuxer::DemuxVideoSample, mStream, &mDemuxerMonitor);
 
     default:
       return nullptr;
   }
 }
 
 size_t
 MP4Reader::SizeOfVideoQueueInFrames()
--- a/dom/media/fmp4/MP4Reader.h
+++ b/dom/media/fmp4/MP4Reader.h
@@ -79,16 +79,17 @@ public:
     MOZ_OVERRIDE;
 
   virtual nsresult ResetDecode() MOZ_OVERRIDE;
 
   virtual nsRefPtr<ShutdownPromise> Shutdown() MOZ_OVERRIDE;
 
 private:
 
+  bool InitDemuxer();
   void ReturnOutput(MediaData* aData, TrackType aTrack);
 
   // Sends input to decoder for aTrack, and output to the state machine,
   // if necessary.
   void Update(TrackType aTrack);
 
   // Enqueues a task to call Update(aTrack) on the decoder task queue.
   // Lock for corresponding track must be held.
@@ -118,16 +119,18 @@ private:
   bool IsSupportedVideoMimeType(const char* aMimeType);
   void NotifyResourcesStatusChanged();
   void RequestCodecResource();
   bool IsWaitingOnCodecResource();
   virtual bool IsWaitingOnCDMResource() MOZ_OVERRIDE;
 
   size_t SizeOfQueue(TrackType aTrack);
 
+  nsRefPtr<MP4Stream> mStream;
+  int64_t mTimestampOffset;
   nsAutoPtr<mp4_demuxer::MP4Demuxer> mDemuxer;
   nsRefPtr<PlatformDecoderModule> mPlatform;
 
   class DecoderCallback : public MediaDataDecoderCallback {
   public:
     DecoderCallback(MP4Reader* aReader,
                     mp4_demuxer::TrackType aType)
       : mReader(aReader)
--- a/dom/media/fmp4/MP4Stream.cpp
+++ b/dom/media/fmp4/MP4Stream.cpp
@@ -4,64 +4,71 @@
  * 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 "MP4Stream.h"
 #include "MediaResource.h"
 
 namespace mozilla {
 
-MP4Stream::MP4Stream(MediaResource* aResource, Monitor* aDemuxerMonitor)
+MP4Stream::MP4Stream(MediaResource* aResource)
   : mResource(aResource)
-  , mDemuxerMonitor(aDemuxerMonitor)
 {
   MOZ_COUNT_CTOR(MP4Stream);
   MOZ_ASSERT(aResource);
 }
 
 MP4Stream::~MP4Stream()
 {
   MOZ_COUNT_DTOR(MP4Stream);
 }
 
 bool
-MP4Stream::ReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
-                  size_t* aBytesRead)
+MP4Stream::BlockingReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
+                          size_t* aBytesRead)
 {
-  // The read call can acquire various monitors, including both the decoder
-  // monitor and gMediaCache's monitor. So we need to unlock ours to avoid
-  // deadlock.
-  mDemuxerMonitor->AssertCurrentThreadOwns();
-  MonitorAutoUnlock unlock(*mDemuxerMonitor);
-
   uint32_t sum = 0;
   uint32_t bytesRead = 0;
   do {
     uint64_t offset = aOffset + sum;
     char* buffer = reinterpret_cast<char*>(aBuffer) + sum;
     uint32_t toRead = aCount - sum;
     nsresult rv = mResource->ReadAt(offset, buffer, toRead, &bytesRead);
     if (NS_FAILED(rv)) {
       return false;
     }
     sum += bytesRead;
   } while (sum < aCount && bytesRead > 0);
   *aBytesRead = sum;
   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,
+                  size_t* aBytesRead)
+{
+  if (mFailedRead.isSome()) {
+    mFailedRead.reset();
+  }
+
+  if (!CachedReadAt(aOffset, aBuffer, aCount, aBytesRead)) {
+    mFailedRead.emplace(aOffset, aCount);
+    return false;
+  }
+
+  return true;
+}
+
 bool
 MP4Stream::CachedReadAt(int64_t aOffset, void* aBuffer, size_t aCount,
                         size_t* aBytesRead)
 {
-  // The read call can acquire various monitors, including both the decoder
-  // monitor and gMediaCache's monitor. So we need to unlock ours to avoid
-  // deadlock.
-  mDemuxerMonitor->AssertCurrentThreadOwns();
-  MonitorAutoUnlock unlock(*mDemuxerMonitor);
 
   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
@@ -4,32 +4,54 @@
  * 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/. */
 
 #ifndef MP4_STREAM_H_
 #define MP4_STREAM_H_
 
 #include "mp4_demuxer/mp4_demuxer.h"
 
-#include "mozilla/Monitor.h"
+#include "MediaResource.h"
+
+#include "mozilla/Maybe.h"
 
 namespace mozilla {
 
-class MediaResource;
+class Monitor;
 
 class MP4Stream : public mp4_demuxer::Stream {
 public:
-  explicit MP4Stream(MediaResource* aResource, Monitor* aDemuxerMonitor);
+  explicit MP4Stream(MediaResource* aResource);
   virtual ~MP4Stream();
+  bool BlockingReadAt(int64_t aOffset, void* aBuffer, size_t aCount, size_t* aBytesRead);
   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) {}
+    bool operator==(const ReadRecord& aOther) { return mOffset == aOther.mOffset && mCount == aOther.mCount; }
+    int64_t mOffset;
+    size_t mCount;
+  };
+  bool LastReadFailed(ReadRecord* aOut)
+  {
+    if (mFailedRead.isSome()) {
+      *aOut = mFailedRead.ref();
+      return true;
+    }
+
+    return false;
+  }
+
+  void Pin() { mResource->Pin(); }
+  void Unpin() { mResource->Unpin(); }
+
 private:
   nsRefPtr<MediaResource> mResource;
-  Monitor* mDemuxerMonitor;
+  Maybe<ReadRecord> mFailedRead;
 };
 
 }
 
 #endif
--- a/dom/media/gtest/TestMP4Demuxer.cpp
+++ b/dom/media/gtest/TestMP4Demuxer.cpp
@@ -19,17 +19,17 @@ public:
 
   nsRefPtr<MockMediaResource> resource;
   Monitor mMonitor;
   nsAutoPtr<MP4Demuxer> demuxer;
 
   explicit MP4DemuxerBinding(const char* aFileName = "dash_dashinit.mp4")
     : resource(new MockMediaResource(aFileName))
     , mMonitor("TestMP4Demuxer monitor")
-    , demuxer(new MP4Demuxer(new MP4Stream(resource, &mMonitor), 0, &mMonitor))
+    , demuxer(new MP4Demuxer(new MP4Stream(resource), 0, &mMonitor))
   {
     EXPECT_EQ(NS_OK, resource->Open(nullptr));
   }
 
 private:
   virtual ~MP4DemuxerBinding()
   {
   }
--- a/media/libstagefright/binding/Index.cpp
+++ b/media/libstagefright/binding/Index.cpp
@@ -85,18 +85,16 @@ SampleIterator::SampleIterator(Index* aI
 
 MP4Sample* SampleIterator::GetNext()
 {
   Sample* s(Get());
   if (!s) {
     return nullptr;
   }
 
-  Next();
-
   nsAutoPtr<MP4Sample> sample(new MP4Sample());
   sample->decode_timestamp = s->mDecodeTime;
   sample->composition_timestamp = s->mCompositionRange.start;
   sample->duration = s->mCompositionRange.Length();
   sample->byte_offset = s->mByteRange.mStart;
   sample->is_sync_point = s->mSync;
   sample->size = s->mByteRange.Length();
 
@@ -126,16 +124,18 @@ MP4Sample* SampleIterator::GetNext()
         sample->crypto.plain_sizes.AppendElement(reader.ReadU16());
         sample->crypto.encrypted_sizes.AppendElement(reader.ReadU32());
       }
       reader.ReadArray(sample->crypto.iv, 16);
       sample->crypto.iv_size = 16;
     }
   }
 
+  Next();
+
   return sample.forget();
 }
 
 Sample* SampleIterator::Get()
 {
   if (!mIndex->mMoofParser) {
     return nullptr;
   }
--- a/media/libstagefright/binding/mp4_demuxer.cpp
+++ b/media/libstagefright/binding/mp4_demuxer.cpp
@@ -90,17 +90,25 @@ MP4Demuxer::~MP4Demuxer()
   }
 }
 
 bool
 MP4Demuxer::Init()
 {
   mMonitor->AssertCurrentThreadOwns();
   sp<MediaExtractor> e = mPrivate->mExtractor;
-  for (size_t i = 0; i < e->countTracks(); i++) {
+
+  // Read the number of tracks. If we can't find any, make sure to bail now before
+  // attempting any new reads to make the retry system work.
+  size_t trackCount = e->countTracks();
+  if (trackCount == 0) {
+    return false;
+  }
+
+  for (size_t i = 0; i < trackCount; i++) {
     sp<MetaData> metaData = e->getTrackMetaData(i);
 
     const char* mimeType;
     if (metaData == nullptr || !metaData->findCString(kKeyMIMEType, &mimeType)) {
       continue;
     }
 
     if (!mPrivate->mAudio.get() && !strncmp(mimeType, "audio/", 6)) {