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: don't clear DASH minimum update period timeout on pause of a media loader #1118

Merged
merged 2 commits into from
Apr 20, 2021

Conversation

gesinger
Copy link
Contributor

@gesinger gesinger commented Apr 9, 2021

Description

Previously, for DASH, pausing a media loader would end up pausing the
main loader as well (via removal of the main loader's minimum update
period). For live streams which required manifest refreshes, this meant
that on media changes, the main loader would be paused and sometimes
never resumed.

This change ensures that when a media loader is paused it won't remove
the main loader's minimum update period timeout.

In addition, a change was made so that loading a main DASH playlist
loader recreates a minimum update period timeout if the main DASH loader
was previously paused (and the timeout cleared).

Requirements Checklist

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

gesinger and others added 2 commits April 9, 2021 13:07
…ia loader

Previously, for DASH, pausing a media loader would end up pausing the
main loader as well (via removal of the main loader's minimum update
period). For live streams which required manifest refreshes, this meant
that on media changes, the main loader would be paused and sometimes
never resumed.

This change ensures that when a media loader is paused it won't remove
the main loader's minimum update period timeout.

In addition, a change was made so that loading a main DASH playlist
loader recreates a minimum update period timeout if the main DASH loader
was previously paused (and the timeout cleared).
@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #1118 (f04173a) into main (aa194d3) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1118      +/-   ##
==========================================
+ Coverage   86.29%   86.32%   +0.02%     
==========================================
  Files          38       38              
  Lines        9068     9072       +4     
  Branches     2052     2054       +2     
==========================================
+ Hits         7825     7831       +6     
+ Misses       1243     1241       -2     
Impacted Files Coverage Δ
src/dash-playlist-loader.js 89.77% <100.00%> (+0.40%) ⬆️
src/source-updater.js 94.83% <0.00%> (+0.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa194d3...f04173a. Read the comment docs.

@gesinger gesinger merged commit 82ff4f5 into main Apr 20, 2021
@gesinger gesinger deleted the fix/dash-loader-mup-removal branch April 20, 2021 15:26
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.

2 participants