fix(HLS): Reset textSequenceModeOffset on discontinuity

[willdharris]

Fixes #5168
Fixes #6325
Fixes #5096
Fixes #4828
Fixes #5604

• Currently the offset used for time.periodStart in computeHlsSequenceModeOffset_, is calculated and set at stream start and never updated.
• In MediaSourceEngine the correct offset is actually being calculated on every call to appendBuffer. But when it is applied to the text engine textEngine_.setTimestampOffset, in order to update time.periodStart, it only ever uses the initial calculated offset.
• this.textSequenceModeOffset_ is initiated as a shaka.util.PublicPromise(). The value is updated by calling this.textSequenceModeOffset_.resolve(value). However, once the promise is resolved, calling resolve on it again has no effect. So the first value passed is locked in and used in all subsequent calculations.
• This PR resets this.textSequenceModeOffset_ to a new shaka.util.PublicPromise() after it's previously fulfilled value has been read.

[shaka-bot]

Incremental code coverage: 100.00%

[avelad]

LGTM, but please, add a test to avoid this error in the future.

[littlespex]

@avelad The only way I can think of to properly test this is with a full integration or E2E test. That would require a public stream which we don't have available. Can we merge the fix while we figure out a way to test this?

[avelad]

@littlespex We have https://github.com/shaka-project/shaka-player/blob/main/test/hls/hls_parser_integration.js for HLS integration tests. Since this problem also occurs on VOD, it should be easy to do a test.

[littlespex]

@avelad The issue is we don't have an asset that we can share publicly.

[theodab]

I... think this new approach should be safe? I don't see any obvious problems with it anyway.

I'll try to make a test for this; I might be able to hack some of our existing sample content into a repro. I'll do it as a follow-up PR so that this can be in the nightly build for practical testing ASAP.

[littlespex]

@avelad @theodab @joeyparrish We've added an integration test for this change.

[avelad]

@shaka-bot test

[shaka-bot]

@avelad: Lab tests started with arguments:

• pr=6388

[theodab]

I've run the test locally, and I can confirm that it passes, and stops passing if the fix is disabled (and that if the fix is disabled the third text cue is positioned very incorrectly).

I don't know if how many times the cuechange even was fired is the right thing to test, though. It seems kind of tangential to the actual problem (that the cues are not being inserted with the correct time offsets).
You can keep that check if you want, but it'd be best to also do something like:

    expect(textTrack.cues.length).toBe(3);
    expect(textTrack.cues[0].startTime).toBeCloseTo(0, 0);
    expect(textTrack.cues[0].endTime).toBeCloseTo(2, 0);
    expect(textTrack.cues[1].startTime).toBeCloseTo(2, 0);
    expect(textTrack.cues[1].endTime).toBeCloseTo(4, 0);
    expect(textTrack.cues[2].startTime).toBeCloseTo(6, 0);
    expect(textTrack.cues[2].endTime).toBeCloseTo(8, 0);

To verify that all of the cues are inserted, and that they have the correct offsets.

[littlespex]

Tests updated and unused assets removed.

[avelad]

@shaka-bot test

[shaka-bot]

@avelad: Lab tests started with arguments:

• pr=6388

Joey is out at the moment. I can confirm that the concern he raised has been addressed.