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

When setMediaItems is called playerCommandsFromPlayer does not include commands included via getAvailableCommands of ForwardingPlayer #1021

Closed
1 task done
aljohnston112 opened this issue Jan 25, 2024 · 13 comments
Assignees
Labels

Comments

@aljohnston112
Copy link

aljohnston112 commented Jan 25, 2024

Version

Media3 1.2.1

More version details

No response

Devices that reproduce the issue

Samsung Galaxy A12 SM-A125U

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

Create a ForwardingPlayer like so

class MyPlayer(
    private var context: Context,
) : ForwardingPlayer(
    ExoPlayer.Builder(context)
        .setSkipSilenceEnabled(true)
        .setSeekParameters(SeekParameters.EXACT)
        .build()
) {

    override fun seekToNextMediaItem() {
        // custom behavior
    }

}

Create a MediaLibrarySession like so

        mediaSession = MediaLibraryService.MediaLibrarySession.Builder(
            service,
            player,
            callback
        ).build()

Create a MediaBrowser like so:

            browser = MediaBrowser.Builder(applicationContext, sessionToken)
                .buildAsync()
                .await()

Then call MediaBrowser.seekToNextMediaItem()

browser.seekToNextMediaItem()

and notice that ForwardingPlayer.seekToNextMediaItem() is not called.

Expected result

For the ForwardingPlayer to get the seekToNextMediaItem() call.

Actual result

The seekToNextMediaItem() method of the ForwardingPlayer is not called.

Media

Not applicable

Bug Report

@aljohnston112
Copy link
Author

This was a misunderstanding on my part. COMMAND_SEEK_TO_NEXT_MEDIA_ITEM was not an available player command.

@aljohnston112
Copy link
Author

aljohnston112 commented Jan 25, 2024

Actually, there still is no call when I add these methods to the ForwardingPlayer implementation

    override fun getAvailableCommands(): Player.Commands {
        return super.getAvailableCommands().buildUpon().add(COMMAND_SEEK_TO_NEXT_MEDIA_ITEM).build()
    }

    override fun isCommandAvailable(command: Int): Boolean {
        return super.isCommandAvailable(command) || command == COMMAND_SEEK_TO_NEXT_MEDIA_ITEM
    }

I even intercepted in the Player.Listener

        @OptIn(UnstableApi::class) override fun onAvailableCommandsChanged(availableCommands: Player.Commands) {
            super.onAvailableCommandsChanged(
                availableCommands.buildUpon().add(COMMAND_SEEK_TO_NEXT_MEDIA_ITEM).build()
            )
        }

