diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 26e39d18fd..b771a251b4 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -26,6 +26,9 @@ * Encapsulate Opus frames in Ogg packets in direct playbacks (offload). * Fix broken gapless MP3 playback on Samsung devices ([#8594](https://github.com/google/ExoPlayer/issues/8594)). + * Fix bug where playback speeds set immediately after disabling audio may + be overridden by a previous speed change + ([#10882](https://github.com/google/ExoPlayer/issues/10882)). * Video: * Map HEVC HDR10 format to `HEVCProfileMain10HDR10` instead of `HEVCProfileMain10`. diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java index fc6bfca39d..ccdc6e74f5 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java @@ -963,7 +963,7 @@ import java.util.concurrent.atomic.AtomicBoolean; livePlaybackSpeedControl.getAdjustedPlaybackSpeed( getCurrentLiveOffsetUs(), getTotalBufferedDurationUs()); if (mediaClock.getPlaybackParameters().speed != adjustedSpeed) { - mediaClock.setPlaybackParameters(playbackInfo.playbackParameters.withSpeed(adjustedSpeed)); + setMediaClockPlaybackParameters(playbackInfo.playbackParameters.withSpeed(adjustedSpeed)); handlePlaybackParameters( playbackInfo.playbackParameters, /* currentPlaybackSpeed= */ mediaClock.getPlaybackParameters().speed, @@ -973,6 +973,12 @@ import java.util.concurrent.atomic.AtomicBoolean; } } + private void setMediaClockPlaybackParameters(PlaybackParameters playbackParameters) { + // Previously sent speed updates from the media clock now become stale. + handler.removeMessages(MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL); + mediaClock.setPlaybackParameters(playbackParameters); + } + private void notifyTrackSelectionRebuffer() { MediaPeriodHolder periodHolder = queue.getPlayingPeriod(); while (periodHolder != null) { @@ -1359,7 +1365,7 @@ import java.util.concurrent.atomic.AtomicBoolean; private void setPlaybackParametersInternal(PlaybackParameters playbackParameters) throws ExoPlaybackException { - mediaClock.setPlaybackParameters(playbackParameters); + setMediaClockPlaybackParameters(playbackParameters); handlePlaybackParameters(mediaClock.getPlaybackParameters(), /* acknowledgeCommand= */ true); } @@ -1674,7 +1680,7 @@ import java.util.concurrent.atomic.AtomicBoolean; nextPendingMessageIndexHint = nextPendingMessageIndex; } - private void ensureStopped(Renderer renderer) throws ExoPlaybackException { + private void ensureStopped(Renderer renderer) { if (renderer.getState() == Renderer.STATE_STARTED) { renderer.stop(); } @@ -1931,14 +1937,20 @@ import java.util.concurrent.atomic.AtomicBoolean; MediaPeriodId newPeriodId, Timeline oldTimeline, MediaPeriodId oldPeriodId, - long positionForTargetOffsetOverrideUs) { + long positionForTargetOffsetOverrideUs) + throws ExoPlaybackException { if (!shouldUseLivePlaybackSpeedControl(newTimeline, newPeriodId)) { // Live playback speed control is unused for the current period, reset speed to user-defined // playback parameters or 1.0 for ad playback. PlaybackParameters targetPlaybackParameters = newPeriodId.isAd() ? PlaybackParameters.DEFAULT : playbackInfo.playbackParameters; if (!mediaClock.getPlaybackParameters().equals(targetPlaybackParameters)) { - mediaClock.setPlaybackParameters(targetPlaybackParameters); + setMediaClockPlaybackParameters(targetPlaybackParameters); + handlePlaybackParameters( + playbackInfo.playbackParameters, + targetPlaybackParameters.speed, + /* updatePlaybackInfo= */ false, + /* acknowledgeCommand= */ false); } return; } @@ -1987,7 +1999,7 @@ import java.util.concurrent.atomic.AtomicBoolean; return maxReadPositionUs; } - private void updatePeriods() throws ExoPlaybackException, IOException { + private void updatePeriods() throws ExoPlaybackException { if (playbackInfo.timeline.isEmpty() || !mediaSourceList.isPrepared()) { // No periods available. return; @@ -2029,7 +2041,7 @@ import java.util.concurrent.atomic.AtomicBoolean; } } - private void maybeUpdateReadingPeriod() { + private void maybeUpdateReadingPeriod() throws ExoPlaybackException { @Nullable MediaPeriodHolder readingPeriodHolder = queue.getReadingPeriod(); if (readingPeriodHolder == null) { return; diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java index 5d7e3b90a2..7fd92a6538 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -91,6 +91,7 @@ import android.media.AudioManager; import android.net.Uri; import android.os.Handler; import android.os.Looper; +import android.util.Pair; import android.view.Surface; import androidx.annotation.Nullable; import androidx.media3.common.AdPlaybackState; @@ -135,14 +136,12 @@ import androidx.media3.exoplayer.source.TrackGroupArray; import androidx.media3.exoplayer.source.WrappingMediaSource; import androidx.media3.exoplayer.source.ads.ServerSideAdInsertionMediaSource; import androidx.media3.exoplayer.text.TextOutput; -import androidx.media3.exoplayer.trackselection.DefaultTrackSelector; import androidx.media3.exoplayer.upstream.Allocation; import androidx.media3.exoplayer.upstream.Allocator; import androidx.media3.exoplayer.upstream.Loader; import androidx.media3.exoplayer.video.VideoRendererEventListener; import androidx.media3.extractor.metadata.id3.BinaryFrame; import androidx.media3.extractor.metadata.id3.TextInformationFrame; -import androidx.media3.test.utils.Action; import androidx.media3.test.utils.ActionSchedule; import androidx.media3.test.utils.ActionSchedule.PlayerRunnable; import androidx.media3.test.utils.ActionSchedule.PlayerTarget; @@ -3851,41 +3850,29 @@ public final class ExoPlayerTest { @Test public void setPlaybackSpeedConsecutivelyNotifiesListenerForEveryChangeOnceAndIsMasked() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); List maskedPlaybackSpeeds = new ArrayList<>(); - Action getPlaybackSpeedAction = - new Action("getPlaybackSpeed", /* description= */ null) { - @Override - protected void doActionImpl( - ExoPlayer player, DefaultTrackSelector trackSelector, @Nullable Surface surface) { - maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed); - } - }; - ActionSchedule actionSchedule = - new ActionSchedule.Builder(TAG) - .pause() - .waitForPlaybackState(Player.STATE_READY) - .setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.1f)) - .apply(getPlaybackSpeedAction) - .setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.2f)) - .apply(getPlaybackSpeedAction) - .setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.3f)) - .apply(getPlaybackSpeedAction) - .play() - .build(); List reportedPlaybackSpeeds = new ArrayList<>(); - Player.Listener listener = + player.addListener( new Player.Listener() { @Override public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) { reportedPlaybackSpeeds.add(playbackParameters.speed); } - }; - new ExoPlayerTestRunner.Builder(context) - .setActionSchedule(actionSchedule) - .setPlayerListener(listener) - .build() - .start() - .blockUntilEnded(TIMEOUT_MS); + }); + player.setMediaSource( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT)); + player.prepare(); + runUntilPlaybackState(player, Player.STATE_READY); + + player.setPlaybackSpeed(1.1f); + maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed); + player.setPlaybackSpeed(1.2f); + maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed); + player.setPlaybackSpeed(1.3f); + maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed); + runUntilPendingCommandsAreFullyHandled(player); + player.release(); assertThat(reportedPlaybackSpeeds).containsExactly(1.1f, 1.2f, 1.3f).inOrder(); assertThat(maskedPlaybackSpeeds).isEqualTo(reportedPlaybackSpeeds); @@ -3895,46 +3882,28 @@ public final class ExoPlayerTest { public void setUnsupportedPlaybackSpeedConsecutivelyNotifiesListenerForEveryChangeOnceAndResetsOnceHandled() throws Exception { - Renderer renderer = - new FakeMediaClockRenderer(C.TRACK_TYPE_AUDIO) { - @Override - public long getPositionUs() { - return 0; - } - - @Override - public void setPlaybackParameters(PlaybackParameters playbackParameters) {} - - @Override - public PlaybackParameters getPlaybackParameters() { - return PlaybackParameters.DEFAULT; - } - }; - ActionSchedule actionSchedule = - new ActionSchedule.Builder(TAG) - .pause() - .waitForPlaybackState(Player.STATE_READY) - .setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.1f)) - .setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.2f)) - .setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.3f)) - .play() + ExoPlayer player = + new TestExoPlayerBuilder(context) + .setRenderers(new AudioClockRendererWithoutSpeedChangeSupport()) .build(); List reportedPlaybackParameters = new ArrayList<>(); - Player.Listener listener = + player.addListener( new Player.Listener() { @Override public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) { reportedPlaybackParameters.add(playbackParameters); } - }; - new ExoPlayerTestRunner.Builder(context) - .setSupportedFormats(ExoPlayerTestRunner.AUDIO_FORMAT) - .setRenderers(renderer) - .setActionSchedule(actionSchedule) - .setPlayerListener(listener) - .build() - .start() - .blockUntilEnded(TIMEOUT_MS); + }); + player.setMediaSource( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT)); + player.prepare(); + runUntilPlaybackState(player, Player.STATE_READY); + + player.setPlaybackSpeed(1.1f); + player.setPlaybackSpeed(1.2f); + player.setPlaybackSpeed(1.3f); + runUntilPendingCommandsAreFullyHandled(player); + player.release(); assertThat(reportedPlaybackParameters) .containsExactly( @@ -3945,6 +3914,51 @@ public final class ExoPlayerTest { .inOrder(); } + @Test + public void + setUnsupportedPlaybackSpeedDirectlyFollowedByDisablingTheRendererAndSupportedPlaybackSpeed_keepsCorrectFinalSpeedAndInformsListenersCorrectly() + throws Exception { + ExoPlayer player = + new TestExoPlayerBuilder(context) + .setRenderers(new AudioClockRendererWithoutSpeedChangeSupport()) + .build(); + List reportedPlaybackParameters = new ArrayList<>(); + player.addListener( + new Player.Listener() { + @Override + public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) { + reportedPlaybackParameters.add(playbackParameters); + } + }); + player.setMediaSource( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT)); + player.prepare(); + runUntilPlaybackState(player, Player.STATE_READY); + + player.setPlaybackSpeed(2f); + // We need to do something that reliably triggers a position sync with the renderer, but no + // further playback progress as we want to test what happens if the parameter reset is still + // pending when we disable the audio renderer below. Calling play and pause will achieve this. + player.play(); + player.pause(); + // Disabling the audio renderer and setting a new speed should work, and should not be affected + // by the still pending parameter reset from above. + player.setTrackSelectionParameters( + player + .getTrackSelectionParameters() + .buildUpon() + .setTrackTypeDisabled(C.TRACK_TYPE_AUDIO, /* disabled= */ true) + .build()); + player.setPlaybackSpeed(5f); + runUntilPendingCommandsAreFullyHandled(player); + player.release(); + + assertThat(reportedPlaybackParameters) + .containsExactly( + new PlaybackParameters(/* speed= */ 2f), new PlaybackParameters(/* speed= */ 5f)) + .inOrder(); + } + @Test public void simplePlaybackHasNoPlaybackSuppression() throws Exception { ActionSchedule actionSchedule = @@ -10089,8 +10103,10 @@ public final class ExoPlayerTest { @Test public void setPlaybackSpeed_withAdPlayback_onlyAppliesToContent() throws Exception { - // Create renderer with media clock to listen to playback parameter changes. + // Create renderer with media clock to listen to playback parameter changes and reported speed + // changes. ArrayList playbackParameters = new ArrayList<>(); + ArrayList> speedChanges = new ArrayList<>(); FakeMediaClockRenderer audioRenderer = new FakeMediaClockRenderer(C.TRACK_TYPE_AUDIO) { private long positionUs; @@ -10118,6 +10134,12 @@ public final class ExoPlayerTest { ? PlaybackParameters.DEFAULT : Iterables.getLast(playbackParameters); } + + @Override + public void setPlaybackSpeed(float currentPlaybackSpeed, float targetPlaybackSpeed) + throws ExoPlaybackException { + speedChanges.add(Pair.create(currentPlaybackSpeed, targetPlaybackSpeed)); + } }; ExoPlayer player = new TestExoPlayerBuilder(context).setRenderers(audioRenderer).build(); AdPlaybackState adPlaybackState = @@ -10150,7 +10172,7 @@ public final class ExoPlayerTest { runUntilPlaybackState(player, Player.STATE_ENDED); player.release(); - // Assert that the renderer received the playback speed updates at each ad/content boundary. + // Assert that the media clock received the playback parameters at each ad/content boundary. assertThat(playbackParameters) .containsExactly( /* preroll ad */ new PlaybackParameters(1f), @@ -10161,6 +10183,18 @@ public final class ExoPlayerTest { /* content after postroll */ new PlaybackParameters(5f)) .inOrder(); + // Assert that the renderer received the speed changes at each ad/content boundary. + assertThat(speedChanges) + .containsExactly( + /* initial setup */ Pair.create(5f, 5f), + /* preroll ad */ Pair.create(1f, 5f), + /* content after preroll */ Pair.create(5f, 5f), + /* midroll ad */ Pair.create(1f, 5f), + /* content after midroll */ Pair.create(5f, 5f), + /* postroll ad */ Pair.create(1f, 5f), + /* content after postroll */ Pair.create(5f, 5f)) + .inOrder(); + // Assert that user-set speed was reported, but none of the ad overrides. verify(mockListener).onPlaybackParametersChanged(any()); verify(mockListener).onPlaybackParametersChanged(new PlaybackParameters(5.0f)); @@ -12532,6 +12566,46 @@ public final class ExoPlayerTest { } } + private static final class AudioClockRendererWithoutSpeedChangeSupport + extends FakeMediaClockRenderer { + + private PlaybackParameters playbackParameters; + private boolean delayingPlaybackParameterReset; + private long positionUs; + + public AudioClockRendererWithoutSpeedChangeSupport() { + super(C.TRACK_TYPE_AUDIO); + playbackParameters = PlaybackParameters.DEFAULT; + } + + @Override + protected void onPositionReset(long positionUs, boolean joining) throws ExoPlaybackException { + super.onPositionReset(positionUs, joining); + this.positionUs = positionUs; + } + + @Override + public long getPositionUs() { + return positionUs; + } + + @Override + public void setPlaybackParameters(PlaybackParameters playbackParameters) { + this.playbackParameters = playbackParameters; + // Similar to a real renderer, the missing speed support is only detected with a delay. + delayingPlaybackParameterReset = true; + } + + @Override + public PlaybackParameters getPlaybackParameters() { + if (delayingPlaybackParameterReset) { + delayingPlaybackParameterReset = false; + return playbackParameters; + } + return PlaybackParameters.DEFAULT; + } + } + /** * Returns an argument matcher for {@link Timeline} instances that ignores period and window uids. */