Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Belhaver1993 opened this issue Mar 14, 2024 · 3 comments
Assignees

Comments

@Belhaver1993
Copy link

Belhaver1993 commented Mar 14, 2024

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
Copy link
Author

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

@marcbaechinger
Copy link
Contributor

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
Copy link
Collaborator

icbaker commented Mar 27, 2024

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.

@tonihei tonihei changed the title Why MediaController does not respect overridden methods in Player? Add forwarding player with simplified extension points that ensure consistency (e.g. ForwardingSimpleBasePlayer) May 7, 2024
copybara-service bot pushed a commit that referenced this issue Jul 8, 2024
This utility helps apps to forward to another Player while overriding
selected behavior or state values. The advantage to a ForwardingPlayer
is that the SimpleBasePlayer base class keeps ensuring correctness,
listener handling etc.

The default forwarding logic tries to stay as close as possible to the
original method calls, even if not strictly required by the Player
interface (e.g. calling single item addMediaItem instead of
addMediaItems if only one item is added).

Issue: #1183
PiperOrigin-RevId: 650155924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants