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: do not call load after mediachange for hls playlist loader #1447

Merged
merged 5 commits into from
Nov 27, 2023

Conversation

dzianis-dashkevich
Copy link
Contributor

Description

During ABR switch or fast quality switch, we call switchMedia_ which will call this.mainPlaylistLoader_.media with a new media. We request a new playlist and trigger a mediachange event right after the request.

In playlist controller we listen to the mediachange and call this.mainPlaylistLoader_.load()

in the load method, we hit the following check

if (media && !media.endList) {
  this.trigger('mediaupdatetimeout');
} else {
  this.trigger('loadedplaylist');
}

So for live, we trigger mediaupdatetimeout , which means that we need to refresh a live playlist, so we start loading the playlist a second time.

So we load the same playlist immediately (right after it was loaded), so we hit playlistunchanged flow.

Specific Changes proposed

This line was introduced mainly to fix dash-specific issues in this PR:
#1339

so the easiest solution is to call it for dash only.

Requirements Checklist

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

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f294133) 86.16% compared to head (944c2c0) 86.15%.

Files Patch % Lines
src/playlist-controller.js 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1447      +/-   ##
==========================================
- Coverage   86.16%   86.15%   -0.01%     
==========================================
  Files          42       42              
  Lines       10613    10614       +1     
  Branches     2445     2446       +1     
==========================================
  Hits         9145     9145              
- Misses       1468     1469       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dzianis-dashkevich dzianis-dashkevich merged commit 28413f8 into main Nov 27, 2023
13 of 15 checks passed
@dzianis-dashkevich dzianis-dashkevich deleted the fix-hls-load-after-media-change branch November 27, 2023 21:11
dzianis-dashkevich added a commit that referenced this pull request Nov 30, 2023
* fix: do not call load after mediachange for hls playlist loader

* chore: bump mux.js 7.0.1 to 7.0.2

* Revert "chore: bump mux.js 7.0.1 to 7.0.2"

This reverts commit de6ff40.

---------

Co-authored-by: Dzianis Dashkevich <[email protected]>
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.

3 participants