Implementing preload=metadata for video

webkit/glue/media/buffered_resource_loader.cc

Index: webkit/glue/media/buffered_resource_loader.cc
diff --git a/webkit/glue/media/buffered_resource_loader.cc b/webkit/glue/media/buffered_resource_loader.cc
index c6295bfd12f87e331f2c5fdb74494e50cde6053e..53bfb7bf1c43154c08debb63fb56cda46742a820 100644
--- a/webkit/glue/media/buffered_resource_loader.cc
+++ b/webkit/glue/media/buffered_resource_loader.cc
@@ -11,6 +11,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebKitClient.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebURLError.h"
+#include "webkit/glue/media/buffered_data_source.h"

[acolwell GONE FROM CHROMIUM, 2011/03/25 23:08:14]

Do you still need this?

[vrk (LEFT CHROMIUM), 2011/03/29 00:53:00]

Nope! Removed :)

 #include "webkit/glue/multipart_response_delegate.h"
 #include "webkit/glue/webkit_glue.h"
 
@@ -50,7 +51,7 @@ BufferedResourceLoader::BufferedResourceLoader(
     int64 last_byte_position)
     : buffer_(new media::SeekableBuffer(kBackwardCapcity, kForwardCapacity)),
       deferred_(false),
-      defer_allowed_(true),
+      defer_strategy_(ReadThenDefer),
       completed_(false),
       range_requested_(false),
       partial_response_(false),
