 Chromium Code Reviews
 Chromium Code Reviews Issue 2616213002:
  Fix WebrtcVideoStream to handle failed capture requests.  (Closed)
    
  
    Issue 2616213002:
  Fix WebrtcVideoStream to handle failed capture requests.  (Closed) 
  | Index: remoting/protocol/webrtc_frame_scheduler_simple.cc | 
| diff --git a/remoting/protocol/webrtc_frame_scheduler_simple.cc b/remoting/protocol/webrtc_frame_scheduler_simple.cc | 
| index 5b90725c2e3eb0609b98703d0cd09e74e378c3ae..429fc95b522958a7daa5cc430f7b00350834c2af 100644 | 
| --- a/remoting/protocol/webrtc_frame_scheduler_simple.cc | 
| +++ b/remoting/protocol/webrtc_frame_scheduler_simple.cc | 
| @@ -84,20 +84,34 @@ void WebrtcFrameSchedulerSimple::EncoderBitrateFilter::SetBandwidthEstimate( | 
| bandwidth_samples_.push(std::make_pair(now, bandwidth_kbps)); | 
| bandwidth_samples_sum_ += bandwidth_kbps; | 
| + | 
| + UpdateTargetBitrate(); | 
| } | 
| -int WebrtcFrameSchedulerSimple::EncoderBitrateFilter::GetTargetBitrateKbps( | 
| - webrtc::DesktopSize size, | 
| - base::TimeTicks now) { | 
| - DCHECK(!bandwidth_samples_.empty()); | 
| +void WebrtcFrameSchedulerSimple::EncoderBitrateFilter::SetFrameSize( | 
| + webrtc::DesktopSize size) { | 
| + minimum_bitrate_ = | 
| + static_cast<int64_t>(kVp8MinimumTargetBitrateKbpsPerMegapixel) * | 
| + size.width() * size.height() / 1000000LL; | 
| + | 
| + UpdateTargetBitrate(); | 
| +} | 
| + | 
| +int WebrtcFrameSchedulerSimple::EncoderBitrateFilter::GetTargetBitrateKbps() | 
| + const { | 
| + DCHECK_GT(current_target_bitrate_, 0); | 
| + return current_target_bitrate_; | 
| +} | 
| + | 
| +void WebrtcFrameSchedulerSimple::EncoderBitrateFilter::UpdateTargetBitrate() { | 
| + if (bandwidth_samples_.empty()) { | 
| + return; | 
| + } | 
| // TODO(sergeyu): This logic is applicable only to VP8. Reconsider it for | 
| 
joedow
2017/01/09 20:00:28
Should this TODO move up with the VP8 constant or
 
Sergey Ulanov
2017/01/09 22:24:06
Done.
 | 
| // VP9. | 
| int bandwidth_estimate = bandwidth_samples_sum_ / bandwidth_samples_.size(); | 
| - int minimum_bitrate = | 
| - static_cast<int64_t>(kVp8MinimumTargetBitrateKbpsPerMegapixel) * | 
| - size.width() * size.height() / 1000000LL; | 
| - int target_bitrate = std::max(minimum_bitrate, bandwidth_estimate); | 
| + int target_bitrate = std::max(minimum_bitrate_, bandwidth_estimate); | 
| // Update encoder bitrate only when it changes by more than 30%. This is | 
| // necessary because the encoder resets internal state when it's reconfigured | 
| @@ -107,7 +121,6 @@ int WebrtcFrameSchedulerSimple::EncoderBitrateFilter::GetTargetBitrateKbps( | 
| current_target_bitrate_ * kEncoderBitrateChangePercentage / 100) { | 
| current_target_bitrate_ = target_bitrate; | 
| } | 
| - return current_target_bitrate_; | 
| } | 
| WebrtcFrameSchedulerSimple::WebrtcFrameSchedulerSimple() | 
| @@ -159,45 +172,51 @@ void WebrtcFrameSchedulerSimple::Pause(bool pause) { | 
| } | 
| } | 
| -bool WebrtcFrameSchedulerSimple::GetEncoderFrameParams( | 
| - const webrtc::DesktopFrame& frame, | 
| +bool WebrtcFrameSchedulerSimple::OnFrameCaptured( | 
| + 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_ && | 
| + if ((!frame || frame->updated_region().is_empty()) && !top_off_is_active_ && | 
| !key_frame_request_) { | 
| frame_pending_ = false; | 
| ScheduleNextFrame(now); | 
| return false; | 
| } | 
| - params_out->bitrate_kbps = | 
| - encoder_bitrate_.GetTargetBitrateKbps(frame.size(), now); | 
| + if (frame) { | 
| + encoder_bitrate_.SetFrameSize(frame->size()); | 
| + } | 
| + params_out->bitrate_kbps = encoder_bitrate_.GetTargetBitrateKbps(); | 
| params_out->duration = kTargetFrameInterval; | 
| params_out->key_frame = key_frame_request_; | 
| key_frame_request_ = false; | 
| params_out->vpx_min_quantizer = 10; | 
| - int64_t updated_area = params_out->key_frame | 
| - ? frame.size().width() * frame.size().height() | 
| - : GetRegionArea(frame.updated_region()); | 
| + int64_t updated_area = 0; | 
| + if (frame) { | 
| + updated_area = params_out->key_frame | 
| + ? frame->size().width() * frame->size().height() | 
| + : GetRegionArea(frame->updated_region()); | 
| + } | 
| // If bandwidth is being underutilized then libvpx is likely to choose the | 
| - // minimum allowed quantizer value, which means that encoded frame size may be | 
| - // significantly bigger than the bandwidth allows. Detect this case and set | 
| + // minimum allowed quantizer value, which means that encoded frame size may | 
| + // be significantly bigger than the bandwidth allows. Detect this case and set | 
| // vpx_min_quantizer to 60. The quality will be topped off later. | 
| if (updated_area - updated_region_area_.Max() > kBigFrameThresholdPixels) { | 
| - int expected_frame_size = updated_area * | 
| - kEstimatedBytesPerMegapixel / kPixelsPerMegapixel; | 
| + int expected_frame_size = | 
| + updated_area * kEstimatedBytesPerMegapixel / kPixelsPerMegapixel; | 
| base::TimeDelta expected_send_delay = base::TimeDelta::FromMicroseconds( | 
| base::Time::kMicrosecondsPerSecond * expected_frame_size / | 
| pacing_bucket_.rate()); | 
| - if (expected_send_delay > kTargetFrameInterval) | 
| + if (expected_send_delay > kTargetFrameInterval) { | 
| params_out->vpx_min_quantizer = 60; | 
| + } | 
| } | 
| updated_region_area_.Record(updated_area); | 
| @@ -210,8 +229,7 @@ bool WebrtcFrameSchedulerSimple::GetEncoderFrameParams( | 
| } | 
| void WebrtcFrameSchedulerSimple::OnFrameEncoded( | 
| - const WebrtcVideoEncoder::EncodedFrame& encoded_frame, | 
| - const webrtc::EncodedImageCallback::Result& send_result, | 
| + const WebrtcVideoEncoder::EncodedFrame* encoded_frame, | 
| HostFrameStats* frame_stats) { | 
| DCHECK(thread_checker_.CalledOnValidThread()); | 
| DCHECK(frame_pending_); | 
| @@ -225,16 +243,17 @@ void WebrtcFrameSchedulerSimple::OnFrameEncoded( | 
| std::max(base::TimeDelta(), pacing_bucket_.GetEmptyTime() - now); | 
| } | 
| - pacing_bucket_.RefillOrSpill(encoded_frame.data.size(), now); | 
| - | 
| - if (encoded_frame.data.empty()) { | 
| + if (!encoded_frame || encoded_frame->data.empty()) { | 
| top_off_is_active_ = false; | 
| } else { | 
| + pacing_bucket_.RefillOrSpill(encoded_frame->data.size(), now); | 
| + | 
| 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; | 
| + top_off_is_active_ = | 
| + encoded_frame->quantizer > kTargetQuantizerForVp8TopOff; | 
| } | 
| ScheduleNextFrame(now); | 
| @@ -266,8 +285,9 @@ void WebrtcFrameSchedulerSimple::ScheduleNextFrame(base::TimeTicks now) { | 
| target_capture_time, last_capture_started_time_ + kTargetFrameInterval); | 
| } | 
| - if (target_capture_time < now) | 
| + if (target_capture_time < now) { | 
| target_capture_time = now; | 
| + } | 
| capture_timer_.Start(FROM_HERE, target_capture_time - now, | 
| base::Bind(&WebrtcFrameSchedulerSimple::CaptureNextFrame, |