Bug 1536766 - Drop frames more accurately when ending and interrupting video track playback. r=padenot
☠☠ backed out by 41f1dcbe9caa ☠ ☠
authorAndreas Pehrson <apehrson@mozilla.com>
Thu, 18 Apr 2019 15:24:27 +0000
changeset 470106 63fc8588506005ac9eb0f1ec45ed34e430c6cf4a
parent 470105 addbb04415cb8b82b5cdd5ff8a3905cafa6221fb
child 470107 72d37a08f281c24833d1c49e947fb6db4df2fb87
push id112843
push useraiakab@mozilla.com
push dateFri, 19 Apr 2019 09:50:22 +0000
treeherdermozilla-inbound@c06f27cbfe40 [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;