videojs / http-streaming

HLS, DASH, and future HTTP streaming protocols library for video.js
https://videojs-http-streaming.netlify.app/
Other
2.53k stars 425 forks source link

Fix: CMAF HLS. Source buffer change type is called with wrong codecs sometimes when append segment without init data because of a race condition. #1375

Closed dzianis-dashkevich closed 1 year ago

dzianis-dashkevich commented 1 year ago

Note This is a cherry-pick of commits from the following 2x branch PR with adaptations for the main branch.

Description

We have a race condition issue that breaks playback during quality switch: CleanShot 2023-02-23 at 22 52 47@2x CleanShot 2023-02-23 at 22 55 32@2x

Here is a visual representation of the race condition: CleanShot 2023-02-25 at 23 25 44@2x

As you can see from the visual representation, we have a pending segment request which is from the previous playlist, because the seeking event kick-starts the segment loader before we have a new playlist loaded. But we receive trackInfo after the new playlist is loaded. That means that during codec calculation it will get codecs from the new playlist, while the segment loader is about to append a segment from the previous one. Because initialization data is not added and codecs do not match with the source buffer's current codecs setup - the source buffer fails to append this segment. The problem is reproducible mainly with CMAF HLS since the transport stream is usually self-initialized.

Specific Changes proposed

I was thinking about possible ways to solve this problem. The first idea was the following: Do not load segments while we are waiting for a playlist. Possible solutions could be: Ignore this seeking event after setting the current time after a segment loader resets everything. Visually it would look like this: CleanShot 2023-02-25 at 23 22 27@2x

It works fine and fixes the issue, but! Users will always see a loading spinner during the quality switch because the player removes everything from the source buffer and does not append anything. It will append only after the playlist + first segment are loaded. So, users will lose a seamless quality switching experience. This problem led me to the second idea: Keep segments loading but ensure that the player always adds initialization data after the quality switch, and the player always uses the segment's playlist for source buffer codecs calculation for consistency.

Requirements Checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #1375 (96ef745) into main (1bd22c9) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1375   +/-   ##
=======================================
  Coverage   85.34%   85.34%           
=======================================
  Files          40       40           
  Lines        9953     9957    +4     
  Branches     2311     2313    +2     
=======================================
+ Hits         8494     8498    +4     
  Misses       1459     1459           
Impacted Files Coverage Δ
src/playlist-controller.js 95.18% <100.00%> (+<0.01%) :arrow_up:
src/segment-loader.js 96.38% <100.00%> (+<0.01%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more