From e9b0207e849b2bdc809d709e6ad9fa51f2340744 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Thu, 9 Nov 2017 13:58:19 -0800 Subject: [PATCH] Throw out references outside the period When shifting content using presentationTimeOffset, especially in combination with SegmentBase and media segment indexes, there can legitimately be segments which are outside the period bounds. Instead of failing an assertion, throw out the unneeded segments. This also drops some largely unnecessary and confusing warnings. Issue #1098 Change-Id: I2addd6d45f1aaf95a1b981cd9373dd24163c13a9 --- lib/media/segment_index.js | 75 ++++++++++++++-------- test/dash/dash_parser_segment_base_unit.js | 4 +- test/media/segment_index_unit.js | 48 +++++++++++++- 3 files changed, 96 insertions(+), 31 deletions(-) diff --git a/lib/media/segment_index.js b/lib/media/segment_index.js index 60afe48be3..caa1538ae1 100644 --- a/lib/media/segment_index.js +++ b/lib/media/segment_index.js @@ -161,9 +161,9 @@ shaka.media.SegmentIndex.prototype.merge = function(references) { } j++; } else { - // When period is changed, fitSegmentReference will expand the last - // segment to the start of the next period. So, it is valid to have end - // time updated to the last segment reference in a period + // When period is changed, fit() will expand the last segment to the start + // of the next period. So, it is valid to have end time updated to the + // last segment reference in a period. if (Math.abs(r1.endTime - r2.endTime) > 0.1) { goog.asserts.assert(r2.endTime > r1.endTime && i == this.references_.length - 1 && @@ -237,16 +237,18 @@ shaka.media.SegmentIndex.prototype.evict = function(time) { /** * Expands the first SegmentReference so it begins at the start of its Period - * if it already begins close to the start of its Period, and expands or - * contracts the last SegmentReference so it ends at the end of its Period for - * VOD presentations. + * if it already begins close to the start of its Period. + * + * Also expands or contracts the last SegmentReference so it ends at the end of + * its Period. + * + * Do not call on the last period of a live presentation (unknown duration). + * It is okay to call on the other periods of a live presentation, where the + * duration is known and another period has been added. * * @param {?number} periodDuration */ shaka.media.SegmentIndex.prototype.fit = function(periodDuration) { - if (this.references_.length == 0) - return; - /** @const {number} */ var tolerance = shaka.util.ManifestParserUtils.GAP_OVERLAP_TOLERANCE_SECONDS; @@ -255,32 +257,51 @@ shaka.media.SegmentIndex.prototype.fit = function(periodDuration) { goog.asserts.assert(periodDuration != Infinity, 'Period duration must be finite for static content!'); - var lastReference = this.references_[this.references_.length - 1]; + // Trim out references we will never use. + while (this.references_.length) { + var lastReference = this.references_[this.references_.length - 1]; + if (lastReference.startTime >= periodDuration) { + this.references_.pop(); + } else { + break; + } + } + while (this.references_.length) { + var firstReference = this.references_[0]; + if (firstReference.endTime <= 0) { + this.references_.shift(); + } else { + break; + } + } - // Sanity check. - goog.asserts.assert( - lastReference.startTime < periodDuration, - 'lastReference cannot begin after the end of the Period'); - if (lastReference.startTime > periodDuration) return; - - // Log warning if necessary. - if (lastReference.endTime <= periodDuration - tolerance) { - shaka.log.warning( - 'The last segment should not end before the end of the Period.', - lastReference); - } else if (lastReference.endTime >= periodDuration + tolerance) { - shaka.log.warning( - 'The last segment should not end after the end of the Period.', - lastReference); + if (this.references_.length == 0) + return; + + // Adjust the first SegmentReference if the start time is smaller than the gap + // tolerance (including negative). + var firstReference = this.references_[0]; + if (firstReference.startTime < tolerance) { + this.references_[0] = + new shaka.media.SegmentReference( + firstReference.position, + /* startTime */ 0, + firstReference.endTime, + firstReference.getUris, + firstReference.startByte, + firstReference.endByte); } // Adjust the last SegmentReference. + var lastReference = this.references_[this.references_.length - 1]; this.references_[this.references_.length - 1] = new shaka.media.SegmentReference( lastReference.position, - lastReference.startTime, periodDuration, + lastReference.startTime, + /* endTime */ periodDuration, lastReference.getUris, - lastReference.startByte, lastReference.endByte); + lastReference.startByte, + lastReference.endByte); }; diff --git a/test/dash/dash_parser_segment_base_unit.js b/test/dash/dash_parser_segment_base_unit.js index e04b291252..dcc03b4a8e 100644 --- a/test/dash/dash_parser_segment_base_unit.js +++ b/test/dash/dash_parser_segment_base_unit.js @@ -290,8 +290,8 @@ describe('DashParser SegmentBase', function() { }) .then(function() { var reference = video.getSegmentReference(0); - expect(reference.startTime).toEqual(-2); - expect(reference.endTime).toEqual(10); + expect(reference.startTime).toEqual(0); // clamped to 0 by fit() + expect(reference.endTime).toEqual(10); // would be 12 without PTO }) .catch(fail) .then(done); diff --git a/test/media/segment_index_unit.js b/test/media/segment_index_unit.js index 21d27b8cbe..cdb09ec672 100644 --- a/test/media/segment_index_unit.js +++ b/test/media/segment_index_unit.js @@ -153,6 +153,49 @@ describe('SegmentIndex', /** @suppress {accessControls} */ function() { }); }); + describe('fit', function() { + it('drops references which are outside the period bounds', function() { + // These negative numbers can occur due to presentationTimeOffset in DASH. + var references = [ + makeReference(0, -10, -3, uri(0)), + makeReference(1, -3, 4, uri(1)), + makeReference(2, 4, 11, uri(2)), + makeReference(3, 11, 18, uri(3)), + makeReference(4, 18, 25, uri(4)) + ]; + var index = new shaka.media.SegmentIndex(references); + expect(index.references_).toEqual(references); + + index.fit(/* periodDuration */ 15); + var newReferences = [ + /* ref 0 dropped because it ends before the period starts */ + makeReference(1, 0, 4, uri(1)), // start time clamped to 0 + makeReference(2, 4, 11, uri(2)), + makeReference(3, 11, 15, uri(3)) // end time clamped to period + /* ref 4 dropped because it starts after the period ends */ + ]; + expect(index.references_).toEqual(newReferences); + }); + + it('drops references which end exactly at zero', function() { + // The end time is meant to be exclusive, so segments ending at zero + // (after PTO adjustments) should be dropped. + var references = [ + makeReference(0, -10, 0, uri(0)), + makeReference(1, 0, 10, uri(1)) + ]; + var index = new shaka.media.SegmentIndex(references); + expect(index.references_).toEqual(references); + + index.fit(/* periodDuration */ 10); + var newReferences = [ + /* ref 0 dropped because it ends before the period starts (at 0) */ + makeReference(1, 0, 10, uri(1)) + ]; + expect(index.references_).toEqual(newReferences); + }); + }); + describe('merge', function() { it('three references into zero references', function() { var index1 = new shaka.media.SegmentIndex([]); @@ -225,8 +268,9 @@ describe('SegmentIndex', /** @suppress {accessControls} */ function() { ]; var index1 = new shaka.media.SegmentIndex(references1); - // when period is changed, fitSegmentReference will - // expand last segment to the start of the next the period + // When the period is changed, fit() will expand last segment to the start + // of the next the period. This simulates an update in which fit() has + // done that. var references2 = [ makeReference(2, 20, 30, uri(20)), makeReference(3, 30, 50, uri(30))