Processing segment reachable even after playlist update removes it (#939)

* Change processing segment reference on playlist refresh
* Test for correct segment references on pending segments
* Fix unreachable segment tests after rebase on async monitor buffer change
* Update media index on playlist refreshes for all requests (including syncs)

src/segment-loader.js

@@ -276,8 +276,15 @@ export default class SegmentLoader extends videojs.EventTarget {
 
         this.mediaIndex -= mediaSequenceDiff;
 
-        if (segmentInfo && !segmentInfo.isSyncRequest) {
+        if (segmentInfo) {
           segmentInfo.mediaIndex -= mediaSequenceDiff;
+
+          // we need to update the referenced segment so that timing information is
+          // saved for the new playlist's segment, however, if the segment fell off the
+          // playlist, we can leave the old reference and just lose the timing info
+          if (segmentInfo.mediaIndex >= 0) {
+            segmentInfo.segment = newPlaylist.segments[segmentInfo.mediaIndex];
+          }
         }
 
         this.syncController_.saveExpiredSegmentInfo(oldPlaylist, newPlaylist);
@@ -526,7 +533,9 @@ export default class SegmentLoader extends videojs.EventTarget {
       // The timeline that the segment is in
       timeline: segment.timeline,
       // The expected duration of the segment in seconds
-      duration: segment.duration
+      duration: segment.duration,
+      // retain the segment in case the playlist updates while doing an async process
+      segment
     };
   }
 
@@ -674,7 +683,7 @@ export default class SegmentLoader extends videojs.EventTarget {
       this.sourceUpdater_.remove(0, removeToTime);
     }
 
-    segment = segmentInfo.playlist.segments[segmentInfo.mediaIndex];
+    segment = segmentInfo.segment;
 
     // optionally, request the decryption key
     if (segment.key) {
@@ -756,7 +765,7 @@ export default class SegmentLoader extends videojs.EventTarget {
     }
 
     segmentInfo = this.pendingSegment_;
-    segment = segmentInfo.playlist.segments[segmentInfo.mediaIndex];
+    segment = segmentInfo.segment;
 
     // if a request times out, reset bandwidth tracking
     if (request.timedout) {
@@ -909,7 +918,7 @@ export default class SegmentLoader extends videojs.EventTarget {
     this.state = 'DECRYPTING';
 
     let segmentInfo = this.pendingSegment_;
-    let segment = segmentInfo.playlist.segments[segmentInfo.mediaIndex];
+    let segment = segmentInfo.segment;
 
     if (segment.key) {
       // this is an encrypted segment
@@ -943,7 +952,7 @@ export default class SegmentLoader extends videojs.EventTarget {
     this.state = 'APPENDING';
 
     let segmentInfo = this.pendingSegment_;
-    let segment = segmentInfo.playlist.segments[segmentInfo.mediaIndex];
+    let segment = segmentInfo.segment;
 
     this.syncController_.probeSegmentInfo(segmentInfo);
 

src/sync-controller.js

@@ -214,7 +214,7 @@ export default class SyncController extends videojs.EventTarget {
    * @param {SegmentInfo} segmentInfo - The current active request information
    */
   probeSegmentInfo(segmentInfo) {
-    let segment = segmentInfo.playlist.segments[segmentInfo.mediaIndex];
+    let segment = segmentInfo.segment;
     let timingInfo;
 
     if (segment.map) {
@@ -239,7 +239,7 @@ export default class SyncController extends videojs.EventTarget {
    * @return {object} The start and end time of the current segment in "media time"
    */
   probeMp4Segment_(segmentInfo) {
-    let segment = segmentInfo.playlist.segments[segmentInfo.mediaIndex];
+    let segment = segmentInfo.segment;
     let timescales = mp4probe.timescale(segment.map.bytes);
     let startTime = mp4probe.startTime(timescales, segmentInfo.bytes);
 
@@ -295,7 +295,7 @@ export default class SyncController extends videojs.EventTarget {
    * @param {object} timingInfo - The start and end time of the current segment in "media time"
    */
   calculateSegmentTimeMapping_(segmentInfo, timingInfo) {
-    let segment = segmentInfo.playlist.segments[segmentInfo.mediaIndex];
+    let segment = segmentInfo.segment;
     let mappingObj = this.timelines[segmentInfo.timeline];
 
     if (segmentInfo.timestampOffset !== null) {
@@ -329,7 +329,7 @@ export default class SyncController extends videojs.EventTarget {
    */
   saveDiscontinuitySyncInfo_(segmentInfo) {
     let playlist = segmentInfo.playlist;
-    let segment = playlist.segments[segmentInfo.mediaIndex];
+    let segment = segmentInfo.segment;
 
     // If the current segment is a discontinuity then we know exactly where
     // the start of the range and it's accuracy is 0 (greater accuracy values

test/segment-loader.test.js

@@ -1063,6 +1063,7 @@ function(assert) {
   this.clock.tick(1);
 
   assert.equal(loader.pendingSegment_.uri, '0.ts', 'retrieving first segment');
+  assert.equal(loader.pendingSegment_.segment.uri, '0.ts', 'correct segment reference');
   assert.equal(loader.state, 'WAITING', 'waiting for response');
 
   this.requests[0].response = new Uint8Array(10).buffer;
@@ -1079,9 +1080,173 @@ function(assert) {
   this.clock.tick(1);
 
   assert.equal(loader.pendingSegment_.uri, '1.ts', 'retrieving second segment');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
   assert.equal(loader.state, 'WAITING', 'waiting for response');
 });
 
+QUnit.test('processing segment reachable even after playlist update removes it',
+function(assert) {
+  let playlist = playlistWithDuration(40);
+
+  playlist.endList = false;
+
+  loader.playlist(playlist);
+  loader.mimeType(this.mimeType);
+  loader.load();
+  this.clock.tick(1);
+
+  assert.equal(loader.state, 'WAITING', 'in waiting state');
+  assert.equal(loader.pendingSegment_.uri, '0.ts', 'first segment pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '0.ts', 'correct segment reference');
+
+  // wrap up the first request to set mediaIndex and start normal live streaming
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+  mediaSource.sourceBuffers[0].buffered = videojs.createTimeRanges([[0, 10]]);
+  mediaSource.sourceBuffers[0].trigger('updateend');
+  this.clock.tick(1);
+
+  assert.equal(loader.state, 'WAITING', 'in waiting state');
+  assert.equal(loader.pendingSegment_.uri, '1.ts', 'second segment pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
+
+  // playlist updated during waiting
+  let playlistUpdated = playlistWithDuration(40);
+
+  playlistUpdated.segments.shift();
+  playlistUpdated.segments.shift();
+  playlistUpdated.mediaSequence += 2;
+  loader.playlist(playlistUpdated);
+
+  assert.equal(loader.pendingSegment_.uri, '1.ts', 'second segment still pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
+
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+
+  // we need to check for the right state, as normally handleResponse would throw an
+  // error under failing cases, but sinon swallows it as part of fake XML HTTP request's
+  // response
+  assert.equal(loader.state, 'APPENDING', 'moved to appending state');
+  assert.equal(loader.pendingSegment_.uri, '1.ts', 'still using second segment');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
+});
+
+QUnit.test('saves segment info to new segment after playlist refresh',
+function(assert) {
+  let playlist = playlistWithDuration(40);
+
+  playlist.endList = false;
+
+  loader.playlist(playlist);
+  loader.mimeType(this.mimeType);
+  loader.load();
+  this.clock.tick(1);
+
+  assert.equal(loader.state, 'WAITING', 'in waiting state');
+  assert.equal(loader.pendingSegment_.uri, '0.ts', 'first segment pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '0.ts', 'correct segment reference');
+
+  // wrap up the first request to set mediaIndex and start normal live streaming
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+  mediaSource.sourceBuffers[0].buffered = videojs.createTimeRanges([[0, 10]]);
+  mediaSource.sourceBuffers[0].trigger('updateend');
+  this.clock.tick(1);
+
+  assert.equal(loader.state, 'WAITING', 'in waiting state');
+  assert.equal(loader.pendingSegment_.uri, '1.ts', 'second segment pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
+
+  // playlist updated during waiting
+  let playlistUpdated = playlistWithDuration(40);
+
+  playlistUpdated.segments.shift();
+  playlistUpdated.mediaSequence++;
+  loader.playlist(playlistUpdated);
+
+  assert.equal(loader.pendingSegment_.uri, '1.ts', 'second segment still pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
+
+  // mock probeSegmentInfo as the response bytes aren't parsable (and won't provide
+  // time info)
+  loader.syncController_.probeSegmentInfo = (segmentInfo) => {
+    segmentInfo.segment.start = 10;
+    segmentInfo.segment.end = 20;
+  };
+
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+
+  assert.equal(playlistUpdated.segments[0].start,
+               10,
+               'set start on segment of new playlist');
+  assert.equal(playlistUpdated.segments[0].end,
+               20,
+               'set end on segment of new playlist');
+  assert.ok(!playlist.segments[1].start, 'did not set start on segment of old playlist');
+  assert.ok(!playlist.segments[1].end, 'did not set end on segment of old playlist');
+});
+
+QUnit.test('saves segment info to old segment after playlist refresh if segment fell off',
+function(assert) {
+  let playlist = playlistWithDuration(40);
+
+  playlist.endList = false;
+
+  loader.playlist(playlist);
+  loader.mimeType(this.mimeType);
+  loader.load();
+  this.clock.tick(1);
+
+  assert.equal(loader.state, 'WAITING', 'in waiting state');
+  assert.equal(loader.pendingSegment_.uri, '0.ts', 'first segment pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '0.ts', 'correct segment reference');
+
+  // wrap up the first request to set mediaIndex and start normal live streaming
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+  mediaSource.sourceBuffers[0].buffered = videojs.createTimeRanges([[0, 10]]);
+  mediaSource.sourceBuffers[0].trigger('updateend');
+  this.clock.tick(1);
+
+  assert.equal(loader.state, 'WAITING', 'in waiting state');
+  assert.equal(loader.pendingSegment_.uri, '1.ts', 'second segment pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
+
+  // playlist updated during waiting
+  let playlistUpdated = playlistWithDuration(40);
+
+  playlistUpdated.segments.shift();
+  playlistUpdated.segments.shift();
+  playlistUpdated.mediaSequence += 2;
+  loader.playlist(playlistUpdated);
+
+  assert.equal(loader.pendingSegment_.uri, '1.ts', 'second segment still pending');
+  assert.equal(loader.pendingSegment_.segment.uri, '1.ts', 'correct segment reference');
+
+  // mock probeSegmentInfo as the response bytes aren't parsable (and won't provide
+  // time info)
+  loader.syncController_.probeSegmentInfo = (segmentInfo) => {
+    segmentInfo.segment.start = 10;
+    segmentInfo.segment.end = 20;
+  };
+
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+
+  assert.equal(playlist.segments[1].start,
+               10,
+               'set start on segment of old playlist');
+  assert.equal(playlist.segments[1].end,
+               20,
+               'set end on segment of old playlist');
+  assert.ok(!playlistUpdated.segments[0].start,
+            'no start info for first segment of new playlist');
+  assert.ok(!playlistUpdated.segments[0].end,
+            'no end info for first segment of new playlist');
+});
+
 QUnit.module('Segment Loading Calculation', {
   beforeEach(assert) {
     this.env = useFakeEnvironment(assert);

test/sync-controller.test.js

@@ -182,13 +182,14 @@ QUnit.test('Correctly updates time mapping and discontinuity info when probing s
       };
     };
 
+    let segment = playlist.segments[0];
     let segmentInfo = {
       mediaIndex: 0,
       playlist,
       timeline: 0,
-      timestampOffset: 0
+      timestampOffset: 0,
+      segment
     };
-    let segment = playlist.segments[0];
 
     syncCon.probeSegmentInfo(segmentInfo);
     assert.ok(syncCon.timelines[0], 'created mapping object for timeline 0');
@@ -203,6 +204,7 @@ QUnit.test('Correctly updates time mapping and discontinuity info when probing s
     segmentInfo.timestampOffset = null;
     segmentInfo.mediaIndex = 1;
     segment = playlist.segments[1];
+    segmentInfo.segment = segment;
 
     syncCon.probeSegmentInfo(segmentInfo);
     assert.equal(segment.start, 10, 'correctly calculated segment start');
@@ -214,6 +216,7 @@ QUnit.test('Correctly updates time mapping and discontinuity info when probing s
     segmentInfo.mediaIndex = 3;
     segmentInfo.timeline = 1;
     segment = playlist.segments[3];
+    segmentInfo.segment = segment;
 
     syncCon.probeSegmentInfo(segmentInfo);
     assert.ok(syncCon.timelines[1], 'created mapping object for timeline 1');