-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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(seeking) #4956 - issue with seeking back when video is paused #5093
fix(seeking) #4956 - issue with seeking back when video is paused #5093
Conversation
@@ -55,7 +55,7 @@ export default class GapController { | |||
this.seeking = seeking; | |||
|
|||
// The playhead is moving, no-op | |||
if (currentTime !== lastCurrentTime) { | |||
if (currentTime !== lastCurrentTime || (media?.paused && seeking)) { | |||
this.moved = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than circumvent all stall tracking and gap jumping when paused and seeking, we can avoid _trySkipBufferHole
by not setting this.moved
to false on line 114 when paused. I included that change in this branch which does the same as long as maxBufferHole
configured to 0
. The later nudge behavior ("Trying to nudge playhead over buffer-hole") is still expected when the playhead is at a point where the combined buffer is empty and media is not loaded.
https://github.com/video-dev/hls.js/compare/bugfix/nudging-over-buffer-hole-when-paused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @robwalch thank you for the fast reply and for covering other cases, I wasn't aware of them.
I tested out your branch.
You mention in #2327 (comment)
As long as maxBufferHole and maxFragLookUpTolerance are set to 0
- maxBufferHole 0 maxFragLookUpTolerance 0 -> not working hitting the wall ❌
- maxBufferHole 0 -> works - it recovers and passes the wall ✅
- default config -> works - it recovers and passes the wall ( a bit slower than (2)) ✅
What is your opinion about [1]? Are there some consequences of setting maxBufferHole 0 (what we can expect)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxBufferHole 0 maxFragLookUpTolerance 0 -> not working hitting the wall ❌
Can you provide more detail - maybe a close-up screenshot of the fragment and buffer source ranges in this state?
I suggested a maxFragLookUpTolerance
of 0
because with higher values, the findFragment...
methods favor selecting the next fragment when within tolerance. The branch introduces some changes to recognize when the first fragment selected is loaded and the previous one is needed, but I'm not exactly sure where you are hitting the wall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @robwalch can you maybe guide what me exact variables should i log?
For example
this.fragmentTracker.getBufferedFrag
"_byteRange": null,
"relurl": "main_1080p6000kbps3954.ts",
"elementaryStreams": {
"audio": {
"startPTS": 7910.3803333333335,
"endPTS": 7912.343,
"startDTS": 7910.3803333333335,
"endDTS": 7912.343,
"partial": false
},
"video": {
"startPTS": 7910.1793333333335,
"endPTS": 7912.1793333333335,
"startDTS": 7910.1793333333335,
"endDTS": 7912.1793333333335,
"partial": false
},
"audiovideo": null
},
"_decryptdata": null,
"rawProgramDateTime": null,
"programDateTime": null,
"tagList": [
[
"INF",
"2.000000"
]
],
"duration": 2,
"sn": 3954,
"type": "main",
"loader": null,
"keyLoader": null,
"level": 2,
"cc": 1,
"startPTS": 7910.1793333333335,
"endPTS": 7912.343,
"appendedPTS": 7912.1793333333335,
"startDTS": 7910.1793333333335,
"endDTS": 7912.343,
"start": 7910.1793333333335,
"deltaPTS": 0.20100000000002183,
"maxStartPTS": 7910.1793333333335,
"minEndPTS": 7912.1793333333335,
"stats": {
"aborted": false,
"loaded": 1674516,
"retry": 0,
"total": 1674516,
"chunkCount": 1,
"bwEstimate": 40905204.688538656,
"loading": {
"start": 41696.09999999963,
"first": 41716.5,
"end": 41730.5
},
"parsing": {
"start": 41730.5,
"end": 41797.20000000112
},
"buffering": {
"start": 41788.09999999963,
"first": 44300.70000000112,
"end": 50793.800000000745
}
},
"urlId": 0,
"bitrateTest": false,
"title": null,
"initSegment": null
bufferedFragAtPos
{
"_byteRange": null,
"relurl": "main_1080p6000kbps3803.ts",
"elementaryStreams": {
"audio": {
"startPTS": 7608.3803333333335,
"endPTS": 7610.343,
"startDTS": 7608.3803333333335,
"endDTS": 7610.343,
"partial": false
},
"video": {
"startPTS": 7608.1793333333335,
"endPTS": 7610.1793333333335,
"startDTS": 7608.1793333333335,
"endDTS": 7610.1793333333335,
"partial": false
},
"audiovideo": null
},
"_decryptdata": null,
"rawProgramDateTime": null,
"programDateTime": null,
"tagList": [
[
"INF",
"2.000000"
]
],
"duration": 2,
"sn": 3803,
"type": "main",
"loader": null,
"keyLoader": null,
"level": 2,
"cc": 1,
"startPTS": 7608.1793333333335,
"endPTS": 7610.343,
"appendedPTS": 7610.1793333333335,
"startDTS": 7608.1793333333335,
"endDTS": 7610.343,
"start": 7608.1793333333335,
"deltaPTS": 0.20100000000002183,
"maxStartPTS": 7608.1793333333335,
"minEndPTS": 7610.1793333333335,
"stats": {
"aborted": false,
"loaded": 1471288,
"retry": 0,
"total": 1471288,
"chunkCount": 1,
"bwEstimate": 39393857.00656811,
"loading": {
"start": 100967,
"first": 100976.90000000037,
"end": 100993
},
"parsing": {
"start": 100993,
"end": 101047.5
},
"buffering": {
"start": 101043.29999999888,
"first": 101059.19999999925,
"end": 101064.29999999888
}
},
"urlId": 0,
"bitrateTest": false,
"title": null,
"initSegment": null
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
friendly reminder ping @robwalch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @salesh,
There are some gap-controller changes coming in #5257. I'll see if I can incorporate changes from bugfix/nudging-over-buffer-hole-when-paused or this PR after that's been merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salesh you can test the upcoming changes at https://feature-gap-tag-handling.hls-js-4zn.pages.dev/demo/
As long as these settings are 0, then the player should load segments at currentTime without jumping forward. So, if you want frame by frame seeking, these settings should be used:
"maxBufferHole": 0,
"maxFragLookUpTolerance": 0
Let me know if this works for you and what you find off the gap handing branch. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[1] tested on https://feature-gap-tag-handling.hls-js-4zn.pages.dev/demo/ with
"maxBufferHole": 0,
"maxFragLookUpTolerance": 0
✔️
[2] created hls.min.js
from #5257 use
"maxBufferHole": 0,
"maxFragLookUpTolerance": 0
and check it in our app
✔️
Everything works @robwalch thank you very much for your efforts!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salesh, would you be OK closing this PR?
I prefer not to merge as it would change the default behavior of the player to snap to the next segment when seeking even when paused. If the solution above is not enough, we'll need to either:
- Add checks for
maxBufferHole === 0 && maxFragLookUpTolerance === 0
to your change - Keep track of lastCurrentTime !== currentTime direction, and prevent forward gap jump seeks when the last difference between currentTime was negative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure @robwalch I agree with you! Thank you!
Closed in favor of #5257 |
This PR will...
Track stuck while paused and seeking back, so that gap-controller will seek over small gaps in this playback state
Why is this Pull Request needed?
[1] Go to https://hls-js.netlify.app/demo
[2] seek somewhere in the middle and pause the video
[3] add function in the console
[4] call
seekFrame()
from console[5] keep calling till you reach to
(example)
[warn] > skipping hole, adjusting currentTime from 459.988593 to 460.088593
=> we are frozen there
Resolves issues:
Allows us to see back and avoid freezing
Checklist