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

Preload waitforFinish API issues #7520

Closed
sridhard opened this issue Oct 30, 2024 · 2 comments · Fixed by #7257 or #7698 · May be fixed by #7597
Closed

Preload waitforFinish API issues #7520

sridhard opened this issue Oct 30, 2024 · 2 comments · Fixed by #7257 or #7698 · May be fixed by #7597
Assignees
Labels
priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly type: enhancement New feature or request
Milestone

Comments

@sridhard
Copy link

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

If the problem is related to FairPlay, have you read the tutorial?
NA

What version of Shaka Player are you using?
Latest from master

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?
Custom app

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

What browser and OS are you using?
Mac OS. Chrome browser

For embedded devices (smart TVs, etc.), what model and firmware version are you using?
NA

What are the manifest and license server URIs?
It happens with any manifest

What configuration are you using? What is the output of player.getNonDefaultConfiguration()?
Using preload sample code

What did you do?
Preload the asset

What did you expect to happen?
When we use preload ro load the asset there are 2 issues:

  1. If any error happens during preload like license acquire failed or seqment fetch failed then no exception is given to the application. Errors should be given to the application to handle failures. Now the exception is just logged by shaka player
  2. When we use preload.waitforfinish(), the api is not waiting for the preload to complete. Preload is complete when the license and the segments are loaded. But this api is not waiting for license and segments to fetch. It returns before that. So application does not know when the preload is completed

What actually happened?
Errors are not given to application during preload and also application does not have a way to wait for preload to complete

Are you planning send a PR to fix it?
No

@sridhard sridhard added the type: bug Something isn't working correctly label Oct 30, 2024
@shaka-bot shaka-bot added this to the v4.12 milestone Oct 30, 2024
@theodab theodab added type: enhancement New feature or request priority: P2 Smaller impact or easy workaround labels Oct 31, 2024
@theodab
Copy link
Contributor

theodab commented Oct 31, 2024

  1. Yes, we probably should be catching those particular errors.
  2. The original design was that the preload was considered finished once the manifest et all were downloaded, and the segment prefetch was just an extension of our normal segment prefetch system and thus not counted as part of the preload. However, counting the segment prefetch as part of the preload sounds like a reasonable change in behavior to me.

I'll make a PR for this.

theodab added a commit that referenced this issue Nov 5, 2024
Previously, the PreloadManager would consider a preload "finished" after a few major files like the manifest had been preloaded. It would start prefetching some segments, but wouldn't wait on it to notify the developer.
This PR changes the PreloadManager so that
PreloadManager.waitForFinish won't return until the prefetched segments have finished loading.
Because of that, this also better surfaces errors thrown during segment prefetching, when preloading.

Issue #7520
@theodab
Copy link
Contributor

theodab commented Nov 6, 2024

With that PR we now track segment prefetches, and surface errors thrown by them. Doing the same for license acquisition will be a future PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly type: enhancement New feature or request
Projects
None yet
4 participants