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: CMAF HLS. Source buffer change type is called with wrong codecs sometimes when append segment without init data because of a race condition. #1375

Conversation

dzianis-dashkevich
Copy link
Contributor

@dzianis-dashkevich dzianis-dashkevich commented Feb 24, 2023

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

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

@misteroneill misteroneill changed the title Get Codecs: Always use playlist for current pending segment instead o… fix: source buffer change type is called with wrong codecs sometimes because of a race condition Feb 24, 2023
@codecov
Copy link

codecov bot commented Feb 24, 2023

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%) ⬆️
src/segment-loader.js 96.38% <100.00%> (+<0.01%) ⬆️

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

@dzianis-dashkevich dzianis-dashkevich self-assigned this Feb 24, 2023
@dzianis-dashkevich dzianis-dashkevich changed the title fix: source buffer change type is called with wrong codecs sometimes because of a race condition Fix: CMAF HLS. Source buffer change type is called with wrong codecs sometimes when append segment without init data because of a race condition. Feb 26, 2023
@dzianis-dashkevich dzianis-dashkevich merged commit 7c3e08e into videojs:main Feb 27, 2023
@dzianis-dashkevich dzianis-dashkevich deleted the fix-race-condition-issue-during-fast-quality-change-main-branch branch November 30, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants