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

PlayerView and Playercontrollview Next button not clickable even when using ForwardingPlayer for the Last mediaitem of Playlist #1588

Closed
1 task
pawaom opened this issue Aug 2, 2024 · 6 comments
Assignees

Comments

@pawaom
Copy link

pawaom commented Aug 2, 2024

Version

Media3 1.4.0

More version details

No response

Devices that reproduce the issue

Samsung Galaxy Tab 7 android version 14
Pixel 2 emulator api 26 Android 8.0.0

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Yes

Reproduction steps

Add a Playercontrolview in activity_player.xml

<androidx.media3.ui.PlayerControlView

      android:background="@color/purple_200"
      android:id="@+id/playerControlView"
      android:layout_width="match_parent"
      android:layout_height="wrap_content"
      android:layout_gravity="center"
      app:controller_layout_id="@layout/custom_exo_controller_view"

      app:show_timeout="0" />

custom_exo_controller_view.xml

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_gravity="bottom"

    android:layoutDirection="ltr"
    android:orientation="vertical"
    android:paddingStart="20dp"
    android:paddingEnd="20dp"
    tools:targetApi="28">

    <LinearLayout
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:gravity="center"
        android:orientation="horizontal"
        android:paddingTop="4dp">

        <ImageButton
            android:id="@id/exo_shuffle"
            style="@style/ExoStyledControls.Button.Bottom.Shuffle"
            app:tint="@color/black"
            app:tintMode="src_in" />

        <ImageButton
            android:id="@id/exo_prev"
            style="@style/ExoMediaButton.Previous"
            app:tint="@color/black"
            app:tintMode="src_in" />

        <ImageButton
            android:id="@+id/exo_play"
            style="@style/ExoMediaButton.Play"
            app:tint="@color/black"
            app:tintMode="src_in" />

        <ImageButton
            android:id="@+id/exo_pause"
            style="@style/ExoMediaButton.Pause"
            app:tint="@color/black"
            app:tintMode="src_in" />

        <ImageButton
            android:id="@id/exo_next"
            style="@style/ExoMediaButton.Next"
            app:tint="@color/black"
            app:tintMode="src_in" />

        <ImageButton
            android:id="@id/exo_repeat_toggle"
            style="@style/ExoMediaButton"
            app:tint="@color/black"
            app:tintMode="src_in" />

    </LinearLayout>

    <LinearLayout
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:layout_marginTop="4dp"
        android:gravity="center_vertical"
        android:orientation="horizontal">

        <TextView
            android:id="@id/exo_position"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:includeFontPadding="false"
            android:paddingLeft="4dp"
            android:paddingRight="4dp"
            android:textColor="#ff323232"
            android:textSize="14sp"
            android:textStyle="bold" />

        <androidx.media3.ui.DefaultTimeBar
            android:id="@id/exo_progress"
            android:layout_width="0dp"
            android:layout_height="26dp"
            android:layout_weight="1"
            app:played_color="@color/teal_200"
            app:unplayed_color="@color/grey" />

        <TextView
            android:id="@id/exo_duration"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:includeFontPadding="false"
            android:paddingLeft="4dp"
            android:paddingRight="4dp"
            android:textColor="#ff323232"
            android:textSize="14sp"
            android:textStyle="bold" />

    </LinearLayout>

</LinearLayout>

in initializeSessionAndPlayer() in DemoPlaybackService

add the following code

val forwardingPlayer : Player = object : ForwardingPlayer(player) {
            //https://github.com/google/ExoPlayer/issues/9971
            // Overrides to always enable COMMAND_SEEK_TO_NEXT
            override fun getAvailableCommands(): Player.Commands {
                return wrappedPlayer
                    .availableCommands
                    .buildUpon()
                    .add(COMMAND_SEEK_TO_NEXT)
                    .build()
            }

            override fun isCommandAvailable(command: @Player.Command Int): Boolean {
                return player.isCommandAvailable(command) || command == COMMAND_SEEK_TO_NEXT
            }

            // Override to implement your custom action when seeking.
            override fun seekToNext() {
                if (wrappedPlayer.hasNextMediaItem()) {
                    // Dispatch to the wrapped player if not at the final media item
                    wrappedPlayer.seekToNextMediaItem()
                }
                else {
                    wrappedPlayer.seekTo(0,
                        C.TIME_UNSET)

                }
            }
        }

        mediaLibrarySession =
            MediaLibrarySession.Builder(this, forwardingPlayer, createLibrarySessionCallback())
                .also { builder -> getSingleTopActivity()?.let { builder.setSessionActivity(it) } }
                .build()

