Bug 1495735 - Properly report updated media details. r=bryce
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 03 Oct 2018 08:23:08 +0000
changeset 495091 34bf7a591f0e8c6861144751a9011f0b2cbc9480
parent 495090 13ee6c203030801e96e5046cad27c696e66a2d95
child 495092 758cfb5e4ddc3e26584d67e81679ad6a9c3e914e
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbryce
bugs1495735
milestone64.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 1495735 - Properly report updated media details. r=bryce Also fix a long-term data race where we could read and write on mInfo.{mVideo,mAudio} on different task queues. Differential Revision: https://phabricator.services.mozilla.com/D7484
dom/media/MediaFormatReader.cpp
dom/media/MediaFormatReader.h
--- a/dom/media/MediaFormatReader.cpp
+++ b/dom/media/MediaFormatReader.cpp
@@ -1145,25 +1145,31 @@ MediaFormatReader::Shutdown()
   }
   if (mVideo.HasPromise()) {
     mVideo.RejectPromise(NS_ERROR_DOM_MEDIA_CANCELED, __func__);
   }
 
   if (HasAudio()) {
     mAudio.ResetDemuxer();
     mAudio.mTrackDemuxer->BreakCycles();
-    mAudio.mTrackDemuxer = nullptr;
+    {
+      MutexAutoLock lock(mAudio.mMutex);
+      mAudio.mTrackDemuxer = nullptr;
+    }
     mAudio.ResetState();
     ShutdownDecoder(TrackInfo::kAudioTrack);
   }
 
   if (HasVideo()) {
     mVideo.ResetDemuxer();
     mVideo.mTrackDemuxer->BreakCycles();
-    mVideo.mTrackDemuxer = nullptr;
+    {
+      MutexAutoLock lock(mVideo.mMutex);
+      mVideo.mTrackDemuxer = nullptr;
+    }
     mVideo.ResetState();
     ShutdownDecoder(TrackInfo::kVideoTrack);
   }
 
   mShutdownPromisePool->Track(mDemuxer->Shutdown());
   mDemuxer = nullptr;
 
   mOnTrackWaitingForKeyListener.Disconnect();
@@ -1382,65 +1388,63 @@ MediaFormatReader::OnDemuxerInitDone(con
   }
 
   // To decode, we need valid video and a place to put it.
   bool videoActive =
     !!mDemuxer->GetNumberTracks(TrackInfo::kVideoTrack) && GetImageContainer();
 
   if (videoActive) {
     // We currently only handle the first video track.
+    MutexAutoLock lock(mVideo.mMutex);
     mVideo.mTrackDemuxer = mDemuxer->GetTrackDemuxer(TrackInfo::kVideoTrack, 0);
     if (!mVideo.mTrackDemuxer) {
       mMetadataPromise.Reject(NS_ERROR_DOM_MEDIA_METADATA_ERR, __func__);
       return;
     }
 
     UniquePtr<TrackInfo> videoInfo = mVideo.mTrackDemuxer->GetInfo();
     videoActive = videoInfo && videoInfo->IsValid();
     if (videoActive) {
       if (platform &&
           !platform->SupportsMimeType(videoInfo->mMimeType, nullptr)) {
         // We have no decoder for this track. Error.
         mMetadataPromise.Reject(NS_ERROR_DOM_MEDIA_METADATA_ERR, __func__);
         return;
       }
-      {
-        MutexAutoLock lock(mVideo.mMutex);
-        mInfo.mVideo = *videoInfo->GetAsVideoInfo();
-      }
+      mInfo.mVideo = *videoInfo->GetAsVideoInfo();
+      mVideo.mWorkingInfo = MakeUnique<VideoInfo>(mInfo.mVideo);
       for (const MetadataTag& tag : videoInfo->mTags) {
         tags->Put(tag.mKey, tag.mValue);
       }
       mVideo.mOriginalInfo = std::move(videoInfo);
       mTrackDemuxersMayBlock |= mVideo.mTrackDemuxer->GetSamplesMayBlock();
     } else {
       mVideo.mTrackDemuxer->BreakCycles();
       mVideo.mTrackDemuxer = nullptr;
     }
   }
 
   bool audioActive = !!mDemuxer->GetNumberTracks(TrackInfo::kAudioTrack);
   if (audioActive) {
+    MutexAutoLock lock(mAudio.mMutex);
     mAudio.mTrackDemuxer = mDemuxer->GetTrackDemuxer(TrackInfo::kAudioTrack, 0);
     if (!mAudio.mTrackDemuxer) {
       mMetadataPromise.Reject(NS_ERROR_DOM_MEDIA_METADATA_ERR, __func__);
       return;
     }
 
     UniquePtr<TrackInfo> audioInfo = mAudio.mTrackDemuxer->GetInfo();
     // We actively ignore audio tracks that we know we can't play.
     audioActive =
       audioInfo && audioInfo->IsValid() &&
       (!platform || platform->SupportsMimeType(audioInfo->mMimeType, nullptr));
 
     if (audioActive) {
-      {
-        MutexAutoLock lock(mAudio.mMutex);
-        mInfo.mAudio = *audioInfo->GetAsAudioInfo();
-      }
+      mInfo.mAudio = *audioInfo->GetAsAudioInfo();
+      mAudio.mWorkingInfo = MakeUnique<AudioInfo>(mInfo.mAudio);
       for (const MetadataTag& tag : audioInfo->mTags) {
         tags->Put(tag.mKey, tag.mValue);
       }
       mAudio.mOriginalInfo = std::move(audioInfo);
       mTrackDemuxersMayBlock |= mAudio.mTrackDemuxer->GetSamplesMayBlock();
     } else {
       mAudio.mTrackDemuxer->BreakCycles();
       mAudio.mTrackDemuxer = nullptr;
@@ -1543,17 +1547,29 @@ MediaFormatReader::OnDemuxerInitFailed(c
 {
   mDemuxerInitRequest.Complete();
   mMetadataPromise.Reject(aError, __func__);
 }
 
 void
 MediaFormatReader::ReadUpdatedMetadata(MediaInfo* aInfo)
 {
-  *aInfo = mInfo;
+  // Called on the MDSM's TaskQueue.
+  {
+    MutexAutoLock lock(mVideo.mMutex);
+    if (HasVideo()) {
+      aInfo->mVideo = *mVideo.GetWorkingInfo()->GetAsVideoInfo();
+    }
+  }
+  {
+    MutexAutoLock lock(mAudio.mMutex);
+    if (HasAudio()) {
+      aInfo->mAudio = *mAudio.GetWorkingInfo()->GetAsAudioInfo();
+    }
+  }
 }
 
 MediaFormatReader::DecoderData&
 MediaFormatReader::GetDecoderData(TrackType aTrack)
 {
   MOZ_ASSERT(aTrack == TrackInfo::kAudioTrack ||
              aTrack == TrackInfo::kVideoTrack);
   if (aTrack == TrackInfo::kAudioTrack) {
@@ -2223,16 +2239,24 @@ MediaFormatReader::HandleDemuxedSamples(
     LOG("%s stream id has changed from:%d to:%d.",
         TrackTypeToStr(aTrack),
         decoder.mLastStreamSourceID,
         info->GetID());
 
     decoder.mNextStreamSourceID.reset();
     decoder.mLastStreamSourceID = info->GetID();
     decoder.mInfo = info;
+    {
+      MutexAutoLock lock(decoder.mMutex);
+      if (aTrack == TrackInfo::kAudioTrack) {
+        decoder.mWorkingInfo = MakeUnique<AudioInfo>(*info->GetAsAudioInfo());
+      } else if (aTrack == TrackInfo::kVideoTrack) {
+        decoder.mWorkingInfo = MakeUnique<VideoInfo>(*info->GetAsVideoInfo());
+      }
+    }
 
     decoder.mMeanRate.Reset();
 
     if (sample->mKeyframe) {
       if (samples.Length()) {
         decoder.mQueuedSamples = std::move(samples);
       }
     } else {
@@ -2670,26 +2694,31 @@ MediaFormatReader::ReturnOutput(MediaDat
     if (audioData->mChannels != mInfo.mAudio.mChannels ||
         audioData->mRate != mInfo.mAudio.mRate) {
       LOG("change of audio format (rate:%d->%d). "
           "This is an unsupported configuration",
           mInfo.mAudio.mRate,
           audioData->mRate);
       mInfo.mAudio.mRate = audioData->mRate;
       mInfo.mAudio.mChannels = audioData->mChannels;
+      MutexAutoLock lock(mAudio.mMutex);
+      mAudio.mWorkingInfo->GetAsAudioInfo()->mRate = audioData->mRate;
+      mAudio.mWorkingInfo->GetAsAudioInfo()->mChannels = audioData->mChannels;
     }
     mAudio.ResolvePromise(audioData, __func__);
   } else if (aTrack == TrackInfo::kVideoTrack) {
     VideoData* videoData = static_cast<VideoData*>(aData);
 
     if (videoData->mDisplay != mInfo.mVideo.mDisplay) {
       LOG("change of video display size (%dx%d->%dx%d)",
           mInfo.mVideo.mDisplay.width, mInfo.mVideo.mDisplay.height,
           videoData->mDisplay.width, videoData->mDisplay.height);
       mInfo.mVideo.mDisplay = videoData->mDisplay;
+      MutexAutoLock lock(mVideo.mMutex);
+      mVideo.mWorkingInfo->GetAsVideoInfo()->mDisplay = videoData->mDisplay;
     }
 
     TimeUnit nextKeyframe;
     if (!mVideo.HasInternalSeekPending() &&
         NS_SUCCEEDED(
           mVideo.mTrackDemuxer->GetNextRandomAccessPoint(&nextKeyframe))) {
       videoData->SetNextKeyFrameTime(nextKeyframe);
     }
@@ -3297,36 +3326,36 @@ void
 MediaFormatReader::GetMozDebugReaderData(nsACString& aString)
 {
   nsCString result;
   nsAutoCString audioDecoderName("unavailable");
   nsAutoCString videoDecoderName = audioDecoderName;
   nsAutoCString audioType("none");
   nsAutoCString videoType("none");
 
-  AudioInfo audioInfo = mAudio.GetCurrentInfo()
-                          ? *mAudio.GetCurrentInfo()->GetAsAudioInfo()
-                          : AudioInfo();
-  if (HasAudio())
+  AudioInfo audioInfo;
   {
     MutexAutoLock lock(mAudio.mMutex);
-    audioDecoderName = mAudio.mDecoder
-                       ? mAudio.mDecoder->GetDescriptionName()
-                       : mAudio.mDescription;
-    audioType = audioInfo.mMimeType;
+    if (HasAudio()) {
+      audioInfo = *mAudio.GetWorkingInfo()->GetAsAudioInfo();
+      audioDecoderName = mAudio.mDecoder ? mAudio.mDecoder->GetDescriptionName()
+                                         : mAudio.mDescription;
+      audioType = audioInfo.mMimeType;
+    }
   }
-  VideoInfo videoInfo = mVideo.GetCurrentInfo()
-                          ? *mVideo.GetCurrentInfo()->GetAsVideoInfo()
-                          : VideoInfo();
-  if (HasVideo()) {
-    MutexAutoLock mon(mVideo.mMutex);
-    videoDecoderName = mVideo.mDecoder
-                       ? mVideo.mDecoder->GetDescriptionName()
-                       : mVideo.mDescription;
-    videoType = videoInfo.mMimeType;
+
+  VideoInfo videoInfo;
+  {
+    MutexAutoLock lock(mVideo.mMutex);
+    if (HasVideo()) {
+      videoInfo = *mVideo.GetWorkingInfo()->GetAsVideoInfo();
+      videoDecoderName = mVideo.mDecoder ? mVideo.mDecoder->GetDescriptionName()
+                                         : mVideo.mDescription;
+      videoType = videoInfo.mMimeType;
+    }
   }
 
   result +=
     nsPrintfCString("Audio Decoder(%s, %u channels @ %0.1fkHz): %s\n",
                     audioType.get(),
                     audioInfo.mChannels,
                     audioInfo.mRate / 1000.0f,
                     audioDecoderName.get());
--- a/dom/media/MediaFormatReader.h
+++ b/dom/media/MediaFormatReader.h
@@ -400,17 +400,20 @@ private:
     MediaFormatReader* mOwner;
     // Disambiguate Audio vs Video.
     MediaData::Type mType;
     RefPtr<MediaTrackDemuxer> mTrackDemuxer;
     // TaskQueue on which decoder can choose to decode.
     // Only non-null up until the decoder is created.
     RefPtr<TaskQueue> mTaskQueue;
 
-    // Mutex protecting mDescription and mDecoder.
+    // Mutex protecting mDescription, mDecoder, mTrackDemuxer and mWorkingInfo
+    // as those can be read outside the TaskQueue.
+    // They are only written on the TaskQueue however, as such mMutex doesn't
+    // need to be held when those members are read on the TaskQueue.
     Mutex mMutex;
     // The platform decoder.
     RefPtr<MediaDataDecoder> mDecoder;
     nsCString mDescription;
     void ShutdownDecoder();
 
     // Only accessed from reader's task queue.
     bool mUpdateScheduled;
@@ -583,16 +586,23 @@ private:
     // with MSE or the WebMDemuxer.
     const TrackInfo* GetCurrentInfo() const
     {
       if (mInfo) {
         return *mInfo;
       }
       return mOriginalInfo.get();
     }
+    // Return the current TrackInfo updated as per the decoder output.
+    // Typically for audio, the number of channels and/or sampling rate can vary
+    // between what was found in the metadata and what the decoder returned.
+    const TrackInfo* GetWorkingInfo() const
+    {
+      return mWorkingInfo.get();
+    }
     bool IsEncrypted() const
     {
       return GetCurrentInfo()->mCrypto.mValid;
     }
 
     // Used by the MDSM for logging purposes.
     Atomic<size_t> mSizeOfQueue;
     // Used by the MDSM to determine if video decoding is hardware accelerated.
@@ -600,16 +610,19 @@ private:
     Atomic<bool> mIsHardwareAccelerated;
     // Sample format monitoring.
     uint32_t mLastStreamSourceID;
     Maybe<uint32_t> mNextStreamSourceID;
     media::TimeIntervals mTimeRanges;
     Maybe<media::TimeUnit> mLastTimeRangesEnd;
     // TrackInfo as first discovered during ReadMetadata.
     UniquePtr<TrackInfo> mOriginalInfo;
+    // Written exclusively on the TaskQueue, can be read on MDSM's TaskQueue.
+    // Must be read with parent's mutex held.
+    UniquePtr<TrackInfo> mWorkingInfo;
     RefPtr<TrackInfoSharedPtr> mInfo;
     Maybe<media::TimeUnit> mFirstDemuxedSampleTime;
     // Use NullDecoderModule or not.
     bool mIsNullDecode;
 
     class
     {
     public: