coreaudio: Refactor iOS audio session management.#15452
Draft
icculus wants to merge 1 commit into
Draft
Conversation
icculus
force-pushed
the
sdl3-coreaudio-ios-session-refactor
branch
from
1afbf53 to
7ffbf71
Compare
This makes UpdateAudioSession only deal with setting the category/options; actually setting the session active and adding interruption listeners is moved to COREAUDIO_OpenDevice. OpenDevice exclusively calls UpdateAudioSession now; there's no reason to change the session during device close...even if we could loosen the session's config when closing a recording device but leaving playback running (or vice-versa), it doesn't seem worth it. Likewise, deactivating the session and removing listeners is now handled in COREAUDIO_CloseDevice. The attempt to try setting a more limited category if setting the session to simultaneous recording and playback fails has been removed; this would presumably cause problems in general, and different problems depending on which device you opened first. It's better to just fail in this case, I think. A bunch of code that proved superfluous is now gone; we don't enumerate all devices to count open ones (we just maintain a simple global counter instead, as atomic ints, just in case this might have a subtle threading concern). The (Pause|Resume)AllDevices() code is gone, as we don't need it anymore with the simplified session management. We still pause/resume per-device in the interruption listener. This should also fix a subtle crash bug, where we sometimes fail to change the session on close, causing an early return from UpdateAudioSession and thus never unregistering the listeners, which would touch a free'd pointer if the the listener fires later. Now the listeners are always unregistered in CloseDevice and UpdateAudioSession is never called from there at all. Closes libsdl-org#15439.
icculus
force-pushed
the
sdl3-coreaudio-ios-session-refactor
branch
from
7ffbf71 to
04bf243
Compare
Collaborator
Author
|
I pushed a simplified version of this for now, and we'll look at this bigger change for 3.6.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes UpdateAudioSession only deal with setting the category/options; actually setting the session active and adding interruption listeners is moved to COREAUDIO_OpenDevice. OpenDevice exclusively calls UpdateAudioSession now; there's no reason to change the session during device close...even if we could loosen the session's config when closing a recording device but leaving playback running (or vice-versa), it doesn't seem worth it.
Likewise, deactivating the session and removing listeners is now handled in COREAUDIO_CloseDevice.
The attempt to try setting a more limited category if setting the session to simultaneous recording and playback fails has been removed; this would presumably cause problems in general, and different problems depending on which device you opened first. It's better to just fail in this case, I think.
A bunch of code that proved superfluous is now gone; we don't enumerate all devices to count open ones (we just maintain a simple global counter instead, as atomic ints, just in case this might have a subtle threading concern). The (Pause|Resume)AllDevices() code is gone, as we don't need it anymore with the simplified session management. We still pause/resume per-device in the interruption listener.
This should also fix a subtle crash bug, where we sometimes fail to change the session on close, causing an early return from UpdateAudioSession and thus never unregistering the listeners, which would touch a free'd pointer if the the listener fires later. Now the listeners are always unregistered in CloseDevice and UpdateAudioSession is never called from there at all.
Closes #15439.
(This is a draft, as I haven't tested this yet. I'll verify on real hardware shortly. Also, I've put this in the 3.4.6 milestone, but this touches a lot of code, so we can decide if we want to stick a bandaid on this for 3.4.6 and use this patch in 3.6.0.)