Skip to content

Commit

Permalink
fix(HLS): Fix discontinuity tracking (#4881)
Browse files Browse the repository at this point in the history
Discontinuity tracking was broken and test coverage was insufficient to
catch this. This fixes the parsing and counting of discontinuities, and
replaces two outdated and useless tests with a new one that covers the
counting properly.

One of the old tests was checking that a timestamp offset was set for
each discontinuity, but this had become irrelevant since the test was
written. Discontinuities do not have anything to do with timestamp
offsets in current versions of Shaka Player.

The other old test was checking that after a discontinuity, we didn't
fetch a segment to parse out the timestamp, but we stopped doing that
entirely in v4.

The new test checks that the initial discontinuity sequence number is
honored, and that after a discontinuity, the number goes up. It also
checks that the correct number is extracted after an update. This test
fails without the fix, and passes with it.

This bug affected v4.2.6 and v4.3.2 only.

Issue #4589
  • Loading branch information
joeyparrish authored Jan 10, 2023
1 parent a292711 commit fc3d5c1
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 31 deletions.
2 changes: 1 addition & 1 deletion lib/hls/hls_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -2632,7 +2632,7 @@ shaka.hls.HlsParser = class {
position = mediaSequenceNumber + skippedSegments + i;

const discontinuityTag = shaka.hls.Utils.getFirstTagWithName(
playlist.tags, 'EXT-X-DISCONTINUITY');
item.tags, 'EXT-X-DISCONTINUITY');
if (discontinuityTag) {
discontinuitySequence++;
}
Expand Down
38 changes: 8 additions & 30 deletions test/hls/hls_live_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -550,20 +550,24 @@ describe('HlsParser live', () => {
});
});

it('sets timestamp offset for segments with discontinuity', async () => {
it('sets discontinuity sequence numbers', async () => {
const ref1 = makeReference(
'test:/main.mp4', 0, 2, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ null,
/* timestampOffset= */ 0);
ref1.discontinuitySequence = 30;

// Expect the timestamp offset to be set for the segment after the
// EXT-X-DISCONTINUITY tag.
const ref2 = makeReference(
'test:/main2.mp4', 2, 4, /* syncTime= */ null,
/* baseUri= */ '', /* startByte= */ 0, /* endByte= */ null,
/* timestampOffset= */ 0);
ref2.discontinuitySequence = 31;

await testInitialManifest(master, mediaWithDiscontinuity, [ref1, ref2]);
const manifest = await testInitialManifest(
master, mediaWithDiscontinuity, [ref1, ref2]);

await testUpdate(
manifest, mediaWithUpdatedDiscontinuitySegment, [ref2]);
});

// Test for https://github.com/shaka-project/shaka-player/issues/4223
Expand Down Expand Up @@ -842,32 +846,6 @@ describe('HlsParser live', () => {
shaka.net.NetworkingEngine.RequestType.MANIFEST);
});

it('reuses cached timestamp offset for segments with discontinuity',
async () => {
const ref1 = makeReference(
'test:/main.mp4', 0, 2, /* syncTime= */ null);
const ref2 = makeReference(
'test:/main2.mp4', 2, 4, /* syncTime= */ null);

const manifest = await testInitialManifest(
master, mediaWithDiscontinuity, [ref1, ref2]);

fakeNetEngine.request.calls.reset();
await testUpdate(
manifest, mediaWithUpdatedDiscontinuitySegment, [ref2]);

// Only one request should be made, and it's for the playlist.
// Expect to use the cached timestamp offset for the main2.mp4
// segment, without fetching the start time again.
expect(fakeNetEngine.request).toHaveBeenCalledTimes(1);
fakeNetEngine.expectRequest(
'test:/video',
shaka.net.NetworkingEngine.RequestType.MANIFEST);
fakeNetEngine.expectNoRequest(
'test:/main.mp4',
shaka.net.NetworkingEngine.RequestType.SEGMENT);
});

it('request playlist delta updates to skip segments', async () => {
const mediaWithDeltaUpdates = [
'#EXTM3U\n',
Expand Down

0 comments on commit fc3d5c1

Please sign in to comment.