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(HLS): Require SegmentIndex to return independent segments only #5288

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions lib/media/segment_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ goog.provide('shaka.media.SegmentIndex');
goog.provide('shaka.media.SegmentIterator');

goog.require('goog.asserts');
goog.require('shaka.log');
goog.require('shaka.media.SegmentReference');
goog.require('shaka.util.IReleasable');
goog.require('shaka.util.Timer');
Expand Down Expand Up @@ -424,10 +425,17 @@ shaka.media.SegmentIndex = class {

/**
* 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. Returns null if we do not find a segment at the
* the given time, or the nearest independent segment before it.
*
* Like the normal iterator, next() must be called first to get to the first
* element. Returns null if we do not find a segment at the
* requested time.
*
* The first segment returned by the iterator _MUST_ be an independent
* segment. Assumes that only partial references can be dependent, based on
* RFC 8216 rev 13, section 8.1: "Each (non-Partial) Media Segment in a Media
* Playlist will contain at least one independent frame."
*
* @param {number} time
* @return {?shaka.media.SegmentIterator}
* @export
Expand All @@ -447,9 +455,18 @@ shaka.media.SegmentIndex = class {
if (ref && ref.hasPartialSegments()) {
// Look for a partial SegmentReference.
for (let i = ref.partialReferences.length - 1; i >= 0; --i) {
const r = ref.partialReferences[i];
let r = ref.partialReferences[i];
// Note that a segment ends immediately before the end time.
if ((time >= r.startTime) && (time < r.endTime)) {
// Find an independent partial segment by moving backwards.
while (i && !r.isIndependent()) {
i--;
r = ref.partialReferences[i];
}
if (!r.isIndependent()) {
shaka.log.alwaysError('No independent partial segment found!');
return null;
}
// Call to next() should move the partial segment, not the full
// segment.
index++;
Expand Down
11 changes: 0 additions & 11 deletions lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -1262,17 +1262,6 @@ shaka.media.StreamingEngine = class {
mediaState.stream.segmentIndex.getIteratorForTime(presentationTime);
ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
// For LL-HLS we need to find the nearest INDEPENDENT segment
let attempt = 0;
const maxAttempts = 5;
while (ref && !ref.isIndependent() && attempt < maxAttempts) {
const newTime = ref.startTime - 0.1;
mediaState.segmentIterator =
mediaState.stream.segmentIndex.getIteratorForTime(newTime);
ref = mediaState.segmentIterator &&
mediaState.segmentIterator.next().value;
attempt++;
}
}
if (ref == null) {
shaka.log.warning(logPrefix, 'cannot find segment',
Expand Down
52 changes: 52 additions & 0 deletions test/media/segment_index_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,58 @@ describe('SegmentIndex', /** @suppress {accessControls} */ () => {
expect(iterator.current()).toBe(null);
});

describe('getIteratorForTime', () => {
it('begins with an independent partial segment', () => {
// This test contains its own segment refs, which are manipulated to
// look a little different from the more general ones used elsewhere.
// In particular, we mark some partial refs as dependent, and we make
// the second partial list much longer.
const partialRefs1 = [
makeReference(uri(10.15), 10, 15),
makeReference(uri(15.20), 15, 20),
];
partialRefs1[1].markAsNonIndependent();

const partialRefs2 = [
makeReference(uri(20.25), 20, 25),
makeReference(uri(25.30), 25, 30),
makeReference(uri(30.35), 30, 35),
makeReference(uri(35.40), 35, 40),
makeReference(uri(40.45), 40, 45),
makeReference(uri(45.50), 45, 50),
makeReference(uri(50.55), 50, 55),
makeReference(uri(55.60), 55, 60),
makeReference(uri(60.65), 60, 65),
makeReference(uri(65.70), 65, 70),
];
// All but the first:
for (const r of partialRefs2.slice(1)) {
r.markAsNonIndependent();
}

const localInputRefs = [
makeReference(uri(0.10), 0, 10),
makeReference(uri(10.20), 10, 20, partialRefs1),
makeReference(uri(20.70), 20, 70, partialRefs2),
];
const index = new shaka.media.SegmentIndex(localInputRefs);

// This time points to partialRefs1[0], which is independent.
const iterator1 = index.getIteratorForTime(11);
expect(iterator1.next().value).toBe(partialRefs1[0]);

// Even though the time would point to partialRefs1[1], that is not
// independent. So it walks back to partialRefs1[0].
const iterator2 = index.getIteratorForTime(16);
expect(iterator2.next().value).toBe(partialRefs1[0]);

// Even though the time would point to partialRefs2[9], that is not
// independent. So it walks all the way back to partialRefs2[0].
const iterator3 = index.getIteratorForTime(69);
expect(iterator3.next().value).toBe(partialRefs2[0]);
});
});

describe('next', () => {
it('starts with the first segment', () => {
const index = new shaka.media.SegmentIndex(inputRefs);
Expand Down