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

Decoder reuse in MediaSessionConnector #5891

Closed
AndrazP opened this issue May 15, 2019 · 6 comments
Closed

Decoder reuse in MediaSessionConnector #5891

AndrazP opened this issue May 15, 2019 · 6 comments
Assignees
Labels

Comments

@AndrazP
Copy link

AndrazP commented May 15, 2019

[REQUIRED] Searched documentation and issues

Recent Improved decoder reuse in ExoPlayer blog post explains how to make sure your app benefits from decoder reuse:

When re-preparing the player with a different video, call prepare() with the new MediaSource without calling stop() first. This is because by default, ExoPlayer will still release decoders when stop() is called.

I noticed MediaSessionConnector.java calls stopPlayerForPrepare(boolean playWhenReady) before preparing new source.

[REQUIRED] Question

Is there a reason why MediaSessionConnector calls player.stop() before preparing new source? Is this not against best practices for decoder reuse?

@ojw28
Copy link
Contributor

ojw28 commented May 15, 2019

Good question ;). I doubt there's a good reason... @marcbaechinger - Think we can safely remove the stop() call, and rename stopPlayerForPrepare to setPlayWhenReady? Or is there a reason those calls are there?

ojw28 pushed a commit that referenced this issue May 15, 2019
Issue: #5891
PiperOrigin-RevId: 248369509
ojw28 pushed a commit that referenced this issue May 15, 2019
Issue: #5891
PiperOrigin-RevId: 248369509
@ojw28 ojw28 closed this as completed May 15, 2019
@tonihei
Copy link
Collaborator

tonihei commented May 16, 2019

Just to document the potential change in behavior after this change: A PlaybackPreparer which tries to start from a non-default start position may now fail if does not call player.stop() itself:

public void onPrepare() {
  player.seekTo(startPosition);
  player.prepare(newMediaSource, /* resetPosition */ false, /* resetState */ true);
}

That is because this seek will be treated as a seek in the old media due to the missing stop operation. This may (or may not) cause unexpected effects for the start position in the new media even if we keep the position. If that problem occurs, it can be fixed by calling player.stop(true) first in PlaybackPreparer methods.

@ojw28 ojw28 reopened this May 16, 2019
@ojw28
Copy link
Contributor

ojw28 commented May 16, 2019

As discussed, we need a slightly bigger change to correctly handle all cases. Reopening.

ojw28 pushed a commit that referenced this issue May 20, 2019
@ojw28 ojw28 closed this as completed May 20, 2019
ojw28 pushed a commit that referenced this issue Jun 3, 2019
Issue: #5891
PiperOrigin-RevId: 248369509
ojw28 pushed a commit that referenced this issue Jun 3, 2019
@loki666
Copy link
Contributor

loki666 commented Jun 26, 2019

I know this now closed, but I have an issue after updating to 2.10.2.

My music applications prepare the player with the latest playing queue when it starts, but now my "play" buttons (which uses the MediaControllerCompat.TransportControls) fails because the player is in READY state, so the onPlay() callback in MediaSessionConnector does nothing

this is because the setPlayWhenReady() has been removed from onPlay()

funny thing, if I use the play button in the Notification from the PlayerNotificationManager, it works since the onReceive calls dispatchSetPlayWhenReady()

how to solve this? TIA

@tonihei
Copy link
Collaborator

tonihei commented Jun 26, 2019

@loki666 Could you please open a new issue to discuss this question/problem?

@loki666
Copy link
Contributor

loki666 commented Jun 26, 2019

#6093

@google google locked and limited conversation to collaborators Sep 27, 2019
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

5 participants