Numerous fixes to audio/video buffered resource loading.

webkit/glue/media/buffered_data_source.cc

Index: webkit/glue/media/buffered_data_source.cc
diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index 4f904426dfb48de15c65d06d5679fa7ea55c7839..43f3f415d51a1cda3f703e3c35e6855c8a33892a 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -61,9 +61,11 @@ BufferedDataSource::BufferedDataSource(
       stopped_on_render_loop_(false),
       media_is_paused_(true),
       media_has_played_(false),
-      preload_(media::METADATA),
+      preload_(media::AUTO),
       using_range_request_(true),
       cache_miss_retries_left_(kNumCacheMissRetries),
+      bitrate_(0),
+      playback_rate_(0.0),
       media_log_(media_log) {
 }
 
@@ -79,6 +81,9 @@ BufferedResourceLoader* BufferedDataSource::CreateResourceLoader(
   return new BufferedResourceLoader(url_,
                                     first_byte_position,
                                     last_byte_position,
+                                    ChooseDeferStrategy(),
+                                    bitrate_,
+                                    playback_rate_,
                                     media_log_);
 }
 
@@ -304,8 +309,6 @@ void BufferedDataSource::RestartLoadingTask() {
   }
 
   loader_ = CreateResourceLoader(read_position_, kPositionNotSpecified);
-  BufferedResourceLoader::DeferStrategy strategy = ChooseDeferStrategy();
-  loader_->UpdateDeferStrategy(strategy);
   loader_->Start(
       NewCallback(this, &BufferedDataSource::PartialReadStartCallback),
       base::Bind(&BufferedDataSource::NetworkEventCallback, this),
@@ -316,6 +319,7 @@ void BufferedDataSource::SetPlaybackRateTask(float playback_rate) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
 
+  playback_rate_ = playback_rate;
   loader_->SetPlaybackRate(playback_rate);
 
   bool previously_paused = media_is_paused_;
@@ -336,28 +340,26 @@ void BufferedDataSource::SetPreloadTask(media::Preload preload) {
 void BufferedDataSource::SetBitrateTask(int bitrate) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
+
+  bitrate_ = bitrate;
   loader_->SetBitrate(bitrate);
 }
 
 BufferedResourceLoader::DeferStrategy
 BufferedDataSource::ChooseDeferStrategy() {
   DCHECK(MessageLoop::current() == render_loop_);
-  // If the user indicates preload=metadata, then just load exactly
-  // what is needed for starting the pipeline and prerolling frames.
-  if (preload_ == media::METADATA && !media_has_played_)
+  // If the page indicated preload=metadata, then load exactly what is needed
+  // needed for starting playback.
+  if (!media_has_played_ && preload_ == media::METADATA)
     return BufferedResourceLoader::kReadThenDefer;
 
-  // In general, we want to try to buffer the entire video when the video
-  // is paused. But we don't want to do this if the video hasn't played yet
-  // and preload!=auto.
-  if (media_is_paused_ &&
-      (preload_ == media::AUTO || media_has_played_)) {
+  // If the playback has started (at which point the preload value is ignored)
+  // and we're paused, then try to load as much as possible.
+  if (media_has_played_ && media_is_paused_)
     return BufferedResourceLoader::kNeverDefer;
-  }
 
-  // When the video is playing, regardless of preload state, we buffer up
-  // to a hard limit and enable/disable deferring when the buffer is
-  // depleted/full.
+  // If playback hasn't started or the page indicated preload=auto,

[vrk (LEFT CHROMIUM), 2011/10/11 18:23:21]

"If playback hasn't started" should be "if video is in the middle of playback"

[scherkus (not reviewing), 2011/10/11 20:13:19]

Done.

+  // buffer enable/disable deferring when the buffer is full/depleted.

[vrk (LEFT CHROMIUM), 2011/10/11 18:23:21]

If playback hasn't started *and* the page indicated preload=auto, the logic has
changed from (before) aggressively download as much of the video as possible to
(in this CL) download only up to a threshold.

Why the change? The spec is shady in the whole preload area but it gives us
permission to load the entire resource
(http://www.w3.org/TR/html5/video.html#attr-media-preload)

Not necessarily opposed, but curious!

[scherkus (not reviewing), 2011/10/11 20:13:19]

kNeverDefer isn't preferable in all cases.

On a video larger than the media cache (i.e., ~50MB), or for a page with many
files, using kNeverDefer will "download" the content hoping that it'll get
cached.

When we actually go to play the content there's an increased risk we'll have an
immediate cache miss and have to re-fetch the media again negating any benefits
from using kNeverDefer.

   return BufferedResourceLoader::kThresholdDefer;
 }
 
@@ -418,7 +420,6 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
   int64 instance_size = loader_->instance_size();
   bool success = error == net::OK;
 
-
   bool initialize_cb_is_null = false;
   {
     base::AutoLock auto_lock(lock_);