Move Request Refresh Frame logic to MediaStreamVideoTrack.

This change removes the RequestRefreshFrame logic from WebRTC sink and
moves it up to MediaStreamVideoTrack so other sinks (like
MediaStreamVideoRendererSink) can also use it.

Bug: 931033, 1100746, 1099280
Change-Id: I6479e9f77a8fdf03325b7ed980091273726cc57a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427563
Commit-Queue: Yash Khandelwal <yakhande@microsoft.com>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Markus Handell <handellm@google.com>
Cr-Commit-Position: refs/heads/master@{#814237}

third_party/blink/public/web/modules/mediastream/media_stream_video_sink.h

@@ -25,6 +25,10 @@ class BLINK_MODULES_EXPORT MediaStreamVideoSink : public WebMediaStreamSink {
  public:
   void OnFrameDropped(media::VideoCaptureFrameDropReason reason);
 
+  // Required minimum frames per second needed for the sink. Default is zero
+  // unless overridden.
+  virtual double GetRequiredMinFramesPerSec() const;
+
  protected:
   MediaStreamVideoSink();
   ~MediaStreamVideoSink() override;

third_party/blink/public/web/modules/mediastream/web_media_stream_utils.h

@@ -17,14 +17,9 @@ namespace blink {
 class WebMediaStreamSink;
 class WebMediaStreamTrack;
 
-// Requests that a refresh frame be sent "soon" (e.g., to resolve picture loss
-// or quality issues).
-//
 // TODO(crbug.com/704136): Move these helper functions out of the Blink
 // public API. Note for while moving it: there is an existing
 // media_stream_utils.h on renderer/modules/mediastream.
-BLINK_MODULES_EXPORT void RequestRefreshFrameFromVideoTrack(
-    const WebMediaStreamTrack& video_track);
 
 // Calls to these methods must be done on the main render thread.
 // Note that |callback| for frame delivery happens on the IO thread.

third_party/blink/renderer/modules/mediarecorder/video_track_recorder.h

@@ -90,6 +90,9 @@ class VideoTrackRecorder : public TrackRecorder<MediaStreamVideoSink> {
       bool is_key_frame)>;
   using OnErrorCB = base::RepeatingClosure;
 
+  // MediaStreamVideoSink implementation
+  double GetRequiredMinFramesPerSec() const override { return 1; }
+
   // Wraps a counter in a class in order to enable use of base::WeakPtr<>.
   // See https://crbug.com/859610 for why this was added.
   class Counter {

third_party/blink/renderer/modules/mediarecorder/video_track_recorder_unittest.cc

@@ -24,6 +24,7 @@
 #include "third_party/blink/renderer/platform/mediastream/media_stream_component.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_source.h"
 #include "third_party/blink/renderer/platform/testing/io_task_runner_testing_platform_support.h"
+#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
 #include "third_party/blink/renderer/platform/testing/video_frame_utils.h"
 #include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
@@ -430,6 +431,19 @@ TEST_F(VideoTrackRecorderTest, ReleasesFrame) {
   Mock::VerifyAndClearExpectations(this);
 }
 
+TEST_F(VideoTrackRecorderTest, RequiredRefreshRate) {
+  // |RequestRefreshFrame| will be called first by |AddSink| and the second time
+  // by the refresh timer using the required min fps.
+  EXPECT_CALL(*mock_source_, OnRequestRefreshFrame).Times(2);
+
+  track_->SetIsScreencastForTesting(true);
+  InitializeRecorder(VideoTrackRecorder::CodecId::VP8);
+
+  EXPECT_EQ(video_track_recorder_->GetRequiredMinFramesPerSec(), 1);
+
+  test::RunDelayedTasks(base::TimeDelta::FromSeconds(1));
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          VideoTrackRecorderTest,
                          ::testing::Combine(ValuesIn(kTrackRecorderTestCodec),

third_party/blink/renderer/modules/mediastream/apply_constraints_processor.cc

@@ -224,6 +224,10 @@ void ApplyConstraintsProcessor::FinalizeVideoRequest() {
   blink::VideoCaptureSettings settings = SelectVideoSettings({format});
 
   if (settings.HasValue()) {
+    if (settings.min_frame_rate().has_value()) {
+      GetCurrentVideoTrack()->SetMinimumFrameRate(
+          settings.min_frame_rate().value());
+    }
     video_source_->ReconfigureTrack(GetCurrentVideoTrack(),
                                     settings.track_adapter_settings());
     ApplyConstraintsSucceeded();

third_party/blink/renderer/modules/mediastream/media_stream_video_sink.cc

@@ -60,4 +60,8 @@ void MediaStreamVideoSink::OnFrameDropped(
     video_track->OnFrameDropped(reason);
 }
 
+double MediaStreamVideoSink::GetRequiredMinFramesPerSec() const {
+  return 0;
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/mediastream/media_stream_video_track.cc

@@ -13,7 +13,9 @@
 #include "base/single_thread_task_runner.h"
 #include "build/build_config.h"
 #include "media/base/bind_to_current_loop.h"
+#include "media/base/limits.h"
 #include "media/capture/video_capture_types.h"
+#include "third_party/blink/public/web/modules/mediastream/media_stream_video_sink.h"
 #include "third_party/blink/public/web/web_local_frame.h"
 #include "third_party/blink/renderer/modules/mediastream/media_stream_constraints_util_video_device.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_component.h"
@@ -25,6 +27,10 @@
 namespace blink {
 namespace {
 
+// A lower-bound for the refresh interval.
+constexpr base::TimeDelta kLowerBoundRefreshInterval =
+    base::TimeDelta::FromHz(media::limits::kMaxFramesPerSecond);
+
 // This alias mimics the definition of VideoCaptureDeliverFrameCB.
 using VideoCaptureDeliverFrameInternalCallback =
     WTF::CrossThreadFunction<void(scoped_refptr<media::VideoFrame> video_frame,
@@ -35,6 +41,28 @@ using EncodedVideoFrameInternalCallback =
     WTF::CrossThreadFunction<void(scoped_refptr<EncodedVideoFrame> frame,
                                   base::TimeTicks estimated_capture_time)>;
 
+base::TimeDelta ComputeRefreshIntervalFromBounds(
+    const base::TimeDelta required_min_refresh_interval,
+    const base::Optional<double>& min_frame_rate,
+    const base::Optional<double>& max_frame_rate) {
+  // Start with the default required refresh interval, and refine based on
+  // constraints. If a minimum frameRate is provided, use that. Otherwise, use
+  // the maximum frameRate if it happens to be less than the default.
+  base::TimeDelta refresh_interval = required_min_refresh_interval;
+  if (min_frame_rate.has_value())
+    refresh_interval = base::TimeDelta::FromHz(*min_frame_rate);
+
+  if (max_frame_rate.has_value()) {
+    refresh_interval =
+        std::max(refresh_interval, base::TimeDelta::FromHz(*max_frame_rate));
+  }
+
+  if (refresh_interval < kLowerBoundRefreshInterval)
+    refresh_interval = kLowerBoundRefreshInterval;
+
+  return refresh_interval;
+}
+
 }  // namespace
 
 // MediaStreamVideoTrack::FrameDeliverer is a helper class used for registering
@@ -87,6 +115,8 @@ class MediaStreamVideoTrack::FrameDeliverer
   void DeliverEncodedVideoFrameOnIO(scoped_refptr<EncodedVideoFrame> frame,
                                     base::TimeTicks estimated_capture_time);
 
+  void SetIsRefreshingForMinFrameRate(bool is_refreshing_for_min_frame_rate);
+
  private:
   friend class WTF::ThreadSafeRefCounted<FrameDeliverer>;
   virtual ~FrameDeliverer();
@@ -104,6 +134,9 @@ class MediaStreamVideoTrack::FrameDeliverer
 
   void SetEnabledOnIO(bool enabled, bool await_key_frame);
 
+  void SetIsRefreshingForMinFrameRateOnIO(
+      bool is_refreshing_for_min_frame_rate);
+
   // Returns a black frame where the size and time stamp is set to the same as
   // as in |reference_frame|.
   scoped_refptr<media::VideoFrame> GetBlackFrame(
@@ -128,6 +161,9 @@ class MediaStreamVideoTrack::FrameDeliverer
   HashMap<VideoSinkId, EncodedVideoFrameInternalCallback> encoded_callbacks_;
   bool await_next_key_frame_;
 
+  // This should only be accessed on the IO thread.
+  bool is_refreshing_for_min_frame_rate_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(FrameDeliverer);
 };
 
@@ -269,6 +305,22 @@ void MediaStreamVideoTrack::FrameDeliverer::SetEnabledOnIO(
   }
 }
 
+void MediaStreamVideoTrack::FrameDeliverer::SetIsRefreshingForMinFrameRate(
+    bool is_refreshing_for_min_frame_rate) {
+  DCHECK_CALLED_ON_VALID_THREAD(main_render_thread_checker_);
+  PostCrossThreadTask(
+      *io_task_runner_, FROM_HERE,
+      CrossThreadBindOnce(&FrameDeliverer::SetIsRefreshingForMinFrameRateOnIO,
+                          WrapRefCounted(this),
+                          is_refreshing_for_min_frame_rate));
+}
+
+void MediaStreamVideoTrack::FrameDeliverer::SetIsRefreshingForMinFrameRateOnIO(
+    bool is_refreshing_for_min_frame_rate) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  is_refreshing_for_min_frame_rate_ = is_refreshing_for_min_frame_rate;
+}
+
 void MediaStreamVideoTrack::FrameDeliverer::DeliverFrameOnIO(
     scoped_refptr<media::VideoFrame> frame,
     base::TimeTicks estimated_capture_time) {
@@ -289,6 +341,18 @@ void MediaStreamVideoTrack::FrameDeliverer::DeliverFrameOnIO(
   auto video_frame = enabled_ ? std::move(frame) : GetBlackFrame(*frame);
   for (const auto& entry : callbacks_)
     entry.second.Run(video_frame, estimated_capture_time);
+
+  // The delay on refresh timer is reset each time a frame is received so that
+  // it will not fire for at least an additional period. This means refresh
+  // frames will only be requested when the source has halted delivery (e.g., a
+  // screen capturer stops sending frames because the screen is not being
+  // updated).
+  if (main_render_task_runner_ && is_refreshing_for_min_frame_rate_) {
+    PostCrossThreadTask(
+        *main_render_task_runner_, FROM_HERE,
+        CrossThreadBindOnce(&MediaStreamVideoTrack::ResetRefreshTimer,
+                            media_stream_video_track_));
+  }
 }
 
 void MediaStreamVideoTrack::FrameDeliverer::DeliverEncodedVideoFrameOnIO(
@@ -482,9 +546,11 @@ void MediaStreamVideoTrack::AddSink(WebMediaStreamSink* sink,
   if (!source_)
     return;
   UpdateSourceHasConsumers();
-  source_->RequestRefreshFrame();
+  RequestRefreshFrame();
   source_->UpdateCapturingLinkSecure(this,
                                      secure_tracker_.is_capturing_secure());
+  if (is_screencast_)
+    StartTimerForRequestingFrames();
 }
 
 void MediaStreamVideoTrack::AddEncodedSink(WebMediaStreamSink* sink,
@@ -507,6 +573,9 @@ void MediaStreamVideoTrack::RemoveSink(WebMediaStreamSink* sink) {
   UpdateSourceHasConsumers();
   source_->UpdateCapturingLinkSecure(this,
                                      secure_tracker_.is_capturing_secure());
+  // Restart the timer with existing sinks.
+  if (is_screencast_)
+    StartTimerForRequestingFrames();
 }
 
 void MediaStreamVideoTrack::RemoveEncodedSink(WebMediaStreamSink* sink) {
@@ -534,7 +603,7 @@ void MediaStreamVideoTrack::SetEnabled(bool enabled) {
   bool maybe_await_key_frame = false;
   if (enabled && source_ && source_->SupportsEncodedOutput() &&
       !encoded_sinks_.IsEmpty()) {
-    source_->RequestRefreshFrame();
+    RequestRefreshFrame();
     maybe_await_key_frame = true;
   }
   frame_deliverer_->SetEnabled(enabled, maybe_await_key_frame);
@@ -567,6 +636,7 @@ void MediaStreamVideoTrack::StopAndNotify(base::OnceClosure callback) {
     std::move(callback).Run();
   }
   OnReadyStateChanged(WebMediaStreamSource::kReadyStateEnded);
+  refresh_timer_.Stop();
 }
 
 void MediaStreamVideoTrack::GetSettings(
@@ -638,4 +708,49 @@ void MediaStreamVideoTrack::OnFrameDropped(
   source_->OnFrameDropped(reason);
 }
 
+void MediaStreamVideoTrack::SetMinimumFrameRate(double min_frame_rate) {
+  DCHECK_CALLED_ON_VALID_THREAD(main_render_thread_checker_);
+  min_frame_rate_ = min_frame_rate;
+}
+
+void MediaStreamVideoTrack::StartTimerForRequestingFrames() {
+  DCHECK_CALLED_ON_VALID_THREAD(main_render_thread_checker_);
+
+  // Find the maximum of all the required min frames per second in the attached
+  // sinks.
+  double required_min_fps = 0;
+  for (auto* web_sink : sinks_) {
+    auto* sink = static_cast<MediaStreamVideoSink*>(web_sink);
+    required_min_fps =
+        std::max(required_min_fps, sink->GetRequiredMinFramesPerSec());
+  }
+
+  base::TimeDelta refresh_interval = ComputeRefreshIntervalFromBounds(
+      base::TimeDelta::FromHz(required_min_fps), min_frame_rate_,
+      max_frame_rate_);
+
+  if (refresh_interval.is_max()) {
+    refresh_timer_.Stop();
+    frame_deliverer_->SetIsRefreshingForMinFrameRate(false);
+  } else {
+    DVLOG(1) << "Starting frame refresh timer with interval "
+             << refresh_interval.InMillisecondsF() << " ms.";
+    refresh_timer_.Start(FROM_HERE, refresh_interval, this,
+                         &MediaStreamVideoTrack::RequestRefreshFrame);
+    frame_deliverer_->SetIsRefreshingForMinFrameRate(true);
+  }
+}
+
+void MediaStreamVideoTrack::RequestRefreshFrame() {
+  DCHECK_CALLED_ON_VALID_THREAD(main_render_thread_checker_);
+  if (source_)
+    source_->RequestRefreshFrame();
+}
+
+void MediaStreamVideoTrack::ResetRefreshTimer() {
+  DCHECK_CALLED_ON_VALID_THREAD(main_render_thread_checker_);
+  if (refresh_timer_.IsRunning())
+    refresh_timer_.Reset();
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/mediastream/media_stream_video_track.h

@@ -139,6 +139,8 @@ class MODULES_EXPORT MediaStreamVideoTrack : public MediaStreamTrackPlatform {
     computed_frame_rate_ = frame_rate;
   }
 
+  void SetMinimumFrameRate(double min_frame_rate);
+
   // Setting information about the source format. The format is computed based
   // on incoming frames and it's used for applying constraints for remote video
   // tracks. Passed as callback on MediaStreamVideoTrack::AddTrack, and run from
@@ -155,6 +157,14 @@ class MODULES_EXPORT MediaStreamVideoTrack : public MediaStreamTrackPlatform {
 
   void OnFrameDropped(media::VideoCaptureFrameDropReason reason);
 
+  bool IsRefreshFrameTimerRunningForTesting() {
+    return refresh_timer_.IsRunning();
+  }
+
+  void SetIsScreencastForTesting(bool is_screencast) {
+    is_screencast_ = is_screencast;
+  }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(MediaStreamRemoteVideoSourceTest, StartTrack);
   FRIEND_TEST_ALL_PREFIXES(MediaStreamRemoteVideoSourceTest, RemoteTrackStop);
@@ -165,6 +175,10 @@ class MODULES_EXPORT MediaStreamVideoTrack : public MediaStreamTrackPlatform {
   void UpdateSourceCapturingSecure();
   void UpdateSourceHasConsumers();
 
+  void RequestRefreshFrame();
+  void StartTimerForRequestingFrames();
+  void ResetRefreshTimer();
+
   // In debug builds, check that all methods that could cause object graph
   // or data flow changes are being called on the main thread.
   THREAD_CHECKER(main_render_thread_checker_);
@@ -200,6 +214,7 @@ class MODULES_EXPORT MediaStreamVideoTrack : public MediaStreamTrackPlatform {
   double frame_rate_ = 0.0;
   base::Optional<double> computed_frame_rate_;
   media::VideoCaptureFormat computed_source_format_;
+  base::RepeatingTimer refresh_timer_;
 
   base::WeakPtrFactory<MediaStreamVideoTrack> weak_factory_{this};
 

third_party/blink/renderer/modules/mediastream/media_stream_video_track_test.cc

@@ -15,14 +15,19 @@
 #include "base/threading/thread_checker.h"
 #include "media/base/video_frame.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h"
 #include "third_party/blink/public/web/web_heap.h"
 #include "third_party/blink/renderer/modules/mediastream/media_stream_video_track.h"
 #include "third_party/blink/renderer/modules/mediastream/mock_encoded_video_frame.h"
 #include "third_party/blink/renderer/modules/mediastream/mock_media_stream_video_sink.h"
 #include "third_party/blink/renderer/modules/mediastream/mock_media_stream_video_source.h"
 #include "third_party/blink/renderer/modules/mediastream/video_track_adapter_settings.h"
+#include "third_party/blink/renderer/modules/peerconnection/media_stream_video_webrtc_sink.h"
+#include "third_party/blink/renderer/modules/peerconnection/mock_peer_connection_dependency_factory.h"
+#include "third_party/blink/renderer/platform/mediastream/media_stream_component.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_source.h"
 #include "third_party/blink/renderer/platform/testing/io_task_runner_testing_platform_support.h"
+#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 
 namespace blink {
@@ -43,6 +48,7 @@ const uint8_t kBlackValue = 0x00;
 const uint8_t kColorValue = 0xAB;
 const int kMockSourceWidth = 640;
 const int kMockSourceHeight = 480;
+const double kMinFrameRate = 30.0;
 
 class MediaStreamVideoTrackTest
     : public testing::TestWithParam<ContentHintType> {
@@ -555,5 +561,147 @@ INSTANTIATE_TEST_SUITE_P(,
                                 ContentHintType::kVideoDetail,
                                 ContentHintType::kVideoText));
 
+class MediaStreamVideoTrackRefreshFrameTimerTest
+    : public MediaStreamVideoTrackTest {
+ public:
+  void SetUp() override { InitializeSource(); }
+};
+
+TEST_F(MediaStreamVideoTrackRefreshFrameTimerTest,
+       SetMinFrameRateForScreenCastTrack) {
+  // |RequestRefreshFrame| should be called exactly twice within kMinFrameRate
+  // interval: First time from |AddSink| and second time from the refresh timer.
+  EXPECT_CALL(*mock_source(), OnRequestRefreshFrame).Times(2);
+  MockMediaStreamVideoSink sink;
+  WebMediaStreamTrack track =
+      CreateTrackWithSettings(VideoTrackAdapterSettings());
+  auto* video_track = MediaStreamVideoTrack::GetVideoTrack(track);
+  video_track->SetMinimumFrameRate(kMinFrameRate);
+  video_track->SetIsScreencastForTesting(true);
+
+  sink.ConnectToTrack(track);
+  test::RunDelayedTasks(base::TimeDelta::FromHz(kMinFrameRate));
+
+  EXPECT_TRUE(video_track->IsRefreshFrameTimerRunningForTesting());
+  video_track->StopAndNotify(base::BindOnce([] {}));
+  EXPECT_FALSE(video_track->IsRefreshFrameTimerRunningForTesting());
+}
+
+TEST_F(MediaStreamVideoTrackRefreshFrameTimerTest,
+       SetMinFrameRateForNonScreenCastTrack) {
+  // |RequestRefreshFrame| should only be called once from |AddSink| since
+  // refresh frame timer is not running.
+  EXPECT_CALL(*mock_source(), OnRequestRefreshFrame).Times(1);
+  MockMediaStreamVideoSink sink;
+  WebMediaStreamTrack track =
+      CreateTrackWithSettings(VideoTrackAdapterSettings());
+
+  auto* video_track = MediaStreamVideoTrack::GetVideoTrack(track);
+  video_track->SetMinimumFrameRate(kMinFrameRate);
+  // Refresh frame timer will not be run when |is_screencast_| is false.
+  video_track->SetIsScreencastForTesting(false);
+
+  sink.ConnectToTrack(track);
+  test::RunDelayedTasks(base::TimeDelta::FromHz(kMinFrameRate));
+
+  EXPECT_FALSE(video_track->IsRefreshFrameTimerRunningForTesting());
+}
+
+TEST_F(MediaStreamVideoTrackRefreshFrameTimerTest, RequiredRefreshRate) {
+  // Sinks that have a required min frames per sec as 0 will not lead
+  // to video track running the refresh frame timer.
+  EXPECT_CALL(*mock_source(), OnRequestRefreshFrame).Times(1);
+
+  MockMediaStreamVideoSink sink;
+  EXPECT_EQ(sink.GetRequiredMinFramesPerSec(), 0);
+
+  WebMediaStreamTrack track =
+      CreateTrackWithSettings(VideoTrackAdapterSettings());
+  auto* video_track = MediaStreamVideoTrack::GetVideoTrack(track);
+  video_track->SetIsScreencastForTesting(true);
+
+  sink.ConnectToTrack(track);
+  test::RunDelayedTasks(base::TimeDelta::FromSeconds(1));
+}
+
+TEST_F(MediaStreamVideoTrackRefreshFrameTimerTest,
+       RequiredRefreshRateWebRTCSink) {
+  // WebRTC sink has a required min frames per sec set to 1 so when we do
+  // not have any min frame rate set on the video track, this required rate will
+  // be used by the timer.
+  EXPECT_CALL(*mock_source(), OnRequestRefreshFrame).Times(2);
+
+  WebMediaStreamTrack track = MediaStreamVideoTrack::CreateVideoTrack(
+      mock_source(), WebPlatformMediaStreamSource::ConstraintsOnceCallback(),
+      true);
+  MediaStreamVideoTrack::GetVideoTrack(track)->SetIsScreencastForTesting(true);
+
+  Persistent<MediaStreamComponent> media_stream_component = *track;
+  blink::MediaStreamVideoWebRtcSink webrtc_sink(
+      media_stream_component, new blink::MockPeerConnectionDependencyFactory(),
+      blink::scheduler::GetSingleThreadTaskRunnerForTesting());
+  EXPECT_EQ(webrtc_sink.GetRequiredMinFramesPerSec(), 1);
+
+  test::RunDelayedTasks(base::TimeDelta::FromSeconds(1));
+}
+
+TEST_F(MediaStreamVideoTrackRefreshFrameTimerTest,
+       RequiredRefreshRateMultipleSinksAdded) {
+  // |RequestRefreshFrame| will be called once for every sink added (twice here)
+  // and third time from the refresh frame timer started by WebRTC sink. We will
+  // pick the maximum of all the required refresh rates to run the timer.
+  EXPECT_CALL(*mock_source(), OnRequestRefreshFrame).Times(3);
+
+  WebMediaStreamTrack track = MediaStreamVideoTrack::CreateVideoTrack(
+      mock_source(), WebPlatformMediaStreamSource::ConstraintsOnceCallback(),
+      true);
+  MediaStreamVideoTrack::GetVideoTrack(track)->SetIsScreencastForTesting(true);
+
+  // First sink.
+  MockMediaStreamVideoSink sink;
+  EXPECT_EQ(sink.GetRequiredMinFramesPerSec(), 0);
+  sink.ConnectToTrack(track);
+
+  // Second sink.
+  Persistent<MediaStreamComponent> media_stream_component = *track;
+  blink::MediaStreamVideoWebRtcSink webrtc_sink(
+      media_stream_component, new blink::MockPeerConnectionDependencyFactory(),
+      blink::scheduler::GetSingleThreadTaskRunnerForTesting());
+  EXPECT_EQ(webrtc_sink.GetRequiredMinFramesPerSec(), 1);
+
+  test::RunDelayedTasks(base::TimeDelta::FromSeconds(1));
+}
+
+TEST_F(MediaStreamVideoTrackRefreshFrameTimerTest,
+       RequiredRefreshRateMultipleSinksAddedAndRemoved) {
+  // |RequestRefreshFrame| will be called once for every sink added (twice
+  // here). The second sink (webrtc sink) does have a required min frames per
+  // sec but it is removed.
+  EXPECT_CALL(*mock_source(), OnRequestRefreshFrame).Times(2);
+
+  WebMediaStreamTrack track = MediaStreamVideoTrack::CreateVideoTrack(
+      mock_source(), WebPlatformMediaStreamSource::ConstraintsOnceCallback(),
+      true);
+  MediaStreamVideoTrack::GetVideoTrack(track)->SetIsScreencastForTesting(true);
+
+  // First sink.
+  MockMediaStreamVideoSink sink;
+  EXPECT_EQ(sink.GetRequiredMinFramesPerSec(), 0);
+  sink.ConnectToTrack(track);
+
+  // Second sink added and then removed. The destructor for
+  // MediaStreamVideoWebRtcSink calls DisconnectFromTrack.
+  {
+    Persistent<MediaStreamComponent> media_stream_component = *track;
+    blink::MediaStreamVideoWebRtcSink webrtc_sink(
+        media_stream_component,
+        new blink::MockPeerConnectionDependencyFactory(),
+        blink::scheduler::GetSingleThreadTaskRunnerForTesting());
+    EXPECT_EQ(webrtc_sink.GetRequiredMinFramesPerSec(), 1);
+  }
+
+  test::RunDelayedTasks(base::TimeDelta::FromSeconds(1));
+}
+
 }  // namespace media_stream_video_track_test
 }  // namespace blink

third_party/blink/renderer/modules/mediastream/web_media_stream_utils.cc

@@ -10,21 +10,13 @@
 #include "media/capture/video_capturer_source.h"
 #include "third_party/blink/public/platform/modules/mediastream/web_media_stream_sink.h"
 #include "third_party/blink/public/platform/modules/mediastream/web_media_stream_track.h"
+#include "third_party/blink/public/web/modules/mediastream/media_stream_video_sink.h"
 #include "third_party/blink/public/web/modules/mediastream/media_stream_video_source.h"
 #include "third_party/blink/renderer/modules/mediastream/media_stream_constraints_util.h"
 #include "third_party/blink/renderer/modules/mediastream/media_stream_video_track.h"
 
 namespace blink {
 
-void RequestRefreshFrameFromVideoTrack(const WebMediaStreamTrack& video_track) {
-  if (video_track.IsNull())
-    return;
-  MediaStreamVideoSource* const source =
-      MediaStreamVideoSource::GetVideoSource(video_track.Source());
-  if (source)
-    source->RequestRefreshFrame();
-}
-
 void AddSinkToMediaStreamTrack(const WebMediaStreamTrack& track,
                                WebMediaStreamSink* sink,
                                const VideoCaptureDeliverFrameCB& callback,

third_party/blink/renderer/modules/peerconnection/media_stream_video_webrtc_sink.cc

@@ -11,8 +11,6 @@
 #include "base/numerics/safe_conversions.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
-#include "base/timer/timer.h"
-#include "media/base/limits.h"
 #include "third_party/blink/public/web/modules/mediastream/web_media_stream_utils.h"
 #include "third_party/blink/renderer/modules/mediastream/media_stream_constraints_util.h"
 #include "third_party/blink/renderer/modules/mediastream/media_stream_video_track.h"
@@ -36,15 +34,6 @@ absl::optional<bool> ToAbslOptionalBool(const base::Optional<bool>& value) {
 
 namespace {
 
-// The default number of microseconds that should elapse since the last video
-// frame was received, before requesting a refresh frame.
-const int64_t kDefaultRefreshIntervalMicros =
-    base::Time::kMicrosecondsPerSecond;
-
-// A lower-bound for the refresh interval.
-const int64_t kLowerBoundRefreshIntervalMicros =
-    base::Time::kMicrosecondsPerSecond / media::limits::kMaxFramesPerSecond;
-
 webrtc::VideoTrackInterface::ContentHint ContentHintTypeToWebRtcContentHint(
     WebMediaStreamTrack::ContentHintType content_hint) {
   switch (content_hint) {
@@ -78,8 +67,6 @@ class MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter
       const scoped_refptr<base::SingleThreadTaskRunner>&
           libjingle_worker_thread,
       const scoped_refptr<WebRtcVideoTrackSource>& source,
-      base::TimeDelta refresh_interval,
-      const base::RepeatingClosure& refresh_callback,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
   // MediaStreamVideoWebRtcSink can be destroyed on the main render thread or
@@ -100,11 +87,6 @@ class MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter
 
   virtual ~WebRtcVideoSourceAdapter();
 
-  // Called whenever a video frame was just delivered on the IO thread. This
-  // restarts the delay period before the |refresh_timer_| will fire the next
-  // time.
-  void ResetRefreshTimerOnMainThread();
-
   scoped_refptr<base::SingleThreadTaskRunner> render_task_runner_;
 
   // |render_thread_checker_| is bound to the main render thread.
@@ -123,38 +105,17 @@ class MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter
   // main render thread in ReleaseSourceOnMainThread() when
   // the owning MediaStreamVideoWebRtcSink is being destroyed.
   base::Lock video_source_stop_lock_;
-
-  // Requests a refresh frame at regular intervals. The delay on this timer is
-  // reset each time a frame is received so that it will not fire for at least
-  // an additional period. This means refresh frames will only be requested when
-  // the source has halted delivery (e.g., a screen capturer stops sending
-  // frames because the screen is not being updated).
-  //
-  // This mechanism solves a number of problems. First, it will ensure that
-  // remote clients that join a distributed session receive a first video frame
-  // in a timely manner. Second, it will allow WebRTC's internal bandwidth
-  // estimation logic to maintain a more optimal state, since sending a video
-  // frame will "prime it." Third, it allows lossy encoders to clean up
-  // artifacts in a still image.  http://crbug.com/486274
-  base::RepeatingTimer refresh_timer_;
 };
 
 MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter::WebRtcVideoSourceAdapter(
     const scoped_refptr<base::SingleThreadTaskRunner>& libjingle_worker_thread,
     const scoped_refptr<WebRtcVideoTrackSource>& source,
-    base::TimeDelta refresh_interval,
-    const base::RepeatingClosure& refresh_callback,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)
     : render_task_runner_(std::move(task_runner)),
       libjingle_worker_thread_(libjingle_worker_thread),
       video_source_(source) {
   DCHECK(render_task_runner_->RunsTasksInCurrentSequence());
   DETACH_FROM_THREAD(io_thread_checker_);
-  if (!refresh_interval.is_zero()) {
-    VLOG(1) << "Starting frame refresh timer with interval "
-            << refresh_interval.InMillisecondsF() << " ms.";
-    refresh_timer_.Start(FROM_HERE, refresh_interval, refresh_callback);
-  }
 }
 
 MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter::
@@ -168,13 +129,6 @@ MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter::
   // is released when MediaStreamVideoWebRtcSink() is destroyed.
 }
 
-void MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter::
-    ResetRefreshTimerOnMainThread() {
-  DCHECK_CALLED_ON_VALID_THREAD(render_thread_checker_);
-  if (refresh_timer_.IsRunning())
-    refresh_timer_.Reset();
-}
-
 void MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter::
     ReleaseSourceOnMainThread() {
   DCHECK_CALLED_ON_VALID_THREAD(render_thread_checker_);
@@ -189,11 +143,6 @@ void MediaStreamVideoWebRtcSink::WebRtcVideoSourceAdapter::OnVideoFrameOnIO(
     scoped_refptr<media::VideoFrame> frame,
     base::TimeTicks estimated_capture_time) {
   DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_);
-  PostCrossThreadTask(
-      *render_task_runner_.get(), FROM_HERE,
-      CrossThreadBindOnce(
-          &WebRtcVideoSourceAdapter::ResetRefreshTimerOnMainThread,
-          WrapRefCounted(this)));
   PostCrossThreadTask(
       *libjingle_worker_thread_.get(), FROM_HERE,
       CrossThreadBindOnce(&WebRtcVideoSourceAdapter::OnVideoFrameOnWorkerThread,
@@ -220,35 +169,6 @@ MediaStreamVideoWebRtcSink::MediaStreamVideoWebRtcSink(
       ToAbslOptionalBool(video_track->noise_reduction());
 
   bool is_screencast = video_track->is_screencast();
-  base::Optional<double> min_frame_rate = video_track->min_frame_rate();
-  base::Optional<double> max_frame_rate = video_track->max_frame_rate();
-
-  // Enable automatic frame refreshes for the screen capture sources, which will
-  // stop producing frames whenever screen content is not changing. Check the
-  // frameRate constraint to determine the rate of refreshes. If a minimum
-  // frameRate is provided, use that. Otherwise, use the maximum frameRate if it
-  // happens to be less than the default.
-  base::TimeDelta refresh_interval = base::TimeDelta::FromMicroseconds(0);
-  if (is_screencast) {
-    // Start with the default refresh interval, and refine based on constraints.
-    refresh_interval =
-        base::TimeDelta::FromMicroseconds(kDefaultRefreshIntervalMicros);
-    if (min_frame_rate.has_value()) {
-      refresh_interval =
-          base::TimeDelta::FromMicroseconds(base::saturated_cast<int64_t>(
-              base::Time::kMicrosecondsPerSecond / *min_frame_rate));
-    }
-    if (max_frame_rate.has_value()) {
-      const base::TimeDelta alternate_refresh_interval =
-          base::TimeDelta::FromMicroseconds(base::saturated_cast<int64_t>(
-              base::Time::kMicrosecondsPerSecond / *max_frame_rate));
-      refresh_interval = std::max(refresh_interval, alternate_refresh_interval);
-    }
-    if (refresh_interval.InMicroseconds() < kLowerBoundRefreshIntervalMicros) {
-      refresh_interval =
-          base::TimeDelta::FromMicroseconds(kLowerBoundRefreshIntervalMicros);
-    }
-  }
 
   // TODO(pbos): Consolidate WebRtcVideoCapturerAdapter into WebRtcVideoSource
   // by removing the need for and dependency on a cricket::VideoCapturer.
@@ -271,10 +191,6 @@ MediaStreamVideoWebRtcSink::MediaStreamVideoWebRtcSink(
 
   source_adapter_ = base::MakeRefCounted<WebRtcVideoSourceAdapter>(
       factory->GetWebRtcNetworkTaskRunner(), video_source_.get(),
-      refresh_interval,
-      ConvertToBaseRepeatingCallback(CrossThreadBindRepeating(
-          &MediaStreamVideoWebRtcSink::RequestRefreshFrame,
-          weak_factory_.GetWeakPtr())),
       std::move(task_runner));
 
   MediaStreamVideoSink::ConnectToTrack(
@@ -307,11 +223,6 @@ void MediaStreamVideoWebRtcSink::OnContentHintChanged(
       ContentHintTypeToWebRtcContentHint(content_hint));
 }
 
-void MediaStreamVideoWebRtcSink::RequestRefreshFrame() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  RequestRefreshFrameFromVideoTrack(connected_track());
-}
-
 absl::optional<bool>
 MediaStreamVideoWebRtcSink::SourceNeedsDenoisingForTesting() const {
   return video_source_->needs_denoising();

third_party/blink/renderer/modules/peerconnection/media_stream_video_webrtc_sink.h

@@ -45,6 +45,8 @@ class MODULES_EXPORT MediaStreamVideoWebRtcSink : public MediaStreamVideoSink {
 
   absl::optional<bool> SourceNeedsDenoisingForTesting() const;
 
+  double GetRequiredMinFramesPerSec() const override { return 1; }
+
  protected:
   // Implementation of MediaStreamSink.
   void OnEnabledChanged(bool enabled) override;
@@ -52,10 +54,6 @@ class MODULES_EXPORT MediaStreamVideoWebRtcSink : public MediaStreamVideoSink {
       WebMediaStreamTrack::ContentHintType content_hint) override;
 
  private:
-  // Helper to request a refresh frame from the source. Called via the callback
-  // passed to WebRtcVideoSourceAdapter.
-  void RequestRefreshFrame();
-
   // Used to DCHECK that we are called on the correct thread.
   THREAD_CHECKER(thread_checker_);