Bug 1307546 - Ensure we don't set timers with negative intervals in to update A/V sync. r=jya,a=ritu
authorChris Pearce <cpearce@mozilla.com>
Fri, 04 Nov 2016 16:42:24 +1300
changeset 350868 8e32fc90a6aadb62ab464ef56a53da8a5d66091a
parent 350867 614a80e577f3757a61a00235f76d961d1c86a587
child 350869 dc617d65c9f0cdbbe4351cc1e5c288b05f25f8f7
push id1249
push usercpearce@mozilla.com
push dateFri, 04 Nov 2016 22:42:00 +0000
treeherdermozilla-release@8e32fc90a6aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya, ritu
bugs1307546
milestone50.0
Bug 1307546 - Ensure we don't set timers with negative intervals in to update A/V sync. r=jya,a=ritu Our logic to do A/V sync sets a timer to drop expired frames based on the start time of the next frame in the queue. If the frames in the queue are badly muxed and don't have monotonically increasing start times, we can end up setting a timer with a negative interval. This causes us to reevaluate the frames in the VideoSink's queue immediately, set the same timer again, and so we end up hot-looping. This is a simple low-risk fix that detects when we're about to set a negative interval timer, and instead sets the timer 1/30th of a second in the future. This fix is deliberately low risk, such that it's suitable for uplift. I have an idea how to do this better, but the lower risk this is most suitable for uplift. MozReview-Commit-ID: CDOqJJodx4l
dom/media/mediasink/VideoSink.cpp
--- a/dom/media/mediasink/VideoSink.cpp
+++ b/dom/media/mediasink/VideoSink.cpp
@@ -17,16 +17,22 @@ extern LazyLogModule gMediaDecoderLog;
 #define VSINK_LOG_V(msg, ...) \
   MOZ_LOG(gMediaDecoderLog, LogLevel::Verbose, \
   ("VideoSink=%p " msg, this, ##__VA_ARGS__))
 
 using namespace mozilla::layers;
 
 namespace media {
 
+// For badly muxed files, if we try to set a timeout to discard expired
+// frames, but the start time of the next frame is less than the clock time,
+// we instead just set a timeout FAILOVER_UPDATE_INTERVAL_US in the future,
+// and run UpdateRenderedVideoFrames() then.
+static const int64_t FAILOVER_UPDATE_INTERVAL_US = 1000000 / 30;
+
 VideoSink::VideoSink(AbstractThread* aThread,
                      MediaSink* aAudioSink,
                      MediaQueue<MediaData>& aVideoQueue,
                      VideoFrameContainer* aContainer,
                      FrameStatistics& aFrameStats,
                      uint32_t aVQueueSentToCompositerSize)
   : mOwnerThread(aThread)
   , mAudioSink(aAudioSink)
@@ -430,18 +436,20 @@ VideoSink::UpdateRenderedVideoFrames()
   // we will run render loops again upon incoming frames.
   nsTArray<RefPtr<MediaData>> frames;
   VideoQueue().GetFirstElements(2, &frames);
   if (frames.Length() < 2) {
     return;
   }
 
   int64_t nextFrameTime = frames[1]->mTime;
+  int64_t delta = (nextFrameTime > clockTime) ? (nextFrameTime - clockTime)
+                                              : FAILOVER_UPDATE_INTERVAL_US;
   TimeStamp target = nowTime + TimeDuration::FromMicroseconds(
-    (nextFrameTime - clockTime) / mAudioSink->GetPlaybackParams().mPlaybackRate);
+     delta / mAudioSink->GetPlaybackParams().mPlaybackRate);
 
   RefPtr<VideoSink> self = this;
   mUpdateScheduler.Ensure(target, [self] () {
     self->UpdateRenderedVideoFramesByTimer();
   }, [self] () {
     self->UpdateRenderedVideoFramesByTimer();
   });
 }