Fix some issues that may arise on BUFFER_FULL situations #1546
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peaBerberian
added
bug
| } | ||
| return bufferGoalRatio; | ||
| if (bufferGoalRatio < 1 && wba === Infinity) { | ||
| return 5 * 60 * 1000 * bufferGoalRatio; |
Collaborator
There was a problem hiding this comment.
maybe we can add a comment to say that 5 minutes is the default for calculation in case wanted buffer head is infinite
I'm working on heuristics to determine adaptively how much the current
device can hold in its audio and video buffers.
To quickly iterate on this, I'm setting a very high (or even `Infinity`)
`wantedBufferAhead` to quickly fill-up memory until the device has to
perform a strategy to free it.
While doing this, I saw a collection of issues:
1. When calling the MSE `SourceBuffer.prototype.remove` API to remove
buffer (which we already do in some clean-up strategies), putting
an end timestamp equal to the start timestamp leads to an Error (as
defined by MSE).
A long-term fix would be to just avoid doing the MSE `remove` call
as close as possible to our MSE abstraction, but for now I also
added a check at the initial call (which also makes sense).
I'm thinking of also adding the long-term fix, but not in this PR
as I want it to have the less risks possible.
2. When a `QuotaExceededError` is received after a push, we trigger a
`BUFFER_FULL_ERROR` error, which is then handled by, waiting a
little, then reducing the `wantedBufferAhead` value progressively
through a ratio and retrying.
If after either the `wantedBufferAhead` is too low (less than 2
seconds) or the ratio is too low (less or equal to 0.05), we
trigger the error through the API.
Turns out that last part was broken. We never triggered the error,
leading to possibilities such as infinite rebuffering (in extreme
cases hopefully never really encountered).
3. The logic in (2) never considered that `wantedBufferAhead` could be
set to `Infinity`, and dividing `Infinity` is not a very bright
idea here. To make it work I decided that when there's a ratio set
to less than `1`, a `wantedBufferAhead` set to `Infinity` would be
equal to 5 minutes.
peaBerberian
force-pushed
the
fix/check-source-buffer-remove-arguments
branch
from
f3b0994 to
fd26e84
Compare
Florent-Bouisset
approved these changes
Sep 19, 2024
peaBerberian
added a commit
that referenced
this pull request
Oct 4, 2024
This is a backport of #1546 for the v3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm working on heuristics to determine adaptively how much the current device can hold in its audio and video buffers.
To quickly iterate on this, I'm setting a very high (or even
Infinity)wantedBufferAheadto quickly fill-up memory until the device has to perform a strategy to free it.While doing this, I saw a collection of issues:
When calling the MSE
SourceBuffer.prototype.removeAPI to remove buffer (which we already do in some clean-up strategies), putting an end timestamp equal to the start timestamp leads to an Error (as defined by MSE).A long-term fix would be to just avoid doing the MSE
removecall as close as possible to our MSE abstraction, but for now I also added a check at the initial call (which also makes sense).I'm thinking of also adding the long-term fix, but not in this PR as I want it to have the less risks possible.
When a
QuotaExceededErroris received after a push, we internally trigger aBUFFER_FULL_ERRORerror, which is then handled by waiting a little, then reducing thewantedBufferAheadvalue progressively through a ratio system and retrying.If after either the
wantedBufferAheadis too low (less than 2 seconds) or the ratio is too low (less or equal to 0.05), we trigger the error through the API and stop the content.Turns out that last part was broken. We never triggered the error, leading to possibilities such as infinite rebuffering (in extreme cases hopefully never really encountered).
The logic in (2) never considered that
wantedBufferAheadcould be set toInfinity, and dividingInfinityis not a very bright idea here. To make it work I decided that when there's a ratio set to less than1, awantedBufferAheadset toInfinitywould be equal to 5 minutes.