Bug 1119456 - Make MP4Demuxer's blocking reads non-blocking and hoist blocking into callers with a hacky retry strategy. r=k17e, a=sledru
authorBobby Holley <bobbyholley@gmail.com>
Sun, 11 Jan 2015 13:24:26 -0800
changeset 242849 fa0128cdef95
parent 242848 2fd2c6de0a87
child 242850 18f7174682d3
push id4321
push userryanvm@gmail.com
push date2015-01-14 15:04 +0000
treeherdermozilla-beta@a78eb4dd84f0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersk17e, sledru
bugs1119456
milestone36.0
Bug 1119456 - Make MP4Demuxer's blocking reads non-blocking and hoist blocking into callers with a hacky retry strategy. r=k17e, a=sledru
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)) {