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: account for difference between duration info in the playlist and the actual duration #1470

Merged
merged 2 commits into from
Dec 27, 2023

Conversation

dzianis-dashkevich
Copy link
Contributor

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

Description

we should account for the difference between the duration info in the manifest and the actual duration of the segment when selecting next segment after quality change.

Assume we have the following playlist:

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-PLAYLIST-TYPE:VOD
#EXT-X-MEDIA-SEQUENCE:0
#EXT-X-TARGETDURATION:10
#EXTINF:10.000,
segment-1.ts
#EXTINF:10.000,
segment-2.ts
#EXTINF:10.000,
segment-3.ts
#EXTINF:10.000,
segment-4.ts
#EXT-X-ENDLIST

Assume we already appended two segments, so bufferend.end should be 20, but we don't have an exact duration match between the playlist and the actual segment duration, so we receive it as 19.99999...
Currently, we select segment-2.ts instead of segment-3.ts because we do not account for this timestamp fluctuation.

Specific Changes proposed

Check whether we hit timestamp fluctuation by adding TIME_FUDGE_FACTOR (1 / 30).

Requirements Checklist

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

@dzianis-dashkevich dzianis-dashkevich changed the title fix: account difference between duration info in the playlist and the… fix: account for difference between duration info in the playlist and the actual duration Dec 26, 2023
Copy link

codecov bot commented Dec 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f12c197) 86.05% compared to head (72e5c05) 86.05%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1470   +/-   ##
=======================================
  Coverage   86.05%   86.05%           
=======================================
  Files          42       42           
  Lines       10728    10731    +3     
  Branches     2469     2470    +1     
=======================================
+ Hits         9232     9235    +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.

Makes sense to me. :shipit:

@dzianis-dashkevich dzianis-dashkevich merged commit 455b020 into main Dec 27, 2023
15 checks passed
@dzianis-dashkevich dzianis-dashkevich deleted the account-timestamp-difference branch December 27, 2023 00:39
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