From 13d29fd06d7cbf14241ea2507114ff34daeb760e Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Tue, 10 Jan 2023 11:36:57 -0800 Subject: [PATCH] fix(HLS): Fix discontinuity tracking (#4881) 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 --- lib/hls/hls_parser.js | 2 +- test/hls/hls_live_unit.js | 38 ++++++++------------------------------ 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/lib/hls/hls_parser.js b/lib/hls/hls_parser.js index 2e5f1685fc..c7bd24e621 100644 --- a/lib/hls/hls_parser.js +++ b/lib/hls/hls_parser.js @@ -2272,7 +2272,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++; } diff --git a/test/hls/hls_live_unit.js b/test/hls/hls_live_unit.js index a19dbbae09..b951d04d5e 100644 --- a/test/hls/hls_live_unit.js +++ b/test/hls/hls_live_unit.js @@ -520,20 +520,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 @@ -725,32 +729,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',