Bug 1423253 - Make future frames in MediaPipeline black when a track is disabled. r=padenot
authorAndreas Pehrson <apehrson@mozilla.com>
Fri, 22 Mar 2019 11:45:15 +0000
changeset 465659 9019e6f0a6c3a52b9215eea8a5f499b4290c8707
parent 465658 f37f5f1fc1e7e912fbffac2fbb8572b9af654867
child 465660 9f3a6410605ac3a8eae9859d24c609e72010f428
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)
reviewerspadenot
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 - Make future frames in MediaPipeline black when a track is disabled. r=padenot Differential Revision: https://phabricator.services.mozilla.com/D22928
dom/media/VideoFrameConverter.h
dom/media/gtest/TestVideoFrameConverter.cpp
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
--- a/dom/media/VideoFrameConverter.h
+++ b/dom/media/VideoFrameConverter.h
@@ -56,17 +56,18 @@ class VideoFrameConverter {
 
   VideoFrameConverter()
       : mTaskQueue(
             new TaskQueue(GetMediaThreadPool(MediaThreadType::WEBRTC_DECODER),
                           "VideoFrameConverter")),
         mPacingTimer(new MediaTimer()),
         mLastImage(-1),  // -1 is not a guaranteed invalid serial (Bug 1262134).
         mBufferPool(false, CONVERTER_BUFFER_POOL_SIZE),
-        mLastFrameQueuedForProcessing(TimeStamp::Now()) {
+        mLastFrameQueuedForProcessing(TimeStamp::Now()),
+        mEnabled(true) {
     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;
     }
@@ -95,16 +96,36 @@ class VideoFrameConverter {
                [self = RefPtr<VideoFrameConverter>(this), this,
                 image = RefPtr<layers::Image>(aChunk.mFrame.GetImage()),
                 t = std::move(t), size = std::move(size), aForceBlack] {
                  QueueForProcessing(std::move(image), t, size, aForceBlack);
                },
                [] {});
   }
 
+  void SetTrackEnabled(bool aEnabled) {
+    nsresult rv = mTaskQueue->Dispatch(NS_NewRunnableFunction(
+        __func__, [self = RefPtr<VideoFrameConverter>(this), this, aEnabled] {
+          MOZ_LOG(gVideoFrameConverterLog, LogLevel::Debug,
+                  ("VideoFrameConverter Track is now %s",
+                   aEnabled ? "enabled" : "disabled"));
+          mEnabled = aEnabled;
+          if (!aEnabled && mLastFrameConverted) {
+            // After disabling, we re-send the last frame as black in case the
+            // source had already stopped and no frame is coming soon.
+            ProcessVideoFrame(nullptr, TimeStamp::Now(),
+                              gfx::IntSize(mLastFrameConverted->width(),
+                                           mLastFrameConverted->height()),
+                              true);
+          }
+        }));
+    MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
+    Unused << rv;
+  }
+
   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);
         }));
     MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
@@ -173,17 +194,17 @@ class VideoFrameConverter {
     }
   }
 
   void QueueForProcessing(RefPtr<layers::Image> aImage, TimeStamp aTime,
                           gfx::IntSize aSize, bool aForceBlack) {
     MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
 
     int32_t serial;
-    if (aForceBlack) {
+    if (aForceBlack || !mEnabled) {
       // 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) {
@@ -202,17 +223,17 @@ class VideoFrameConverter {
     mLastImage = serial;
     mLastFrameQueuedForProcessing = aTime;
 
     nsresult rv = mTaskQueue->Dispatch(
         NewRunnableMethod<StoreCopyPassByRRef<RefPtr<layers::Image>>, TimeStamp,
                           gfx::IntSize, bool>(
             "VideoFrameConverter::ProcessVideoFrame", this,
             &VideoFrameConverter::ProcessVideoFrame, std::move(aImage), aTime,
-            aSize, aForceBlack));
+            aSize, aForceBlack || !mEnabled));
     MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
     Unused << rv;
   }
 
   void ProcessVideoFrame(RefPtr<layers::Image> aImage, TimeStamp aTime,
                          gfx::IntSize aSize, bool aForceBlack) {
     MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn());
 
@@ -312,14 +333,15 @@ class VideoFrameConverter {
   TimeStamp mLastFrameQueuedForPacing;
 
   // Accessed only from mTaskQueue.
   int32_t mLastImage;  // Serial number of last processed Image
   webrtc::I420BufferPool mBufferPool;
   nsCOMPtr<nsITimer> mSameFrameTimer;
   TimeStamp mLastFrameQueuedForProcessing;
   UniquePtr<webrtc::VideoFrame> mLastFrameConverted;
+  bool mEnabled;
   nsTArray<RefPtr<VideoConverterListener>> mListeners;
 };
 
 }  // namespace mozilla
 
 #endif // VideoFrameConverter_h
