fix: allow buffer removes when there's no current media info in loader (

#1070)

Previously, if there wasn't any current media info in the loader, then a
remove of buffered contents would not be processed. This would
intuitively make sense, as if there wasn't current media info, then in
theory there shouldn't be any buffered contents. However, it is possible
for there to be buffered contents without current media info in the event
of a non destructive rendition switch. This was noticed when switching
renditions, expecting a fast quality change to clear the contents and
continue playback at a new rendition. Sometimes the remove wouldn't
happen, so the content would continue to play back the old buffered
content from a different rendition. This fix instead allows the removes
so long as there is starting media info (which is available after first
segment download).

src/master-playlist-controller.js

@@ -750,6 +750,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
  */
  fastQualityChange_(media = this.selectPlaylist()) {
  if (media === this.masterPlaylistLoader_.media()) {
+ this.logger_('skipping fastQualityChange because new media is same as old');
  return;
  }
 

src/segment-loader.js

@@ -1096,7 +1096,8 @@ export default class SegmentLoader extends videojs.EventTarget {
  end = this.duration_();
  }
 
- if (!this.sourceUpdater_ || !this.currentMediaInfo_) {
+ if (!this.sourceUpdater_ || !this.startingMediaInfo_) {
+ this.logger_('skipping remove because no source updater or starting media info');
  // nothing to remove if we haven't processed any media
  return;
  }
@@ -1115,7 +1116,15 @@ export default class SegmentLoader extends videojs.EventTarget {
  this.sourceUpdater_.removeAudio(start, end, removeFinished);
  }
 
- if (this.loaderType_ === 'main' && this.currentMediaInfo_ && this.currentMediaInfo_.hasVideo) {
+ // While it would be better to only remove video if the main loader has video, this
+ // should be safe with audio only as removeVideo will call back even if there's no
+ // video buffer.
+ //
+ // In theory we can check to see if there's video before calling the remove, but in
+ // the event that we're switching between renditions and from video to audio only
+ // (when we add support for that), we may need to clear the video contents despite
+ // what the new media will contain.
+ if (this.loaderType_ === 'main') {
  this.gopBuffer_ = removeGopBuffer(this.gopBuffer_, start, end, this.timeMapping_);
  removesRemaining++;
  this.sourceUpdater_.removeVideo(start, end, removeFinished);

test/master-playlist-controller.test.js

@@ -534,7 +534,7 @@ QUnit.test('resets everything for a fast quality change', function(assert) {
  origRemove.call(segmentLoader, start, end);
  };
 
- segmentLoader.currentMediaInfo_ = { hasVideo: true };
+ segmentLoader.startingMediaInfo_ = { hasVideo: true };
  segmentLoader.audioDisabled_ = true;
 
  segmentLoader.sourceUpdater_.removeVideo = function(start, end) {

test/segment-loader.test.js

@@ -2556,6 +2556,44 @@ QUnit.module('SegmentLoader', function(hooks) {
  });
  });
 
+ QUnit.test('does not remove until starting media info', function(assert) {
+ let audioRemoves = 0;
+ let videoRemoves = 0;
+
+ return setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {
+ const playlist = playlistWithDuration(40);
+
+ loader.playlist(playlist);
+ loader.load();
+ this.clock.tick(1);
+
+ loader.sourceUpdater_.removeAudio = (start, end) => {
+ audioRemoves++;
+ };
+ loader.sourceUpdater_.removeVideo = (start, end) => {
+ videoRemoves++;
+ };
+
+ // segment is requested but not yet downloaded, therefore there's no starting
+ // media info
+ //
+ // callback won't be called
+ loader.remove(0, 100, () => {});
+ assert.equal(audioRemoves, 0, 'no audio removes');
+ assert.equal(videoRemoves, 0, 'no video removes');
+
+ standardXHRResponse(this.requests.shift(), muxedSegment());
+ return new Promise((resolve, reject) => {
+ loader.one('appended', resolve);
+ loader.one('error', reject);
+ });
+ }).then(() => {
+ loader.remove(0, 100, () => {});
+ assert.equal(audioRemoves, 1, 'one audio remove');
+ assert.equal(videoRemoves, 1, 'one video remove');
+ });
+ });
+
  QUnit.test('triggers appenderror when append errors', function(assert) {
 
  return setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {
@@ -4176,8 +4214,7 @@ QUnit.module('SegmentLoader: FMP4', function(hooks) {
  endTime: 2,
  text: 'test'
  });
- // set currentMediaInfo_
- loader.currentMediaInfo_ = {hasVideo: true, hasAudio: true};
+ loader.startingMediaInfo_ = {hasVideo: true, hasAudio: true};
  loader.remove(0, 2);
  assert.equal(this.inbandTextTracks.CC1.cues.length, 0, 'all cues have been removed');