Merge pull request #223 from videojs/hotfix/src-changes

Clean up the playlist loader when sources change

src/videojs-hls.js

@@ -10,6 +10,9 @@ var
   // a fudge factor to apply to advertised playlist bitrates to account for
   // temporary flucations in client bandwidth
   bandwidthVariance = 1.1,
+
+  // the amount of time to wait between checking the state of the buffer
+  bufferCheckInterval = 500,
   keyXhr,
   keyFailed,
   resolveUrl;
@@ -36,6 +39,11 @@ videojs.Hls = videojs.Flash.extend({
     this.currentTime = videojs.Hls.prototype.currentTime;
     this.setCurrentTime = videojs.Hls.prototype.setCurrentTime;
 
+    // periodically check if new data needs to be downloaded or
+    // buffered data should be appended to the source buffer
+    this.segmentBuffer_ = [];
+    this.startCheckingBuffer_();
+
     videojs.Hls.prototype.src.call(this, options.source && options.source.src);
   }
 });
@@ -49,7 +57,10 @@ videojs.Hls.GOAL_BUFFER_LENGTH = 30;
 videojs.Hls.prototype.src = function(src) {
   var
     tech = this,
+    player = this.player(),
+    settings = player.options().hls || {},
     mediaSource,
+    oldMediaPlaylist,
     source;
 
   // do nothing if the src is falsey
@@ -115,46 +126,13 @@ videojs.Hls.prototype.src = function(src) {
   // load the MediaSource into the player
   this.mediaSource.addEventListener('sourceopen', videojs.bind(this, this.handleSourceOpen));
 
-  this.player().ready(function() {
-    // do nothing if the tech has been disposed already
-    // this can occur if someone sets the src in player.ready(), for instance
-    if (!tech.el()) {
-      return;
-    }
-    tech.el().vjs_src(source.src);
-  });
-};
-
-videojs.Hls.setMediaIndexForLive = function(selectedPlaylist) {
-  var tailIterator = selectedPlaylist.segments.length,
-      tailDuration = 0,
-      targetTail = (selectedPlaylist.targetDuration || 10) * 3;
-
-  while (tailDuration < targetTail && tailIterator > 0) {
-    tailDuration += selectedPlaylist.segments[tailIterator - 1].duration;
-    tailIterator--;
-  }
-
-  return tailIterator;
-};
-
-videojs.Hls.prototype.handleSourceOpen = function() {
-  // construct the video data buffer and set the appropriate MIME type
-  var
-    player = this.player(),
-    settings = player.options().hls || {},
-    sourceBuffer = this.mediaSource.addSourceBuffer('video/flv; codecs="vp6,aac"'),
-    oldMediaPlaylist;
-
-  this.sourceBuffer = sourceBuffer;
-  sourceBuffer.appendBuffer(this.segmentParser_.getFlvHeader());
-
-  this.mediaIndex = 0;
-
+  // cleanup the old playlist loader, if necessary
   if (this.playlists) {
     this.playlists.dispose();
   }
 
+  this.mediaIndex = 0;
+
   this.playlists = new videojs.Hls.PlaylistLoader(this.src_, settings.withCredentials);
 
   this.playlists.on('loadedmetadata', videojs.bind(this, function() {
@@ -164,11 +142,7 @@ videojs.Hls.prototype.handleSourceOpen = function() {
     setupEvents = function() {
       this.fillBuffer();
 
-      // periodically check if new data needs to be downloaded or
-      // buffered data should be appended to the source buffer
-      player.on('timeupdate', videojs.bind(this, this.fillBuffer));
-      player.on('timeupdate', videojs.bind(this, this.drainBuffer));
-      player.on('waiting', videojs.bind(this, this.drainBuffer));
+
 
       player.trigger('loadedmetadata');
     };
@@ -255,6 +229,39 @@ videojs.Hls.prototype.handleSourceOpen = function() {
     player.trigger('mediachange');
   }));
 
+  this.player().ready(function() {
+    // do nothing if the tech has been disposed already
+    // this can occur if someone sets the src in player.ready(), for instance
+    if (!tech.el()) {
+      return;
+    }
+    tech.el().vjs_src(source.src);
+  });
+};
+
+videojs.Hls.setMediaIndexForLive = function(selectedPlaylist) {
+  var tailIterator = selectedPlaylist.segments.length,
+      tailDuration = 0,
+      targetTail = (selectedPlaylist.targetDuration || 10) * 3;
+
+  while (tailDuration < targetTail && tailIterator > 0) {
+    tailDuration += selectedPlaylist.segments[tailIterator - 1].duration;
+    tailIterator--;
+  }
+
+  return tailIterator;
+};
+
+videojs.Hls.prototype.handleSourceOpen = function() {
+  // construct the video data buffer and set the appropriate MIME type
+  var
+    player = this.player(),
+    sourceBuffer = this.mediaSource.addSourceBuffer('video/flv; codecs="vp6,aac"');
+
+  this.sourceBuffer = sourceBuffer;
+  sourceBuffer.appendBuffer(this.segmentParser_.getFlvHeader());
+
+
   // if autoplay is enabled, begin playback. This is duplicative of
   // code in video.js but is required because play() must be invoked
   // *after* the media source has opened.
@@ -377,12 +384,7 @@ videojs.Hls.prototype.cancelSegmentXhr = function() {
  * Abort all outstanding work and cleanup.
  */
 videojs.Hls.prototype.dispose = function() {
-  var player = this.player();
-
-  // remove event handlers
-  player.off('timeupdate', this.fillBuffer);
-  player.off('timeupdate', this.drainBuffer);
-  player.off('waiting', this.drainBuffer);
+  this.stopCheckingBuffer_();
 
   if (this.playlists) {
     this.playlists.dispose();
@@ -467,6 +469,46 @@ videojs.Hls.prototype.selectPlaylist = function () {
   return resolutionBestVariant || bandwidthBestVariant || sortedPlaylists[0];
 };
 
+/**
+ * Periodically request new segments and append video data.
+ */
+videojs.Hls.prototype.checkBuffer_ = function() {
+  // calling this method directly resets any outstanding buffer checks
+  if (this.checkBufferTimeout_) {
+    window.clearTimeout(this.checkBufferTimeout_);
+    this.checkBufferTimeout_ = null;
+  }
+
+  this.fillBuffer();
+  this.drainBuffer();
+
+  // wait awhile and try again
+  this.checkBufferTimeout_ = window.setTimeout(videojs.bind(this, this.checkBuffer_),
+                                               bufferCheckInterval);
+};
+
+/**
+ * Setup a periodic task to request new segments if necessary and
+ * append bytes into the SourceBuffer.
+ */
+videojs.Hls.prototype.startCheckingBuffer_ = function() {
+  // if the player ever stalls, check if there is video data available
+  // to append immediately
+  this.player().on('waiting', videojs.bind(this, this.drainBuffer));
+
+  this.checkBuffer_();
+};
+
+/**
+ * Stop the periodic task requesting new segments and feeding the
+ * SourceBuffer.
+ */
+videojs.Hls.prototype.stopCheckingBuffer_ = function() {
+  window.clearTimeout(this.checkBufferTimeout_);
+  this.checkBufferTimeout_ = null;
+  this.player().off('waiting', this.drainBuffer);
+};
+
 /**
  * Determines whether there is enough video data currently in the buffer
  * and downloads a new segment if the buffered time is less than the goal.
@@ -481,6 +523,11 @@ videojs.Hls.prototype.fillBuffer = function(offset) {
     segment,
     segmentUri;
 
+  // if a video has not been specified, do nothing
+  if (!player.currentSrc() || !this.playlists) {
+    return;
+  }
+
   // if there is a request already in flight, do nothing
   if (this.segmentXhr_) {
     return;
@@ -488,6 +535,7 @@ videojs.Hls.prototype.fillBuffer = function(offset) {
 
   // if no segments are available, do nothing
   if (this.playlists.state === "HAVE_NOTHING" ||
+      !this.playlists.media() ||
       !this.playlists.media().segments) {
     return;
   }

test/videojs-hls_test.js

@@ -26,6 +26,7 @@ var
   oldMediaSourceOpen,
   oldSegmentParser,
   oldSetTimeout,
+  oldClearTimeout,
   oldSourceBuffer,
   oldFlashSupported,
   oldNativeHlsSupport,
@@ -114,6 +115,16 @@ var
         on: Function.prototype
       };
     };
+  },
+
+  // return an absolute version of a page-relative URL
+  absoluteUrl = function(relativeUrl) {
+    return window.location.origin +
+      (window.location.pathname
+         .split('/')
+         .slice(0, -1)
+         .concat(relativeUrl)
+         .join('/'));
   };
 
 module('HLS', {
@@ -136,6 +147,7 @@ module('HLS', {
     // store functionality that some tests need to mock
     oldSegmentParser = videojs.Hls.SegmentParser;
     oldSetTimeout = window.setTimeout;
+    oldClearTimeout = window.clearTimeout;
 
     oldNativeHlsSupport = videojs.Hls.supportsNativeHls;
 
@@ -156,7 +168,6 @@ module('HLS', {
   },
 
   teardown: function() {
-    player.dispose();
     videojs.Flash.isSupported = oldFlashSupported;
     videojs.MediaSource.open = oldMediaSourceOpen;
     videojs.Hls.SegmentParser = oldSegmentParser;
@@ -164,6 +175,8 @@ module('HLS', {
     videojs.Hls.decrypt = oldDecrypt;
     videojs.SourceBuffer = oldSourceBuffer;
     window.setTimeout = oldSetTimeout;
+    window.clearTimeout = oldClearTimeout;
+    player.dispose();
     xhr.restore();
   }
 });
@@ -207,6 +220,39 @@ test('creates a PlaylistLoader on init', function() {
               'the playlist is selected');
 });
 
+test('re-initializes the playlist loader when switching sources', function() {
+  // source is set
+  player.src({
+    src:'manifest/media.m3u8',
+    type: 'application/vnd.apple.mpegurl'
+  });
+  openMediaSource(player);
+  // loader gets media playlist
+  standardXHRResponse(requests.shift());
+  // request a segment
+  standardXHRResponse(requests.shift());
+  // change the source
+  player.src({
+    src:'manifest/master.m3u8',
+    type: 'application/vnd.apple.mpegurl'
+  });
+  ok(!player.hls.playlists.media(), 'no media playlist');
+  equal(player.hls.playlists.state,
+        'HAVE_NOTHING',
+        'reset the playlist loader state');
+  equal(requests.length, 1, 'requested the new src');
+
+  // buffer check
+  player.hls.checkBuffer_();
+  equal(requests.length, 1, 'did not request a stale segment');
+
+  // sourceopen
+  openMediaSource(player);
+
+  equal(requests.length, 1, 'made one request');
+  ok(requests[0].url.indexOf('master.m3u8') >= 0, 'requested only the new playlist');
+});
+
 test('sets the duration if one is available on the playlist', function() {
   var calls = 0;
   player.duration = function(value) {
@@ -261,9 +307,7 @@ test('starts downloading a segment on loadedmetadata', function() {
   standardXHRResponse(requests[0]);
   standardXHRResponse(requests[1]);
   strictEqual(requests[1].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media-00001.ts',
+              absoluteUrl('manifest/media-00001.ts'),
               'the first segment is requested');
 });
 
@@ -359,14 +403,10 @@ test('downloads media playlists after loading the master', function() {
 
   strictEqual(requests[0].url, 'manifest/master.m3u8', 'master playlist requested');
   strictEqual(requests[1].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media.m3u8',
+              absoluteUrl('manifest/media.m3u8'),
               'media playlist requested');
   strictEqual(requests[2].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media-00001.ts',
+              absoluteUrl('manifest/media-00001.ts'),
               'first segment requested');
 });
 
@@ -390,19 +430,13 @@ test('upshift if initial bandwidth is high', function() {
 
   strictEqual(requests[0].url, 'manifest/master.m3u8', 'master playlist requested');
   strictEqual(requests[1].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media.m3u8',
+              absoluteUrl('manifest/media.m3u8'),
               'media playlist requested');
   strictEqual(requests[2].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media3.m3u8',
+              absoluteUrl('manifest/media3.m3u8'),
               'media playlist requested');
   strictEqual(requests[3].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media3-00001.ts',
+              absoluteUrl('manifest/media3-00001.ts'),
               'first segment requested');
 });
 
@@ -424,27 +458,96 @@ test('dont downshift if bandwidth is low', function() {
 
   strictEqual(requests[0].url, 'manifest/master.m3u8', 'master playlist requested');
   strictEqual(requests[1].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media.m3u8',
+              absoluteUrl('manifest/media.m3u8'),
               'media playlist requested');
   strictEqual(requests[2].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media-00001.ts',
+              absoluteUrl('manifest/media-00001.ts'),
               'first segment requested');
 });
 
-test('timeupdates do not check to fill the buffer until a media playlist is ready', function() {
+test('starts checking the buffer on init', function() {
+  var player, i, calls, callbacks = [], fills = 0, drains = 0;
+  // capture timeouts
+  window.setTimeout = function(callback) {
+    callbacks.push(callback);
+    return callbacks.length - 1;
+  };
+  window.clearTimeout = function(index) {
+    callbacks[index] = Function.prototype;
+  };
+
+  player = createPlayer();
+  player.hls.fillBuffer = function() {
+    fills++;
+  };
+  player.hls.drainBuffer = function() {
+    drains++;
+  };
+  player.src({
+    src: 'manifest/master.m3u8',
+    type: 'application/vnd.apple.mpegurl'
+  });
+  ok(callbacks.length > 0, 'set timeouts');
+
+  // run the initial set of callbacks. this should cause
+  // fill/drainBuffer to be run
+  calls = callbacks.slice();
+  for (i = 0; i < calls.length; i++) {
+    calls[i]();
+  }
+  equal(fills, 1, 'called fillBuffer');
+  equal(drains, 1, 'called drainBuffer');
+
+  player.dispose();
+  // the remaining callbacks do not run any buffer checks
+  calls = callbacks.slice();
+  for (i = 0; i < calls.length; i++) {
+    calls[i]();
+  }
+  equal(fills, 1, 'did not call fillBuffer again');
+  equal(drains, 1, 'did not call drainBuffer again');
+});
+
+test('buffer checks are noops until a media playlist is ready', function() {
   player.src({
     src: 'manifest/media.m3u8',
     type: 'application/vnd.apple.mpegurl'
   });
   openMediaSource(player);
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
+
+  strictEqual(1, requests.length, 'one request was made');
+  strictEqual(requests[0].url, 'manifest/media.m3u8', 'media playlist requested');
+});
+
+test('buffer checks are noops when only the master is ready', function() {
+  player.src({
+    src: 'manifest/master.m3u8',
+    type: 'application/vnd.apple.mpegurl'
+  });
+  openMediaSource(player);
+  standardXHRResponse(requests.shift());
+  standardXHRResponse(requests.shift());
+  // ignore any outstanding segment requests
+  requests.length = 0;
+
+  // load in a new playlist which will cause playlists.media() to be
+  // undefined while it is being fetched
+  player.src({
+    src: 'manifest/master.m3u8',
+    type: 'application/vnd.apple.mpegurl'
+  });
+  openMediaSource(player);
+
+  // respond with the master playlist but don't send the media playlist yet
+  standardXHRResponse(requests.shift());
+  // trigger fillBuffer()
+  player.hls.checkBuffer_();
 
   strictEqual(1, requests.length, 'one request was made');
-  strictEqual('manifest/media.m3u8', requests[0].url, 'media playlist requested');
+  strictEqual(requests[0].url,
+              absoluteUrl('manifest/media.m3u8'),
+              'media playlist requested');
 });
 
 test('calculates the bandwidth after downloading a segment', function() {
@@ -493,7 +596,7 @@ test('selects a playlist after segment downloads', function() {
   player.buffered = function() {
     return videojs.createTimeRange(0, 2);
   };
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
 
   standardXHRResponse(requests[3]);
 
@@ -587,10 +690,7 @@ test('downloads additional playlists if required', function() {
 
   strictEqual(4, requests.length, 'requests were made');
   strictEqual(requests[3].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/' +
-              playlist.uri,
+              absoluteUrl('manifest/' + playlist.uri),
               'made playlist request');
   strictEqual(playlist.uri,
               player.hls.playlists.media().uri,
@@ -766,15 +866,13 @@ test('downloads the next segment if the buffer is getting low', function() {
   player.buffered = function() {
     return videojs.createTimeRange(0, 19.999);
   };
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
 
   standardXHRResponse(requests[2]);
 
   strictEqual(requests.length, 3, 'made a request');
   strictEqual(requests[2].url,
-              window.location.origin +
-              window.location.pathname.split('/').slice(0, -1).join('/') +
-              '/manifest/media-00002.ts',
+              absoluteUrl('manifest/media-00002.ts'),
               'made segment request');
 });
 
@@ -1267,7 +1365,7 @@ test('waits until the buffer is empty before appending bytes at a discontinuity'
   // play to 6s to trigger the next segment request
   currentTime = 6;
   bufferEnd = 10;
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
   strictEqual(aborts, 0, 'no aborts before the buffer empties');
 
   standardXHRResponse(requests.pop());
@@ -1315,7 +1413,7 @@ test('clears the segment buffer on seek', function() {
   // play to 6s to trigger the next segment request
   currentTime = 6;
   bufferEnd = 10;
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
 
   standardXHRResponse(requests.pop());
 
@@ -1365,7 +1463,7 @@ test('continues playing after seek to discontinuity', function() {
 
   currentTime = 1;
   bufferEnd = 10;
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
 
   standardXHRResponse(requests.pop());
 
@@ -1435,10 +1533,7 @@ test('remove event handlers on dispose', function() {
     oldOn.call(player, type, handler);
   };
   player.off = function(type, handler) {
-    // ignore the top-level videojs removals that aren't relevant to HLS
-    if (type && type !== 'dispose') {
-      offhandlers++;
-    }
+    offhandlers++;
     oldOff.call(player, type, handler);
   };
   player.src({
@@ -1452,10 +1547,8 @@ test('remove event handlers on dispose', function() {
 
   player.dispose();
 
-  equal(offhandlers, onhandlers, 'the amount of on and off handlers is the same');
-
-  player.off = oldOff;
-  player.on = oldOn;
+  ok(offhandlers > onhandlers, 'more handlers were removed than were registered');
+  equal(offhandlers - onhandlers, 1, 'one handler was registered during init');
 });
 
 test('aborts the source buffer on disposal', function() {
@@ -1538,7 +1631,7 @@ test('tracks the bytes downloaded', function() {
 
   strictEqual(player.hls.bytesReceived, 17, 'tracked bytes received');
 
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
 
   // transmit some more
   requests[0].response = new ArrayBuffer(5);
@@ -1918,7 +2011,7 @@ test('skip segments if key requests fail more than once', function() {
 
   player.hls.playlists.trigger('loadedplaylist');
 
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
 
   // respond to ts segment
   standardXHRResponse(requests.pop());
@@ -1934,7 +2027,7 @@ test('skip segments if key requests fail more than once', function() {
 
   equal(bytes.length, 1, 'bytes from the ts segments should not be added');
 
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
 
   tags.length = 0;
   tags.push({pts: 0, bytes: 1});
@@ -2106,7 +2199,7 @@ test('treats invalid keys as a key request failure', function() {
   requests.shift().respond(200, null, '');
 
   // the first segment should be dropped and playback moves on
-  player.trigger('timeupdate');
+  player.hls.checkBuffer_();
   equal(bytes.length, 1, 'did not append bytes');
   equal(bytes[0], 'flv', 'appended the flv header');