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

[MultiThread] Fix error not being thrown on manifest update #1653

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

peaBerberian
Copy link
Collaborator

Fixes #1651

We had an issue in multithreading mode where we would not trigger an error (and stop playback) if a Manifest request failed in all its attempt unless it was the first manifest request.

The reason was simple: we didn't link a ManifestFetcher's error event to an actual RxPlayer error. It worked for the initial request because it leads to the rejection of our internal initial-loading promise with that error.

Fixes #1651

We had an issue in multithreading mode where we would not trigger an
error (and stop playback) if a Manifest request failed in all its
attempt unless it was the first manifest request.

The reason was simple: we didn't link a `ManifestFetcher`'s `error`
event to an actual RxPlayer error. It worked for the initial request
because it leads to the rejection of our internal initial-loading
promise with that error.
@peaBerberian peaBerberian added Priority: 0 (Very high) This issue or PR has a very high priority. Efforts should be concentrated on it first. MultiThread Concerns specifically the multithreaded mode of the RxPlayer labels Feb 26, 2025
Copy link

Automated performance checks have been performed on commit bf86f74c90384a1cfaabdc7a5db407aea291570d with the base branch dev.

Tests results

✅ Tests have passed.

Performance tests 1st run output

No significative change in performance for tests:

Name Mean Median
loading 19.84ms -> 19.85ms (-0.013ms, z: 1.06146) 27.30ms -> 27.15ms
seeking 14.78ms -> 12.84ms (1.941ms, z: 1.64038) 11.40ms -> 11.40ms
audio-track-reload 26.88ms -> 26.79ms (0.094ms, z: 0.10779) 39.00ms -> 39.00ms
cold loading multithread 48.42ms -> 47.60ms (0.825ms, z: 11.98525) 70.80ms -> 69.60ms
seeking multithread 18.74ms -> 18.63ms (0.104ms, z: 0.41651) 10.50ms -> 10.50ms
audio-track-reload multithread 26.57ms -> 26.39ms (0.174ms, z: 2.84281) 39.00ms -> 38.85ms
hot loading multithread 16.69ms -> 16.55ms (0.143ms, z: 3.41665) 24.30ms -> 24.15ms

If you want to skip performance checks for latter commits, add the skip-performance-checks label to this Pull Request.

@peaBerberian peaBerberian merged commit b010819 into dev Feb 26, 2025
13 checks passed
@peaBerberian peaBerberian added this to the 4.3.0 milestone Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MultiThread Concerns specifically the multithreaded mode of the RxPlayer Priority: 0 (Very high) This issue or PR has a very high priority. Efforts should be concentrated on it first.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue] [MultiThread] No error thrown when manifest request failed and exceed maximum of retry
2 participants