-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Implement picture in picture for iOS #1325
Conversation
This is a very cool feature, thanks for submitting and providing all the documentation! I'd like to see if we can condense the interface so that it's as simple as possible. onIsPictureInPictureSupported - React Native only supports iOS >= 9.0 so I don't think we need this. |
3994e73
to
c809dde
Compare
Ops, didn't realize my debug commits were showing up here. This is not ready to be reviewed again. There are some issues. I'll resolve them when I get a chance. In terms of |
fe04674
to
c2a6d91
Compare
@cobarx I made 4 of the 5 changes you mentioned. Please let me know what you think about the restoration piece. I tried to mirror Apple's restoration function with: |
Test Plan: - Run on ipad - test out onIsPictureInPictureSupported, onIsPictureInPictureActive, restoreUserInterfaceForPictureInPictureStop, startPictureInPicture, stopPictureInPicture
@cobarx any feedback about the updated code? |
Sorry for the delay in getting to review this. This is working great, I'm going to merge it. Two things I noticed: Also, I'm not seeing events when I put the video in fullscreen and then engage PiP from there. If possible, can you add a PR to capture those using the same framework you've done here. |
Btw this works great. Thanks so much for putting it together, great feature. It would be wonderful to be able to support this on ExoPlayer as well. |
I think this PR breaks compatibility with tvOS, as PiP is only on AVKit (ios) Probably is can be fixed with a solution like this (probably some condition has to be added also on AVPictureInPictureControllerDelegate delegate's methods ) or
|
Also it fails if initial value is set to true (it should be UI/JS triggered) of it is set as initial value, trigger the function on |
@cobarx As noted by @danielmarino24i, this created a regression on tvOS. We'll need to get a PR up to hotfix it. |
I've spun up a work branch to complete this work for Android, we'll see how it goes. |
- controls TheWidlarzGroup/react-native-video#1414 - PictureInPicture TheWidlarzGroup/react-native-video#1325 - minLoadRetryCount TheWidlarzGroup/react-native-video@420d88d
- controls TheWidlarzGroup/react-native-video#1414 - PictureInPicture TheWidlarzGroup/react-native-video#1325 - minLoadRetryCount TheWidlarzGroup/react-native-video@420d88d
Implement picture in picture for iOS (rebased from commit d5fe47f)
Test Plan:
onIsPictureInPictureSupported
,onIsPictureInPictureActive
,restoreUserInterfaceForPictureInPictureStop
,startPictureInPicture
,stopPictureInPicture
.