Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix management of dash segment template index references for multiperiod live DASH streams #3419

Closed
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Benjamin Wallberg <[email protected]>
Bonnier Broadcasting <*@bonnierbroadcasting.com>
Bryan Huh <[email protected]>
Code It <*@code-it.fr>
Charter Communications Inc <*@charter.com>
Damien Deis <[email protected]>
Dany L'Hébreux <[email protected]>
Esteban Dosztal <[email protected]>
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
# Please keep the list sorted.

Aaron Vaage <[email protected]>
Aidan Ridley <[email protected]>
Alex Jones <[email protected]>
Alvaro Velad Galvan <[email protected]>
Amila Sampath <[email protected]>
Expand Down
12 changes: 11 additions & 1 deletion lib/dash/dash_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ shaka.dash.DashParser = class {
*/
this.segmentIndexMap_ = {};

/**
* A map of period ids to their durations
* @private {!Object.<string, number>}
*/
this.periodDurations_ = {};

/** @private {shaka.util.PeriodCombiner} */
this.periodCombiner_ = new shaka.util.PeriodCombiner();

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.');
Expand Down
53 changes: 40 additions & 13 deletions lib/dash/segment_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.<string, number>} 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;
Expand Down Expand Up @@ -77,7 +78,8 @@ shaka.dash.SegmentTemplate = class {
return {
generateSegmentIndex: () => {
return SegmentTemplate.generateSegmentIndexFromDuration_(
shallowCopyOfContext, info, segmentLimit, initSegmentReference);
shallowCopyOfContext, info, segmentLimit, initSegmentReference,
periodDurationMap);
},
};
} else {
Expand Down Expand Up @@ -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.<string, number>} periodDurationMap
* @return {!Promise.<shaka.media.SegmentIndex>}
* @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');

Expand All @@ -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(
Expand All @@ -304,7 +318,7 @@ shaka.dash.SegmentTemplate = class {

Math.min(
presentationTimeline.getSegmentAvailabilityEnd(),
periodEnd),
getPeriodEnd()),
];
};

Expand Down Expand Up @@ -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.
Expand All @@ -394,7 +409,7 @@ shaka.dash.SegmentTemplate = class {
initSegmentReference,
timestampOffset,
/* appendWindowStart= */ periodStart,
/* appendWindowEnd= */ periodEnd);
/* appendWindowEnd= */ getPeriodEnd());
};

for (let position = minPosition; position <= maxPosition; ++position) {
Expand All @@ -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
Expand All @@ -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();
Expand All @@ -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;
}
Expand Down
17 changes: 11 additions & 6 deletions lib/media/segment_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,22 +363,25 @@ shaka.media.SegmentIndex = class {

/** @return {!shaka.media.SegmentIterator} */
[Symbol.iterator]() {
return this.getIteratorForTime(0);
const iter = this.getIteratorForTime(0);
goog.asserts.assert(iter != null, 'Iterator for 0 should never be null!');
return iter;
}

/**
* Returns a new iterator that initially points to the segment that contains
* the given time. Like the normal iterator, next() must be called first to
* get to the first element.
* get to the first element. Returns null if we do not find a segment at the
* requested time.
*
* @param {number} time
* @return {!shaka.media.SegmentIterator}
* @return {?shaka.media.SegmentIterator}
* @export
*/
getIteratorForTime(time) {
let index = this.find(time);
if (index == null) {
index = -1;
return null;
} else {
index--;
}
Expand Down Expand Up @@ -497,8 +500,10 @@ shaka.media.SegmentIterator = class {
'Please use SegmentIndex.getIteratorForTime instead of seek().');

const iter = this.segmentIndex_.getIteratorForTime(time);
this.currentPosition_ = iter.currentPosition_;
this.currentPartialPosition_ = iter.currentPartialPosition_;
if (iter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the effect of making seek() equivalent to next() on time values in the future. Although seek() is deprecated, I think this might break the contract of seek() for anyone who is still using it in their applications.

If iter is null, please set currentPosition_ to something large, like Number.MAX_VALUE, to ensure seek() works as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do..

The only use of this seek() function that I could find is in the unit tests, and it doesn't seem like this would be accessible for direct use by applications. Can we just remove it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately, yes, we will remove it. But for backward compatibility reasons, we can't remove it until Shaka Player v4. It's exported to the application, and may be used by private plugins. So for now, we need to make sure it still works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

this.currentPosition_ = iter.currentPosition_;
this.currentPartialPosition_ = iter.currentPartialPosition_;
}
return this.next().value;
}

Expand Down
12 changes: 6 additions & 6 deletions lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -1113,10 +1113,8 @@ shaka.media.StreamingEngine = class {

mediaState.segmentIterator =
mediaState.stream.segmentIndex.getIteratorForTime(time);
const ref = mediaState.segmentIterator.next().value;
if (ref == null) {
shaka.log.warning(logPrefix, 'cannot find segment', 'endTime:', time);
}
const ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
return ref;
} else {
// Nothing is buffered. Start at the playhead time.
Expand All @@ -1135,14 +1133,16 @@ shaka.media.StreamingEngine = class {
if (inaccurateTolerance) {
mediaState.segmentIterator =
mediaState.stream.segmentIndex.getIteratorForTime(lookupTime);
ref = mediaState.segmentIterator.next().value;
ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
}
if (!ref) {
// If we can't find a valid segment with the drifted time, look for a
// segment with the presentation time.
mediaState.segmentIterator =
mediaState.stream.segmentIndex.getIteratorForTime(presentationTime);
ref = mediaState.segmentIterator.next().value;
ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
}
if (ref == null) {
shaka.log.warning(logPrefix, 'cannot find segment',
Expand Down