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

v1.4.0-beta regression gap controller jumps large gaps #5360

Closed
robwalch opened this issue Mar 30, 2023 · 2 comments · Fixed by #5366
Closed

v1.4.0-beta regression gap controller jumps large gaps #5360

robwalch opened this issue Mar 30, 2023 · 2 comments · Fixed by #5366
Labels
Bug Confirmed Regression A bug introduced in a recent release
Milestone

Comments

@robwalch
Copy link
Collaborator

robwalch commented Mar 30, 2023

I noticed one undesirable action: gap-controller.ts:264 [warn] > skipping hole, adjusting currentTime from 105.275674 to 170.05533300000002

To reproduce the issue, you need to watch the entire video and seek to 105.
"backBufferLength": 90
08b938f3.hls-js-dev.pages.dev-1680185898252.log

Originally posted by @mtoczko in #2940 (comment)

@robwalch robwalch mentioned this issue Mar 30, 2023
5 tasks
@robwalch
Copy link
Collaborator Author

Hi @mtoczko,

In this case is there a GAP tag or no loadable media at 105? Is this reproducible in streams without GAP tags?

First thing to try is to clear out any gap fragments from the fragmentTracker on seeking. My guess is that if we don't do this with your test stream the player will not not error->switch to find bufferable fragments and encounter a stall situation that it determines it should jump because of all the tracked gaps.

@robwalch robwalch added this to the 1.4.0 milestone Mar 30, 2023
@mtoczko
Copy link
Collaborator

mtoczko commented Mar 30, 2023

Hi @robwalch
To observe this better, you need to enable throttling in the browser (3G). Seek to 260, seek to 0.
We can observe that almost the entire video is skipped. We only seek to "safe" places.

We should probably clear the buffer when we seek.

[log] > [audio-stream-controller]: Buffered audio sn: 13 of track 0 (frag:[26.005-28.011] > buffer:[0.000-28.011][260.011-266.027])
base-stream-controller.ts:1682 [log] > [audio-stream-controller]: PARSED->IDLE
[warn] > skipping hole, adjusting currentTime from 18.83739 to 260.060666
[log] > [stream-controller]: media seeking to 260.061, state: IDLE

targetTime - It is not validated in any way and you can jump through the entire video

const targetTime = Math.max(
startTime + SKIP_BUFFER_RANGE_START,
currentTime + SKIP_BUFFER_HOLE_STEP_SECONDS
);

 if ((startTime + SKIP_BUFFER_RANGE_START) - (currentTime + SKIP_BUFFER_HOLE_STEP_SECONDS) > backBufferLength )
return 0 

robwalch added a commit that referenced this issue Apr 1, 2023
@robwalch robwalch added Bug Confirmed Regression A bug introduced in a recent release labels Apr 4, 2023
robwalch added a commit that referenced this issue Apr 6, 2023
robwalch added a commit that referenced this issue Apr 6, 2023
…range are partial (#5366)

* Only allow large gaps to be skipped if start gap or all fragments in range are partial
Fixes #5360

* Fix adaptation and fragment tracking after fragment errors (gap tags)
#2940
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed Regression A bug introduced in a recent release
Projects
None yet
2 participants