androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
Apache License 2.0
1.36k stars 322 forks source link

Add forwarding player with simplified extension points that ensure consistency (e.g. ForwardingSimpleBasePlayer) #1183

Open Belhaver1993 opened 3 months ago

Belhaver1993 commented 3 months ago

I have a similar issue to the one described here.

Based on the data I have in MediaMetadata I want to have control whether COMMAND_SEEK_TO_NEXT is available or not.

To reproduce issue from demo application, I altered data in catalog.json from demos/session_service and changed artist for few media items to empty strings in Electronic category. Then I wrapped ExoPlayer with ForwardingPlayer in DemoPlaybackService

val player = object : ForwardingPlayer(ExoPlayer.Builder(this)
        .setAudioAttributes(AudioAttributes.DEFAULT, /* handleAudioFocus= */ true)
        .build()
        .apply {
            addAnalyticsListener(EventLogger())
        }) {

    override fun getAvailableCommands(): Player.Commands {
        return super.getAvailableCommands().buildUpon().apply {
            if (currentMediaItem?.mediaMetadata?.artist?.isNotBlank() == true) {
                add(COMMAND_SEEK_TO_NEXT)
            } else {
                remove(COMMAND_SEEK_TO_NEXT)
            }
        }.build()
    }

    override fun isCommandAvailable(command: Int): Boolean {
        if (command == Player.COMMAND_SEEK_TO_NEXT) {
            return currentMediaItem?.mediaMetadata?.artist?.isNotBlank() == true
        }
        return super.isCommandAvailable(command)
    }
}

When I run the code, ExoPlayer in PlayerActivity displays that exo_next button is always available, even though when I click this button it sets some strange state in the player (no progress, old media item still plays).

But, when I add exactly the same code to the PlayerActivity

playerView.player = object : ForwardingPlayer(controller) {

  override fun getAvailableCommands(): Player.Commands {
    return super.getAvailableCommands().buildUpon().apply {
      if (currentMediaItem?.mediaMetadata?.artist?.isNotBlank() == true) {
        add(COMMAND_SEEK_TO_NEXT)
      } else {
        remove(COMMAND_SEEK_TO_NEXT)
      }
    }.build()
  }

  override fun isCommandAvailable(command: Int): Boolean {
    if (command == Player.COMMAND_SEEK_TO_NEXT) {
      return currentMediaItem?.mediaMetadata?.artist?.isNotBlank() == true
    }
    return super.isCommandAvailable(command)
  }
}

exo_next button displays state correctly.

With this info I have few questions:

  1. Why does that happen? Shouldn't MediaController respect what's inside ForwardingPlayer to be correctly shown in the playerView.player?
  2. Why media controller on the notification is still able to seekToNext? This one is strange, because in my application overriding those methods in our implementation of Player interface is enough to control the behaviour of media notification.
  3. How to wrap ExoPlayer in only one place (service) to get it work correctly?
Belhaver1993 commented 3 months ago

Hello, is there any development on this question? :)

marcbaechinger commented 3 months ago

I think the session gets the available commands via the Player.Listener.onAvailableCommandsChanged().

So for that to work with a ForwardingPlayer in the current version, you'd have to intercept the listener calls that add and remove listeners and register you own listener to the upstream player. Then when your listener is called you'd delegate to the registered listeners.

It would be an option to make this easier when MediaSessionImpl would use playerWrapper.getAvailableCommands() on that callsite instead of the arguments passed in by the listener. For now you would need to go the route with listener interception I think.

icbaker commented 3 months ago

Correctly changing the available commands using ForwardingPlayer is described here: https://developer.android.com/media/media3/exoplayer/customization#intercepting-method

We're aware this is fiddly and error-prone, and we have an internal issue (b/323900817) to experiment with adding a ForwardingSimpleBasePlayer which we hope will help make it easier to consistently modify the behaviour (or available commands) of a delegate/inner Player instance.