Tab/Desktop Capture: Use requests instead of timer-based refreshing.

media/capture/content/video_capture_oracle.cc

 // Copyright (c) 2015 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 "media/capture/content/video_capture_oracle.h"
 
 #include <algorithm>
 
 #include "base/format_macros.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/stringprintf.h"
 
 namespace media {
 
 namespace {
 
-// This value controls how many redundant, timer-base captures occur when the
-// content is static. Redundantly capturing the same frame allows iterative
-// quality enhancement, and also allows the buffer to fill in "buffered mode".
-//
-// TODO(nick): Controlling this here is a hack and a layering violation, since
-// it's a strategy specific to the WebRTC consumer, and probably just papers
-// over some frame dropping and quality bugs. It should either be controlled at
-// a higher level, or else redundant frame generation should be pushed down
-// further into the WebRTC encoding stack.
-const int kNumRedundantCapturesOfStaticContent = 200;
+// When estimating frame durations, this is the hard upper-bound on the
+// estimate.
+const int kUpperBoundDurationEstimateMicros = 1000000;  // 1 second

[Irfan, 2016/04/06 19:53:05]

kUpperBoundFrameDuration... ?

[miu, 2016/04/06 22:33:53]

IMHO, adding "Frame" to this long name doesn't really make it that much clearer.
 It's already easily understood this is about frame durations, and it'd cause
some weird line-wrapping where this is used in the code, if the name were even
longer.

 
 // The half-life of data points provided to the accumulator used when evaluating
 // the recent utilization of the buffer pool.  This value is based on a
 // simulation, and reacts quickly to change to avoid depleting the buffer pool
 // (which would cause hard frame drops).
 const int kBufferUtilizationEvaluationMicros = 200000;  // 0.2 seconds
 
 // The half-life of data points provided to the accumulator used when evaluating
 // the recent resource utilization of the consumer.  The trade-off made here is
 // reaction time versus over-reacting to outlier data points.
@@ skipping 62 matching lines @@
 
 VideoCaptureOracle::VideoCaptureOracle(
     base::TimeDelta min_capture_period,
     const gfx::Size& max_frame_size,
     media::ResolutionChangePolicy resolution_change_policy,
     bool enable_auto_throttling)
     : auto_throttling_enabled_(enable_auto_throttling),
       next_frame_number_(0),
       last_successfully_delivered_frame_number_(-1),
       num_frames_pending_(0),
-      smoothing_sampler_(min_capture_period,
-                         kNumRedundantCapturesOfStaticContent),
+      smoothing_sampler_(min_capture_period),
       content_sampler_(min_capture_period),
       resolution_chooser_(max_frame_size, resolution_change_policy),
       buffer_pool_utilization_(base::TimeDelta::FromMicroseconds(
           kBufferUtilizationEvaluationMicros)),
       estimated_capable_area_(base::TimeDelta::FromMicroseconds(
           kConsumerCapabilityEvaluationMicros)) {
   VLOG(1) << "Auto-throttling is "
           << (auto_throttling_enabled_ ? "enabled." : "disabled.");
 }
 
@@ skipping 16 matching lines @@
   DCHECK_LT(event, kNumEvents);
   if (event_time < last_event_time_[event]) {
     LOG(WARNING) << "Event time is not monotonically non-decreasing.  "
                  << "Deciding not to capture this frame.";
     return false;
   }
   last_event_time_[event] = event_time;
 
   bool should_sample = false;
   duration_of_next_frame_ = base::TimeDelta();
   switch (event) {

[xjz, 2016/04/06 17:02:28]

nit: maybe just use if (event == kCompositorUpdate) { } else { }?

[miu, 2016/04/06 22:33:53]

I would, except for two things:

1. This forces compile-time checking that all possible enum values are addressed
in this code.  (i.e., you get an error if someone adds a new item in the enum,
but doesn't decide how to handle it here)

2. I'm not sure whether I'm going to keep the refresh requests as being handled
the same way as the mouse cursor updates in the long-term.  I figured we'd start
with this behavior and see if things need to be improved.

-------

Actually, it just occurred to me that we should not allow refreshes (or mouse
updates) to happen if content was very-recently animating, since that could mess
up smoothness.  So, I made a logic change here to account for that.  PTAL.

     case kCompositorUpdate: {
       smoothing_sampler_.ConsiderPresentationEvent(event_time);
       const bool had_proposal = content_sampler_.HasProposal();
       content_sampler_.ConsiderPresentationEvent(damage_rect, event_time);
       if (content_sampler_.HasProposal()) {
         VLOG_IF(1, !had_proposal) << "Content sampler now detects animation.";
         should_sample = content_sampler_.ShouldSample();
         if (should_sample) {
           event_time = content_sampler_.frame_timestamp();
           duration_of_next_frame_ = content_sampler_.sampling_period();
         }
         last_time_animation_was_detected_ = event_time;
       } else {
         VLOG_IF(1, had_proposal) << "Content sampler detects animation ended.";
         should_sample = smoothing_sampler_.ShouldSample();
       }
       break;
     }
 
-    case kTimerPoll:
-      // While the timer is firing, only allow a sampling if there are none
-      // currently in-progress.
-      if (num_frames_pending_ == 0)
-        should_sample = smoothing_sampler_.IsOverdueForSamplingAt(event_time);
-      break;
-
+    case kActiveRefreshRequest:
+    case kPassiveRefreshRequest:
     case kMouseCursorUpdate:
       // Only allow a sampling if there are none currently in-progress.
       if (num_frames_pending_ == 0) {
         smoothing_sampler_.ConsiderPresentationEvent(event_time);
         should_sample = smoothing_sampler_.ShouldSample();
       }
       break;
+
     case kNumEvents:
       NOTREACHED();
       break;
   }
 
   if (!should_sample)
     return false;
 
   // If the exact duration of the next frame has not been determined, estimate
   // it using the difference between the current and last frame.
   if (duration_of_next_frame_.is_zero()) {
     if (next_frame_number_ > 0) {
       duration_of_next_frame_ =
           event_time - GetFrameTimestamp(next_frame_number_ - 1);
     }
-    const base::TimeDelta upper_bound = base::TimeDelta::FromMilliseconds(
-        SmoothEventSampler::OVERDUE_DIRTY_THRESHOLD_MILLIS);
+    const base::TimeDelta upper_bound =
+        base::TimeDelta::FromMilliseconds(kUpperBoundDurationEstimateMicros);
     duration_of_next_frame_ =
         std::max(std::min(duration_of_next_frame_, upper_bound),
                  smoothing_sampler_.min_capture_period());
   }
 
   // Update |capture_size_| and reset all feedback signal accumulators if
   // either: 1) this is the first frame; or 2) |resolution_chooser_| has an
   // updated capture size and sufficient time has passed since the last size
   // change.
   if (next_frame_number_ == 0) {
@@ skipping 125 matching lines @@
   // Translate the utilization metric to be in terms of the capable frame area
   // and update the feedback accumulators.  Research suggests utilization is at
   // most linearly proportional to area, and typically is sublinear.  Either
   // way, the end-to-end system should converge to the right place using the
   // more-conservative assumption (linear).
   const int area_at_full_utilization =
       base::saturated_cast<int>(capture_size_.GetArea() / resource_utilization);
   estimated_capable_area_.Update(area_at_full_utilization, timestamp);
 }
 
+// static
+const char* VideoCaptureOracle::EventAsString(Event event) {
+  switch (event) {
+    case kCompositorUpdate:
+      return "compositor";
+    case kActiveRefreshRequest:
+      return "active_refresh";
+    case kPassiveRefreshRequest:
+      return "passive_refresh";
+    case kMouseCursorUpdate:
+      return "mouse";
+    case kNumEvents:
+      break;
+  }
+  NOTREACHED();
+  return "unknown";
+}
+
 base::TimeTicks VideoCaptureOracle::GetFrameTimestamp(int frame_number) const {
   DCHECK(IsFrameInRecentHistory(frame_number));
   return frame_timestamps_[frame_number % kMaxFrameTimestamps];
 }
 
 void VideoCaptureOracle::SetFrameTimestamp(int frame_number,
                                            base::TimeTicks timestamp) {
   DCHECK(IsFrameInRecentHistory(frame_number));
   frame_timestamps_[frame_number % kMaxFrameTimestamps] = timestamp;
 }
@@ skipping 180 matching lines @@
   // Content is not animating, so permit an immediate increase in the capture
   // area.  This allows the system to quickly improve the quality of
   // non-animating content (frame drops are not much of a concern).
   VLOG(2) << "Proposing a "
           << (100.0 * (increased_area - current_area) / current_area)
           << "% increase in capture area for non-animating content.  :-)";
   return increased_area;
 }
 
 }  // namespace media