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

[expo-av] Fix instability issues when changing source and useNativeControls #9381

Merged
merged 4 commits into from
Jul 24, 2020

Conversation

IjzerenHein
Copy link
Contributor

Why

Fixes #7137. Code that changes the UI is not always executed in the main UI thread, causing immediate or delayed crashes in the EXVideoView. This was reproducible by changing the source icw setting useNativeControls to true, however it is not limited to this situation and may occur on other scenarios as well.

How

  • Update ViewController & Layer code to always be executed in main UI thread
  • Fix external call to setUseNativeControls not checking whether player-load is complete
  • Add test-suite tests to verify stability when changing sources & useNativeControls
  • Add 'Change source` button to NCL to test & verify changing source ability

Change source button in NCL

image

Test Plan

image

@github-actions
Copy link
Contributor

Native Component List for this branch is ready

@IjzerenHein IjzerenHein merged commit e9aa9df into master Jul 24, 2020
@IjzerenHein IjzerenHein deleted the @hein/expo-av/video-layout-engine-crash branch July 24, 2020 13:50
}
// Any ViewController/layer updates need to be
// executed on the main UI thread.
__weak EXVideoView *weakSelf = self;
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, we have macros for handling those nasty __weak self references.
Instead of __weak VariableType *var = variable use UM_WEAKIFY(variable)
and instead of __strong ... use UM_STRONGIFY or even UM_ENSURE_STRONGIFY (the latter comes with auto-checking for nullability).

Jamedjo pushed a commit to Jamedjo/expo that referenced this pull request Feb 4, 2021
…ntrols (expo#9381)

* [ncl] Add multiple sources switching for video-player

* [expo-av] Fix stability issues when changing source & useNativeControls

* [test] Add video tests for changing source & useNativeControls

* [expo-av] Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[expo-av] EXVideoView - Modifications to the layout engine must not be performed from a background thread
2 participants