Backed out changeset 8c8ff2c7bfa1 (bug 1485536) for crashes at VideoSink. CLOSED TREE
authorCsoregi Natalia <ncsoregi@mozilla.com>
Sun, 26 Aug 2018 00:15:16 +0300
changeset 481742 b0a28bbe750bdac8ac5b3eaa65787fcb74fb6f9c
parent 481741 8c8ff2c7bfa19deaca39766ad5ca47983039cc1e
child 481743 78ced2d7b8df4d9e014d9eb07019dd0ecccd409d
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
bugs1485536
milestone63.0a1
backs out8c8ff2c7bfa19deaca39766ad5ca47983039cc1e
Backed out changeset 8c8ff2c7bfa1 (bug 1485536) for crashes at VideoSink. CLOSED TREE
dom/media/FrameStatistics.h
dom/media/MediaFormatReader.cpp
dom/media/mediasink/VideoSink.cpp
dom/media/mediasink/VideoSink.h
--- a/dom/media/FrameStatistics.h
+++ b/dom/media/FrameStatistics.h
@@ -34,26 +34,21 @@ struct FrameStatisticsData
   uint64_t mInterKeyframeSum_us = 0;
   // Number of inter-keyframe segments summed so far.
   size_t mInterKeyframeCount = 0;
 
   // Maximum inter-keyframe segment duration, in microseconds.
   uint64_t mInterKeyFrameMax_us = 0;
 
   FrameStatisticsData() = default;
-  FrameStatisticsData(uint64_t aParsed,
-                      uint64_t aDecoded,
-                      uint64_t aDropped,
-                      uint64_t aPresented)
+  FrameStatisticsData(uint64_t aParsed, uint64_t aDecoded, uint64_t aDropped)
     : mParsedFrames(aParsed)
     , mDecodedFrames(aDecoded)
-    , mPresentedFrames(aPresented)
     , mDroppedFrames(aDropped)
-  {
-  }
+  {}
 
   void
   Accumulate(const FrameStatisticsData& aStats)
   {
     mParsedFrames += aStats.mParsedFrames;
     mDecodedFrames += aStats.mDecodedFrames;
     mPresentedFrames += aStats.mPresentedFrames;
     mDroppedFrames += aStats.mDroppedFrames;
@@ -115,17 +110,17 @@ public:
   uint64_t GetDroppedFrames() const
   {
     ReentrantMonitorAutoEnter mon(mReentrantMonitor);
     return mFrameStatisticsData.mDroppedFrames;
   }
 
   // Increments the parsed and decoded frame counters by the passed in counts.
   // Can be called on any thread.
-  void Accumulate(const FrameStatisticsData& aStats)
+  void NotifyDecodedFrames(const FrameStatisticsData& aStats)
   {
     ReentrantMonitorAutoEnter mon(mReentrantMonitor);
     mFrameStatisticsData.Accumulate(aStats);
   }
 
   // Increments the presented frame counters.
   // Can be called on any thread.
   void NotifyPresentedFrame()
@@ -142,17 +137,17 @@ public:
   public:
     explicit AutoNotifyDecoded(FrameStatistics* aFrameStats)
       : mFrameStats(aFrameStats)
     {
     }
     ~AutoNotifyDecoded()
     {
       if (mFrameStats) {
-        mFrameStats->Accumulate(mStats);
+        mFrameStats->NotifyDecodedFrames(mStats);
       }
     }
 
     FrameStatisticsData mStats;
 
   private:
     FrameStatistics* mFrameStats;
   };
--- a/dom/media/MediaFormatReader.cpp
+++ b/dom/media/MediaFormatReader.cpp
@@ -2803,17 +2803,17 @@ MediaFormatReader::DropDecodedSamples(Tr
     if (time >= decoder.mTimeThreshold.ref().Time()) {
       // We would have reached our internal seek target.
       decoder.mTimeThreshold.reset();
     }
   }
   decoder.mOutput.Clear();
   decoder.mSizeOfQueue -= lengthDecodedQueue;
   if (aTrack == TrackInfo::kVideoTrack && mFrameStats) {
-    mFrameStats->Accumulate({ 0, 0, lengthDecodedQueue, 0 });
+    mFrameStats->NotifyDecodedFrames({ 0, 0, lengthDecodedQueue });
   }
 }
 
 void
 MediaFormatReader::SkipVideoDemuxToNextKeyFrame(TimeUnit aTimeThreshold)
 {
   MOZ_ASSERT(OnTaskQueue());
   LOG("Skipping up to %" PRId64, aTimeThreshold.ToMicroseconds());
@@ -2835,25 +2835,25 @@ MediaFormatReader::VideoSkipReset(uint32
 {
   MOZ_ASSERT(OnTaskQueue());
 
   // Some frames may have been output by the decoder since we initiated the
   // videoskip process and we know they would be late.
   DropDecodedSamples(TrackInfo::kVideoTrack);
   // Report the pending frames as dropped.
   if (mFrameStats) {
-    mFrameStats->Accumulate({ 0, 0, SizeOfVideoQueueInFrames(), 0 });
+    mFrameStats->NotifyDecodedFrames({ 0, 0, SizeOfVideoQueueInFrames() });
   }
 
   // Cancel any pending demux request and pending demuxed samples.
   mVideo.mDemuxRequest.DisconnectIfExists();
   Reset(TrackType::kVideoTrack);
 
   if (mFrameStats) {
-    mFrameStats->Accumulate({ aSkipped, 0, aSkipped, 0 });
+    mFrameStats->NotifyDecodedFrames({ aSkipped, 0, aSkipped });
   }
 
   mVideo.mNumSamplesSkippedTotal += aSkipped;
 }
 
 void
 MediaFormatReader::OnVideoSkipCompleted(uint32_t aSkipped)
 {
--- a/dom/media/mediasink/VideoSink.cpp
+++ b/dom/media/mediasink/VideoSink.cpp
@@ -42,19 +42,17 @@ VideoSink::VideoSink(AbstractThread* aTh
                      FrameStatistics& aFrameStats,
                      uint32_t aVQueueSentToCompositerSize)
   : mOwnerThread(aThread)
   , mAudioSink(aAudioSink)
   , mVideoQueue(aVideoQueue)
   , mContainer(aContainer)
   , mProducerID(ImageContainer::AllocateProducerID())
   , mFrameStats(aFrameStats)
-  , mOldCompositorDroppedCount(mContainer ? mContainer->GetDroppedImageCount()
-                                          : 0)
-  , mPendingDroppedCount(0)
+  , mOldDroppedCount(0)
   , mHasVideo(false)
   , mUpdateScheduler(aThread)
   , mVideoQueueSendToCompositorSize(aVQueueSentToCompositerSize)
   , mMinVideoQueueSize(StaticPrefs::MediaRuinAvSyncEnabled() ? 1 : 0)
 #ifdef XP_WIN
   , mHiResTimersRequested(false)
 #endif
 
@@ -62,18 +60,16 @@ VideoSink::VideoSink(AbstractThread* aTh
   MOZ_ASSERT(mAudioSink, "AudioSink should exist.");
 }
 
 VideoSink::~VideoSink()
 {
 #ifdef XP_WIN
   MOZ_ASSERT(!mHiResTimersRequested);
 #endif
-  MOZ_DIAGNOSTIC_ASSERT(mPendingDroppedCount == 0,
-                        "All dropped frames should have been reported");
 }
 
 const MediaSink::PlaybackParams&
 VideoSink::GetPlaybackParams() const
 {
   AssertOwnerThread();
 
   return mAudioSink->GetPlaybackParams();
@@ -464,69 +460,52 @@ VideoSink::RenderVideoFrames(int32_t aMa
 
     VSINK_LOG_V("playing video frame %" PRId64 " (id=%x) (vq-queued=%zu)",
                 frame->mTime.ToMicroseconds(), frame->mFrameID,
                 VideoQueue().GetSize());
   }
 
   if (images.Length() > 0) {
     mContainer->SetCurrentFrames(frames[0]->mDisplay, images);
+    uint32_t droppedCount = mContainer->GetDroppedImageCount();
+    uint32_t dropped = droppedCount - mOldDroppedCount;
+    if (dropped > 0) {
+      mFrameStats.NotifyDecodedFrames({0, 0, dropped});
+      mOldDroppedCount = droppedCount;
+      VSINK_LOG_V("%u video frame discarded by compositor", dropped);
+    }
   }
 }
 
 void
 VideoSink::UpdateRenderedVideoFrames()
 {
   AssertOwnerThread();
   MOZ_ASSERT(mAudioSink->IsPlaying(), "should be called while playing.");
 
   // Get the current playback position.
   TimeStamp nowTime;
   const auto clockTime = mAudioSink->GetPosition(&nowTime);
   MOZ_ASSERT(!clockTime.IsNegative(), "Should have positive clock time.");
 
-  uint32_t sentToCompositorCount = 0;
-  uint32_t droppedCount = 0;
-
   // Skip frames up to the playback position.
   TimeUnit lastFrameEndTime;
   while (VideoQueue().GetSize() > mMinVideoQueueSize &&
          clockTime >= VideoQueue().PeekFront()->GetEndTime()) {
     RefPtr<VideoData> frame = VideoQueue().PopFront();
     lastFrameEndTime = frame->GetEndTime();
     if (frame->IsSentToCompositor()) {
-      sentToCompositorCount++;
+      mFrameStats.NotifyPresentedFrame();
     } else {
-      droppedCount++;
+      mFrameStats.NotifyDecodedFrames({ 0, 0, 1 });
       VSINK_LOG_V("discarding video frame mTime=%" PRId64 " clock_time=%" PRId64,
                   frame->mTime.ToMicroseconds(), clockTime.ToMicroseconds());
     }
   }
 
-  if (droppedCount || sentToCompositorCount) {
-    uint32_t totalCompositorDroppedCount = mContainer->GetDroppedImageCount();
-    uint32_t compositorDroppedCount =
-      totalCompositorDroppedCount - mOldCompositorDroppedCount;
-    if (compositorDroppedCount > 0) {
-      mOldCompositorDroppedCount = totalCompositorDroppedCount;
-      VSINK_LOG_V("%u video frame previously discarded by compositor",
-                  compositorDroppedCount);
-    }
-    mPendingDroppedCount += compositorDroppedCount;
-    uint32_t droppedReported = mPendingDroppedCount > sentToCompositorCount
-                                 ? sentToCompositorCount
-                                 : mPendingDroppedCount;
-    mPendingDroppedCount -= droppedReported;
-
-    mFrameStats.Accumulate({ 0,
-                             0,
-                             droppedCount + droppedReported,
-                             sentToCompositorCount - droppedReported });
-  }
-
   // The presentation end time of the last video frame displayed is either
   // the end time of the current frame, or if we dropped all frames in the
   // queue, the end time of the last frame we removed from the queue.
   RefPtr<VideoData> currentFrame = VideoQueue().PeekFront();
   mVideoFrameEndTime = std::max(mVideoFrameEndTime,
     currentFrame ? currentFrame->GetEndTime() : lastFrameEndTime);
 
   RenderVideoFrames(
@@ -564,24 +543,17 @@ VideoSink::MaybeResolveEndPromise()
   AssertOwnerThread();
   // All frames are rendered, Let's resolve the promise.
   if (VideoQueue().IsFinished() &&
       VideoQueue().GetSize() <= 1 &&
       !mVideoSinkEndRequest.Exists()) {
     if (VideoQueue().GetSize() == 1) {
       // Remove the last frame since we have sent it to compositor.
       RefPtr<VideoData> frame = VideoQueue().PopFront();
-      if (mPendingDroppedCount > 0) {
-        // We won't get a chance to report the frames dropped later so report
-        // them all now.
-        mFrameStats.Accumulate({ 0, 0, mPendingDroppedCount, 0 });
-        mPendingDroppedCount = 0;
-      } else {
-        mFrameStats.NotifyPresentedFrame();
-      }
+      mFrameStats.NotifyPresentedFrame();
     }
     mEndPromiseHolder.ResolveIfExists(true, __func__);
   }
 }
 
 nsCString
 VideoSink::GetDebugInfo()
 {
--- a/dom/media/mediasink/VideoSink.h
+++ b/dom/media/mediasink/VideoSink.h
@@ -127,18 +127,17 @@ private:
 
   RefPtr<GenericPromise> mEndPromise;
   MozPromiseHolder<GenericPromise> mEndPromiseHolder;
   MozPromiseRequestHolder<GenericPromise> mVideoSinkEndRequest;
 
   // The presentation end time of the last video frame which has been displayed.
   TimeUnit mVideoFrameEndTime;
 
-  uint32_t mOldCompositorDroppedCount;
-  uint32_t mPendingDroppedCount;
+  uint32_t mOldDroppedCount;
 
   // Event listeners for VideoQueue
   MediaEventListener mPushListener;
   MediaEventListener mFinishListener;
 
   // True if this sink is going to handle video track.
   bool mHasVideo;