--- a/dom/media/gtest/TestVideoFrameConverter.cpp
+++ b/dom/media/gtest/TestVideoFrameConverter.cpp
@@ -145,8 +145,31 @@ TEST_F(VideoFrameConverterTest, DropsOld
   mConverter->QueueVideoChunk(GenerateChunk(640, 480, future2), false);
   auto frames = WaitForNConverted(1);
   EXPECT_GT(TimeStamp::Now(), future2);
   ASSERT_EQ(frames.size(), 1U);
   EXPECT_EQ(frames[0].first().width(), 640);
   EXPECT_EQ(frames[0].first().height(), 480);
   EXPECT_GT(frames[0].second(), future2);
 }
+
+// We check that the disabling code was triggered by sending multiple,
+// different, frames to the converter within one second. While black, it shall
+// treat all frames identical and issue one black frame per second.
+TEST_F(VideoFrameConverterTest, BlackOnDisable) {
+  TimeStamp now = TimeStamp::Now();
+  TimeStamp future1 = now + TimeDuration::FromMilliseconds(100);
+  TimeStamp future2 = now + TimeDuration::FromMilliseconds(200);
+  TimeStamp future3 = now + TimeDuration::FromMilliseconds(400);
+  mConverter->SetTrackEnabled(false);
+  mConverter->QueueVideoChunk(GenerateChunk(640, 480, future1), false);
+  mConverter->QueueVideoChunk(GenerateChunk(640, 480, future2), false);
+  mConverter->QueueVideoChunk(GenerateChunk(640, 480, future3), 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/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -685,16 +685,17 @@ class MediaPipelineTransmit::PipelineLis
     MOZ_RELEASE_ASSERT(mConduit->type() == MediaSessionConduit::VIDEO);
     static_cast<VideoSessionConduit*>(mConduit.get())
         ->SendVideoFrame(aVideoFrame);
   }
 
   // Implement MediaStreamTrackListener
   void NotifyQueuedChanges(MediaStreamGraph* aGraph, StreamTime aTrackOffset,
                            const MediaSegment& aQueuedMedia) override;
+  void NotifyEnabledStateChanged(bool aEnabled) override;
 
   // Implement DirectMediaStreamTrackListener
   void NotifyRealtimeTrackData(MediaStreamGraph* aGraph,
                                StreamTime aTrackOffset,
                                const MediaSegment& aMedia) override;
   void NotifyDirectListenerInstalled(InstallationResult aResult) override;
   void NotifyDirectListenerUninstalled() override;
 
@@ -823,16 +824,17 @@ void MediaPipelineTransmit::Stop() {
 
   mTransmitting = false;
 
   if (mDomTrack->AsAudioStreamTrack()) {
     mDomTrack->RemoveDirectListener(mListener);
     mDomTrack->RemoveListener(mListener);
   } else if (mDomTrack->AsVideoStreamTrack()) {
     mDomTrack->RemoveDirectListener(mListener);
+    mDomTrack->RemoveListener(mListener);
   } else {
     MOZ_ASSERT(false, "Unknown track type");
   }
 
   mConduit->StopTransmitting();
 }
 
 bool MediaPipelineTransmit::Transmitting() const {
@@ -873,16 +875,17 @@ void MediaPipelineTransmit::Start() {
       // We also register it as a non-direct listener so we fall back to that
       // if installing the direct listener fails. As a direct listener we get
       // access to direct unqueued (and not resampled) data.
       mDomTrack->AddDirectListener(mListener);
     }
     mDomTrack->AddListener(mListener);
   } else if (mDomTrack->AsVideoStreamTrack()) {
     mDomTrack->AddDirectListener(mListener);
+    mDomTrack->AddListener(mListener);
   } else {
     MOZ_ASSERT(false, "Unknown track type");
   }
 }
 
 bool MediaPipelineTransmit::IsVideo() const { return mIsVideo; }
 
 void MediaPipelineTransmit::UpdateSinkIdentity_m(
@@ -1066,16 +1069,25 @@ void MediaPipelineTransmit::PipelineList
     rate = aGraph->GraphRate();
   } else {
     // When running tests, graph may be null. In that case use a default.
     rate = 16000;
   }
   NewData(aQueuedMedia, rate);
 }
 
+void MediaPipelineTransmit::PipelineListener::NotifyEnabledStateChanged(
+    bool aEnabled) {
+  if (mConduit->type() != MediaSessionConduit::VIDEO) {
+    return;
+  }
+  MOZ_ASSERT(mConverter);
+  mConverter->SetTrackEnabled(aEnabled);
+}
+
 void MediaPipelineTransmit::PipelineListener::NotifyDirectListenerInstalled(
     InstallationResult aResult) {
   MOZ_LOG(gMediaPipelineLog, LogLevel::Info,
           ("MediaPipeline::NotifyDirectListenerInstalled() listener=%p,"
            " result=%d",
            this, static_cast<int32_t>(aResult)));
 
   mDirectConnect = InstallationResult::SUCCESS == aResult;