ExoPlayer setPlaybackParameters() speed overwritten by Live Adjustment

[stevemayhew]

Media3 Version

1.0.0-beta03

Devices that reproduce the issue

• OMNI Fire TV
• Broadcom STB's running Android 10 and 11

Devices that do not reproduce the issue

unknown

Reproducible in the demo app?

No

Reproduction steps

Difficult to reproduce, I'm working back from the exact sequence of events on the player thread to produce a test case.

Basic idea flow is, set speed > 1, disable audio (and text) renderers

  player.setPlaybackParameters(new PlaybackParameters(15.0));
  DefaultTrackSelector.Parameters trackSelectorParameters = trackSelector.getParameters();
  savedParameters = trackSelectorParameters;
  
  DefaultTrackSelector.ParametersBuilder builder = trackSelectorParameters.buildUpon();
  for (int i = 0; i < player.getRendererCount(); i++) {
    if (player.getRendererType(i) != C.TRACK_TYPE_VIDEO) {
        builder.setRendererDisabled(i, true);
    }
  }
  trackSelector.setParameters(builder);

Expected result

Speed set as expected.

I'll have a pull request with a suggestion to fix this, simply don't update PlaybackParameters on the callback from MediaClock.

My notes on why think this is valid, first here are all the reads of PlaybackParameters in EPII:

1. updatePlaybackPositions() — checked for speed adjustment, if != 1f:
   • Value set into MediaClock.setPlaybackParameters()
   • passed to handleParameters() with no update (so value just passed to renderers and track selections)
2. resetInternal() — copied to a new playbackInfo
3. updateLivePlaybackSpeedControl() — resets MediaClock parameters to value that is in playbackInfo.playbackParameters in if change indicates timeline was empty or new media period is not valid for live speed control. Commit 8dc63dd added this change, clearly intent was to use virgin value from playbackInfo.playbackParameters

The two commits around the area to change are:

• d640ced 2021-01-12 | Forward live playback speed changes to track selections and renderers. [@tonihei]
• 74c493f 2020-08-27 | Add back support for setting audio pitch [@andrewlewis]

My suggested fix is simply not to update the EPII PlaybackParameter in the changed callback from MediaClock, this call:

        case MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL:
          handlePlaybackParameters((PlaybackParameters) msg.obj, /* acknowledgeCommand= */ false);
          break;

@andrewlewis this was your change for supporting slow motion streams (sorry, long time ago)

Actual result

Player internal logic overwrites speed change to 1.0

Logging:

--  Here live edge speed adjustment is evident

12-16 14:55:21.270 23913 25900 I ExoPlayerImplInternal: handlePlaybackParameters()  updatePlaybackInfo:true acknowledgeCommand:false currentPlaybackSpeed: 1.0 currentParameters: PlaybackParameters(speed=1.00, pitch=1.00) playbackParameters: PlaybackParameters(speed=1.00, pitch=1.00)
12-16 14:55:21.297 23913 25900 I ExoPlayerImplInternal: updatePlaybackPositions() changing speed - old: PlaybackParameters(speed=1.00, pitch=1.00) new: 1.03 isUsingStandaloneClock: false standaloneClockIsStarted: true
12-16 14:55:21.297 23913 25900 I ExoPlayerImplInternal: handlePlaybackParameters()  updatePlaybackInfo:false acknowledgeCommand:false currentPlaybackSpeed: 1.03 currentParameters: PlaybackParameters(speed=1.00, pitch=1.00) playbackParameters: PlaybackParameters(speed=1.00, pitch=1.00)
12-16 14:55:21.302 23913 25900 I ExoPlayerImplInternal: handlePlaybackParameters()  updatePlaybackInfo:true acknowledgeCommand:false currentPlaybackSpeed: 1.0 currentParameters: PlaybackParameters(speed=1.00, pitch=1.00) playbackParameters: PlaybackParameters(speed=1.00, pitch=1.00)


--- Set speed 15 Player side 1.03 is captured in the standalone clock, but is correctly ignored

12-16 14:55:21.395 23913 25900 I ExoPlayerImplInternal: setPlaybackParametersInternal() - parameters: PlaybackParameters(speed=15.00, pitch=1.00) mediaClock: PlaybackParameters(speed=1.03, pitch=1.00) isUsingStandlone: true isUsingStandloneStarted: true
12-16 14:55:21.395 23913 25900 I ExoPlayerImplInternal: handlePlaybackParameters()  
			updatePlaybackInfo:true 
			acknowledgeCommand:true
			currentPlaybackSpeed: 15.0 
			currentParameters: PlaybackParameters(speed=1.00, pitch=1.00) 
			playbackParameters: PlaybackParameters(speed=15.00, pitch=1.00)


12-16 14:55:21.395  3491  3581 D OmxVideoDecoder: setConfig 2668 0x6f800003
12-16 14:55:21.396  3491  3581 V OmxVideoDecoder: setConfig 2828 nIndex 0x6f800003
12-16 14:55:21.396  3491  3581 D OmxComponent: setConfig 1221
12-16 14:55:21.396  3491  3581 E OMXNodeInstance: setConfig(0xf2c363fc:amlogic.avc.decoder.awesome2, ConfigOperatingRate(0x6f800003)) ERROR: UnsupportedIndex(0x8000101a)
12-16 14:55:21.396 23913 25919 I ACodec  : codec does not support config operating rate (err -1010)
12-16 14:55:21.396  3491 26176 V OMX_WorkerPeer: vendor/omx/omx_framework/WorkerPeer.cpp:runWorkerStatic:155 --------------------
12-16 14:55:21.396  3491 26176 V OMX_WorkerPeer: vendor/omx/omx_framework/WorkerPeer.cpp:runWorker:161 --------------------
	

**** Call is from MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL, it sets the parameters to 1x.  This is triggered by MediaClock.syncClocks()

12-16 14:55:21.396 23913 25900 I ExoPlayerImplInternal: handlePlaybackParameters()  
			updatePlaybackInfo:true 				<--  This forces the speed change back to 1.0 into EPII.playbackInfo
			acknowledgeCommand:false 
			currentPlaybackSpeed: 1.0 
			currentParameters: PlaybackParameters(speed=15.00, pitch=1.00) 
			playbackParameters: PlaybackParameters(speed=1.00, pitch=1.00)

Media

I can send a URL if needed, but I think a test case is better.

Bug Report

• You will email the zip file produced by adb bugreport to dev.exoplayer@gmail.com after filing this issue.

[christosts]

@stevemayhew is this a dup of google/ExoPlayer#10882?

[stevemayhew]

Yes. Do you want us to put pull requests in both repo or is it easier just to AndroidX?

…

On Wed, Jan 11, 2023 at 6:28 AM christosts ***@***.***> wrote: @stevemayhew <https://github.com/stevemayhew> is this a dup of google/ExoPlayer#10882 <google/ExoPlayer#10882>? — Reply to this email directly, view it on GitHub <#230 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADBF6ELYQ437PYO32MW62LWR27STANCNFSM6AAAAAATCIBGZ4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[christosts]

Def one is enough, since both github repos are merged into the same codebase internally, and ExoPlayer/media3 releases are synced. In the future we will be using the androidx repo (but not sure when!). At the moment it doesn't make a difference which one you choose, perhaps let's use the androidx one to warm up the transition.

[stevemayhew]

Sounds good, can close out the ExoPlayer pull then

…

On Wed, Jan 11, 2023 at 11:47 AM christosts ***@***.***> wrote: Def one is enough, since both github repos are merged into the same codebase internally, and ExoPlayer/media3 releases are synced. In the future we will be using the androidx repo (but not sure when!). At the moment it doesn't make a difference which one you choose, perhaps let's use the androidx one to warm up the transition. — Reply to this email directly, view it on GitHub <#230 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADBF6GG3WNR5JGOXNKE6MDWR4E5VANCNFSM6AAAAAATCIBGZ4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[tonihei]

Closing as duplicate of google/ExoPlayer#10882.