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

--HG--
extra : moz-landing-system : lando

dom/media/MediaFormatReader.cpp

@@ -1150,7 +1150,10 @@ MediaFormatReader::Shutdown()
   if (HasAudio()) {
     mAudio.ResetDemuxer();
     mAudio.mTrackDemuxer->BreakCycles();
-    mAudio.mTrackDemuxer = nullptr;
+    {
+      MutexAutoLock lock(mAudio.mMutex);
+      mAudio.mTrackDemuxer = nullptr;
+    }
     mAudio.ResetState();
     ShutdownDecoder(TrackInfo::kAudioTrack);
   }
@@ -1158,7 +1161,10 @@ MediaFormatReader::Shutdown()
   if (HasVideo()) {
     mVideo.ResetDemuxer();
     mVideo.mTrackDemuxer->BreakCycles();
-    mVideo.mTrackDemuxer = nullptr;
+    {
+      MutexAutoLock lock(mVideo.mMutex);
+      mVideo.mTrackDemuxer = nullptr;
+    }
     mVideo.ResetState();
     ShutdownDecoder(TrackInfo::kVideoTrack);
   }
@@ -1387,6 +1393,7 @@ MediaFormatReader::OnDemuxerInitDone(const MediaResult& aResult)
 
   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__);
@@ -1402,10 +1409,8 @@ MediaFormatReader::OnDemuxerInitDone(const MediaResult& aResult)
         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);
       }
@@ -1419,6 +1424,7 @@ MediaFormatReader::OnDemuxerInitDone(const MediaResult& aResult)
 
   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__);
@@ -1432,10 +1438,8 @@ MediaFormatReader::OnDemuxerInitDone(const MediaResult& aResult)
       (!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);
       }
@@ -1548,7 +1552,19 @@ MediaFormatReader::OnDemuxerInitFailed(const MediaResult& aError)
 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&
@@ -2228,6 +2244,14 @@ MediaFormatReader::HandleDemuxedSamples(
     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();
 
@@ -2675,6 +2699,9 @@ MediaFormatReader::ReturnOutput(MediaData* aData, TrackType aTrack)
           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) {
@@ -2685,6 +2712,8 @@ MediaFormatReader::ReturnOutput(MediaData* aData, TrackType aTrack)
           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;
@@ -3302,26 +3331,26 @@ MediaFormatReader::GetMozDebugReaderData(nsACString& aString)
   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 +=

dom/media/MediaFormatReader.h

@@ -405,7 +405,10 @@ private:
     // 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;
@@ -588,6 +591,13 @@ private:
       }
       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;
@@ -605,6 +615,9 @@ private:
     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.