fix: fix repeated segments (#1489)

* chore: add buffered ranges logs after updateend * chore: move media-sequence-sync to a separate module * chore: move media-sequence-sync to a separate module * chore: update logic * chore: update logs * update sync logic * use hls * update logs * fix typo * add media sequence for mpeg-dash * update logging * dont use media sequence for getExpired * fix subtitles * fix tests * fix unit * fix playback unit * update logs * add check for null * fix typo * add description in js doc * fix typo * add more logs --------- Co-authored-by: Dzianis Dashkevich <ddashkevich@brightcove.com>
videojs · Feb 28, 2024 · ed8f6bd · ed8f6bd
1 parent 39a5622
commit ed8f6bd
Show file tree

Hide file tree

Showing 11 changed files with 462 additions and 157 deletions.
diff --git a/src/segment-loader.js b/src/segment-loader.js
@@ -11,7 +11,7 @@ import segmentTransmuxer from './segment-transmuxer';
 import { TIME_FUDGE_FACTOR, timeUntilRebuffer as timeUntilRebuffer_ } from './ranges';
 import { minRebufferMaxBandwidthSelector } from './playlist-selectors';
 import logger from './util/logger';
-import { concatSegments } from './util/segment';
+import {compactSegmentUrlDescription, concatSegments} from './util/segment';
 import {
   createCaptionsTrackIfNotExists,
   addCaptionData,
@@ -678,6 +678,18 @@ export default class SegmentLoader extends videojs.EventTarget {
     }
   }
 
+  /**
+   * TODO: Current sync controller consists of many hls-specific strategies
+   * media sequence sync is also hls-specific, and we would like to be protocol-agnostic on this level
+   * this should be a part of the sync-controller and sync controller should expect different strategy list based on the protocol.
+   *
+   * @return {MediaSequenceSync|null}
+   * @private
+   */
+  get mediaSequenceSync_() {
+    return this.syncController_.getMediaSequenceSync(this.loaderType_);
+  }
+
   createTransmuxer_() {
     return segmentTransmuxer.createTransmuxer({
       remux: false,
@@ -1034,8 +1046,14 @@ export default class SegmentLoader extends videojs.EventTarget {
     }
 
     this.logger_(`playlist update [${oldId} => ${newPlaylist.id || newPlaylist.uri}]`);
-    this.syncController_.updateMediaSequenceMap(newPlaylist, this.currentTime_(), this.loaderType_);
 
+    if (this.mediaSequenceSync_) {
+      this.mediaSequenceSync_.update(newPlaylist, this.currentTime_());
+      this.logger_(`Playlist update:
+currentTime: ${this.currentTime_()}
+bufferedEnd: ${lastBufferedEnd(this.buffered_())}
+`, this.mediaSequenceSync_.diagnostics);
+    }
     // in VOD, this is always a rendition switch (or we updated our syncInfo above)
     // in LIVE, we always want to update with new playlists (including refreshes)
     this.trigger('syncinfoupdate');
@@ -1200,6 +1218,9 @@ export default class SegmentLoader extends videojs.EventTarget {
    */
   resetLoader() {
     this.fetchAtBuffer_ = false;
+    if (this.mediaSequenceSync_) {
+      this.mediaSequenceSync_.resetAppendedStatus();
+    }
     this.resyncLoader();
   }
 
@@ -1216,7 +1237,11 @@ export default class SegmentLoader extends videojs.EventTarget {
     this.partIndex = null;
     this.syncPoint_ = null;
     this.isPendingTimestampOffset_ = false;
-    this.shouldForceTimestampOffsetAfterResync_ = true;
+    // this is mainly to sync timing-info when switching between renditions with and without timestamp-rollover,
+    // so we don't want it for DASH
+    if (this.sourceType_ === 'hls') {
+      this.shouldForceTimestampOffsetAfterResync_ = true;
+    }
     this.callQueue_ = [];
     this.loadQueue_ = [];
     this.metadataQueue_.id3 = [];
@@ -1452,18 +1477,50 @@ export default class SegmentLoader extends videojs.EventTarget {
         next.mediaIndex = this.mediaIndex + 1;
       }
     } else {
-      // Find the segment containing the end of the buffer or current time.
-      const {segmentIndex, startTime, partIndex} = Playlist.getMediaInfoForTime({
-        exactManifestTimings: this.exactManifestTimings,
-        playlist: this.playlist_,
-        currentTime: this.fetchAtBuffer_ ? bufferedEnd : this.currentTime_(),
-        startingPartIndex: this.syncPoint_.partIndex,
-        startingSegmentIndex: this.syncPoint_.segmentIndex,
-        startTime: this.syncPoint_.time
-      });
+      let segmentIndex; let partIndex; let startTime;
+      const targetTime = this.fetchAtBuffer_ ? bufferedEnd : this.currentTime_();
+
+      if (this.mediaSequenceSync_) {
+        this.logger_(`chooseNextRequest_ request after Quality Switch:
+For TargetTime: ${targetTime}.
+CurrentTime: ${this.currentTime_()}
+BufferedEnd: ${bufferedEnd}
+Fetch At Buffer: ${this.fetchAtBuffer_}
+`, this.mediaSequenceSync_.diagnostics);
+      }
 
-      next.getMediaInfoForTime = this.fetchAtBuffer_ ?
-        `bufferedEnd ${bufferedEnd}` : `currentTime ${this.currentTime_()}`;
+      if (this.mediaSequenceSync_ && this.mediaSequenceSync_.isReliable) {
+        const syncInfo = this.getSyncInfoFromMediaSequenceSync_(targetTime);
+
+        if (!syncInfo) {
+          this.logger_('chooseNextRequest_ - no sync info found using media sequence sync');
+          // no match
+          return null;
+        }
+
+        this.logger_(`chooseNextRequest_ mediaSequence syncInfo (${syncInfo.start} --> ${syncInfo.end})`);
+
+        segmentIndex = syncInfo.segmentIndex;
+        partIndex = syncInfo.partIndex;
+        startTime = syncInfo.start;
+      } else {
+        this.logger_('chooseNextRequest_ - fallback to a regular segment selection algorithm, based on a syncPoint.');
+        // fallback
+        const mediaInfoForTime = Playlist.getMediaInfoForTime({
+          exactManifestTimings: this.exactManifestTimings,
+          playlist: this.playlist_,
+          currentTime: targetTime,
+          startingPartIndex: this.syncPoint_.partIndex,
+          startingSegmentIndex: this.syncPoint_.segmentIndex,
+          startTime: this.syncPoint_.time
+        });
+
+        segmentIndex = mediaInfoForTime.segmentIndex;
+        partIndex = mediaInfoForTime.partIndex;
+        startTime = mediaInfoForTime.startTime;
+      }
+
+      next.getMediaInfoForTime = this.fetchAtBuffer_ ? `bufferedEnd ${targetTime}` : `currentTime ${targetTime}`;
       next.mediaIndex = segmentIndex;
       next.startOfSegment = startTime;
       next.partIndex = partIndex;
@@ -1536,6 +1593,47 @@ export default class SegmentLoader extends videojs.EventTarget {
     return this.generateSegmentInfo_(next);
   }
 
+  getSyncInfoFromMediaSequenceSync_(targetTime) {
+    if (!this.mediaSequenceSync_) {
+      return null;
+    }
+
+    // we should pull the target time to the least available time if we drop out of sync for any reason
+    const finalTargetTime = Math.max(targetTime, this.mediaSequenceSync_.start);
+
+    if (targetTime !== finalTargetTime) {
+      this.logger_(`getSyncInfoFromMediaSequenceSync_. Pulled target time from ${targetTime} to ${finalTargetTime}`);
+    }
+
+    const mediaSequenceSyncInfo = this.mediaSequenceSync_.getSyncInfoForTime(finalTargetTime);
+
+    if (!mediaSequenceSyncInfo) {
+      // no match at all
+      return null;
+    }
+
+    if (!mediaSequenceSyncInfo.isAppended) {
+      // has a perfect match
+      return mediaSequenceSyncInfo;
+    }
+
+    // has match, but segment was already appended.
+    // attempt to auto-advance to the nearest next segment:
+    const nextMediaSequenceSyncInfo = this.mediaSequenceSync_.getSyncInfoForTime(mediaSequenceSyncInfo.end);
+
+    if (!nextMediaSequenceSyncInfo) {
+      // no match at all
+      return null;
+    }
+
+    if (nextMediaSequenceSyncInfo.isAppended) {
+      this.logger_('getSyncInfoFromMediaSequenceSync_: We encounter unexpected scenario where next media sequence sync info is also appended!');
+    }
+
+    // got match with the nearest next segment
+    return nextMediaSequenceSyncInfo;
+  }
+
   generateSegmentInfo_(options) {
     const {
       independent,
@@ -2493,7 +2591,9 @@ export default class SegmentLoader extends videojs.EventTarget {
       segmentInfo.timeline > 0;
     const isEndOfTimeline = isEndOfStream || (isWalkingForward && isDiscontinuity);
 
-    this.logger_(`Requesting ${segmentInfoString(segmentInfo)}`);
+    this.logger_(`Requesting
+${compactSegmentUrlDescription(segmentInfo.uri)}
+${segmentInfoString(segmentInfo)}`);
 
     // If there's an init segment associated with this segment, but it is not cached (identified by a lack of bytes),
     // then this init segment has never been seen before and should be appended.
@@ -3020,6 +3120,14 @@ export default class SegmentLoader extends videojs.EventTarget {
 
     const segmentInfo = this.pendingSegment_;
 
+    if (segmentInfo.part && segmentInfo.part.syncInfo) {
+      // low-latency flow
+      segmentInfo.part.syncInfo.markAppended();
+    } else if (segmentInfo.segment.syncInfo) {
+      // normal flow
+      segmentInfo.segment.syncInfo.markAppended();
+    }
+
     // Now that the end of the segment has been reached, we can set the end time. It's
     // best to wait until all appends are done so we're sure that the primary media is
     // finished (and we have its end time).

diff --git a/src/source-updater.js b/src/source-updater.js
@@ -9,7 +9,7 @@ import {getMimeForCodec} from '@videojs/vhs-utils/es/codecs.js';
 import window from 'global/window';
 import toTitleCase from './util/to-title-case.js';
 import { QUOTA_EXCEEDED_ERR } from './error-codes';
-import {createTimeRanges} from './util/vjs-compat';
+import {createTimeRanges, bufferedRangesToString} from './util/vjs-compat';
 
 const bufferTypes = [
   'video',
@@ -314,6 +314,11 @@ const onUpdateend = (type, sourceUpdater) => (e) => {
   // updateend events on source buffers. This does not appear to be in the spec. As such,
   // if we encounter an updateend without a corresponding pending action from our queue
   // for that source buffer type, process the next action.
+  const bufferedRangesForType = sourceUpdater[`${type}Buffered`]();
+  const descriptiveString = bufferedRangesToString(bufferedRangesForType);
+
+  sourceUpdater.logger_(`received "updateend" event for ${type} Source Buffer: `, descriptiveString);
+
   if (sourceUpdater.queuePending[type]) {
     const doneFn = sourceUpdater.queuePending[type].doneFn;