Bug 1054153 - Fix MP4 demuxer is init vs buffered range race; r=edwin
authorAnthony Jones <ajones@mozilla.com>
Tue, 19 Aug 2014 14:13:56 +1200
changeset 200235 28a26c0a23b042d550d096d2a3e5110702ab87f1
parent 200234 3998dcd6b14766d51f3364f5edfe742703ecf8a7
child 200236 cdf1cbfe5c4943094960d8f1b3176e05a921fb14
push id47845
push userajones@mozilla.com
push dateTue, 19 Aug 2014 02:14:22 +0000
treeherdermozilla-inbound@cdf1cbfe5c49 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1054153
milestone34.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 1054153 - Fix MP4 demuxer is init vs buffered range race; r=edwin
content/media/fmp4/MP4Reader.cpp
content/media/fmp4/MP4Reader.h
media/libstagefright/binding/Index.cpp
media/libstagefright/binding/include/mp4_demuxer/Index.h
--- a/content/media/fmp4/MP4Reader.cpp
+++ b/content/media/fmp4/MP4Reader.cpp
@@ -106,16 +106,18 @@ private:
 MP4Reader::MP4Reader(AbstractMediaDecoder* aDecoder)
   : MediaDecoderReader(aDecoder)
   , mAudio("MP4 audio decoder data", Preferences::GetUint("media.mp4-audio-decode-ahead", 2))
   , mVideo("MP4 video decoder data", Preferences::GetUint("media.mp4-video-decode-ahead", 2))
   , mLastReportedNumDecodedFrames(0)
   , mLayersBackendType(layers::LayersBackend::LAYERS_NONE)
   , mDemuxerInitialized(false)
   , mIsEncrypted(false)
