Bug 1423253 - Pace future video frames in MediaPipeline. r=dminor,padenot
authorAndreas Pehrson <apehrson@mozilla.com>
Fri, 22 Mar 2019 11:45:27 +0000
changeset 465653 b5e813887d3ffc1bf1cd9f6869351c1c6b96c1f8
parent 465652 5fdd6c8a0d2f1a5ed849ce167cd31a08e58de041
child 465654 1437c364fe9948d98c09578f6ccba79ae97af212
push id35744
push userapavel@mozilla.com
push dateFri, 22 Mar 2019 16:44:08 +0000
treeherdermozilla-central@e66a2b59914d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdminor, padenot
bugs1423253
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 1423253 - Pace future video frames in MediaPipeline. r=dminor,padenot WebRTC.org doesn't handle receiving multiple future frames. This will buffer future frames from a direct listener in MediaPipeline and pass them on when the frame's wall clock timestamp has been reached. Differential Revision: https://phabricator.services.mozilla.com/D22921
dom/media/VideoFrameConverter.h
dom/media/VideoSegment.cpp
dom/media/VideoSegment.h
dom/media/gtest/TestVideoFrameConverter.cpp
dom/media/gtest/moz.build
--- a/dom/media/VideoFrameConverter.h
+++ b/dom/media/VideoFrameConverter.h
@@ -3,155 +3,103 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef VideoFrameConverter_h
 #define VideoFrameConverter_h
 
 #include "ImageContainer.h"
 #include "ImageToI420.h"
+#include "MediaTimer.h"
 #include "VideoSegment.h"
 #include "VideoUtils.h"
 #include "nsISupportsImpl.h"
 #include "nsThreadUtils.h"
 #include "mozilla/TaskQueue.h"
 #include "mozilla/dom/ImageBitmapBinding.h"
 #include "mozilla/dom/ImageUtils.h"
+#include "webrtc/api/video/video_frame.h"
 #include "webrtc/common_video/include/i420_buffer_pool.h"
 #include "webrtc/common_video/include/video_frame_buffer.h"
 #include "webrtc/rtc_base/keep_ref_until_done.h"
