From 9dc466475a857041aa485ac7a804aae9de9c63c1 Mon Sep 17 00:00:00 2001 From: Aidan Ridley Date: Wed, 19 May 2021 14:00:05 -0600 Subject: [PATCH] fix: Fix buffering due to re-fetch (SegmentTemplate+duration) (#3419) In DASH with SegmentTemplate and a fixed duration, indexes would continue to grow so long as they had not ended. However, this was based on a callback which captured an end time variable that could become out-of-date in a multi-period live stream. This prevents indexes from continuing to grow indefinitely by always using the most up-to-date end time for the period. This also fixes eviction of segments in the same scenario, which fixes SegmentIterator access and related assertions later. Issue #3354 (partial fix) b/186457868 Change-Id: I2b34ee52dd12b59e1c1237258050b50e3189bee3 --- AUTHORS | 1 + CONTRIBUTORS | 1 + lib/dash/dash_parser.js | 12 +++++++- lib/dash/segment_template.js | 53 +++++++++++++++++++++++++++--------- 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/AUTHORS b/AUTHORS index 288bdfff4e..558ae8f050 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,6 +24,7 @@ Benjamin Wallberg Bonnier Broadcasting <*@bonnierbroadcasting.com> Bryan Huh Code It <*@code-it.fr> +Charter Communications Inc <*@charter.com> Damien Deis Dany L'Hébreux Esteban Dosztal diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 302e241b20..db37181245 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -23,6 +23,7 @@ # Please keep the list sorted. Aaron Vaage +Aidan Ridley Alex Jones Alvaro Velad Galvan Amila Sampath diff --git a/lib/dash/dash_parser.js b/lib/dash/dash_parser.js index 7a3c28fa55..83b18dccb4 100644 --- a/lib/dash/dash_parser.js +++ b/lib/dash/dash_parser.js @@ -65,6 +65,12 @@ shaka.dash.DashParser = class { */ this.segmentIndexMap_ = {}; + /** + * A map of period ids to their durations + * @private {!Object.} + */ + this.periodDurations_ = {}; + /** @private {shaka.util.PeriodCombiner} */ this.periodCombiner_ = new shaka.util.PeriodCombiner(); @@ -570,6 +576,10 @@ shaka.dash.DashParser = class { const period = this.parsePeriod_(context, baseUris, info); periods.push(period); + if (context.period.id && periodDuration) { + this.periodDurations_[context.period.id] = periodDuration; + } + if (periodDuration == null) { if (next) { // If the duration is still null and we aren't at the end, then we @@ -1060,7 +1070,7 @@ shaka.dash.DashParser = class { const hasManifest = !!this.manifest_; streamInfo = shaka.dash.SegmentTemplate.createStreamInfo( context, requestInitSegment, this.segmentIndexMap_, hasManifest, - this.config_.dash.initialSegmentLimit); + this.config_.dash.initialSegmentLimit, this.periodDurations_); } else { goog.asserts.assert(isText, 'Must have Segment* with non-text streams.'); diff --git a/lib/dash/segment_template.js b/lib/dash/segment_template.js index c20eca832b..275993e564 100644 --- a/lib/dash/segment_template.js +++ b/lib/dash/segment_template.js @@ -35,11 +35,12 @@ shaka.dash.SegmentTemplate = class { * @param {boolean} isUpdate True if the manifest is being updated. * @param {number} segmentLimit The maximum number of segments to generate for * a SegmentTemplate with fixed duration. + * @param {!Object.} periodDurationMap * @return {shaka.dash.DashParser.StreamInfo} */ static createStreamInfo( context, requestInitSegment, segmentIndexMap, isUpdate, - segmentLimit) { + segmentLimit, periodDurationMap) { goog.asserts.assert(context.representation.segmentTemplate, 'Should only be called with SegmentTemplate'); const SegmentTemplate = shaka.dash.SegmentTemplate; @@ -77,7 +78,8 @@ shaka.dash.SegmentTemplate = class { return { generateSegmentIndex: () => { return SegmentTemplate.generateSegmentIndexFromDuration_( - shallowCopyOfContext, info, segmentLimit, initSegmentReference); + shallowCopyOfContext, info, segmentLimit, initSegmentReference, + periodDurationMap); }, }; } else { @@ -259,11 +261,12 @@ shaka.dash.SegmentTemplate = class { * @param {shaka.dash.SegmentTemplate.SegmentTemplateInfo} info * @param {number} segmentLimit The maximum number of segments to generate. * @param {shaka.media.InitSegmentReference} initSegmentReference + * @param {!Object.} periodDurationMap * @return {!Promise.} * @private */ static generateSegmentIndexFromDuration_( - context, info, segmentLimit, initSegmentReference) { + context, info, segmentLimit, initSegmentReference, periodDurationMap) { goog.asserts.assert(info.mediaTemplate, 'There should be a media template with duration'); @@ -275,9 +278,20 @@ shaka.dash.SegmentTemplate = class { // Capture values that could change as the parsing context moves on to // other parts of the manifest. const periodStart = context.periodInfo.start; - const periodDuration = context.periodInfo.duration; - const periodEnd = periodDuration ? - periodStart + periodDuration : Infinity; + const periodId = context.period.id; + const initialPeriodDuration = context.periodInfo.duration; + + // For multi-period live streams the period duration may not be known until + // the following period appears in an updated manifest. periodDurationMap + // provides the updated period duration. + const getPeriodEnd = () => { + const periodDuration = + (periodId != null && periodDurationMap[periodId]) || + initialPeriodDuration; + const periodEnd = periodDuration ? + (periodStart + periodDuration) : Infinity; + return periodEnd; + }; const segmentDuration = info.segmentDuration; goog.asserts.assert( @@ -304,7 +318,7 @@ shaka.dash.SegmentTemplate = class { Math.min( presentationTimeline.getSegmentAvailabilityEnd(), - periodEnd), + getPeriodEnd()), ]; }; @@ -378,7 +392,8 @@ shaka.dash.SegmentTemplate = class { const segmentStart = segmentPeriodTime + periodStart; // Cap the segment end at the period end so that references from the // next period will fit neatly after it. - const segmentEnd = Math.min(segmentStart + segmentDuration, periodEnd); + const segmentEnd = Math.min(segmentStart + segmentDuration, + getPeriodEnd()); // This condition will be true unless the segmentStart was >= periodEnd. // If we've done the position calculations correctly, this won't happen. @@ -394,7 +409,7 @@ shaka.dash.SegmentTemplate = class { initSegmentReference, timestampOffset, /* appendWindowStart= */ periodStart, - /* appendWindowEnd= */ periodEnd); + /* appendWindowEnd= */ getPeriodEnd()); }; for (let position = minPosition; position <= maxPosition; ++position) { @@ -407,7 +422,15 @@ shaka.dash.SegmentTemplate = class { // If the availability timeline currently ends before the period, we will // need to add references over time. - if (presentationTimeline.getSegmentAvailabilityEnd() < periodEnd) { + const willNeedToAddReferences = + presentationTimeline.getSegmentAvailabilityEnd() < getPeriodEnd(); + + // When we start a live stream with a period that ends within the + // availability window we will not need to add more references, but we will + // need to evict old references. + const willNeedToEvictReferences = presentationTimeline.isLive(); + + if (willNeedToAddReferences || willNeedToEvictReferences) { // The period continues to get longer over time, so check for new // references once every |segmentDuration| seconds. // We clamp to |minPosition| in case the initial range was reversed and no @@ -416,7 +439,9 @@ shaka.dash.SegmentTemplate = class { let nextPosition = Math.max(minPosition, maxPosition + 1); segmentIndex.updateEvery(segmentDuration, () => { // Evict any references outside the window. - segmentIndex.evict(presentationTimeline.getSegmentAvailabilityStart()); + const availabilityStartTime = + presentationTimeline.getSegmentAvailabilityStart(); + segmentIndex.evict(availabilityStartTime); // Compute any new references that need to be added. const [_, maxPosition] = computeAvailablePositionRange(); @@ -426,8 +451,10 @@ shaka.dash.SegmentTemplate = class { references.push(reference); nextPosition++; } - if (presentationTimeline.getSegmentAvailabilityEnd() >= periodEnd && - !references.length) { + + // The timer must continue firing until the entire period is + // unavailable, so that all references will be evicted. + if (availabilityStartTime > getPeriodEnd() && !references.length) { // Signal stop. return null; }