Handle Android 15+ audio focus changes / allow platform to handle focus in AV fragment #17054
Conversation
|
Sounds interesting to me, can do so |
|
Sound tags are currently only played if the app is the top app, so theoretically there shouldn't be any problems with keep using MediaPlayer. So, if something isn't broken, it doesn't need to be fixed. IIRC, SoundTagPlayer handles errors well, but David will have a better opinion than me. |
| continuation.ensureActive() | ||
| Timber.d("starting sound tag") | ||
| start() | ||
| if (requestAudioFocus() == AudioManager.AUDIOFOCUS_REQUEST_GRANTED) { |
There was a problem hiding this comment.
It feels like the error case doesn't affect the continuation, meaning that we block the coroutine
There was a problem hiding this comment.
okay will look more closely - this is important but not urgent, just something I tossed up while skimming android 15 changelist. So there is time to investigate
@BrayanDSO agreed using base MediaPlayer is not a bad thing. I just think ExoPlayer may allow for less code in our repo as it offers more functionality out of the box and our requirements match it pretty well. Agreed SoundTagPlayer implementation here is pretty well done though, so it would be a low-priority task IMHO
There was a problem hiding this comment.
Exo player removes the headache of handling any error manually though, so less maintenance in future :)
There was a problem hiding this comment.
Took a minute to read up on CancellableContinuation and how it is supposed to work, or how I think it is supposed to work
And you are correct, the continuation was ignored but there is now seemingly a branch now where it may be left uncontinued indefinitely
I added an else to the conditional then, logged out that we couldn't get focus and cancelled the continuation
I think that's how this stuff is supposed to work but I have never played with these APIs before so definitely needs a look
mikehardy
added
the
Needs Author Reply
Recommended style, see: https://developer.android.com/media/optimize/audio-focus "Note: If you use ExoPlayer, consider letting ExoPlayer automatically manage audio focus by calling setAudioAttributes with true for the handleAudioFocus parameter. With this behavior enabled, your app shouldn't include any code for requesting or responding to audio focus changes"
Do not attempt to play sounds if audio focus request failed See: https://developer.android.com/about/versions/15/behavior-changes-15#audio-focus "Apps that target Android 15 (API level 35) must be the top app or running a foreground service in order to request audio focus. If an app attempts to request focus when it does not meet one of these requirements, the call returns AUDIOFOCUS_REQUEST_FAILED"
mikehardy
force-pushed
the
platform-handles-audio-focus
branch
from
f36271a
to
c5e5d23
Compare
mikehardy
removed
the
Needs Author Reply
Purpose / Description
Android 15+ apps have audio focus restrictions applied
See: https://developer.android.com/about/versions/15/behavior-changes-15#audio-focus
It is recommended, if you are using ExoPlayer (which the new AV fragment in #16769 does), to allow the platform to handle audio focus, and this PR also includes a commit to enable that.
The SoundTagPlayer should be refactored to use ExoPlayer in my opinion, now that we have that as a dependency, then audio focus code should be removed from SoundTagPlayer. That is out of scope for this PR though as it requires a large refactor
Approach
How Has This Been Tested?
Warning
automated tests only, testing while not in foreground is not something I'm sure how to do
@david-allison If I recall correctly you just did a big refactor to handle auto-play / start / stop etc, so you will know if shortcutting out of the "play sounds" series of steps on request failure will result in a state issue that will cause problems later when "stop" is attempted. I believe it is always valid to attempt to stop playback or to attempt focus abandon even if you don't have it so it should be okay? The playback stop has state exceptions handled, and focus abandon failure is unimportant...
@criticalAY the ExoPlayer setup change might be interesting for you? And/or you may want to attempt the SoundTagPlayer port to ExoPlayer if that seems like a good idea
Learning (optional, can help others)
Scan of https://developer.android.com/about/versions/15/behavior-changes-15 (and the 14 changes as well...)
Checklist
Please, go through these checks before submitting the PR.