fix(android): only the active player may stop the shared MediaPlaybackService - #919
Open
tvanlaerhoven wants to merge 1 commit into
Open
tvanlaerhoven wants to merge 1 commit into
tvanlaerhoven wants to merge 1 commit into
Conversation
…kService Destroying an inactive player context, or disabling its background audio, unconditionally called stopForegroundService() on the shared service. This removed the foreground notification of the player that was still active. Gate the service stop on ownsMediaSession(); always unbind. Fixes #918 Co-Authored-By: tom.vanlaerhoven <tom.vanlaerhoven@dolby.com>
Contributor
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
This branch has not been deployed
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
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.
Summary
Fixes #918. Split out from the review discussion on #917 (#917 (comment)); not part of the 11.7.0 release.
All player contexts with background audio bind to one shared
MediaPlaybackService.destroy()already gates the media session connector teardown onownsMediaSession(), but the service stop one line above it was unconditional. Destroying an inactive player therefore removed the foreground notification and stopped the service of the player that was still active.applyBackgroundPlaybackConfig()(disabling background audio at runtime) had the same unconditional call.Unbinding stays unconditional: when the last client unbinds, Android destroys the service and
onDestroy()cleans up the session.ownsMediaSession()returnstruewhenbinder == null, so single-player behaviour is unchanged.Not done here (optional hardening from the issue): making
MediaPlaybackBinder.stopForegroundService()take the calling context and ignore non-owners.Link to Devin session: https://dolby.devinenterprise.com/sessions/56f1f4577f7044829520dd18f54efc76
Open in Devin Desktop: https://dolby.devinenterprise.com/desktop/session/56f1f4577f7044829520dd18f54efc76?variant=devin
Requested by: @tvanlaerhoven