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

feat: iOS Picture in Picture support #206

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

23doors
Copy link
Contributor

@23doors 23doors commented May 24, 2023

PIP with texture API is only available since iOS 15+ so this is a must.

To enable PIP app needs to have audio background mode enabled in Info.plist:

	<key>UIBackgroundModes</key>
	<array>
		<string>audio</string>
	</array>

and that's pretty much it. New APIs:

controller.isPictureInPictureAvailable() -> only returns true on iOS15+ platform
controller.enablePictureInPicture()
controller.disablePictureInPicture()
controller.enableAutoPictureInPicture() -> when app goes into background, currently playing video will automatically go into PiP
controller.disableAutoPictureInPicture()
controller.enterPictureInPicture() -> requires PiP to be enabled first (or auto PIP)

@birros
Copy link
Member

birros commented May 24, 2023

I just tested it, awesome!

However, I have encountered some bugs:

  1. sometimes the PIP displays a black screen, especially when I switch to network videos, but I have trouble reproducing
  2. using software rendering when the app is unfocused causes colorimetry problems with HDR videos, but this problem also exists with non-PIP software rendering. @23doors are you sure there is no way to keep hardware rendering when the app is unfocused?

Finally, there is still some work to do before we can merge:

  1. @alexmercerind needs to confirm that he's ok with adding this kind of API to the VideoController, maybe review the syntax
  2. certainly review the VideoOutput.swift code, probably by moving it to a dedicated file…

@23doors
Copy link
Contributor Author

23doors commented May 24, 2023

Regarding 1. Does it fix itself or stays black? I can try to fix it if it is reproducable but as you know, I have near zero experience with MacOS/iOS and swift in general.

Regarding 2. Yes sadly this seems to be the case. Any kind of opengl calls when app is in background cause a bunch of warnings about app not having access to GPU. This is also consistent with what I found in docs about iOS background mode.

https://developer.apple.com/documentation/metal/gpu_devices_and_work_submission/preparing_your_metal_app_to_run_in_the_background

iOS and tvOS restrict a background app’s access to the GPU, to guarantee foreground app performance.

Regarding review.
Feel free to suggest whatever you feel is necessary. This is pretty much the first time I touched swift. Separated VideoOutputPIP.swift for now

@birros
Copy link
Member

birros commented Jun 7, 2023

So, after 2 weeks of considerations, I'm not OK to merge this PR until a solution is found for HEVC videos rendering in PIP:

  • by finding a way to enable the GPU when the app is in background
  • or by fixing colors in software rendering

If we merge with this "bug", people will think this feature is ready, and will lead to countless issues about it…

@23doors sorry, that doesn't stop you from keeping your branch up to date, until we find a solution, or someone help us…

@alexmercerind I set this PR as draft with the help-wanted label if you agree.

@birros birros marked this pull request as draft June 7, 2023 22:32
@birros birros added the help wanted Extra attention is needed label Jun 7, 2023
@alexmercerind alexmercerind marked this pull request as ready for review June 8, 2023 08:19
@alexmercerind alexmercerind marked this pull request as draft June 8, 2023 08:19
@alexmercerind
Copy link
Member

Hi! I apologize for the delay.

I'm still thinking over the code style. Ideally, we'd land this after the first-stable 1.0.0 release, once it's well-tested & (hopefully) implemented for other platforms too.

@kkkkkkkkkkkkkkeee
Copy link

Excuse me, everyone. Is this still going on

@AlvaroVasconcelos
Copy link

I really need this functionality, is there anything new?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants