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

Using ForwardingPlayer in MusicService #9897

Closed
marekkondracki opened this issue Jan 25, 2022 · 4 comments
Closed

Using ForwardingPlayer in MusicService #9897

marekkondracki opened this issue Jan 25, 2022 · 4 comments

Comments

@marekkondracki
Copy link

I'm tried to set new ForwardingPlayer with custom callbacks in my project, but it didn't work. So I tried with UAMP project. It also didn't work.

I swapped code, according to 2.16.0 release notes, in this file: MusicService.kt

mediaSessionConnector.setPlayer(newPlayer)

with:

        mediaSessionConnector.setPlayer(object : ForwardingPlayer(newPlayer) {
            override fun prepare() {
                super.prepare()
                Log.i(TAG, "prepare")
            }

            override fun seekBack() {
                super.seekBack()
                Log.i(TAG, "seekBack")
            }

            override fun seekForward() {
                super.seekForward()
                Log.i(TAG, "seekForward")
            }

            override fun getSeekBackIncrement() = 12000L

            override fun getSeekForwardIncrement() = 18000L
        }
        )

No one of callbacks above never fired. I can't find any information about how to use ForwardingExoplayer.

@marcbaechinger
Copy link
Contributor

The ForwardingPlayeras a class is working fine, if it gets called. I used it various times. It may be that it is not called if you create an anonymous inner class as above for the MediaSessionConnector exclusively.

Are you sure that the commands arrive through the MediaSession? In UAMP for example it could be that a command comes from a PendingIntent of the notification manager. If this is the case, the ForwardingPlayer that you pass to mediaSessionConnector.setPlayer() would not be called. In the case of UAMP you want to look for notificationManager.showNotificationForPlayer(currentPlayer) as well.

So probably this makes the forwarding player being called:

Player forwardingPlayer = new ForwardingPlayer(player);

mediaSessionConnector.setPlayer(forwardingPlayer);
notificationManager.showNotificationForPlayer(forwardingPlayer)

@marekkondracki
Copy link
Author

So I pass ForwardingPlayer by:

mediaSessionConnector?.setPlayer(CustomForwardingPlayer((currentPlayer)))
playerNotificationManager?.setPlayer(CustomForwardingPlayer((currentPlayer)))

After click seek forward on notification, callback seekForward() in ForwardingPlayer was fired, but getSeekForwardIncrement() did not. I checked ExoPlayer sources and assumed that only external calls on ForwardingPlayer, from mediaSessionConnector or notificationManager will work cause they pass through ForwardingPlayer.

I assumed that ExoPlayer.Builder() returns SimpleExoPlayer which is ExoPlayerImpl() and method getSeekForwardIncrement() returns value passed by ExoPlayerImpl constructor, not ForwardingPlayer.

Before 2.16.0, I used DefaultControlDispatcher in case, I want to change seek/forward values dynamically, I just passplayerNotificationManager.setControlDispatcher(defaultDispatcher).

Do now I need to build new ExoPlayer every time I want to change these values?

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Jan 25, 2022

No, you don't need to build ExoPlayer each time when you change the increment.

But you are right this is confusing and we need at least to document this properly on the ForwardingPlayer because I think it is quite likely that other people make the same assumption. You actually hit a special case where two methods depend on each other and this is not ideal for the delegation that ForwardingPlayer is implemented.

Let me explain in more detail:

The ForwardingPlayer is using composition and delegates all calls to the player that you pass to the constructor. When now seekBack() is called and delegated to the player, then that player uses the result of getSeekBackIncrement() of that player to get the seek increment. It never asks the ForwardingPlayer due to how this is implemented. Instead when you attempt to change the seek increment you need to override both method. This isn't clear and needs at least to be documented clearly on ForwardingPlayer.getSeekBackIncrement() and ForwardingPlayer.getSeekForwardIncrement().

What you want to achieve probably could look like this in code:

    ForwardingPlayer forwardingPlayer = new ForwardingPlayer(player) {

      long seekBackIncrement = 3_000; // 3 seconds
      long seekForwardIncrement = 20_000; // 20 seconds

      public void setSeekBackIncrement(long seekBackIncrement) {
        this.seekBackIncrement = seekBackIncrement;
      }

      public void setSeekForwardIncrement(long seekForwardIncrement) {
        this.seekForwardIncrement = seekForwardIncrement;
      }
      
      @Override
      public long getSeekBackIncrement() {
        return seekBackIncrement;
      }

      @Override
      public void seekBack() {
        seekToOffset(-seekBackIncrement);
      }

      @Override
      public long getSeekForwardIncrement() {
        return seekForwardIncrement;
      }

      @Override
      public void seekForward() {
        seekToOffset(seekForwardIncrement);
      }

      private void seekToOffset(long offsetMs) {
        long positionMs = getCurrentPosition() + offsetMs;
        long durationMs = getDuration();
        if (durationMs != C.TIME_UNSET) {
          positionMs = min(positionMs, durationMs);
        }
        positionMs = max(positionMs, 0);
        seekTo(positionMs);
      }
    };

I marked this issue as a documentation candidate.

@marekkondracki
Copy link
Author

Thank you for clear explanation and fast response!

icbaker added a commit to androidx/media that referenced this issue Feb 4, 2022
This makes the delegation model more explicit, and prevents the javadoc
compiler from just pulling in the Player javadoc automatically - which
can lead to some confusion when some method definitions in Player depend
on other methods (e.g. seekForward() is defined in terms of
getSeekForwardIncrement()).

Issue: google/ExoPlayer#9897

#minor-release

PiperOrigin-RevId: 426359004
icbaker added a commit that referenced this issue Feb 4, 2022
This makes the delegation model more explicit, and prevents the javadoc
compiler from just pulling in the Player javadoc automatically - which
can lead to some confusion when some method definitions in Player depend
on other methods (e.g. seekForward() is defined in terms of
getSeekForwardIncrement()).

Issue: #9897

#minor-release

PiperOrigin-RevId: 426359004
icbaker added a commit that referenced this issue Feb 23, 2022
This makes the delegation model more explicit, and prevents the javadoc
compiler from just pulling in the Player javadoc automatically - which
can lead to some confusion when some method definitions in Player depend
on other methods (e.g. seekForward() is defined in terms of
getSeekForwardIncrement()).

Issue: #9897

#minor-release

PiperOrigin-RevId: 426359004
icbaker added a commit to androidx/media that referenced this issue Feb 25, 2022
This makes the delegation model more explicit, and prevents the javadoc
compiler from just pulling in the Player javadoc automatically - which
can lead to some confusion when some method definitions in Player depend
on other methods (e.g. seekForward() is defined in terms of
getSeekForwardIncrement()).

Issue: google/ExoPlayer#9897

#minor-release

PiperOrigin-RevId: 426359004
@google google locked and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants