Bug 1536766 - Drop frames more accurately when ending and interrupting video track playback. r=padenot
authorAndreas Pehrson <apehrson@mozilla.com>
Wed, 24 Apr 2019 10:56:11 +0000
changeset 529448 93eb3684768757b78558d261f0db56aea2b797bd
parent 529447 8b2771254100edaff6d738f1335d4253f9ede203
child 529449 2787160c8537aa22a3c20ab22b3f1febe4d51715
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1536766
milestone68.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 1536766 - Drop frames more accurately when ending and interrupting video track playback. r=padenot Differential Revision: https://phabricator.services.mozilla.com/D27269
dom/media/VideoStreamTrack.cpp
--- a/dom/media/VideoStreamTrack.cpp
+++ b/dom/media/VideoStreamTrack.cpp
@@ -30,18 +30,16 @@ static bool SetImageToBlackPixel(PlanarY
   return aImage->CopyData(data);
 }
 
 class VideoOutput : public DirectMediaStreamTrackListener {
  protected:
   virtual ~VideoOutput() = default;
 
   void DropPastFrames() {
-    mMutex.AssertCurrentThreadOwns();
-
     TimeStamp now = TimeStamp::Now();
     size_t nrChunksInPast = 0;
     for (const auto& idChunkPair : mFrames) {
       const VideoChunk& chunk = idChunkPair.second();
       if (chunk.mTimeStamp > now) {
         break;
       }
       ++nrChunksInPast;
@@ -49,19 +47,22 @@ class VideoOutput : public DirectMediaSt
     if (nrChunksInPast > 1) {
       // We need to keep one frame that starts in the past, because it only ends
       // when the next frame starts (which also needs to be in the past for it
       // to drop).
       mFrames.RemoveElementsAt(0, nrChunksInPast - 1);
     }
   }
 
-  void SendFrames() {
+  void SendFramesEnsureLocked() {
     mMutex.AssertCurrentThreadOwns();
+    SendFrames();
+  }
 
+  void SendFrames() {
     DropPastFrames();
 
     if (mFrames.IsEmpty()) {
       return;
     }
 
     // Collect any new frames produced in this iteration.
     AutoTArray<ImageContainer::NonOwningImage, 16> images;
@@ -138,38 +139,59 @@ class VideoOutput : public DirectMediaSt
         // it seeks, as the previously buffered frames would stretch into the
         // future. If this happens, we clear the buffered frames and start over.
         mFrames.ClearAndRetainStorage();
       }
       mFrames.AppendElement(MakePair(mVideoFrameContainer->NewFrameID(), *i));
       mLastFrameTime = i->mTimeStamp;
     }
 
-    SendFrames();
+    SendFramesEnsureLocked();
   }
   void NotifyRemoved() override {
     // Doesn't need locking by mMutex, since the direct listener is removed from
     // the track before we get notified.
+    if (mFrames.Length() <= 1) {
+      // The compositor has already received the last frame.
+      mFrames.ClearAndRetainStorage();
+      mVideoFrameContainer->ClearFutureFrames();
+      return;
+    }
+
+    // The compositor has multiple frames. ClearFutureFrames() would only retain
+    // the first as that's normally the current one. We however stop doing
+    // SetCurrentFrames() once we've received the last frame in a track, so
+    // there might be old frames lingering. We'll find the current one and
+    // re-send that.
+    DropPastFrames();
+    mFrames.RemoveElementsAt(1, mFrames.Length() - 1);
+    SendFrames();
     mFrames.ClearAndRetainStorage();
-    mVideoFrameContainer->ClearFutureFrames();
   }
   void NotifyEnded() override {
     // Doesn't need locking by mMutex, since for the track to end, it must have
     // been ended by the source, meaning that the source won't append more data.
+    if (mFrames.IsEmpty()) {
+      return;
+    }
+
+    // Re-send only the last one to the compositor.
+    mFrames.RemoveElementsAt(0, mFrames.Length() - 1);
+    SendFrames();
     mFrames.ClearAndRetainStorage();
   }
   void NotifyEnabledStateChanged(bool aEnabled) override {
     MutexAutoLock lock(mMutex);
     mEnabled = aEnabled;
     // Since mEnabled will affect whether frames are real, or black, we assign
     // new FrameIDs whenever this changes.
     for (auto& idChunkPair : mFrames) {
       idChunkPair.first() = mVideoFrameContainer->NewFrameID();
     }
-    SendFrames();
+    SendFramesEnsureLocked();
   }
 
   Mutex mMutex;
   TimeStamp mLastFrameTime;
   // Once the frame is forced to black, we initialize mBlackImage for use in any
   // following forced-black frames.
   RefPtr<Image> mBlackImage;
   bool mEnabled = true;