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

Variant failover feature doesn't apply to missing (404) segments #4728

Closed
friday opened this issue Nov 18, 2022 · 0 comments · Fixed by #4769
Closed

Variant failover feature doesn't apply to missing (404) segments #4728

friday opened this issue Nov 18, 2022 · 0 comments · Fixed by #4769
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@friday
Copy link
Contributor

friday commented Nov 18, 2022

Have you read the FAQ and checked for duplicate open issues?
Yes

What version of Shaka Player are you using?
4.3.0/main

Can you reproduce the issue with our latest release version?
Yes

Can you reproduce the issue with the latest code from main?
Yes

Are you using the demo app or your own custom app?
Yes

If custom app, can you reproduce the issue using our demo app?
Yes

What browser and OS are you using?
Doesn't matter, but Chromium Browser/Firefox, Arch Linux

What did you do?
This can be replicated with this prepared asset using https://github.com/Eyevinn/chaos-stream-proxy to 404 at the segment at 0:35 on the highest bandwidth variant. If you switch the variant manually you can see that it will continue to buffer past that point.

Link to your demo with this asset

What did you expect to happen?
#4189 should activate for 404, or at least there should be a way to configure it to activate for 404.

What actually happened?

Same issue as #4093 (it was closed, but not quite fixed).

tryToRecoverFromError_ from #4189 works great, but it's guarded with conditions that means it won't be used in many cases. Only for HTTP_ERROR and SEGMENT_MISSING.

shaka-player/lib/player.js

Lines 6033 to 6037 in 0db6070

if ((error.code != shaka.util.Error.Code.HTTP_ERROR &&
error.code != shaka.util.Error.Code.SEGMENT_MISSING) ||
error.category != shaka.util.Error.Category.NETWORK) {
return false;
}

If a VOD segment is missing and the server responds with 404 it actually means the request was successful, but the status was not. So it results in BAD_HTTP_STATUS

Maybe shaka.net.HttpPluginUtils.makeResponse could be considered to be the culprit, but I think it's the tryToRecoverFromError_ condition that is too strict and should also at least optionally include BAD_HTTP_STATUS and TIMEOUT, but maybe other statuses also. Maybe we can add a configuration option for this?

Example:

shakaPlayer.configure({streaming: { variantFailoverErrors: [
  shaka.util.Error.Code.HTTP_ERROR,
  shaka.util.Error.Code.SEGMENT_MISSING,
  shaka.util.Error.Code.BAD_HTTP_STATUS,
  shaka.util.Error.Code.TIMEOUT,
]}})
@friday friday added the type: bug Something isn't working correctly label Nov 18, 2022
@github-actions github-actions bot added this to the v4.4 milestone Nov 18, 2022
@friday friday changed the title 404 segment failover doesn't work Variant failover feature doesn't apply missing (404) segments Nov 22, 2022
@friday friday changed the title Variant failover feature doesn't apply missing (404) segments Variant failover feature doesn't apply to missing (404) segments Nov 27, 2022
avelad pushed a commit that referenced this issue Dec 1, 2022
This enables #4189 for two more error types. As mentioned in #4728 404
status results in `BAD_HTTP_STATUS` rather than `HTTP_ERROR`. I also
think `TIMEOUT` should be included, and another issue was just created
for this (#4764), so I included that as well.

Closes #4728
Closes #4764
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jan 30, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant