media
media copied to clipboard
ForwardingPlayer that stops events gets MediaController out of sync
Media3 Version
1.0.0-beta02
Devices that reproduce the issue
Emulator
Devices that do not reproduce the issue
No response
Reproducible in the demo app?
Not tested
Reproduction steps
Use the horologist media-sample.
MediaController -> MediaSession -> WearConfiguredPlayer -> ExoPlayer.
WearConfiguredPlayer will open the Bluetooth fragment if no headphones are connected, ignoring the play() command. If you close the fragment without selecting headphones, you end up on the player screen. Hitting play again will open the fragment again. After closing the fragment again, the player State is out of sync.
Workaround is to fire an event - see PR in https://github.com/google/horologist/pull/558/files
Expected result
MediaController - playWhenReady = true ExoPlayer - playWhenReady = false
Actual result
MediaController - playWhenReady = false ExoPlayer - playWhenReady = false
Media
Uamp content in media-sample
Bug Report
- [ ] You will email the zip file produced by
adb bugreport
to [email protected] after filing this issue.
Thanks for reporting! Interesting case. :)
Reason for the wrong behaviour you describe: We are masking the state change to playWhenReady=true
(and other state changes) in MediaControllerImplBase.play()
. This means we change the local playWhenReady
of the controller before we send the message over the binder to the session/player. The ForwardingPlayer
now makes the call a no-op, and no state (change) is sent back to the controller which would correct the wrongly predicted playWhenReady
state.
Your case of the very example of play
is specifically interesting. I would argue this is WAI and that the correct behaviour of your ForwardingPlayer
would be to call Player.Listener.onPlaybackSuppressionReasonChanged(@PlaybackSuppressionReason int playbackSuppressionReason)
with an accurate reason (if there is an accurate one). That resets playhenReady
. Then I could declare this an enhancement
to add sufficient @PlaybackSuppressionReason constants if needed. :)
For other Player
commands, there isn't such a reason and callback though, so by design the player actually needs to full-fill and either trigger a state change or throw an exception (I think).
MediaController
advertise their availableCommands
. If an app knows statically that a command is a no-op, the command needs to be made unavailable, for instance when not supported. This avoids the situation of sending a command when it's predictably not possible.
That works dynamically as well. Your app could (besides suppression reason) do so when playbackRule
changes and remove COMMAND_PLAY_PAUSE
. The controller receives an onAvailableCommandsChanged
event and can reflect that in the UI. The user knows ahead they can't play. Technically further when attempting to send either way, the library would then intercept the play
call on the controller side and avoid the masking failure by not sending the command.
With the above, an app has an API to cover the play
command with a suppression reason and other commands with the available commands to avoid the scenario that breaks masking in the described way.
Said all this, I didn't go through all the Player
methods to check whether there are edge-cases that aren't covered and would need a suppression reason or similar too. Happy to hear thoughts! :)
In this case, play is not a no-op, it triggers a UI flow that if completed in 15 seconds, will start playback. We just can't start yet. So we need the command to be enabled. Being enabled also means this flow can work when this comes externally via the service, like assistant or the system media controls.
It sounds like my current clunky workaround is close to the best course of action. Interestingly, I think @tonihei suggested @PlayWhenReadyChangeReason instead of @PlaybackSuppressionReason, but I'm happy to change.
If it's really WAI, then perhaps just a change to document what you've described above. To me this is the point of the Forwarding player, it can override the effect of each command. But if it breaks things we should document how to fix.
Do you think setting the suppression reason PLAYBACK_SUPPRESSION_REASON_TRANSIENT_AUDIO_FOCUS_LOSS
wouldn't be the accurate action for the ForwardingPlayer
intercepting a play
call? I'm not a believer of a single truth in design questions but for me that seems to be an accurate behaviour more than a clunky workaround. Technically it's an if-else instead of an else to inform your users.
We should look into a mechanism that sends a single update of the PlaybackInfo
only after all commands have been completed on the session side. This update can then be sent in either case to send an update for cases that do not have a suppression
reason. In cases where the API provides such a reason, an app should imo make use of it to behave according to the API.