Skip to content

Commit

Permalink
Throw out references outside the period
Browse files Browse the repository at this point in the history
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
  • Loading branch information
joeyparrish committed Nov 10, 2017
1 parent d90821a commit e9b0207
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 31 deletions.
75 changes: 48 additions & 27 deletions lib/media/segment_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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;

Expand All @@ -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);
};


Expand Down
4 changes: 2 additions & 2 deletions test/dash/dash_parser_segment_base_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
48 changes: 46 additions & 2 deletions test/media/segment_index_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit e9b0207

Please sign in to comment.