Use maximum capacity instead of a ratio of capacity for BufferedResourceLoader.

webkit/media/buffered_resource_loader.cc

 // Copyright (c) 2012 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 "webkit/media/buffered_resource_loader.h"
 
 #include "base/bits.h"
 #include "base/callback_helpers.h"
 #include "base/format_macros.h"
 #include "base/metrics/histogram.h"
@@ skipping 35 matching lines @@
 // effectively the largest single read the code path can handle.
 // 20MB is an arbitrary limit; it just seems to be "good enough" in practice.
 static const int kMaxBufferCapacity = 20 * kMegabyte;
 
 // Maximum number of bytes outside the buffer we will wait for in order to
 // fulfill a read. If a read starts more than 2MB away from the data we
 // currently have in the buffer, we will not wait for buffer to reach the read's
 // location and will instead reset the request.
 static const int kForwardWaitThreshold = 2 * kMegabyte;
 
-// The lower bound on our buffer (expressed as a fraction of the buffer size)
-// where we'll disable deferring and continue downloading data.
-//
-// TODO(scherkus): refer to http://crbug.com/124719 for more discussion on
-// how we could improve our buffering logic.
-static const double kDisableDeferThreshold = 0.9;
-
 // Computes the suggested backward and forward capacity for the buffer
 // if one wants to play at |playback_rate| * the natural playback speed.
 // Use a value of 0 for |bitrate| if it is unknown.
 static void ComputeTargetBufferWindow(float playback_rate, int bitrate,
                                       int* out_backward_capacity,
                                       int* out_forward_capacity) {
   static const int kDefaultBitrate = 200 * 1024 * 8;  // 200 Kbps.
   static const int kMaxBitrate = 20 * kMegabyte * 8;  // 20 Mbps.
   static const float kMaxPlaybackRate = 25.0;
   static const int kTargetSecondsBufferedAhead = 10;
@@ skipping 495 matching lines @@
 }
 
 bool BufferedResourceLoader::DidPassCORSAccessCheck() const {
   DCHECK(start_cb_.is_null())
       << "Start() must complete before calling DidPassCORSAccessCheck()";
   return !loader_failed_ && cors_mode_ != kUnspecified;
 }
 
 void BufferedResourceLoader::UpdateDeferStrategy(DeferStrategy strategy) {
   if (!might_be_reused_from_cache_in_future_ && strategy == kNeverDefer)
-    strategy = kThresholdDefer;
+    strategy = kCapacityDefer;
   defer_strategy_ = strategy;
   UpdateDeferBehavior();
 }
 
 void BufferedResourceLoader::SetPlaybackRate(float playback_rate) {
   playback_rate_ = playback_rate;
 
   // This is a pause so don't bother updating the buffer window as we'll likely
   // get unpaused in the future.
   if (playback_rate_ == 0.0)
@@ skipping 46 matching lines @@
 
   switch(defer_strategy_) {
     // Never defer at all, so never enable defer.
     case kNeverDefer:
       return false;
 
     // Defer if nothing is being requested.
     case kReadThenDefer:
       return read_cb_.is_null();
 
-    // Defer if we've reached the max capacity of the threshold.
-    case kThresholdDefer:
+    // Defer if we've reached max capacity.
+    case kCapacityDefer:
       return buffer_.forward_bytes() >= buffer_.forward_capacity();
   }
   // Otherwise don't enable defer.
   return false;
 }
 
 bool BufferedResourceLoader::ShouldDisableDefer() const {

[Ami GONE FROM CHROMIUM, 2012/07/11 05:05:58]

Didn't notice until now, but it's hilarious that we have ShouldEnable and
ShouldDisable and they each return a bool (and have very similar but inverted
logic).  IWBN to collapse them into a single ShouldDefer().  Another day,
another CL.

[scherkus (not reviewing), 2012/07/11 18:15:38]

lol.

see http://codereview.chromium.org/10694138/

   // If we're not deferring, then disabling makes no sense.
   if (!active_loader_->deferred())
     return false;
 
   switch(defer_strategy_) {
     // Always disable deferring.
     case kNeverDefer:
       return true;
 
     // We have an outstanding read request, and we have not buffered enough
     // yet to fulfill the request; disable defer to get more data.
     case kReadThenDefer:
       return !read_cb_.is_null() && last_offset_ > buffer_.forward_bytes();
 
     // Disable deferring whenever our forward-buffered amount falls beneath our
-    // threshold.
-    //
-    // TODO(scherkus): refer to http://crbug.com/124719 for more discussion on
-    // how we could improve our buffering logic.
-    case kThresholdDefer: {
-      int buffered = buffer_.forward_bytes();
-      int threshold = buffer_.forward_capacity() * kDisableDeferThreshold;
-      return buffered < threshold;
-    }
+    // capacity.
+    case kCapacityDefer:
+      return buffer_.forward_bytes() < buffer_.forward_capacity();
   }
 
   // Otherwise keep deferring.
   return false;
 }
 
 bool BufferedResourceLoader::CanFulfillRead() const {
   // If we are reading too far in the backward direction.
   if (first_offset_ < 0 && (first_offset_ + buffer_.backward_bytes()) < 0)
     return false;
@@ skipping 160 matching lines @@
 
 void BufferedResourceLoader::Log() {
   media_log_->AddEvent(
       media_log_->CreateBufferedExtentsChangedEvent(
           offset_ - buffer_.backward_bytes(),
           offset_,
           offset_ + buffer_.forward_bytes()));
 }
 
 }  // namespace webkit_media