Update WebrtcFrameScheduler to avoid concurrent captures.

remoting/protocol/webrtc_frame_scheduler_simple.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/protocol/webrtc_frame_scheduler_simple.h"
 
 #include <algorithm>
 
 #include "remoting/protocol/webrtc_dummy_video_encoder.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
@@ skipping 46 matching lines @@
 WebrtcFrameSchedulerSimple::WebrtcFrameSchedulerSimple()
     : pacing_bucket_(LeakyBucket::kUnlimitedDepth, 0),
       frame_processing_delay_us_(kStatsWindow),
       updated_region_area_(kStatsWindow),
       weak_factory_(this) {}
 WebrtcFrameSchedulerSimple::~WebrtcFrameSchedulerSimple() {}
 
 void WebrtcFrameSchedulerSimple::OnKeyFrameRequested() {
   DCHECK(thread_checker_.CalledOnValidThread());
   key_frame_request_ = true;
-  if (!capture_timer_.IsRunning())
-    ScheduleNextFrame(base::TimeTicks::Now());
+  ScheduleNextFrame(base::TimeTicks::Now());
 }
 
 void WebrtcFrameSchedulerSimple::OnChannelParameters(int packet_loss,
                                                      base::TimeDelta rtt) {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 void WebrtcFrameSchedulerSimple::OnTargetBitrateChanged(int bitrate_kbps) {
   DCHECK(thread_checker_.CalledOnValidThread());
   base::TimeTicks now = base::TimeTicks::Now();
@@ skipping 23 matching lines @@
 
 bool WebrtcFrameSchedulerSimple::GetEncoderFrameParams(
     const webrtc::DesktopFrame& frame,
     WebrtcVideoEncoder::FrameParams* params_out) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   base::TimeTicks now = base::TimeTicks::Now();
 
   if (frame.updated_region().is_empty() && !top_off_is_active_ &&
       !key_frame_request_) {
+    frame_pending_ = false;
     ScheduleNextFrame(now);
     return false;
   }
 
   // TODO(sergeyu): This logic is applicable only to VP8. Reconsider it for VP9.
   int minimum_bitrate =
       static_cast<int64_t>(kVp8MinimumTargetBitrateKbpsPerMegapixel) *
       frame.size().width() * frame.size().height() / 1000000LL;
   params_out->bitrate_kbps =
       std::max(minimum_bitrate, pacing_bucket_.rate() * 8 / 1000);
@@ skipping 28 matching lines @@
 
   params_out->clear_active_map = !top_off_is_active_;
 
   return true;
 }
 
 void WebrtcFrameSchedulerSimple::OnFrameEncoded(
     const WebrtcVideoEncoder::EncodedFrame& encoded_frame,
     const webrtc::EncodedImageCallback::Result& send_result) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(frame_pending_);
+  frame_pending_ = false;
 
   base::TimeTicks now = base::TimeTicks::Now();
   pacing_bucket_.RefillOrSpill(encoded_frame.data.size(), now);
 
   if (encoded_frame.data.empty()) {
     top_off_is_active_ = false;
   } else {
     frame_processing_delay_us_.Record(
         (now - last_capture_started_time_).InMicroseconds());
 
     // Top-off until the target quantizer value is reached.
     top_off_is_active_ = encoded_frame.quantizer > kTargetQuantizerForVp8TopOff;
   }
 
   ScheduleNextFrame(now);
 }
 
 void WebrtcFrameSchedulerSimple::ScheduleNextFrame(base::TimeTicks now) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Don't capture frames when paused or target bitrate is 0 or there is
   // no capture callback set.

[Jamie, 2016/10/18 22:40:26]

Update this comment. As written now, it's just an English transition of the
conditional, which doesn't seem very useful. You could just remove it, or
rephrase it in terms of why frames should not be scheduled under these
circumstances.

[Sergey Ulanov, 2016/10/18 23:11:02]

Removed it.

-  if (paused_ || pacing_bucket_.rate() == 0 || capture_callback_.is_null())
+  if (paused_ || pacing_bucket_.rate() == 0 || capture_callback_.is_null() ||
+      frame_pending_) {
     return;
+  }
 
   // If this is not the first frame then capture next frame after the previous
   // one has finished sending.
   base::TimeDelta expected_processing_time =
       base::TimeDelta::FromMicroseconds(frame_processing_delay_us_.Max());
   base::TimeTicks target_capture_time =
       pacing_bucket_.GetEmptyTime() - expected_processing_time;
 
   // Cap interval between frames to kTargetFrameInterval.
   if (!last_capture_started_time_.is_null()) {
     target_capture_time = std::max(
         target_capture_time, last_capture_started_time_ + kTargetFrameInterval);
   }
 
   if (target_capture_time < now)
     target_capture_time = now;
 
   capture_timer_.Start(FROM_HERE, target_capture_time - now,
                        base::Bind(&WebrtcFrameSchedulerSimple::CaptureNextFrame,
                                   base::Unretained(this)));
 }
 
 void WebrtcFrameSchedulerSimple::CaptureNextFrame() {
   DCHECK(thread_checker_.CalledOnValidThread());
-
+  DCHECK(!frame_pending_);
   last_capture_started_time_ = base::TimeTicks::Now();
+  frame_pending_ = true;
   capture_callback_.Run();
 }
 
 }  // namespace protocol
 }  // namespace remoting