Fire canplaythrough event at the proper time for audio/video

media/base/download_rate_monitor.cc

Index: media/base/download_rate_monitor.cc
diff --git a/media/base/download_rate_monitor.cc b/media/base/download_rate_monitor.cc
new file mode 100644
index 0000000000000000000000000000000000000000..776d44b9bd4afcc9ddc483aed52bfea49bb80239
--- /dev/null
+++ b/media/base/download_rate_monitor.cc
@@ -0,0 +1,213 @@
+// Copyright (c) 2011 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/base/download_rate_monitor.h"
+
+#include "base/bind.h"
+#include "base/time.h"
+
+namespace media {
+
+// Number of samples to use to collect and average for each measurement of
+// download rate.
+static const size_t kNumberOfSamples = 5;
+
+// Minimum number of seconds represented in a sample period.
+static const float kSamplePeriod = 1.0;
+
+DownloadRateMonitor::Sample::Sample() {
+  Reset();
+}
+
+void DownloadRateMonitor::Sample::SetStartPoint(const Point& new_start) {
+  if (!end_.second.is_null())

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

Change to CHECK based on out of band discussion.

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

I actually got rid of SetStartPoint altogether and changed some things around...
Take a look; I don't know if I did the right things! I'll talk to you in person
about it :)

+    DCHECK(new_start <= end_);
+  start_ = new_start;
+}
+
+void DownloadRateMonitor::Sample::SetEndPoint(const Point& new_end) {
+  if (!start_.second.is_null())

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

ditto

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Done.

+    DCHECK(new_end >= start_);
+  end_ = new_end;
+}
+
+float DownloadRateMonitor::Sample::bytes_per_second() const {
+  if (seconds_elapsed() > 0.0 && bytes_downloaded() > 0)

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

This will hide situations like not getting any bytes for 10 seconds. Why do we
want to do this?

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

You're right, we don't want to hide that situation; this should be >= 0.

+    return bytes_downloaded() / seconds_elapsed();
+  return -1.0;
+}
+
+float DownloadRateMonitor::Sample::seconds_elapsed() const {
+  if (start_.second.is_null() || end_.second.is_null())
+    return -1.0;
+  return (end_.second - start_.second).InSecondsF();
+}
+
+int64 DownloadRateMonitor::Sample::bytes_downloaded() const {
+  return end_.first - start_.first;

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

no is_null() checks? I think this breaks if start_ is set, but end_ is not.

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Done.

+}
+
+bool DownloadRateMonitor::Sample::is_null() const {
+  return start_.second.is_null() && end_.second.is_null();
+}
+
+void DownloadRateMonitor::Sample::Reset() {
+  start_ = std::make_pair(0, base::Time());
+  end_ = std::make_pair(0, base::Time());
+}
+
+void DownloadRateMonitor::Sample::RestartAtEndPoint() {
+  start_ = end_;

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

does end_ need to be set to null here?

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

It doesn't *need* to be, but maybe it makes more sense to have it that way.
Done.

+}
+
+DownloadRateMonitor::DownloadRateMonitor()
+    : has_notified_can_play_through_(false),
+      is_downloading_data_(false),
+      total_bytes_(0),
+      buffered_bytes_(0),
+      loaded_(false),
+      bitrate_(0),
+      started_(false) {
+}
+
+void DownloadRateMonitor::Start(
+    const CanPlayThroughCB& canplaythrough_cb, int media_bitrate) {
+  canplaythrough_cb_ = canplaythrough_cb;
+  started_ = true;
+  bitrate_ = media_bitrate;
+  current_sample_.Reset();
+
+  NotifyCanPlayThroughIfNeeded();
+}
+
+void DownloadRateMonitor::SetBufferedBytes(
+    int64 buffered_bytes, const base::Time& timestamp) {
+  if (!started_)
+    return;
+  buffered_bytes_ = buffered_bytes;

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

Probably should add a CHECK for your monotonically nondecreasing constraint
here.

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Done.

+  is_downloading_data_ = true;
+
+  // If the current sample's buffering mark is farther along than the incoming,
+  // invalidate the current sample.
+  if (current_sample_.start_point().first > buffered_bytes)

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

Doesn't this violate your monotonically nondecreasing expectation? Is this for
handling seeks? If so you may want to provide a way to explicitly signal a seek
to this object.

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

I'll answer this slightly out of order :)
On 2011/11/09 00:50:22, acolwell wrote:
More generally, it's for anything that causes the resource loader to be
recreated, which often includes a seek. The resource loader is also recreated
when the video pauses and buffers for a long time then plays again.
Yes... I think I don't want/need that expectation anymore :) It makes sense for
a buffer to drop (like if buffer data gets evicted). The expectation I really
care about is for the timestamps to be monotonically nondecreasing. I modified
the comment to say so.
Now I Stop() the monitor when a Seek starts and Start() again when playback
starts again.

+    current_sample_.Reset();
+
+  Sample::Point latest_point = std::make_pair(buffered_bytes, timestamp);
+  if (current_sample_.is_null())
+    current_sample_.SetStartPoint(latest_point);
+  current_sample_.SetEndPoint(latest_point);
+
+  UpdateSampleWindow();
+  NotifyCanPlayThroughIfNeeded();
+}
+
+void DownloadRateMonitor::SetNetworkActivity(bool is_downloading_data) {
+  // Invalidate the current sample if downloading is going from start to stopped
+  // or vice versa.
+  if (is_downloading_data != is_downloading_data_)

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

consider reversing condition & early return.

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Done.

+    current_sample_.Reset();
+  is_downloading_data_ = is_downloading_data;
+}
+
+int64 DownloadRateMonitor::bytes_downloaded_in_window() const {
+  // There are max |kNumberOfSamples|; might as well recompute each time.

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

remove ;

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Done.

+  int64 total = 0;
+  for (size_t i = 0; i < sample_window_.size(); ++i)
+    total += sample_window_[i].bytes_downloaded();
+  return total;
+}
+
+float DownloadRateMonitor::seconds_elapsed_in_window() const {
+  float total = 0.0;
+  // There are max |kNumberOfSamples|; might as well recompute each time.
+  for (size_t i = 0; i < sample_window_.size(); ++i)
+    total += sample_window_[i].seconds_elapsed();
+  return total;
+}
+
+void DownloadRateMonitor::UpdateSampleWindow() {
+  if (current_sample_.seconds_elapsed() < kSamplePeriod)
+    return;
+
+  // Add latest sample and remove oldest sample.
+  sample_window_.push_back(current_sample_);
+  if (sample_window_.size() > kNumberOfSamples)
+    sample_window_.pop_front();
+
+  // Prepare for next measurement.
+  current_sample_.RestartAtEndPoint();
+}
+
+float DownloadRateMonitor::ApproximateDownloadByteRate() const {
+  // Compute and return the average download byte rate from within the sample
+  // window.
+  // XXX: If the data is arriving really bursty-ly, say getting a big chunk

[scherkus (not reviewing), 2011/11/09 02:55:15]

hmm... hard to say whether we'll see that in practice

we require 5 samples at a minimum, correct?

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

This actually does happen in practice, in the contrived example in the bug
report:
http://flim.org/~kinetik/tests/bug627153/

+  // of data every 5 seconds, then with this implementation it will take 25
+  // seconds until bitrate is calculated ... is that ok?
+  if (sample_window_.size() >= kNumberOfSamples) {
+    DCHECK(seconds_elapsed_in_window() > 0.0);
+    if (seconds_elapsed_in_window() > 0.0)

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

I don't think we need the DCHECK AND the if().

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Without the if, there might be a divide-by-0 in Release mode. 

+      return bytes_downloaded_in_window() / seconds_elapsed_in_window();
+  }
+
+  // Could not determine approximate download byte rate.
+  return -1.0;
+}
+
+bool DownloadRateMonitor::ShouldNotifyCanPlayThrough() {
+  // A stream with |total_bytes_| == 0 means the size of the stream cannot be
+  // determined or is undefined in the streaming case. In these scenarios,
+  // CanPlayThrough should never fire.
+  if (has_notified_can_play_through_ || total_bytes_ == 0)

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

so does this mean that autoplay can't be implemented for live streams?

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

What it means is, one wouldn't be able to implement their own version of
autoplay by waiting for the canplaythrough event on a live stream. Sounds like a
potential issue to raise regarding the spec?

+    return false;
+
+  // If the media is from a local file (|loaded_|) or if all bytes are
+  // buffered, fire CanPlayThrough.
+  if (loaded_ || buffered_bytes_ == total_bytes_)
+    return true;
+
+  // Cannot approximate when the media can play through if bitrate is unknown.
+  if (bitrate_ <= 0)
+    return false;
+
+  float bytes_needed_per_second = bitrate_ / 8;
+  float download_rate = ApproximateDownloadByteRate();
+
+  // If we are downloading at or faster than the media's bitrate, then we can
+  // play through to the end of the media without stopping to buffer.
+  if (download_rate > 0)
+    return download_rate > bytes_needed_per_second;
+
+  // If download rate is unknown, it may be because the media is being
+  // downloaded so fast that it cannot collect an adequate number of samples
+  // before the download gets deferred.
+  //
+  // To catch this case, we also look at how much data is being downloaded
+  // immediately after the download begins.
+  if (sample_window_.size() < kNumberOfSamples) {
+    int64 bytes_downloaded_since_start =
+        bytes_downloaded_in_window() + current_sample_.bytes_downloaded();
+    float seconds_elapsed_since_start =
+        seconds_elapsed_in_window() + current_sample_.seconds_elapsed();
+
+    // If we download 4 seconds of data in less than 2 seconds of time, we're
+    // probably downloading at a fast enough rate that we can play through.
+    // This is an arbitrary metric that will likely need tweaking.
+    if (seconds_elapsed_since_start < 2.0 &&
+        bytes_downloaded_since_start > 4.0 * bytes_needed_per_second) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+void DownloadRateMonitor::NotifyCanPlayThroughIfNeeded() {
+  if (ShouldNotifyCanPlayThrough() && !canplaythrough_cb_.is_null()) {
+    canplaythrough_cb_.Run();
+    has_notified_can_play_through_ = true;
+  }
+}
+
+}  // namespace media