Bug 1367950 - Only throttle download of src=url video if the download is 'fast' on desktop. r?jwwang draft
authorChris Pearce <cpearce@mozilla.com>
Fri, 26 May 2017 13:55:48 +1200
changeset 584851 a9f06ce85331229699da5bb927605d56d8f5f0a9
parent 582183 74566d5345f4cab06c5683d4b620124104801e65
child 630535 da5ec74c10649ce232b531620139db8bab6ed6e5
push id60904
push userbmo:cpearce@mozilla.com
push dateFri, 26 May 2017 03:50:47 +0000
Bug 1367950 - Only throttle download of src=url video if the download is 'fast' on desktop. r?jwwang Our canplaythrough logic is opaque to the users, so I expect that our recent change to throttle when we hit the readahead limit would be confusing to users; those on a slow connection would want their media to prebuffer, and not expect the download to stop part way through. They would think that Firefox had stalled at an arbitrary point for some unknown reason, i.e., they'd think Firefox was broken. So I think we're better to instead only throttle if the network is good enough that the user probably doesn't worry about the download not keeping up with playback. We should restore the previous behaviour on mobile of throttling when the download reached the readaheadd limit regardless of canplaythrough or network speed, as the calculus is different on mobile; the user may also be concerned about battery life, or hitting their data cap. And often the faster the cellular network is, the more expensive data on it is. So this patch changes us to throttle when we reach the readahead limit only if the network is fast, where fast is defined as being able to stream at twice the rate estimated to be required to playback without stalling. It also adds a pref to revert to the old behaviour of not considering the network speed, which we enable on mobile to restore it to its previous behaviour. MozReview-Commit-ID: KLIGaQZV6dX
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -1047,24 +1047,47 @@ MediaDecoder::NotifySuspendedStatusChang
   if (mResource) {
     bool suspended = mResource->IsSuspendedByCache();
+  // We throttle the download if either the throttle override pref is set
+  // (so that we can always throttle in Firefox on mobile) or if the download
+  // is fast enough that there's no concern about playback being interrupted.
+  MOZ_ASSERT(NS_IsMainThread());
+  NS_ENSURE_TRUE(mDecoderStateMachine, false);
+  if (Preferences::GetBool("media.throttle-regardless-of-download-rate",
+                           false)) {
+    return true;
+  }
+  MediaStatistics stats = GetStatistics();
+  if (!stats.mDownloadRateReliable || !stats.mPlaybackRateReliable) {
+    return false;
+  }
+  uint32_t factor =
+    std::max(2u, Preferences::GetUint("media.throttle-factor", 2));
+  return stats.mDownloadRate > factor * stats.mPlaybackRate;
-  mResource->ThrottleReadahead(CanPlayThrough());
+  mResource->ThrottleReadahead(ShouldThrottleDownload());
 MediaDecoder::NotifyDownloadEnded(nsresult aStatus)
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -637,16 +637,18 @@ protected:
   // Ensures our media stream has been unpinned.
   void UnpinForSeek();
   const char* PlayStateStr();
   void OnMetadataUpdate(TimedMetadata&& aMetadata);
+  bool ShouldThrottleDownload();
   // This should only ever be accessed from the main thread.
   // It is set in the constructor and cleared in Shutdown when the element goes
   // away. The decoder does not add a reference the element.
   MediaDecoderOwner* mOwner;
   // The AbstractThread from mOwner.
   const RefPtr<AbstractThread> mAbstractMainThread;
--- a/mobile/android/app/mobile.js
+++ b/mobile/android/app/mobile.js
@@ -584,16 +584,19 @@ pref("browser.meta_refresh_when_inactive
 // prevent video elements from preloading too much data
 pref("media.preload.default", 1); // default to preload none
 pref("media.preload.auto", 2);    // preload metadata if preload=auto
 pref("media.cache_size", 32768);    // 32MB media cache
 // Try to save battery by not resuming reading from a connection until we fall
 // below 10s of buffered data.
 pref("media.cache_resume_threshold", 10);
 pref("media.cache_readahead_limit", 30);
+// On mobile we'll throttle the download once the readahead_limit is hit,
+// even if the download is slow. This is to preserve battery and data.
+pref("media.throttle-regardless-of-download-rate", true);
 // Number of video frames we buffer while decoding video.
 // On Android this is decided by a similar value which varies for
 // each OMX decoder |OMX_PARAM_PORTDEFINITIONTYPE::nBufferCountMin|. This
 // number must be less than the OMX equivalent or gecko will think it is
 // chronically starved of video frames. All decoders seen so far have a value
 // of at least 4.
 pref("media.video-queue.default-size", 3);
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -307,16 +307,28 @@ pref("media.cache_size", 512000);
 // When a network connection is suspended, don't resume it until the
 // amount of buffered data falls below this threshold (in seconds).
 pref("media.cache_resume_threshold", 30);
 // Stop reading ahead when our buffered data is this many seconds ahead
 // of the current playback position. This limit can stop us from using arbitrary
 // amounts of network bandwidth prefetching huge videos.
 pref("media.cache_readahead_limit", 60);
+// We'll throttle the download if the download rate is throttle-factor times
+// the estimated playback rate, AND we satisfy the cache readahead_limit
+// above. The estimated playback rate is time_duration/length_in_bytes.
+// This means we'll only throttle the download if there's no concern that
+// throttling would cause us to stop and buffer.
+pref("media.throttle-factor", 2);
+// By default, we'll throttle media download once we've reached the the
+// readahead_limit if the download is fast. This pref toggles the "and the
+// download is fast" check off, so that we can always throttle the download
+// once the readaheadd limit is reached even on a slow network.
+pref("media.throttle-regardless-of-download-rate", false);
 // Master HTML5 media volume scale.
 pref("media.volume_scale", "1.0");
 // Timeout for wakelock release
 pref("media.wakelock_timeout", 2000);
 // Whether we should play videos opened in a "video document", i.e. videos
 // opened as top-level documents, as opposed to inside a media element.