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

fix: Disable seek retry cooldown on most platforms #7010

Merged
merged 2 commits into from
Jul 9, 2024

Conversation

theodab
Copy link
Contributor

@theodab theodab commented Jul 9, 2024

Previously, we added a 1 second "cooldown" period between attempts to perform a corrective seek.
This was for the benefit of old v1 Chromecasts, which found the process of seeking so slow that they would sometimes get stuck in an infinite loop trying to start a presentation.
However, that cooldown period was causing issues in some situations during seeking, so this PR removes the restriction on everything but pre-Android Chromecast devices.

Fixes #4393
Fixes #5202

Previously, we added a 1 second "cooldown" period between attempts
to perform a corrective seek.
This was for the benefit of old v1 Chromecasts, which found the
process of seeking so slow that they would sometimes get stuck in
an infinite loop trying to start a presentation.
However, that cooldown period was causing issues in some situations
during seeking, so this PR removes the restriction on everything but
pre-Android Chromecast devices.

Issue shaka-project#4393
@theodab theodab added the type: bug Something isn't working correctly label Jul 9, 2024
@theodab
Copy link
Contributor Author

theodab commented Jul 9, 2024

Based on some practical testing I did, it seems like v2 Chromecasts are probably safe from this issue?
But just in case I'm being purposefully safe here, and leaving in the cooldown on all pre-Android-based Chromecasts.

@shaka-bot
Copy link
Collaborator

Incremental code coverage: 65.62%

@avelad avelad added the priority: P1 Big impact or workaround impractical; resolve before feature release label Jul 9, 2024
@avelad avelad added this to the v4.11 milestone Jul 9, 2024
@avelad avelad self-requested a review July 9, 2024 11:18
static isSeekingSlow() {
const Platform = shaka.util.Platform;
if (Platform.isChromecast()) {
if (Platform.isAndroidCastDevice()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And Fuchsia?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference...
I didn't have a Fuchsia device to test with, so I can't say for sure it's not a problem in them. It's probably fine, I was just being purposefully conservative with this change.

@avelad
Copy link
Member

avelad commented Jul 9, 2024

@shaka-bot test

@shaka-bot
Copy link
Collaborator

@avelad: Lab tests started with arguments:

  • pr=7010

@avelad avelad merged commit dcc60f9 into shaka-project:main Jul 9, 2024
22 of 30 checks passed
@theodab theodab deleted the infiniteSeekRetryBranch branch July 10, 2024 01:20
avelad added a commit that referenced this pull request Jul 10, 2024
Previously, we added a 1 second "cooldown" period between attempts to
perform a corrective seek.
This was for the benefit of old v1 Chromecasts, which found the process
of seeking so slow that they would sometimes get stuck in an infinite
loop trying to start a presentation.
However, that cooldown period was causing issues in some situations
during seeking, so this PR removes the restriction on everything but
pre-Android Chromecast devices.

Fixes #4393
Fixes #5202

---------

Co-authored-by: Álvaro Velad Galván <[email protected]>
avelad added a commit that referenced this pull request Jul 10, 2024
Previously, we added a 1 second "cooldown" period between attempts to
perform a corrective seek.
This was for the benefit of old v1 Chromecasts, which found the process
of seeking so slow that they would sometimes get stuck in an infinite
loop trying to start a presentation.
However, that cooldown period was causing issues in some situations
during seeking, so this PR removes the restriction on everything but
pre-Android Chromecast devices.

Fixes #4393
Fixes #5202

---------

Co-authored-by: Álvaro Velad Galván <[email protected]>
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Sep 7, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Sep 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible to seek beyond live edge. Not corrected Playback is broken after seeking to video end
3 participants