in setController() in PlayerActivity add the code

playerControlView = findViewById(R.id.playerControlView)
playerControlView.player = controller
        playerControlView.showShuffleButton = true
        playerControlView.repeatToggleModes = Player.REPEAT_MODE_OFF

I have referred this

here is a demo video

VideoCook_20240802_112417680.mp4

Expected result

The next button should work even for the Last MediaItem with forwardingplayer, in the playerview and playercontrolview like it does in the notification

Actual result

The next button does not work even for the Last MediaItem with forwardingplayer, in the playerview and playercontrolview like it does in the notification

Media

Jazz and Blues or any other album or list of mediaitems

Bug Report

@tianyif
Copy link
Contributor

tianyif commented Aug 2, 2024

Hi @pawaom,

Thanks for reporting! I think the referred approach only works for player UI (google/ExoPlayer#9971) and the legacy MediaController (notification), but don't work for the media3 MediaController.

For the player UI case, the Player set to the PlayerControlView is exactly your custom ForwardingPlayer. Then whether to enable the "next" button is determined by the result of Player.isCommandAvailable, where your custom code will take effect.

For the legacy controller case (media notification is using the legacy controller to control the session), the legacy MediaController doesn't have isCommandAvailable method (it doesn't implement the Player interface) and system UI doesn't use the PlayerControlView in our library. Also legacy controller doesn't check if the command is available upon the corresponding method call at its side (eg. skipToNext), then whether this method call can actually go through directly depends on the call to the player in the session, thus your custom code can take effect either.

However, for media3 MediaController, it implements Player interface. If you set the controller as the player in the PlayerControlView, it will call Player.isCommandAvailable to determine whether to enable the "next" button as well, however, this call will firstly flow through the MediaController's implementation. Taking look at it's implementation, it checks with a maintained property intersectedPlayerCommands. It is a intersection of the available player commands set on the session and from the player, which means that if one of them doesn't contain the Command.SEEK_TO_NEXT, then it will be unavailable. Then why the SEEK_TO_NEXT becomes unavailable? In fact, when the underlying ExoPlayer plays the last item, the SEEK_TO_NEXT becomes unavailable at the ExoPlayer side, then it will send an event onAvailableCommandsChanged to its listener, then ForwardingPlayer receives it and further forwards this event with a new availableCommands (no SEEK_TO_NEXT) to the session and then controller. This process doesn't consider your custom implementation in ForwardingPlayer.isCommandAvailable, thus, the controller will finally get a updated intersectedPlayerCommands without SEEK_TO_NEXT.

I'll discuss internally to see if we already have some approach somewhere to workaround this problem or we need to add one. Will update this issue next week.

@tianyif
Copy link
Contributor

tianyif commented Aug 5, 2024

Hi @pawaom,

I spoke with the team today, and it seems that we already had the similar issue reported before, which caused by the inconsistency in the custom ForwardingPlayer.

In fact, we have a ForwardingSimpleBasePlayer that addresses this issue. It allows you to wrap your underlying Player and override the state that should be forwarded to another player, including the available commands that you hope to declare aside from those coming from the underlying player. However, this class hasn't been included in the latest release, so you may want to copy this class and manually add it into your code base and extend it until it is officially released.

If your problem is just about letting the next button show for the last item and the player play the first item, then you can probably set the repeat mode to REPEAT_MODE_ALL, without bothering to copy the above unreleased code.

Hope the above can help!

@pawaom
Copy link
Author

pawaom commented Aug 6, 2024

I know about the REPEAT_MODE_ALL however that is not what we require.

I copied the ForwardingSimpleBasePlayer how ever it gives me the following error Expected 1 arguments but found 3 here

image

@tonihei
Copy link
Collaborator

tonihei commented Aug 6, 2024

@pawaom This is because there were some additional changes to the SimpleBasePlayer super class to make this possible. If you keep a temporary fork of ForwardingSimpleBasePlayer until the class is officially released in 1.5.0, you will also need to keep a forked version of SimpleBasePlayer I'm afraid.

@pawaom
Copy link
Author

pawaom commented Aug 6, 2024

any idea when the alpha or beta version of media3 1.5.0 will be released and will the above changes be included in those alpha and beta releases, if its within a few days I will wait.

@icbaker
Copy link
Collaborator

icbaker commented Aug 6, 2024

1.5.0-alpha01 is planned for the first half of September.

@tianyif tianyif closed this as completed Aug 9, 2024
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