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

setPlaybackParameters() not overwriten by live adjusment #10883

Closed

Conversation

stevemayhew
Copy link
Contributor

There are multiple sequences of handler messages which can cause a user request to update playback speed to be overwritten by the LivePlaybackSpeedControl value.

All the sequeunces share a common feature that:

  1. a queued MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL executes after the MSG_SET_PLAYBACK_PARAMETERS
  2. The PlaybackParameters in this internal change are not the intended speed.

The easiest way to reproduce this is with AC-3 pass-thru audio, in this case the audio renderer does not properly support speed change, see bug:
#10865

According to this commit,

Add back support for setting audio pitch [andrewlewis]

This callback to report the internal speed change is needed for the renderers to run slow motion properly.

In any event, there is no reason to propegate this internal speed change to the user speed setting (ExoPlayerImplInternal.playbackParameters). The change simply fixes MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL not to update the user setting.

There are multiple sequences of handler messages which can cause a user
request to update playback speed to be overwritten by the `LivePlaybackSpeedControl` value.

All the sequeunces share a common feature that:

1. a queued `MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL` executes after the `MSG_SET_PLAYBACK_PARAMETERS`
2. The `PlaybackParameters` in this internal change are not the intended speed.

The easiest way to reproduce this is with AC-3 pass-thru audio, in this case the audio renderer does not properly
support speed change, see bug:
  google#10865

According to this commit,

 [Add back support for setting audio pitch](google@74c493f51e) [andrewlewis]

This callback to report the internal speed change is needed for the renderers to run slow motion properly.

In any event, there is no reason to propegate this internal speed change to the user speed setting (`ExoPlayerImplInternal.playbackParameters`).
The change simply fixes `MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL` not to update the user setting.
@stevemayhew
Copy link
Contributor Author

stevemayhew commented Dec 22, 2022

This is the fix for #10882

@stevemayhew
Copy link
Contributor Author

I'll open the same pull request on AndroidX.media if this one is acceptable.

@tonihei
Copy link
Collaborator

tonihei commented Jan 26, 2023

I'd suggest we continue the discussion on #10882. This PR doesn't fully work anyway I believe because it cuts out all potential updates to PlaybackInfo when a speed change is triggered from MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL. This may be correct for the speed changes related to automatic live stream adjustments, but we definitely want to see the PlaybackInfo update for cases where a manually set speed can't be applied or has to be changed.

@tonihei tonihei closed this Jan 26, 2023
@google google locked and limited conversation to collaborators Mar 28, 2023
@stevemayhew stevemayhew deleted the p-live-speed-overwrites-user-fix branch September 1, 2023 17:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants