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

Support onSetPlaybackSpeed(speed, pitch) in MediaSessionConnector.ComponentListener #8229

Closed
mhelder opened this issue Nov 16, 2020 · 5 comments
Assignees

Comments

@mhelder
Copy link

mhelder commented Nov 16, 2020

Use case description

Android API 29 MediaController added setPlaybackSpeed(float), which was backported in MediaControllerCompat with androidx.media:media:1.2.0-alpha01. 1.2.0 become stable mid-September.

On the receiving end, MediaSessionCompat.Callback also added onSetPlaybackSpeed(float speed). Interestingly, the docs do not yet show this method, but it's definitely there for media:1.2.0. It's easy to miss anyways, because the Callback is an abstract class with empty methods, so nothing it forcing extending classes to actually implement this functionality. The same goes for ExoPlayer's MediaSessionConnector, which has a ComponentListener that extends MediaSessionCompat.Callback, but does not yet override onSetPlaybackSpeed(float speed).

TLDR: support for setting the playback speed is currently missing in ComponentListener.

Proposed solution

Override MediaSessionCompat.Callback.onSetPlaybackSpeed(float speed) in ComponentListener and forward the playback speed to the actual Player instance. In its most simple form, this could look somewhat like this:

@Override
public void onSetPlaybackSpeed(float speed) {
  float playbackPitch = player.getPlaybackParameters().pitch;
  player.setPlaybackParameters(new PlaybackParameters(speed, playbackPitch));
}

Alternatives considered

I wasn't sure whether to file this as bug or feature request, but settles on the latter, because as long as the mediasession extension is not targeting media:1.2.0, this is working as expected.

If you accept PRs for this matter, I'd be happy to submit one addressing the missing functionality. Although I'm not quite sure if there are any other features from media:1.1.0 or 1.2.0 that would need to be picked up at the same time?

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Nov 16, 2020

Thanks for reporting. I will look into that.

I'm a bit surprised that there is no constant ACTION_SET_PLAYBACK_SPEED speed, because player may not support that. In the case of ExoPlayer, we do support it (unless tunneling is enabled). So I'd have expected that we can signal that, so that the client knows whether this is supported.

I check whether we need some more stuff beside this. Many thanks!

@mhelder
Copy link
Author

mhelder commented Dec 8, 2020

@marcbaechinger I noticed 2.12.2 was released about a week ago and I had been kind of hoping that adding support for changing playback speed would be small enough to make it into the next minor/patch update. Unfortunately it didn't, which leads me to the following question(s):

Any chance you could provide an indication of whether adding support for changing playback speed is planned for a future release? Is there any more stuff to be done before that's preventing us from targeting media:1.2.0?

I'd be happy to submit a PR if would speed up (pun intended) the process.

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Dec 8, 2020

We are working on this. I started the implementation in MediaSessionConnector and I noticed that the constant PlaybackStateCompat.ACTON_SET_PLAYBACK_SPEED is missing with media-1.2.0. I filed a bug internally as we want to have the implementation in MediaSessionConnector in the same way as for other playback actions. The constant has been added internally now (I think yesterday). With this we know the value of that constant and we can provide correct support for this before that is released. Even more important, we now can provide this with the MediaSessionConnector without risking that it will change when the media:1.2.x will include this constant.

It's planned to work on this asap and it will land on the dev-v2 branch as soon as possible. It will be released latest with 2.13 I think, which will be in Q1/2021. I can't give you any guarantees it will land sooner in another release I'm afraid.

I will update this issue when the change lands in dev-v2.

@mhelder
Copy link
Author

mhelder commented Dec 8, 2020

That's great news, and I appreciate the update and rough timeline 👍

I'll keep an eye on dev-v2 too!

icbaker pushed a commit that referenced this issue Dec 14, 2020
Issue: #8229
#exofixit
PiperOrigin-RevId: 346968046
@marcbaechinger
Copy link
Contributor

The change landed in dev-v2 and will be released with 2.13.0.

@google google locked and limited conversation to collaborators Feb 20, 2021
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