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: drop fragment when currentLevel is undefined #6730

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

gjanblaszczyk
Copy link
Contributor

This PR will...

It will iron logic for dropping a level fragment after resetting the details.

Why is this Pull Request needed?

It fixes TypeError: Cannot read properties of undefined (reading 'details') which sometimes happens on mobile devices.
Especially on Android 10.

Are there any points in the code the reviewer needs to double check?

No idea.

Resolves issues:

I don't think someone reported an issue.

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@gjanblaszczyk
Copy link
Contributor Author

I am submitting this fix because it seems there is a very rare case where currentLevel is undefined. From my testing, it appears to happen primarily on mobile devices, especially running Android. I have logs indicating that the issue occurs on devices such as:

Oppo CPH2205 running Android 12 and Chrome Mobile WebView 129,
Xiaomi Redmi Note 8 Pro,
2201117TY,
Oppo CPH2465, but it is not limited to these. I have also encountered errors reported on Android 10 and Chrome Mobile 128, although without device names.
The tests were conducted on hls.js version 1.5.8, but I don't believe that updating to the latest version would resolve the issue...

So, my question is: does anyone know why currentLevel might sometimes be undefined, and does my fix actually resolve the issue or just mask it, replacing a critical error with a warning?

@robwalch
Copy link
Collaborator

Is removeLevel getting called somewhere?

@gjanblaszczyk
Copy link
Contributor Author

Is removeLevel getting called somewhere?

Thank you for the quick response!
I checked this suggestion, and it looks like I’m not using this function anywhere in my code. However, the removeLevel() method might be called internally by hls.js here: AbrController -> findBestLevel(), but you probably already know that. I will try to gather more logs from these devices to be 100% sure if this method is indeed being called somewhere earlier.

src/controller/stream-controller.ts Outdated Show resolved Hide resolved
@robwalch robwalch added this to the 1.6.0 milestone Oct 4, 2024
@robwalch robwalch merged commit 09c6747 into video-dev:master Oct 7, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants