-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Prevent playback freeze by checking buffer availability before updating playback position #7210
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
Conversation
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.
Prevent playback freeze by verifying buffer before seeking to sync position
This change does more than that. It only seeks to the sync position when the buffer is contiguous between the current position and the sync position. When it is not, it seeks to the start of the buffered range containing the sync position. It also flushes the buffer while seeking and you haven't provided the reasoning for this.
There's an expectation that HLS.js will move to the edge when out of sync with live. The sync position is determined by targetLatency
. If you want to break that contract, one solution would be to set hls.targetLatency
on stall, increasing it to match the buffered position you would like it to aim for.
If you'd like hls.js to ignore targetLatency
after it has falled behind and play through every buffered range as it tries to keep up, we should try to get this code down to fewer lines and put it behind a configuration option, as an alternate route for advancing playback in live with less than ideal streaming conditions.
src/controller/stream-controller.ts
Outdated
return; | ||
} | ||
|
||
media.currentTime = nextBufferedRange.start; |
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.
Is the idea that you want to play everything buffered between the currentTime and liveSyncPosition?
The problem is that live is supposed to maintain a prescribed distance from the edge. The purpose of this method is to catch up to that position. It is not meant to skip between unbuffered regions.
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.
Hello @robwalch, thanks for the feedback. I agree that our primary goal is to maintain a prescribed distance from the live edge. However, my suggestion was made with unstable network conditions in mind.
In my experiments, under poor network connectivity, once playback had fallen behind the live edge and reached the if (media.readyState) block, executing:
media.currentTime = liveSyncPosition;
in the HLS.js demo resulted in:
The buffer spinner stopping entirely
The screen freezing for an extended period
No buffer being accumulated around liveSyncPosition, leaving playback stuck
This produced a very poor playback experience. I’ll further analyze the code to understand why no buffer is built when jumping to liveSyncPosition under weak network conditions.
However, to mitigate the long freezing when jumping to liveSyncPosition, I propose adding a new HLS.js configuration option. For example:
interface HlsConfig {
// …existing options…
/**
* liveSyncMode:
* - 'edge' (default): always jump straight to liveSyncPosition
* - 'buffered': if liveSyncPosition is buffered, jump there; otherwise, play from the next buffered segment
*/
liveSyncMode?: 'edge' | 'buffered'; // default: 'edge'
}
this.warn(
`Playback: ${currentTime.toFixed(
3,
)} is located too far from the end of live sliding playlist: ${end}, reset currentTime to :
${liveSyncPosition.toFixed(
3,
)}`,
);
if (this.config.liveSyncMode === 'buffered') {
const bufferInfo = BufferHelper.bufferInfo(
this.media,
liveSyncPosition,
0,
);
if (!bufferInfo) {
media.currentTime = liveSyncPosition;
return;
}
const isLiveSyncInBuffer = bufferInfo.start <= currentTime;
if (isLiveSyncInBuffer) {
media.currentTime = liveSyncPosition;
return;
}
if (!bufferInfo.buffered) {
return;
}
let nextBufferedRange: BufferTimeRange | null = null;
for (let i = 0; i < bufferInfo.buffered.length; i++) {
const range = bufferInfo.buffered[i];
if (range.start > currentTime) {
nextBufferedRange = range;
break;
}
}
if (!nextBufferedRange) {
return;
}
media.currentTime = nextBufferedRange.start;
} else {
media.currentTime = liveSyncPosition;
}
Could you please let me know if I should submit the PR again with these suggestions, or if my proposal is not valid?
If this proposal is accepted, I will also add documentation for the liveSyncMode.
Thank you.
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.
Check for no bufferInfo or buffered at once so that the default seek is performed:
if (!bufferInfo?.buffered) {
media.currentTime = liveSyncPosition;
return;
}
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.
Regarding nextBufferedRange
, bufferInfo has a nextStart
property (number when there in another buffered range otherwise undefined). Use that instead of looping.
https://github.com/video-dev/hls.js/blob/master/src/utils/buffer-helper.ts
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.
@robwalch I’ve incorporated your requested changes into this PR. Could you please take a final look and let me know if there’s anything else that needs tweaking? Thanks!
docs/API.md
Outdated
### `liveSyncMode` | ||
|
||
(default: `'edge'`) | ||
|
||
Controls how playback synchronizes to the live edge: | ||
- `'edge'`: When the playhead moves outside the prescribed distance from the live edge, immediately jump to `liveSyncPosition`. | ||
- `'buffered'`: When the playhead moves outside the prescribed distance from the live edge, if `liveSyncPosition` is buffered, seek there; otherwise, continue playback from the start of the next buffered segment. | ||
|
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.
This would work well with startOnSegmentBoundary
added in #7211
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.
@whdudtod1273, Please run npm run prettier && npm run sanity-check
and commit any formatting fixes and api-extractor generated changes.
Hello @robwalch I ran npm run prettier && npm run sanity-check and committed all formatting fixes and API-Extractor generated changes. Thank you. |
@whdudtod1273 I cant wait to try this feature thanks all |
Prevent playback freeze by verifying buffer before seeking to sync position
The issue is reproducible in poor network conditions, such as when using a 3G connection
This PR will...
Improve the screen freezing issue when moving the playback position to the live sync position in a live streaming environment. By adding logic to check buffer status and move to the optimal playback position, it provides a smoother playback experience.
Why is this Pull Request needed?
This typically occurs when a viewer is far behind the live edge; since buffer status is not taken into account when seeking to the live sync position, it can result in the following issues
Are there any points in the code the reviewer needs to double check?
Resolves issues:
Checklist