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): Fix duplicate hinted segments #4258

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions lib/hls/hls_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -1893,9 +1893,20 @@ shaka.hls.HlsParser = class {
const pPreviousReference = i == 0 ?
previousReference : partialSegmentRefs[partialSegmentRefs.length - 1];
const pStartTime = (i == 0) ? startTime : pPreviousReference.endTime;
const pDuration = Number(item.getAttributeValue('DURATION'));
// A preload hinted partial segment doesn't have duration information,
// so its startTime and endTime are the same.

// If DURATION is missing from this partial segment, use the target
// partial duration from the top of the playlist, which is a required
// attribute for content with partial segments.
const pDuration = Number(item.getAttributeValue('DURATION')) ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically this approach means that if a manifest has a tag with DURATION=0, we will ignore that. But I don't know why someone would want to do that, so it's probably fine to not account for it.

this.partialTargetDuration_;

// If for some reason we have neither an explicit duration, nor a target
// partial duration, we should SKIP this partial segment to avoid
// duplicating content in the presentation timeline.
if (!pDuration) {
continue;
}

const pEndTime = pStartTime + pDuration;

let pStartByte = 0;
Expand Down
64 changes: 57 additions & 7 deletions test/hls/hls_live_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,13 @@ describe('HlsParser live', () => {
ManifestParser.verifySegmentIndex(video, [ref1, ref2]);
});

// Test for https://github.com/shaka-project/shaka-player/issues/4223
it('parses streams with partial and preload hinted segments', async () => {
playerInterface.isLowLatencyMode = () => true;
const mediaWithPartialSegments = [
'#EXTM3U\n',
'#EXT-X-TARGETDURATION:5\n',
'#EXT-X-PART-INF:PART-TARGET=1.5\n',
'#EXT-X-MAP:URI="init.mp4",BYTERANGE="616@0"\n',
'#EXT-X-MEDIA-SEQUENCE:0\n',
// ref includes partialRef, partialRef2
Expand Down Expand Up @@ -576,26 +578,73 @@ describe('HlsParser live', () => {
'test:/partial2.mp4', 2, 4, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 200, /* endByte= */ 429);

const ref = makeReference(
'test:/main.mp4', 0, 4, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ 429,
/* timestampOffset= */ 0, [partialRef, partialRef2]);

const partialRef3 = makeReference(
'test:/partial.mp4', 4, 6, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ 209);

// A preload hinted partial segment doesn't have duration information,
// so its startTime and endTime are the same.
const preloadRef = makeReference(
'test:/partial.mp4', 6, 6, /* syncTime= */ null,
'test:/partial.mp4', 6, 7.5, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 210, /* endByte= */ null);

// ref2 is not fully published yet, so it doesn't have a segment uri.
const ref2 = makeReference(
'', 4, 7.5, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ null,
/* timestampOffset= */ 0, [partialRef3, preloadRef]);

const manifest = await parser.start('test:/master', playerInterface);
const video = manifest.variants[0].video;
await video.createSegmentIndex();
ManifestParser.verifySegmentIndex(video, [ref, ref2]);
});

// Test for https://github.com/shaka-project/shaka-player/issues/4223
it('ignores preload hinted segments without target duration', async () => {
playerInterface.isLowLatencyMode = () => true;

// Missing PART-TARGET, so preload hints are skipped.
const mediaWithPartialSegments = [
'#EXTM3U\n',
'#EXT-X-TARGETDURATION:5\n',
'#EXT-X-MAP:URI="init.mp4",BYTERANGE="616@0"\n',
'#EXT-X-MEDIA-SEQUENCE:0\n',
'#EXTINF:4,\n',
// ref1
'main.mp4\n',
// ref2 includes partialRef, but not preloadRef
// partialRef
'#EXT-X-PART:DURATION=2,URI="partial.mp4",BYTERANGE=210@0\n',
// preloadRef
'#EXT-X-PRELOAD-HINT:TYPE=PART,URI="partial.mp4",BYTERANGE-START=210\n',
].join('');

fakeNetEngine
.setResponseText('test:/master', master)
.setResponseText('test:/video', mediaWithPartialSegments)
.setResponseValue('test:/init.mp4', initSegmentData)
.setResponseValue('test:/main.mp4', segmentData)
.setResponseValue('test:/partial.mp4', segmentData)
.setResponseValue('test:/partial2.mp4', segmentData);

const ref = makeReference(
'test:/main.mp4', 0, 4, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ 429,
/* timestampOffset= */ 0, [partialRef, partialRef2]);
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ null,
/* timestampOffset= */ 0, []);

const partialRef = makeReference(
'test:/partial.mp4', 4, 6, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ 209);

// ref2 is not fully published yet, so it doesn't have a segment uri.
const ref2 = makeReference(
'', 4, 6, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ null,
/* timestampOffset= */ 0, [partialRef3, preloadRef]);
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ 209,
/* timestampOffset= */ 0, [partialRef]);

const manifest = await parser.start('test:/master', playerInterface);
const video = manifest.variants[0].video;
Expand All @@ -611,6 +660,7 @@ describe('HlsParser live', () => {
const mediaWithPartialSegments = [
'#EXTM3U\n',
'#EXT-X-TARGETDURATION:5\n',
'#EXT-X-PART-INF:PART-TARGET=1.5\n',
'#EXTINF:4,\n',
'main.mp4\n',
'#EXT-X-PART:DURATION=2,URI="partial.mp4",BYTERANGE=210@0\n',
Expand Down