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(ios): Do not crash when accessLog return nil #3549

Merged

Conversation

gkueny
Copy link
Contributor

@gkueny gkueny commented Feb 27, 2024

Summary

accessLog method can return nil if no logging information are currently available (see https://developer.apple.com/documentation/avfoundation/avplayeritem/1388499-accesslog).

Motivation

should fix #3424

Changes

I handle the case where accessLog return nil & do not call onVideoBandwidthUpdate if it's nil. That should avoid some crash.

Test plan

Sadly I can not reproduce the crash. I did not find a way to trigger onVideoBandwidthUpdate. Someone maybe can point me a way to reproduce this beahavior ?

@douzal34
Copy link

I have tested his branch and my issue from #3424 is not freezing anymore.
Great job @gkueny !

@KrzysztofMoch KrzysztofMoch self-requested a review February 28, 2024 09:04
Copy link
Member

@KrzysztofMoch KrzysztofMoch left a comment

Choose a reason for hiding this comment

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

Thank you! I left one small comment, but it looks good

ios/Video/RCTVideo.swift Outdated Show resolved Hide resolved
accessLog method can return nil if no logging information are currently available (see https://developer.apple.com/documentation/avfoundation/avplayeritem/1388499-accesslog).
So we handle this case & do not call onVideoBandwidthUpdate

fix TheWidlarzGroup#3424
@gkueny gkueny force-pushed the fix/3424-onBandwidthUpdate-crash branch from 7b8bcda to ce938e7 Compare February 28, 2024 09:37
@gkueny gkueny requested a review from KrzysztofMoch February 28, 2024 09:38
@gkueny
Copy link
Contributor Author

gkueny commented Feb 28, 2024

@KrzysztofMoch thanks for the review.

I did not find a way to trigger onVideoBandwidthUpdate. Someone maybe can point me a way to reproduce this beahavior ?

For my own knowledge, do you know how I can trigger AVPlayerItemNewAccessLogEntry notification ?

Copy link
Member

@KrzysztofMoch KrzysztofMoch left a comment

Choose a reason for hiding this comment

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

@gkueny I am not sure... 😅 - it's sometimes just work for me

@KrzysztofMoch KrzysztofMoch merged commit 4d4b56c into TheWidlarzGroup:master Feb 29, 2024
6 checks passed
@gkueny gkueny deleted the fix/3424-onBandwidthUpdate-crash branch February 29, 2024 17:12
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.

(IOS) App Freeze on handleAVPlayerAccess function [v6.0.0-beta.2]
4 participants