+  , mIndexReady(false)
+  , mIndexMonitor("MP4 index")
 {
   MOZ_ASSERT(NS_IsMainThread(), "Must be on main thread.");
   MOZ_COUNT_CTOR(MP4Reader);
 }
 
 MP4Reader::~MP4Reader()
 {
   MOZ_ASSERT(NS_IsMainThread(), "Must be on main thread.");
@@ -298,16 +300,21 @@ MP4Reader::IsSupportedAudioMimeType(cons
 nsresult
 MP4Reader::ReadMetadata(MediaInfo* aInfo,
                         MetadataTags** aTags)
 {
   if (!mDemuxerInitialized) {
     bool ok = mDemuxer->Init();
     NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
 
+    {
+      MonitorAutoLock mon(mIndexMonitor);
+      mIndexReady = true;
+    }
+
     mInfo.mVideo.mHasVideo = mVideo.mActive = mDemuxer->HasValidVideo();
     const VideoDecoderConfig& video = mDemuxer->VideoConfig();
     // If we have video, we *only* allow H.264 to be decoded.
     if (mInfo.mVideo.mHasVideo && strcmp(video.mime_type, "video/avc")) {
       return NS_ERROR_FAILURE;
     }
 
     {
@@ -411,16 +418,18 @@ MP4Reader::ReadMetadata(MediaInfo* aInfo
   if (duration != -1) {
     ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
     mDecoder->SetMediaDuration(duration);
   }
 
   *aInfo = mInfo;
   *aTags = nullptr;
 
+  UpdateIndex();
+
   return NS_OK;
 }
 
 bool
 MP4Reader::IsMediaSeekable()
 {
   // We can seek if we get a duration *and* the reader reports that it's
   // seekable.
@@ -768,46 +777,63 @@ MP4Reader::NotifyDataArrived(const char*
   } else {
     UpdateIndex();
   }
 }
 
 void
 MP4Reader::UpdateIndex()
 {
+  MonitorAutoLock mon(mIndexMonitor);
+  if (!mIndexReady) {
+    return;
+  }
+
   MediaResource* resource = mDecoder->GetResource();
   resource->Pin();
   nsTArray<MediaByteRange> ranges;
   if (NS_SUCCEEDED(resource->GetCachedRanges(ranges))) {
     mDemuxer->UpdateIndex(ranges);
   }
   resource->Unpin();
 }
 
 int64_t
 MP4Reader::GetEvictionOffset(double aTime)
 {
+  MonitorAutoLock mon(mIndexMonitor);
+  if (!mIndexReady) {
+    return 0;
+  }
+
   return mDemuxer->GetEvictionOffset(aTime * 1000000.0);
 }
 
 nsresult
 MP4Reader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
 {
+  MonitorAutoLock mon(mIndexMonitor);
+  if (!mIndexReady) {
+    return NS_OK;
+  }
+
   MediaResource* resource = mDecoder->GetResource();
+  nsTArray<MediaByteRange> ranges;
   resource->Pin();
-  nsTArray<MediaByteRange> ranges;
-  if (NS_SUCCEEDED(resource->GetCachedRanges(ranges))) {
+  nsresult rv = resource->GetCachedRanges(ranges);
+  resource->Unpin();
+
+  if (NS_SUCCEEDED(rv)) {
     nsTArray<Interval<Microseconds>> timeRanges;
     mDemuxer->ConvertByteRangesToTime(ranges, &timeRanges);
     for (size_t i = 0; i < timeRanges.Length(); i++) {
       aBuffered->Add((timeRanges[i].start - aStartTime) / 1000000.0,
                      (timeRanges[i].end - aStartTime) / 1000000.0);
     }
   }
-  resource->Unpin();
 
   return NS_OK;
 }
 
 bool MP4Reader::IsDormantNeeded()
 {
 #ifdef MOZ_GONK_MEDIACODEC
   return mVideo.mDecoder && mVideo.mDecoder->IsDormantNeeded();
--- a/content/media/fmp4/MP4Reader.h
+++ b/content/media/fmp4/MP4Reader.h
@@ -184,13 +184,16 @@ private:
 
   nsTArray<nsTArray<uint8_t>> mInitDataEncountered;
 
   // True if we've read the streams' metadata.
   bool mDemuxerInitialized;
 
   // Synchronized by decoder monitor.
   bool mIsEncrypted;
+
+  bool mIndexReady;
+  Monitor mIndexMonitor;
 };
 
 } // namespace mozilla
 
 #endif
--- a/media/libstagefright/binding/Index.cpp
+++ b/media/libstagefright/binding/Index.cpp
@@ -72,17 +72,16 @@ RangeFinder::Contains(MediaByteRange aBy
     }
   }
 
   return false;
 }
 
 Index::Index(const stagefright::Vector<MediaSource::Indice>& aIndex,
              Stream* aSource, uint32_t aTrackId)
-  : mMonitor("mp4_demuxer::Index")
 {
   if (aIndex.isEmpty()) {
     mMoofParser = new MoofParser(aSource, aTrackId);
   } else {
     mIndex.AppendElements(&aIndex[0], aIndex.size());
   }
 }
 
@@ -90,27 +89,24 @@ Index::~Index() {}
 
 void
 Index::UpdateMoofIndex(const nsTArray<MediaByteRange>& aByteRanges)
 {
   if (!mMoofParser) {
     return;
   }
 
-  MonitorAutoLock mon(mMonitor);
   mMoofParser->RebuildFragmentedIndex(aByteRanges);
 }
 
 void
 Index::ConvertByteRangesToTimeRanges(
   const nsTArray<MediaByteRange>& aByteRanges,
   nsTArray<Interval<Microseconds>>* aTimeRanges)
 {
-  MonitorAutoLock mon(mMonitor);
-
   RangeFinder rangeFinder(aByteRanges);
   nsTArray<Interval<Microseconds>> timeRanges;
 
   nsTArray<nsTArray<stagefright::MediaSource::Indice>*> indexes;
   if (mMoofParser) {
     // We take the index out of the moof parser and move it into a local
     // variable so we don't get concurrency issues. It gets freed when we
     // exit this function.
--- a/media/libstagefright/binding/include/mp4_demuxer/Index.h
+++ b/media/libstagefright/binding/include/mp4_demuxer/Index.h
@@ -24,15 +24,14 @@ public:
 
   void UpdateMoofIndex(const nsTArray<mozilla::MediaByteRange>& aByteRanges);
   void ConvertByteRangesToTimeRanges(
     const nsTArray<mozilla::MediaByteRange>& aByteRanges,
     nsTArray<Interval<Microseconds>>* aTimeRanges);
   uint64_t GetEvictionOffset(Microseconds aTime);
 
 private:
-  mozilla::Monitor mMonitor;
   nsTArray<stagefright::MediaSource::Indice> mIndex;
   nsAutoPtr<MoofParser> mMoofParser;
 };
 }
 
 #endif