fix(ios): avoid prematurely activating audio session #2146
Merged
Conversation
This file contains 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
puckey
force-pushed
the
fix/premature-audio-session-activation
branch
from
18983ce to
dc73578
Compare
This commit fixes the issue where the audio session is prematurely activated when `TrackPlayer.play()` or `TrackPlayer.setPlayWhenReady(true)` is called with an empty queue. - move configureAudioSession from play & setPlayWhenReady to handlePlayWhenReadyChange - add configureAudioSession to handleAudioPlayerCurrentItemChange when we go from no queue items to the first or the other way around - remove setActive parameter from configureAudioSession. Instead, activate the audio session when there is a `currentItem` and `playWhenReady == true`. Deactivate the session when there is no `currentItem`. - in configureAudioSession remove unnecessary ios 13.0 available check, since there is already one for ios 11 - remove `try? audioSessionController.deactivateSession()` before `try? audioSessionController.activateSession()` until we can figure out why it is necessary (see doublesymmetry#2110)
puckey
force-pushed
the
fix/premature-audio-session-activation
branch
from
dc73578 to
f92c84f
Compare
dcvz
previously approved these changes
Oct 10, 2023
| } else if #available(iOS 11.0, *) { | ||
| try? AVAudioSession.sharedInstance().setCategory(sessionCategory, mode: sessionCategoryMode, policy: sessionCategoryPolicy, options: sessionCategoryOptions) | ||
| private func configureAudioSession() { | ||
| if (player.currentItem != nil) { |
There was a problem hiding this comment.
small nitpick for readability: I'd reverse the order of the conditional to early return "the smaller case"
if (player.currentItem == nil) {
try? audioSessionController.deactivateSession()
return
}
if (player.playWhenReady) {
try? audioSessionController.activateSession()
}
if #available(iOS 11.0, *) {
try? AVAudioSession.sharedInstance().setCategory(sessionCategory, mode: sessionCategoryMode, policy: sessionCategoryPolicy, options: sessionCategoryOptions)
} else {
try? AVAudioSession.sharedInstance().setCategory(sessionCategory, mode: sessionCategoryMode, options: sessionCategoryOptions)
}There was a problem hiding this comment.
I just added a commit to improve this: 424c326
- deactivate the session when there is no current item to be played
- activate & configure the audio session when there is an item to be played and the player has been configured to start when it is ready loading
| @@ -860,6 +856,10 @@ public class RNTrackPlayer: RCTEventEmitter, AudioSessionControllerDelegate { | |||
| } | |||
| } | |||
|
|
|||
| if ((item != nil && lastItem == nil) || item == nil) { | |||
There was a problem hiding this comment.
non-blocking: There might be some redundancy here, because if you go from no item, to having an item. You'll likely have to have pressed play first to start the playback which would already be covered by handlePlayWhenReadyChange.
There was a problem hiding this comment.
configureAudioSession now only activates the session when playWhenReady is true
dcvz
reviewed
Oct 10, 2023
| @@ -908,6 +908,7 @@ public class RNTrackPlayer: RCTEventEmitter, AudioSessionControllerDelegate { | |||
| } | |||
|
|
|||
| func handlePlayWhenReadyChange(playWhenReady: Bool) { | |||
| configureAudioSession(); | |||
There was a problem hiding this comment.
One more thing I just thought of. This will also trigger when going into pause mode, which really shouldn't do anything right (it'll try to activate session again)? Should we be filtering here for just when playWhenReady is true?
There was a problem hiding this comment.
we are now checking for playWhenReady in configureAudioSession
- deactivate the session when there is no current item to be played - activate & configure the audio session when there is an item to be played and the player has been configured to start when it is ready loading
puckey
force-pushed
the
fix/premature-audio-session-activation
branch
from
657eb21 to
424c326
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 commit fixes the issue where the audio session is prematurely activated when
TrackPlayer.play()orTrackPlayer.setPlayWhenReady(true)is called with an empty queue.play&setPlayWhenReadytohandlePlayWhenReadyChangeconfigureAudioSessiontohandleAudioPlayerCurrentItemChangewhen we go from no queue items to the first or the other way aroundsetActiveparameter fromconfigureAudioSession. Instead, activate the audio session when there is acurrentItemandplayWhenReady == true. Deactivate the session when there is nocurrentItem.try? audioSessionController.deactivateSession()beforetry? audioSessionController.activateSession()until we can figure out why it is necessary (see [4.0.0-rc06] [iOS] Media player remote center remains invisible while playing audio after running a Unity game on iOS 16 #2110)