+#include "webrtc/system_wrappers/include/clock.h"
 
 // The number of frame buffers VideoFrameConverter may create before returning
 // errors.
 // Sometimes these are released synchronously but they can be forwarded all the
 // way to the encoder for asynchronous encoding. With a pool size of 5,
 // we allow 1 buffer for the current conversion, and 4 buffers to be queued at
 // the encoder.
 #define CONVERTER_BUFFER_POOL_SIZE 5
 
 namespace mozilla {
 
-mozilla::LazyLogModule gVideoFrameConverterLog("VideoFrameConverter");
+static mozilla::LazyLogModule gVideoFrameConverterLog("VideoFrameConverter");
 
 class VideoConverterListener {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(VideoConverterListener)
 
   virtual void OnVideoFrameConverted(const webrtc::VideoFrame& aVideoFrame) = 0;
 
  protected:
   virtual ~VideoConverterListener() {}
 };
 
 // An async video frame format converter.
 //
-// Input is typically a MediaStream(Track)Listener driven by MediaStreamGraph.
-//
-// We keep track of the size of the TaskQueue so we can drop frames if
-// conversion is taking too long.
+// Input is typically a MediaStreamTrackListener driven by MediaStreamGraph.
 //
 // Output is passed through to all added VideoConverterListeners on a TaskQueue
 // thread whenever a frame is converted.
 class VideoFrameConverter {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(VideoFrameConverter)
 
   VideoFrameConverter()
-      : mLength(0),
-        mTaskQueue(
+      : mTaskQueue(
             new TaskQueue(GetMediaThreadPool(MediaThreadType::WEBRTC_DECODER),
                           "VideoFrameConverter")),
-        mBufferPool(false, CONVERTER_BUFFER_POOL_SIZE),
-        mLastImage(
-            -1)  // -1 is not a guaranteed invalid serial. See bug 1262134.
-#ifdef DEBUG
-        ,
-        mThrottleCount(0),
-        mThrottleRecord(0)
-#endif
-        ,
-        mLastFrame(nullptr, 0, 0, webrtc::kVideoRotation_0) {
+        mPacingTimer(new MediaTimer()),
+        mLastImage(-1),  // -1 is not a guaranteed invalid serial (Bug 1262134).
+        mBufferPool(false, CONVERTER_BUFFER_POOL_SIZE) {
     MOZ_COUNT_CTOR(VideoFrameConverter);
   }
 
   void QueueVideoChunk(const VideoChunk& aChunk, bool aForceBlack) {
     gfx::IntSize size = aChunk.mFrame.GetIntrinsicSize();
     if (size.width == 0 || size.width == 0) {
       return;
     }
 
-    if (aChunk.IsNull()) {
-      aForceBlack = true;
-    } else {
-      aForceBlack = aChunk.mFrame.GetForceBlack();
-    }
-
-    int32_t serial;
-    if (aForceBlack) {
-      // Reset the last-img check.
-      // -1 is not a guaranteed invalid serial. See bug 1262134.
-      serial = -1;
-    } else {
-      serial = aChunk.mFrame.GetImage()->GetSerial();
-    }
-
     TimeStamp t = aChunk.mTimeStamp;
     MOZ_ASSERT(!t.IsNull());
-    if (serial == mLastImage && !mLastFrameSent.IsNull()) {
-      // With a non-direct listener we get passed duplicate frames every ~10ms
-      // even with no frame change.
-      return;
+
+    if (!mLastFrameSent.IsNull() && t < mLastFrameSent) {
+      // With a direct listener we can have buffered up future frames in
+      // mPacingTimer. The source could start sending us frames that start
+      // before some previously buffered frames (e.g., a MediaDecoder does that
+      // when it seeks). We don't want to just append these to the pacing timer,
+      // as frames at different times on the MediaDecoder timeline would get
+      // passed to the encoder in a mixed order. We don't have an explicit way
+      // of signaling this, so we must detect here if time goes backwards.
+      MOZ_LOG(gVideoFrameConverterLog, LogLevel::Debug,
+              ("Clearing pacer because of source reset (%.3f)",
+               (mLastFrameSent - t).ToSeconds()));
+      mPacingTimer->Cancel();
     }
     mLastFrameSent = t;
-    mLastImage = serial;
 
-    // A throttling limit of 1 allows us to convert 2 frames concurrently.
-    // It's short enough to not build up too significant a delay, while
-    // giving us a margin to not cause some machines to drop every other frame.
-    const int32_t queueThrottlingLimit = 1;
-    if (mLength > queueThrottlingLimit) {
-      MOZ_LOG(gVideoFrameConverterLog, LogLevel::Debug,
-              ("VideoFrameConverter %p queue is full. Throttling by "
-               "throwing away a frame.",
-               this));
-#ifdef DEBUG
-      ++mThrottleCount;
-      mThrottleRecord = std::max(mThrottleCount, mThrottleRecord);
-#endif
-      return;
-    }
-
-#ifdef DEBUG
-    if (mThrottleCount > 0) {
-      if (mThrottleCount > 5) {
-        // Log at a higher level when we have large drops.
-        MOZ_LOG(gVideoFrameConverterLog, LogLevel::Info,
-                ("VideoFrameConverter %p stopped throttling after throwing "
-                 "away %d frames. Longest throttle so far was %d frames.",
-                 this, mThrottleCount, mThrottleRecord));
-      } else {
-        MOZ_LOG(gVideoFrameConverterLog, LogLevel::Debug,
-                ("VideoFrameConverter %p stopped throttling after throwing "
-                 "away %d frames. Longest throttle so far was %d frames.",
-                 this, mThrottleCount, mThrottleRecord));
-      }
-      mThrottleCount = 0;
-    }
-#endif
-
-    ++mLength;  // Atomic
-
-    nsCOMPtr<nsIRunnable> runnable =
-        NewRunnableMethod<StoreRefPtrPassByPtr<layers::Image>, gfx::IntSize,
-                          bool>("VideoFrameConverter::ProcessVideoFrame", this,
-                                &VideoFrameConverter::ProcessVideoFrame,
-                                aChunk.mFrame.GetImage(), size, aForceBlack);
-    nsresult rv = mTaskQueue->Dispatch(runnable.forget());
-    MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
-    Unused << rv;
+    mPacingTimer->WaitUntil(t, __func__)
+        ->Then(mTaskQueue, __func__,
+               [self = RefPtr<VideoFrameConverter>(this), this,
+                image = RefPtr<layers::Image>(aChunk.mFrame.GetImage()),
+                t = std::move(t), size = std::move(size), aForceBlack] {
+                 ProcessVideoFrame(image, t, size, aForceBlack);
+               });
   }
 
   void AddListener(const RefPtr<VideoConverterListener>& aListener) {
     nsresult rv = mTaskQueue->Dispatch(NS_NewRunnableFunction(
         "VideoFrameConverter::AddListener",
         [self = RefPtr<VideoFrameConverter>(this), this, aListener] {
           MOZ_ASSERT(!mListeners.Contains(aListener));
           mListeners.AppendElement(aListener);
@@ -166,16 +114,18 @@ class VideoFrameConverter {
         [self = RefPtr<VideoFrameConverter>(this), this, aListener] {
           mListeners.RemoveElement(aListener);
         }));
     MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
     Unused << rv;
   }
 
   void Shutdown() {
+    mPacingTimer->Cancel();
+
     nsresult rv = mTaskQueue->Dispatch(NS_NewRunnableFunction(
         "VideoFrameConverter::Shutdown",
         [self = RefPtr<VideoFrameConverter>(this), this] {
           if (mSameFrameTimer) {
             mSameFrameTimer->Cancel();
           }
           mListeners.Clear();
         }));
@@ -186,50 +136,65 @@ class VideoFrameConverter {
  protected:
   virtual ~VideoFrameConverter() { MOZ_COUNT_DTOR(VideoFrameConverter); }
 
   static void SameFrameTick(nsITimer* aTimer, void* aClosure) {
     MOZ_ASSERT(aClosure);
     VideoFrameConverter* self = static_cast<VideoFrameConverter*>(aClosure);
     MOZ_ASSERT(self->mTaskQueue->IsCurrentThreadIn());
 
-    if (!self->mLastFrame.video_frame_buffer()) {
+    if (!self->mLastFrame) {
       return;
     }
 
     for (RefPtr<VideoConverterListener>& listener : self->mListeners) {
-      listener->OnVideoFrameConverted(self->mLastFrame);
+      listener->OnVideoFrameConverted(*self->mLastFrame);
     }
   }
 
   void VideoFrameConverted(const webrtc::VideoFrame& aVideoFrame) {
     MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
 
     if (mSameFrameTimer) {
       mSameFrameTimer->Cancel();
     }
 
     const int sameFrameIntervalInMs = 1000;
     NS_NewTimerWithFuncCallback(
         getter_AddRefs(mSameFrameTimer), &SameFrameTick, this,
         sameFrameIntervalInMs, nsITimer::TYPE_REPEATING_SLACK,
         "VideoFrameConverter::mSameFrameTimer", mTaskQueue);
 
-    mLastFrame = aVideoFrame;
+    mLastFrame = MakeUnique<webrtc::VideoFrame>(aVideoFrame);
 
     for (RefPtr<VideoConverterListener>& listener : mListeners) {
       listener->OnVideoFrameConverted(aVideoFrame);
     }
   }
 
-  void ProcessVideoFrame(layers::Image* aImage, gfx::IntSize aSize,
-                         bool aForceBlack) {
-    --mLength;  // Atomic
-    MOZ_ASSERT(mLength >= 0);
+  void ProcessVideoFrame(layers::Image* aImage, TimeStamp aTime,
+                         gfx::IntSize aSize, bool aForceBlack) {
+    MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
 
+    int32_t serial;
+    if (aForceBlack) {
+      // Set the last-img check to indicate black.
+      // -1 is not a guaranteed invalid serial. See bug 1262134.
+      serial = -1;
+    } else {
+      serial = aImage->GetSerial();
+    }
+
+    if (serial == mLastImage) {
+      // With a non-direct listener we get passed duplicate frames every ~10ms
+      // even with no frame change.
+      return;
+    }
+
+    mLastImage = serial;
     // See Bug 1529581 - Ideally we'd use the mTimestamp from the chunk
     // passed into QueueVideoChunk rather than the webrtc.org clock here.
     int64_t now = webrtc::Clock::GetRealTimeClock()->TimeInMilliseconds();
 
     if (aForceBlack) {
       // Send a black image.
       rtc::scoped_refptr<webrtc::I420Buffer> buffer =
           mBufferPool.CreateBuffer(aSize.width, aSize.height);
@@ -299,29 +264,28 @@ class VideoFrameConverter {
       return;
     }
 
     webrtc::VideoFrame frame(buffer, 0, //not setting rtp timestamp
                              now, webrtc::kVideoRotation_0);
     VideoFrameConverted(frame);
   }
 
-  Atomic<int32_t, Relaxed> mLength;
   const RefPtr<TaskQueue> mTaskQueue;
-  webrtc::I420BufferPool mBufferPool;
+
+  // Used to pace future frames close to their rendering-time. Thread-safe.
+  const RefPtr<MediaTimer> mPacingTimer;
 
   // Written and read from the queueing thread (normally MSG).
-  int32_t mLastImage;        // serial number of last Image
-  TimeStamp mLastFrameSent;  // The time we sent the last frame.
-#ifdef DEBUG
-  uint32_t mThrottleCount;
-  uint32_t mThrottleRecord;
-#endif
+  // Last time we queued a frame in the pacer
+  TimeStamp mLastFrameSent;
 
   // Accessed only from mTaskQueue.
+  int32_t mLastImage;  // Serial number of last processed Image
+  webrtc::I420BufferPool mBufferPool;
   nsCOMPtr<nsITimer> mSameFrameTimer;
-  webrtc::VideoFrame mLastFrame;
+  UniquePtr<webrtc::VideoFrame> mLastFrame;
   nsTArray<RefPtr<VideoConverterListener>> mListeners;
 };
 
 }  // namespace mozilla
 
 #endif // VideoFrameConverter_h
--- a/dom/media/VideoSegment.cpp
+++ b/dom/media/VideoSegment.cpp
@@ -9,17 +9,17 @@
 #include "ImageContainer.h"
 #include "Layers.h"
 #include "mozilla/UniquePtr.h"
 
 namespace mozilla {
 
 using namespace layers;
 
-VideoFrame::VideoFrame(already_AddRefed<Image>& aImage,
+VideoFrame::VideoFrame(already_AddRefed<Image> aImage,
                        const gfx::IntSize& aIntrinsicSize)
     : mImage(aImage),
       mIntrinsicSize(aIntrinsicSize),
       mForceBlack(false),
       mPrincipalHandle(PRINCIPAL_HANDLE_NONE) {}
 
 VideoFrame::VideoFrame()
     : mIntrinsicSize(0, 0),
@@ -86,17 +86,17 @@ already_AddRefed<Image> VideoFrame::Crea
 }
 
 void VideoSegment::AppendFrame(already_AddRefed<Image>&& aImage,
                                const IntSize& aIntrinsicSize,
                                const PrincipalHandle& aPrincipalHandle,
                                bool aForceBlack, TimeStamp aTimeStamp) {
   VideoChunk* chunk = AppendChunk(0);
   chunk->mTimeStamp = aTimeStamp;
-  VideoFrame frame(aImage, aIntrinsicSize);
+  VideoFrame frame(std::move(aImage), aIntrinsicSize);
   MOZ_ASSERT_IF(!IsNull(), !aTimeStamp.IsNull());
   frame.SetForceBlack(aForceBlack);
   frame.SetPrincipalHandle(aPrincipalHandle);
   chunk->mFrame.TakeFrom(&frame);
 }
 
 VideoSegment::VideoSegment()
     : MediaSegmentBase<VideoSegment, VideoChunk>(VIDEO) {}
--- a/dom/media/VideoSegment.h
+++ b/dom/media/VideoSegment.h
@@ -16,17 +16,17 @@ namespace mozilla {
 namespace layers {
 class Image;
 }  // namespace layers
 
 class VideoFrame {
  public:
   typedef mozilla::layers::Image Image;
 
-  VideoFrame(already_AddRefed<Image>& aImage,
+  VideoFrame(already_AddRefed<Image> aImage,
              const gfx::IntSize& aIntrinsicSize);
   VideoFrame();
   ~VideoFrame();
 
   bool operator==(const VideoFrame& aFrame) const {
     return mIntrinsicSize == aFrame.mIntrinsicSize &&
            mForceBlack == aFrame.mForceBlack &&
            ((mForceBlack && aFrame.mForceBlack) || mImage == aFrame.mImage) &&
new file mode 100644
--- /dev/null
+++ b/dom/media/gtest/TestVideoFrameConverter.cpp
@@ -0,0 +1,138 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "gtest/gtest.h"
+#include "VideoFrameConverter.h"
+#include "YUVBufferGenerator.h"
+
+using namespace mozilla;
+
+class VideoFrameConverterTest;
+
+class FrameListener : public VideoConverterListener {
+ public:
+  explicit FrameListener(VideoFrameConverterTest* aTest);
+  void OnVideoFrameConverted(const webrtc::VideoFrame& aVideoFrame) override;
+
+ private:
+  VideoFrameConverterTest* mTest;
+};
+
+class VideoFrameConverterTest : public ::testing::Test {
+ protected:
+  using FrameType = Pair<webrtc::VideoFrame, TimeStamp>;
+  Monitor mMonitor;
+  RefPtr<VideoFrameConverter> mConverter;
+  RefPtr<FrameListener> mListener;
+  std::vector<FrameType> mConvertedFrames;
+
+  VideoFrameConverterTest()
+      : mMonitor("PacingFixture::mMonitor"),
+        mConverter(MakeAndAddRef<VideoFrameConverter>()),
+        mListener(MakeAndAddRef<FrameListener>(this)) {
+    mConverter->AddListener(mListener);
+  }
+
+  void TearDown() override { mConverter->Shutdown(); }
+
+  size_t NumConvertedFrames() {
+    MonitorAutoLock lock(mMonitor);
+    return mConvertedFrames.size();
+  }
+
+  std::vector<FrameType> WaitForNConverted(size_t aN) {
+    MonitorAutoLock l(mMonitor);
+    while (mConvertedFrames.size() < aN) {
+      l.Wait();
+    }
+    std::vector<FrameType> v(mConvertedFrames.begin(),
+                             mConvertedFrames.begin() + aN);
+    return v;
+  }
+
+ public:
+  void OnVideoFrameConverted(const webrtc::VideoFrame& aVideoFrame) {
+    MonitorAutoLock lock(mMonitor);
+    mConvertedFrames.push_back(MakePair(aVideoFrame, TimeStamp::Now()));
+    mMonitor.Notify();
+  }
+};
+
+FrameListener::FrameListener(VideoFrameConverterTest* aTest) : mTest(aTest) {}
+void FrameListener::OnVideoFrameConverted(
+    const webrtc::VideoFrame& aVideoFrame) {
+  mTest->OnVideoFrameConverted(aVideoFrame);
+}
+
+VideoChunk GenerateChunk(int32_t aWidth, int32_t aHeight, TimeStamp aTime) {
+  YUVBufferGenerator generator;
+  generator.Init(gfx::IntSize(aWidth, aHeight));
+  VideoFrame f(generator.GenerateI420Image(), gfx::IntSize(aWidth, aHeight));
+  VideoChunk c;
+  c.mFrame.TakeFrom(&f);
+  c.mTimeStamp = aTime;
+  c.mDuration = 0;
+  return c;
+}
+
+TEST_F(VideoFrameConverterTest, BasicConversion) {
+  TimeStamp now = TimeStamp::Now();
+  VideoChunk chunk = GenerateChunk(640, 480, now);
+  mConverter->QueueVideoChunk(chunk, false);
+  auto frames = WaitForNConverted(1);
+  ASSERT_EQ(frames.size(), 1U);
+  EXPECT_EQ(frames[0].first().width(), 640);
+  EXPECT_EQ(frames[0].first().height(), 480);
+  EXPECT_GT(frames[0].second(), now);
+}
+
+TEST_F(VideoFrameConverterTest, BasicPacing) {
+  TimeStamp now = TimeStamp::Now();
+  TimeStamp future = now + TimeDuration::FromMilliseconds(100);
+  VideoChunk chunk = GenerateChunk(640, 480, future);
+  mConverter->QueueVideoChunk(chunk, false);
+  auto frames = WaitForNConverted(1);
+  EXPECT_GT(TimeStamp::Now(), future);
+  ASSERT_EQ(frames.size(), 1U);
+  EXPECT_EQ(frames[0].first().width(), 640);
+  EXPECT_EQ(frames[0].first().height(), 480);
+  EXPECT_GT(frames[0].second(), future);
+}
+
+TEST_F(VideoFrameConverterTest, MultiPacing) {
+  TimeStamp now = TimeStamp::Now();
+  TimeStamp future1 = now + TimeDuration::FromMilliseconds(100);
+  TimeStamp future2 = now + TimeDuration::FromMilliseconds(200);
+  VideoChunk chunk = GenerateChunk(640, 480, future1);
+  mConverter->QueueVideoChunk(chunk, false);
+  chunk = GenerateChunk(640, 480, future2);
+  mConverter->QueueVideoChunk(chunk, false);
+  auto frames = WaitForNConverted(2);
+  EXPECT_GT(TimeStamp::Now(), future2);
+  ASSERT_EQ(frames.size(), 2U);
+  EXPECT_EQ(frames[0].first().width(), 640);
+  EXPECT_EQ(frames[0].first().height(), 480);
+  EXPECT_GT(frames[0].second(), future1);
+  EXPECT_EQ(frames[1].first().width(), 640);
+  EXPECT_EQ(frames[1].first().height(), 480);
+  EXPECT_GT(frames[1].second(), future2);
+  EXPECT_GT(frames[1].second(), frames[0].second());
+}
+
+TEST_F(VideoFrameConverterTest, Duplication) {
+  TimeStamp now = TimeStamp::Now();
+  TimeStamp future1 = now + TimeDuration::FromMilliseconds(100);
+  VideoChunk chunk = GenerateChunk(640, 480, future1);
+  mConverter->QueueVideoChunk(chunk, false);
+  auto frames = WaitForNConverted(2);
+  EXPECT_GT(TimeStamp::Now(), now + TimeDuration::FromMilliseconds(1100));
+  ASSERT_EQ(frames.size(), 2U);
+  EXPECT_EQ(frames[0].first().width(), 640);
+  EXPECT_EQ(frames[0].first().height(), 480);
+  EXPECT_GT(frames[0].second(), future1);
+  EXPECT_EQ(frames[1].first().width(), 640);
+  EXPECT_EQ(frames[1].first().height(), 480);
+  EXPECT_GT(frames[1].second(), now + TimeDuration::FromMilliseconds(1100));
+}
--- a/dom/media/gtest/moz.build
+++ b/dom/media/gtest/moz.build
@@ -34,16 +34,17 @@ UNIFIED_SOURCES += [
     'TestMediaDataDecoder.cpp',
     'TestMediaDataEncoder.cpp',
     'TestMediaEventSource.cpp',
     'TestMediaMIMETypes.cpp',
     'TestMP3Demuxer.cpp',
     'TestMP4Demuxer.cpp',
     'TestOpusParser.cpp',
     'TestRust.cpp',
+    'TestVideoFrameConverter.cpp',
     'TestVideoSegment.cpp',
     'TestVideoUtils.cpp',
     'TestVPXDecoding.cpp',
     'TestWebMBuffered.cpp',
 ]
 
 if CONFIG['MOZ_WEBM_ENCODER']:
     UNIFIED_SOURCES += [