From 7e50866bb539ddd7e70ed6d6c245793b353262a6 Mon Sep 17 00:00:00 2001 From: ismena Date: Tue, 30 Mar 2021 14:04:47 -0700 Subject: [PATCH] fix(dash): Fix stalls on a live dash stream. In live streams, we can evict segments outside the availability window faster than they disappear from the manifest. If that happens, we used to evict them several times (add them back in and then evict again). This caused the eviction counter to increase beyond what it should be and we had trouble finding segments afterwards. Closes #3139. Change-Id: Iafebfaf8e1e9ebb09a64cdf7e09a882115fd8eb6 --- docs/tutorials/manifest-parser.md | 7 ++++--- lib/dash/segment_list.js | 3 +-- lib/dash/segment_template.js | 3 +-- lib/hls/hls_parser.js | 3 ++- lib/media/segment_index.js | 26 ++++++++++++++++++++++++++ test/media/segment_index_unit.js | 25 +++++++++++++++++++++++++ 6 files changed, 59 insertions(+), 8 deletions(-) diff --git a/docs/tutorials/manifest-parser.md b/docs/tutorials/manifest-parser.md index 99139f8c75..a596d0b477 100644 --- a/docs/tutorials/manifest-parser.md +++ b/docs/tutorials/manifest-parser.md @@ -199,9 +199,10 @@ const references = refs.map(function(r) { const index = new shaka.media.SegmentIndex(references); ``` -To merge updates, simply create a new array of segments and call `merge`. Any -existing segments will be updated and new segments will be added. You can also -call `evict` to remove old references to reduce the memory footprint. +To merge updates and remove old references to reduce the memory footprint, +simply create a new array of segments and call `mergeAndEvict`. Any +existing segments will be updated, new segments will be added, and old +unavailable references will be removed. To expand the list of references on a timer, as is done for DASH's SegmentTemplate, call `index.updateEvery` with a callback that evicts old diff --git a/lib/dash/segment_list.js b/lib/dash/segment_list.js index 3fcf5d2041..11a24a8a41 100644 --- a/lib/dash/segment_list.js +++ b/lib/dash/segment_list.js @@ -60,9 +60,8 @@ shaka.dash.SegmentList = class { const isNew = !segmentIndex; if (segmentIndex) { - segmentIndex.merge(references); const start = context.presentationTimeline.getSegmentAvailabilityStart(); - segmentIndex.evict(start); + segmentIndex.mergeAndEvict(references, start); } else { context.presentationTimeline.notifySegments(references); segmentIndex = new shaka.media.SegmentIndex(references); diff --git a/lib/dash/segment_template.js b/lib/dash/segment_template.js index ddf56e5e2c..c20eca832b 100644 --- a/lib/dash/segment_template.js +++ b/lib/dash/segment_template.js @@ -111,8 +111,7 @@ shaka.dash.SegmentTemplate = class { wrapper.fit(periodStart, periodEnd, /* isNew= */ true); } - segmentIndex.merge(references); - segmentIndex.evict( + segmentIndex.mergeAndEvict(references, context.presentationTimeline.getSegmentAvailabilityStart()); } else { context.presentationTimeline.notifySegments(references); diff --git a/lib/hls/hls_parser.js b/lib/hls/hls_parser.js index cc4503e195..6f06803ad0 100644 --- a/lib/hls/hls_parser.js +++ b/lib/hls/hls_parser.js @@ -333,7 +333,8 @@ shaka.hls.HlsParser = class { stream.mimeType, streamInfo.mediaSequenceToStartTime, mediaVariables, streamInfo.discontinuityToMediaSequence); - stream.segmentIndex.merge(segments); + stream.segmentIndex.mergeAndEvict( + segments, this.presentationTimeline_.getSegmentAvailabilityStart()); if (segments.length) { const mediaSequenceNumber = shaka.hls.Utils.getFirstTagWithNameAsNumber( playlist.tags, 'EXT-X-MEDIA-SEQUENCE', 0); diff --git a/lib/media/segment_index.js b/lib/media/segment_index.js index 0ea6085222..61acc14188 100644 --- a/lib/media/segment_index.js +++ b/lib/media/segment_index.js @@ -205,6 +205,7 @@ shaka.media.SegmentIndex = class { if (!references.length) { return; } + this.references = this.references.filter((r) => { return r.startTime < references[0].startTime; }); @@ -216,6 +217,31 @@ shaka.media.SegmentIndex = class { } } + /** + * Merges the given SegmentReferences and evicts the ones that end before the + * given time. Supports extending the original references only. + * Will not replace old references or interleave new ones. + * Used, for example, by the DASH and HLS parser, where manifests may not list + * all available references, so we must keep available references in memory to + * fill the availability window. + * + * @param {!Array.} references The list of + * SegmentReferences, which must be sorted first by their start times + * (ascending) and second by their end times (ascending). + * @param {number} windowStart The start of the availability window to filter + * out the references that are no longer available. + * @export + */ + mergeAndEvict(references, windowStart) { + // FIlter out the references that are no longer available to avoid + // repeatedly evicting them and messing up eviction count. + references = references.filter((r) => { + return r.endTime > windowStart; + }); + + this.merge(references); + this.evict(windowStart); + } /** * Removes all SegmentReferences that end before the given time. diff --git a/test/media/segment_index_unit.js b/test/media/segment_index_unit.js index 2386f7deca..b6502072ed 100644 --- a/test/media/segment_index_unit.js +++ b/test/media/segment_index_unit.js @@ -468,6 +468,31 @@ describe('SegmentIndex', /** @suppress {accessControls} */ () => { }); }); + describe('mergeAndEvict', () => { + it('discards segments that end before the availabilityWindowStart', () => { + /** @type {!Array.} */ + const references1 = [ + // Assuming ref(0, 10) has been already evicted + makeReference(uri(10), 10, 20), + ]; + const index1 = new shaka.media.SegmentIndex(references1); + + /** @type {!Array.} */ + const references2 = [ + makeReference(uri(0), 0, 10), + makeReference(uri(10), 10, 20), + makeReference(uri(20), 20, 30), + ]; + + // The first reference ends before the availabilityWindowStart, so it + // should be discarded. + index1.mergeAndEvict(references2, 19); + expect(index1.references.length).toBe(2); + expect(index1.references[0]).toEqual(references2[1]); + expect(index1.references[1]).toEqual(references2[2]); + }); + }); + describe('evict', () => { /** @type {!shaka.media.SegmentIndex} */ let index1;