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: select next if we are at the of the current segment #1467

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

dzianis-dashkevich
Copy link
Contributor

@dzianis-dashkevich dzianis-dashkevich commented Dec 18, 2023

Description

Assume we have the following playlist:

#EXTINF:6.000,
segment-1.ts
#EXTINF:6.000,
segment-2.ts
#EXTINF:6.000,
segment-3.ts
#EXTINF:6.000,
segment-4.ts
#EXTINF:6.000,
segment-5.ts
#EXTINF:6.000,
segment-6.ts
...

Assume we switch rendition based on bandwidth update.
We are selecting the next segment to load based on the buffered.end (assume current buffered is [0 - 12]).

Currently, we select segment-2.ts, but it is already buffered.

or assume we switch rendition based on a fast quality switch.
We are selecting the next segment to load based on the current time (current time is 12).

Currently, we select segment-2.ts but it is already played.

in both cases, we should select the next segment (segment-3.ts)

Specific Changes proposed

Select the next segment when we are at the exact segment.end boundary.

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 Dec 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2878b27) 86.03% compared to head (e71202e) 86.03%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1467   +/-   ##
=======================================
  Coverage   86.03%   86.03%           
=======================================
  Files          42       42           
  Lines       10712    10715    +3     
  Branches     2462     2464    +2     
=======================================
+ Hits         9216     9219    +3     
  Misses       1496     1496           

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

Copy link
Contributor

@adrums86 adrums86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find.

Copy link
Contributor

@wseymour15 wseymour15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, definitely fixes the issue. +1 after response to nit

src/playlist.js Show resolved Hide resolved
@dzianis-dashkevich dzianis-dashkevich merged commit 7debc17 into main Dec 19, 2023
15 checks passed
@dzianis-dashkevich dzianis-dashkevich deleted the select-next-segment-when-on-the-edge branch December 19, 2023 18:47
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