-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: reBufferingGoal is not respected #6433
Conversation
Incremental code coverage: 100.00% |
@avelad, you made the change to stop using playbackRate of 0 to manage buffering state. This would reintroduce a smaller version of that, by explicitly setting playbackRate to 0 when we enter a buffering state, but without so much machinery to manage things. It also has the side-effect of reverting to defaultPlaybackRate after a buffering event, instead of the playbackRate explicitly chosen by the user or by the catch-up logic. @avelad, I would prefer you look at this and make recommendations. |
Change the buffering logic to use isBuffering_ This will not change any playback rate already set
@avelad thanks |
@joeyparrish can you review it? Thanks! |
Gentle ask to @joeyparrish if you have time ;) thanks a lot |
@theodab @joeyparrish can you review it again? Thanks! |
This appears to be causing test failures on Tizen. |
Specifically, this causes these two failures on Tizen 100% of the time:
As seen here when testing v4.8.2: https://github.com/shaka-project/shaka-player/actions/runs/8976237248/job/24654981074 I am reverting this before v4.8.2 goes out. It has not been in a release yet. |
Fixes these failures on Tizen: Player rebufferGoal ✗ state orchestration and buffer length [Safari 3.0 (Tizen 3.0)] Player Src Equals ✗ can control trick play rate [Safari 3.0 (Tizen 3.0)] This was not in any releases. Reverts "fix: reBufferingGoal is not respected (shaka-project#6433)" This reverts commit 99ed5db. Reopens shaka-project#6355
Reverted by #6540 |
Fixes these failures on Tizen: Player rebufferGoal ✗ state orchestration and buffer length [Safari 3.0 (Tizen 3.0)] Player Src Equals ✗ can control trick play rate [Safari 3.0 (Tizen 3.0)] This was not in any releases. Reverts "fix: reBufferingGoal is not respected (#6433)" This reverts commit 99ed5db. Reopens #6355
Fixes these failures on Tizen: Player rebufferGoal ✗ state orchestration and buffer length [Safari 3.0 (Tizen 3.0)] Player Src Equals ✗ can control trick play rate [Safari 3.0 (Tizen 3.0)] This was not in any releases. Reverts "fix: reBufferingGoal is not respected (#6433)" This reverts commit 99ed5db. Reopens #6355
Fixes these failures on Tizen: Player rebufferGoal ✗ state orchestration and buffer length [Safari 3.0 (Tizen 3.0)] Player Src Equals ✗ can control trick play rate [Safari 3.0 (Tizen 3.0)] This was not in any releases. Reverts "fix: reBufferingGoal is not respected (#6433)" This reverts commit 99ed5db. Reopens #6355
Fixes these failures on Tizen: Player rebufferGoal ✗ state orchestration and buffer length [Safari 3.0 (Tizen 3.0)] Player Src Equals ✗ can control trick play rate [Safari 3.0 (Tizen 3.0)] This was not in any releases. Reverts "fix: reBufferingGoal is not respected (#6433)" This reverts commit 99ed5db. Reopens #6355
Fixes #6355
it's still in draft until having proper unit tests to control this
This PR is doing the following
At startup time and at buffering it will set the playback rate to 0. like it was before and reset it to the default value we no buffer.
I'm still passing all the unit test but i'm still thinging if we have an impact with the live catchup feature that is changing the playback rate.
Maybe we should just re introduce the isBuffering_ value in playbackrate class.
Happy to do it if you think so
here the link the PR that introduce the regression
https://github.com/shaka-project/shaka-player/pull/5696/files#diff-274a4cc3948f85eed18f86e04c6260b93faa97b995d38f0e3b1af618fd01fab7L34
BEGIN_COMMIT_OVERRIDE
chore: Reverted, do not include in release notes
END_COMMIT_OVERRIDE