@@ -180,10 +181,15 @@ void BufferedResourceLoader::Read(int64 position,
   // If we can serve the request now, do the actual read.
   if (CanFulfillRead()) {
     ReadInternal();
-    DisableDeferIfNeeded();
+    UpdateDeferBehavior();
     return;
   }
 
+  // If you're deferred and you can't fulfill the read because you don't have
+  // enough data, you will never fulfill the read.
+  // Update defer behavior to re-enable deferring if need be.
+  UpdateDeferBehavior();
+
   // If we expected the read request to be fulfilled later, returns
   // immediately and let more data to flow in.
   if (WillFulfillRead())
@@ -199,11 +205,6 @@ int64 BufferedResourceLoader::GetBufferedPosition() {
   return kPositionNotSpecified;
 }
 
-void BufferedResourceLoader::SetAllowDefer(bool is_allowed) {
-  defer_allowed_ = is_allowed;
-  DisableDeferIfNeeded();
-}
-
 int64 BufferedResourceLoader::content_length() {
   return content_length_;
 }
@@ -334,21 +335,19 @@ void BufferedResourceLoader::didReceiveData(
   buffer_->Append(reinterpret_cast<const uint8*>(data), data_length);
 
   // If there is an active read request, try to fulfill the request.
-  if (HasPendingRead() && CanFulfillRead()) {
+  if (HasPendingRead() && CanFulfillRead())
     ReadInternal();
-  } else if (!defer_allowed_) {
-    // If we're not allowed to defer, slide the buffer window forward instead
-    // of deferring.
-    if (buffer_->forward_bytes() > buffer_->forward_capacity()) {
-      size_t excess = buffer_->forward_bytes() - buffer_->forward_capacity();
-      bool success = buffer_->Seek(excess);
-      DCHECK(success);
-      offset_ += first_offset_ + excess;
-    }
-  }
 
   // At last see if the buffer is full and we need to defer the downloading.
-  EnableDeferIfNeeded();
+  UpdateDeferBehavior();
+
+  // Consume excess bytes from our in-memory buffer if necessary.
+  if (buffer_->forward_bytes() > buffer_->forward_capacity()) {

[acolwell GONE FROM CHROMIUM, 2011/03/25 23:08:14]

This looks like a change in behavior. why is this run all the time instead of
only when deferral is not allowed?

[vrk (LEFT CHROMIUM), 2011/03/29 00:53:00]

Ah I meant to tell you about this! Brief storytime:

As you probably know, the SeekableBuffer buffer_ does not actually impose a
limit on the amount of data it holds.

In the olden days, we only had the "ThresholdDefer" behavior, and we would defer
loading whenever we buffered 10mb or more; if there was a little bit more than
10mb in the buffer before we deferred loading, we wouldn't consume the small
amount of extra data.

Then I added the "NeverDefer" behavior. In this strategy, it's super important
to keep consuming the in-memory buffer to keep it at 10mb, otherwise you can end
up storing the whole video in memory. At this point I added the logic: if we
never defer, then we must consume memory when it hits 10mb; otherwise we don't
care to consume when we go over.

Now I have added the 3rd buffering strategy, and it also falls in the, "I don't
care about keeping the in-memory buffer exactly capped at 10mb because I will
defer before it becomes an issue" category. So originally I had some like
"AllowsDefer" method for which NeverDefer returned false and the other things
returned true.

But this is a weird/hard thing to explain, and it seems simpler just to keep a
hard 10mb limit for every strategy. What do you think, though?

[acolwell GONE FROM CHROMIUM, 2011/03/29 05:52:03]

Ok. I think I understand. It sounds reasonable to me.

On 2011/03/29 00:53:00, Victoria Kirst wrote:

+    size_t excess = buffer_->forward_bytes() - buffer_->forward_capacity();
+    bool success = buffer_->Seek(excess);
+    DCHECK(success);
+    offset_ += first_offset_ + excess;
+  }
 
   // Notify that we have received some data.
   NotifyNetworkEvent();
@@ -431,32 +430,80 @@ bool BufferedResourceLoader::HasSingleOrigin() const {
 
 /////////////////////////////////////////////////////////////////////////////
 // Helper methods.
-void BufferedResourceLoader::EnableDeferIfNeeded() {
-  if (!defer_allowed_)
-    return;
+void BufferedResourceLoader::UpdateDeferBehavior() {
+  if ((deferred_ && ShouldDisableDefer()) ||
+      (!deferred_ && ShouldEnableDefer())) {
+    bool eventOccurred = ToggleDeferring();
+    if (eventOccurred)
+      NotifyNetworkEvent();
+  }
+}
 
-  if (!deferred_ &&
-      buffer_->forward_bytes() >= buffer_->forward_capacity()) {
-    deferred_ = true;
+void BufferedResourceLoader::SetDeferStrategy(DeferStrategy strategy) {
+  defer_strategy_ = strategy;
+}
+
+bool BufferedResourceLoader::ShouldEnableDefer() {
+  // If we're already deferring, then enabling makes no sense.
+  if (deferred_)
+    return false;
 
-  if (url_loader_.get())
-    url_loader_->setDefersLoading(true);
+  switch(defer_strategy_) {
+    // Never defer at all, so never enable defer.
+    case NeverDefer:
+      return false;
 
-    NotifyNetworkEvent();
+    // Defer if nothing is being requested.
+    case ReadThenDefer:
+      return !read_callback_.get();
+
+    // Defer if we've reached the max capacity of the threshold.
+    case ThresholdDefer:
+      return buffer_->forward_bytes() >= buffer_->forward_capacity();
   }
+  // Otherwise no longer defer.
+  return false;
 }
 
-void BufferedResourceLoader::DisableDeferIfNeeded() {
-  if (deferred_ &&
-      (!defer_allowed_ ||
-       buffer_->forward_bytes() < buffer_->forward_capacity() / 2)) {
-    deferred_ = false;
+bool BufferedResourceLoader::ShouldDisableDefer() {
+  // If we're not deferring, then disabling makes no sense.
+  if (!deferred_)
+    return false;
 
-    if (url_loader_.get())
-      url_loader_->setDefersLoading(false);
+  switch(defer_strategy_) {
+    // Always disable deferring.
+    case NeverDefer:
+      return true;
+
+    // We have an outstanding read request, and we have not buffered enoug

[acolwell GONE FROM CHROMIUM, 2011/03/25 23:08:14]

enoug -> enough

[vrk (LEFT CHROMIUM), 2011/03/29 00:53:00]

Done.

+    // yet to fulfill the request; disable defer to get more data.
+    case ReadThenDefer: {
+      size_t amount_buffered = buffer_->forward_bytes();
+      size_t amount_to_read = static_cast<size_t>(read_size_);
+      return read_callback_.get() && amount_buffered < amount_to_read;
+    }
 
-    NotifyNetworkEvent();
+    // We have less than half the capacity of our threshold, so
+    // disable defer to get more data.
+    case ThresholdDefer: {
+      size_t amount_buffered = buffer_->forward_bytes();
+      size_t half_capacity = buffer_->forward_capacity() / 2;
+      return amount_buffered < half_capacity;
+    }
   }
+
+  // Otherwise keep deferring.
+  return false;
+}
+
+bool BufferedResourceLoader::ToggleDeferring() {
+    bool newDeferSetting = !deferred_;
+    deferred_ = newDeferSetting;

[acolwell GONE FROM CHROMIUM, 2011/03/25 23:08:14]

deferred_ = !deferred_

[vrk (LEFT CHROMIUM), 2011/03/29 00:53:00]

Done.

+    if (url_loader_.get()) {
+      url_loader_->setDefersLoading(newDeferSetting);

[acolwell GONE FROM CHROMIUM, 2011/03/25 23:08:14]

deferred_

[vrk (LEFT CHROMIUM), 2011/03/29 00:53:00]

Done.

+      return true;
+    }
+    return false;
 }
 
 bool BufferedResourceLoader::CanFulfillRead() {