The seekToNextMediaItem method of the MediaontrollerImplBase returns here

  @Override
  public void seekToNextMediaItem() {
    if (!isPlayerCommandAvailable(Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM)) {
      return;
    }

@aljohnston112 aljohnston112 reopened this Jan 25, 2024
@aljohnston112
Copy link
Author

aljohnston112 commented Jan 25, 2024

The intersectedPlayerCommands of the MediaControllerImplBase do not get the commands set in getAvailableCommands of the ForwardingPlayer

@aljohnston112
Copy link
Author

aljohnston112 commented Jan 25, 2024

I put breakpoints on the 4 writes of intersectedPlayerCommands in MediaControllerImplBase, and before the MediaBrowser calls seekToNextMediaItem the COMMAND_SEEK_TO_NEXT_MEDIA_ITEM flag is included in playerCommandsFromPlayer in onConnected, but not later when onAvailableCommandsChangedFromPlayer is called in the setMediaItems call.

@aljohnston112
Copy link
Author

aljohnston112 commented Jan 25, 2024

Ok, so it looks like setMediaItems is not getting flags set by the ForwardingPlayer. Stepping through some code, it looks like ExoPlayerImpl is overwriting the commands available in the updatePlaybackInfo method. Also, updateAvailableCommands is using the ExoPlayerImpl as the wrappingPlayer rather than the ForwardingPlayer.

Looks like setting up the ForwardingPlayer here would fix the issue, but I am not familiar with the code and whether that would cause any issues.

    public ExoPlayer build() {
      checkState(!buildCalled);
      buildCalled = true;
      return new ExoPlayerImpl(/* builder= */ this, /* wrappingPlayer= */ null);
    }

@aljohnston112 aljohnston112 changed the title ForwardingPlayer.seekToNextMediaItem is not called when MediaBrowser.seekToNextMediaItem is called setMediaItems does not get commands included by getAvailableCommands of ForwardingPlayer Jan 25, 2024
@aljohnston112 aljohnston112 changed the title setMediaItems does not get commands included by getAvailableCommands of ForwardingPlayer When setMediaItems is called playerCommandsFromPlayer does not include commands included via getAvailableCommands of ForwardingPlayer Jan 25, 2024
@aljohnston112
Copy link
Author

Is there some way to force an update of the flags after ExoPlayerImpl overwrites them?

@tonihei
Copy link
Collaborator

tonihei commented Jan 26, 2024

Can you explain a bit more what you'd like to achieve? If it's just about changing the available commands for media controllers, the recommended way is to configure that in MediaSession.Callback.onConnect as shown here: https://developer.android.com/media/media3/session/background-playback#media-notification-controller

@tonihei tonihei self-assigned this Jan 26, 2024
@aljohnston112
Copy link
Author

aljohnston112 commented Jan 28, 2024

I was trying to get MyPlayer to overwrite the available commands after setMediaItems is called because the next media item may not be known until the playing of the current media item is done.

Setting the COMMAND_SEEK_TO_NEXT_MEDIA_ITEM in the onConnect callback did not work either. This is what I did.

        @OptIn(UnstableApi::class)
        override fun onConnect(
            session: MediaSession,
            controller: MediaSession.ControllerInfo
        ): MediaSession.ConnectionResult {
            val playerCommands =
                session.player.availableCommands.buildUpon()
                    .add(COMMAND_SEEK_TO_NEXT_MEDIA_ITEM)
                    .build()
            return MediaSession.ConnectionResult.AcceptedResultBuilder(session)
                .setAvailablePlayerCommands(playerCommands)
                .build()
        }

I then thought instead that I could add a next media item to the playlist, and of course, since ExoPlayer sees a next media item, it makes the command available.

This works, but it seems as though the commands made available through the onConnect callback, the onAvailableCommandsChanged callback, the ForwardingPlayer.getAvailableCommands, method and the ForwardingPlayer.isCommandAvailable method are not enough to make a command, that ExoPlayer does not make available, available which seems like a bug to me.

@tonihei
Copy link
Collaborator

tonihei commented Jan 29, 2024

I tested this out and I think there may be some confusion around COMMAND_SEEK_TO_NEXT_MEDIA_ITEM and COMMAND_SEEK_TO_NEXT. If both are available, the button click in the System notification maps to seekToNext() whereas seekToNextMediaItem() is used as a fallback. They may have different behaviors depending on the media (e.g. seekToNext in a live stream skips forward to the live edge. Can you try overriding COMMAND_SEEK_TO_NEXT/seekToNext() only?

@aljohnston112
Copy link
Author

That makes sense, but the seek to next command is still overwritten by ExoPlayer when setMediaItem is called.

@tonihei
Copy link
Collaborator

tonihei commented Jan 29, 2024

This may be related to you override the listener. If all listener calls get intercepted, then the you always overwrite whatever commands ExoPlayer makes available.

@aljohnston112
Copy link
Author

It is the other way around. ExoPlayer is overwriting the commands I tried to make available via the listener.

@tonihei
Copy link
Collaborator

tonihei commented Jan 29, 2024

If you feel something is broken, please provide a more complete example that allows us to reproduce the issue. Otherwise I think there must be some missing piece in your forwarding player somewhere.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants