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: Ambient session not working #1366

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

grubicv
Copy link
Contributor

@grubicv grubicv commented Jan 12, 2022

.longForm / .longFormAudtio can't be used with the .ambient session category - this PR fixes that issue as it uses .default policy instead of longForm

@bradfloodx
Copy link
Collaborator

Great, thanks @grubicv. As a JavaScript dev I have no idea what this is change is brining/improving - could you explain the what and why in some more detail here?

Thanks.

@grubicv
Copy link
Contributor Author

grubicv commented Jan 13, 2022

@bradleyflood basically you want to use ambient session category on iOS devices when you want to play a sound without affecting the current media being played (such as apple music using avaudiosession) and only when the app is open - in our case it's playing a beat playlist as a backing track for apple music.
https://developer.apple.com/documentation/avfaudio/avaudiosession/category/1616560-ambient

The issue is that iOS doesn't allow you to use ambient as a session category if you put a policy for longForm/longFormAudio since that one is used for playing tracks on the main audio route so it's basically conflicting (you don't want to play it as a regular track, but rather as an ambient sound, but you're trying to route it to the longForm route). In that case an exception occurs on that line stating that and basically blocks you from playing sounds in ambient mode since it can't be set. Therefore you need to use the default policy route that's used for playing ambient audio, so I just added a simple condition that uses .default policy route if user wants to use ambient session category, and longForm/longFormAudio for other things.

@bradfloodx
Copy link
Collaborator

Great that all makes sense, thank you @grubicv

@dcvz are you happy to merge this?

@dcvz
Copy link
Contributor

dcvz commented Jan 14, 2022

Let’s merge this @bradleyflood . I think longer term we’ll want to also make the policy configurable

@dcvz dcvz merged commit 0064219 into doublesymmetry:main Jan 14, 2022
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.

3 participants