diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 90695c3c0e..2e898d948b 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -3,6 +3,10 @@ ### Unreleased changes * Common Library: + * Forward presumed no-op seek calls to the protected `BasePlayer.seekTo` + and `SimpleBasePlayer.handleSeek` methods instead of ignoring them. If + you are implementing these methods in a custom player, you may need to + handle these additional calls with `mediaItemIndex == C.INDEX_UNSET`. * ExoPlayer: * Add `reset` to `BasePreloadManager` to release all the holding sources while keep the preload manager instance. diff --git a/libraries/cast/src/main/java/androidx/media3/cast/CastPlayer.java b/libraries/cast/src/main/java/androidx/media3/cast/CastPlayer.java index 884169151e..f18fde331c 100644 --- a/libraries/cast/src/main/java/androidx/media3/cast/CastPlayer.java +++ b/libraries/cast/src/main/java/androidx/media3/cast/CastPlayer.java @@ -417,6 +417,9 @@ public final class CastPlayer extends BasePlayer { long positionMs, @Player.Command int seekCommand, boolean isRepeatingCurrentItem) { + if (mediaItemIndex == C.INDEX_UNSET) { + return; + } checkArgument(mediaItemIndex >= 0); if (!currentTimeline.isEmpty() && mediaItemIndex >= currentTimeline.getWindowCount()) { return; diff --git a/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java b/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java index 7910d474a8..31036bf9fb 100644 --- a/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java @@ -197,12 +197,15 @@ public abstract class BasePlayer implements Player { public final void seekToPrevious() { Timeline timeline = getCurrentTimeline(); if (timeline.isEmpty() || isPlayingAd()) { + ignoreSeek(Player.COMMAND_SEEK_TO_PREVIOUS); return; } boolean hasPreviousMediaItem = hasPreviousMediaItem(); if (isCurrentMediaItemLive() && !isCurrentMediaItemSeekable()) { if (hasPreviousMediaItem) { seekToPreviousMediaItemInternal(Player.COMMAND_SEEK_TO_PREVIOUS); + } else { + ignoreSeek(Player.COMMAND_SEEK_TO_PREVIOUS); } } else if (hasPreviousMediaItem && getCurrentPosition() <= getMaxSeekToPreviousPosition()) { seekToPreviousMediaItemInternal(Player.COMMAND_SEEK_TO_PREVIOUS); @@ -261,12 +264,15 @@ public abstract class BasePlayer implements Player { public final void seekToNext() { Timeline timeline = getCurrentTimeline(); if (timeline.isEmpty() || isPlayingAd()) { + ignoreSeek(Player.COMMAND_SEEK_TO_NEXT); return; } if (hasNextMediaItem()) { seekToNextMediaItemInternal(Player.COMMAND_SEEK_TO_NEXT); } else if (isCurrentMediaItemLive() && isCurrentMediaItemDynamic()) { seekToDefaultPositionInternal(getCurrentMediaItemIndex(), Player.COMMAND_SEEK_TO_NEXT); + } else { + ignoreSeek(Player.COMMAND_SEEK_TO_NEXT); } } @@ -287,9 +293,13 @@ public abstract class BasePlayer implements Player { /** * Seeks to a position in the specified {@link MediaItem}. * - * @param mediaItemIndex The index of the {@link MediaItem}. + * @param mediaItemIndex The index of the {@link MediaItem}. If the original seek operation did + * not directly specify an index, this is the most likely implied index based on the available + * player state. If the implied action is to do nothing, this will be {@link C#INDEX_UNSET}. * @param positionMs The seek position in the specified {@link MediaItem} in milliseconds, or - * {@link C#TIME_UNSET} to seek to the media item's default position. + * {@link C#TIME_UNSET} to seek to the media item's default position. If the original seek + * operation did not directly specify a position, this is the most likely implied position + * based on the available player state. * @param seekCommand The {@link Player.Command} used to trigger the seek. * @param isRepeatingCurrentItem Whether this seeks repeats the current item. */ @@ -459,6 +469,14 @@ public abstract class BasePlayer implements Player { return repeatMode == REPEAT_MODE_ONE ? REPEAT_MODE_OFF : repeatMode; } + private void ignoreSeek(@Player.Command int seekCommand) { + seekTo( + /* mediaItemIndex= */ C.INDEX_UNSET, + /* positionMs= */ C.TIME_UNSET, + seekCommand, + /* isRepeatingCurrentItem= */ false); + } + private void seekToCurrentItem(long positionMs, @Player.Command int seekCommand) { seekTo( getCurrentMediaItemIndex(), positionMs, seekCommand, /* isRepeatingCurrentItem= */ false); @@ -485,6 +503,7 @@ public abstract class BasePlayer implements Player { private void seekToNextMediaItemInternal(@Player.Command int seekCommand) { int nextMediaItemIndex = getNextMediaItemIndex(); if (nextMediaItemIndex == C.INDEX_UNSET) { + ignoreSeek(seekCommand); return; } if (nextMediaItemIndex == getCurrentMediaItemIndex()) { @@ -497,6 +516,7 @@ public abstract class BasePlayer implements Player { private void seekToPreviousMediaItemInternal(@Player.Command int seekCommand) { int previousMediaItemIndex = getPreviousMediaItemIndex(); if (previousMediaItemIndex == C.INDEX_UNSET) { + ignoreSeek(seekCommand); return; } if (previousMediaItemIndex == getCurrentMediaItemIndex()) { diff --git a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java index a46ff5ddba..0c6b6e12b3 100644 --- a/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java @@ -2352,19 +2352,24 @@ public abstract class SimpleBasePlayer extends BasePlayer { @Player.Command int seekCommand, boolean isRepeatingCurrentItem) { verifyApplicationThreadAndInitState(); - checkArgument(mediaItemIndex >= 0); + checkArgument(mediaItemIndex == C.INDEX_UNSET || mediaItemIndex >= 0); // Use a local copy to ensure the lambda below uses the current state value. State state = this.state; - if (!shouldHandleCommand(seekCommand) - || isPlayingAd() - || (!state.playlist.isEmpty() && mediaItemIndex >= state.playlist.size())) { + if (!shouldHandleCommand(seekCommand)) { return; } + boolean ignoreSeekForPlaceholderState = + mediaItemIndex == C.INDEX_UNSET + || isPlayingAd() + || (!state.playlist.isEmpty() && mediaItemIndex >= state.playlist.size()); updateStateForPendingOperation( /* pendingOperation= */ handleSeek(mediaItemIndex, positionMs, seekCommand), /* placeholderStateSupplier= */ () -> - getStateWithNewPlaylistAndPosition(state, state.playlist, mediaItemIndex, positionMs), - /* seeked= */ true, + ignoreSeekForPlaceholderState + ? state + : getStateWithNewPlaylistAndPosition( + state, state.playlist, mediaItemIndex, positionMs), + /* forceSeekDiscontinuity= */ !ignoreSeekForPlaceholderState, isRepeatingCurrentItem); } @@ -2921,7 +2926,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { return; } updateStateAndInformListeners( - getState(), /* seeked= */ false, /* isRepeatingCurrentItem= */ false); + getState(), /* forceSeekDiscontinuity= */ false, /* isRepeatingCurrentItem= */ false); } /** @@ -3344,10 +3349,13 @@ public abstract class SimpleBasePlayer extends BasePlayer { *

Will only be called if the appropriate {@link Player.Command}, for example {@link * Player#COMMAND_SEEK_TO_MEDIA_ITEM} or {@link Player#COMMAND_SEEK_TO_NEXT}, is available. * - * @param mediaItemIndex The media item index to seek to. The index is in the range 0 <= {@code - * mediaItemIndex} < {@code mediaItems.size()}. + * @param mediaItemIndex The media item index to seek to. If the original seek operation did not + * directly specify an index, this is the most likely implied index based on the available + * player state. If the implied action is to do nothing, this will be {@link C#INDEX_UNSET}. * @param positionMs The position in milliseconds to start playback from, or {@link C#TIME_UNSET} - * to start at the default position in the media item. + * to start at the default position in the media item. If the original seek operation did not + * directly specify a position, this is the most likely implied position based on the + * available player state. * @param seekCommand The {@link Player.Command} used to trigger the seek. * @return A {@link ListenableFuture} indicating the completion of all immediate {@link State} * changes caused by this call. @@ -3385,7 +3393,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { @SuppressWarnings("deprecation") // Calling deprecated listener methods. @RequiresNonNull("state") private void updateStateAndInformListeners( - State newState, boolean seeked, boolean isRepeatingCurrentItem) { + State newState, boolean forceSeekDiscontinuity, boolean isRepeatingCurrentItem) { State previousState = state; // Assign new state immediately such that all getters return the right values, but use a // snapshot of the previous and new state so that listener invocations are triggered correctly. @@ -3407,7 +3415,8 @@ public abstract class SimpleBasePlayer extends BasePlayer { MediaMetadata previousMediaMetadata = getMediaMetadataInternal(previousState); MediaMetadata newMediaMetadata = getMediaMetadataInternal(newState); int positionDiscontinuityReason = - getPositionDiscontinuityReason(previousState, newState, seeked, window, period); + getPositionDiscontinuityReason( + previousState, newState, forceSeekDiscontinuity, window, period); boolean timelineChanged = !previousState.timeline.equals(newState.timeline); int mediaItemTransitionReason = getMediaItemTransitionReason( @@ -3618,7 +3627,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { updateStateForPendingOperation( pendingOperation, placeholderStateSupplier, - /* seeked= */ false, + /* forceSeekDiscontinuity= */ false, /* isRepeatingCurrentItem= */ false); } @@ -3626,22 +3635,26 @@ public abstract class SimpleBasePlayer extends BasePlayer { private void updateStateForPendingOperation( ListenableFuture pendingOperation, Supplier placeholderStateSupplier, - boolean seeked, + boolean forceSeekDiscontinuity, boolean isRepeatingCurrentItem) { if (pendingOperation.isDone() && pendingOperations.isEmpty()) { - updateStateAndInformListeners(getState(), seeked, isRepeatingCurrentItem); + updateStateAndInformListeners(getState(), forceSeekDiscontinuity, isRepeatingCurrentItem); } else { pendingOperations.add(pendingOperation); State suggestedPlaceholderState = placeholderStateSupplier.get(); updateStateAndInformListeners( - getPlaceholderState(suggestedPlaceholderState), seeked, isRepeatingCurrentItem); + getPlaceholderState(suggestedPlaceholderState), + forceSeekDiscontinuity, + isRepeatingCurrentItem); pendingOperation.addListener( () -> { castNonNull(state); // Already checked by method @RequiresNonNull pre-condition. pendingOperations.remove(pendingOperation); if (pendingOperations.isEmpty() && !released) { updateStateAndInformListeners( - getState(), /* seeked= */ false, /* isRepeatingCurrentItem= */ false); + getState(), + /* forceSeekDiscontinuity= */ false, + /* isRepeatingCurrentItem= */ false); } }, this::postOrRunOnApplicationHandler); @@ -3740,14 +3753,14 @@ public abstract class SimpleBasePlayer extends BasePlayer { private static int getPositionDiscontinuityReason( State previousState, State newState, - boolean seeked, + boolean forceSeekDiscontinuity, Timeline.Window window, Timeline.Period period) { if (newState.hasPositionDiscontinuity) { // We were asked to report a discontinuity. return newState.positionDiscontinuityReason; } - if (seeked) { + if (forceSeekDiscontinuity) { return Player.DISCONTINUITY_REASON_SEEK; } if (previousState.playlist.isEmpty()) { diff --git a/libraries/common/src/test/java/androidx/media3/common/BasePlayerTest.java b/libraries/common/src/test/java/androidx/media3/common/BasePlayerTest.java index 4f3c677f66..a47f52c4fd 100644 --- a/libraries/common/src/test/java/androidx/media3/common/BasePlayerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/BasePlayerTest.java @@ -119,6 +119,32 @@ public class BasePlayerTest { /* isRepeatingCurrentItem= */ false); } + @Test + public void seekToNext_withoutNextItem_forwardsCallWithUnsetMediaItemIndex() { + BasePlayer player = + spy( + new TestBasePlayer() { + @Override + public Timeline getCurrentTimeline() { + return new FakeTimeline(/* windowCount= */ 3); + } + + @Override + public int getCurrentMediaItemIndex() { + return 2; + } + }); + + player.seekToNext(); + + verify(player) + .seekTo( + /* mediaItemIndex= */ C.INDEX_UNSET, + /* positionMs= */ C.TIME_UNSET, + Player.COMMAND_SEEK_TO_NEXT, + /* isRepeatingCurrentItem= */ false); + } + @Test public void seekToNextMediaItem_usesCommandSeekToNextMediaItem() { BasePlayer player = @@ -140,6 +166,32 @@ public class BasePlayerTest { /* isRepeatingCurrentItem= */ false); } + @Test + public void seekToNextMediaItem_withoutNextItem_forwardsCallWithUnsetMediaItemIndex() { + BasePlayer player = + spy( + new TestBasePlayer() { + @Override + public Timeline getCurrentTimeline() { + return new FakeTimeline(/* windowCount= */ 3); + } + + @Override + public int getCurrentMediaItemIndex() { + return 2; + } + }); + + player.seekToNextMediaItem(); + + verify(player) + .seekTo( + /* mediaItemIndex= */ C.INDEX_UNSET, + /* positionMs= */ C.TIME_UNSET, + Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, + /* isRepeatingCurrentItem= */ false); + } + @Test public void seekForward_usesCommandSeekForward() { BasePlayer player = @@ -223,6 +275,27 @@ public class BasePlayerTest { /* isRepeatingCurrentItem= */ false); } + @Test + public void seekToPreviousMediaItem_withoutPreviousItem_forwardsCallWithUnsetMediaItemIndex() { + BasePlayer player = + spy( + new TestBasePlayer() { + @Override + public int getCurrentMediaItemIndex() { + return 0; + } + }); + + player.seekToPreviousMediaItem(); + + verify(player) + .seekTo( + /* mediaItemIndex= */ C.INDEX_UNSET, + /* positionMs= */ C.TIME_UNSET, + Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM, + /* isRepeatingCurrentItem= */ false); + } + @Test public void seekBack_usesCommandSeekBack() { BasePlayer player = diff --git a/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java b/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java index 37f91ea27e..26d33ff606 100644 --- a/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java @@ -8013,6 +8013,62 @@ public class SimpleBasePlayerTest { verifyNoMoreInteractions(listener); } + @SuppressWarnings("deprecation") // Verifying deprecated listener calls. + @Test + public void seekTo_asyncHandlingForNoImpliedActionSeeks_usesCurrentStateAsPlaceholderState() { + MediaItem mediaItem = new MediaItem.Builder().setMediaId("2").build(); + State state = + new State.Builder() + .setAvailableCommands(new Commands.Builder().addAllCommands().build()) + .setPlaylist( + ImmutableList.of( + new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ 1) + .setMediaItem(mediaItem) + .build(), + new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ 2).build())) + .setCurrentMediaItemIndex(1) + .build(); + // Change updated state to see a difference to the placeholder state. + State updatedState = + state + .buildUpon() + .setCurrentMediaItemIndex(0) + .setPositionDiscontinuity( + Player.DISCONTINUITY_REASON_SEEK, /* discontinuityPositionMs= */ 0) + .build(); + SettableFuture future = SettableFuture.create(); + SimpleBasePlayer player = + new SimpleBasePlayer(Looper.myLooper()) { + @Override + protected State getState() { + return future.isDone() ? updatedState : state; + } + + @Override + protected ListenableFuture handleSeek( + int mediaItemIndex, long positionMs, @Player.Command int seekCommand) { + return future; + } + }; + Listener listener = mock(Listener.class); + player.addListener(listener); + + player.seekToNext(); + + // Verify placeholder state is the same as before with no listener updates. + assertThat(player.getCurrentMediaItemIndex()).isEqualTo(1); + verifyNoMoreInteractions(listener); + + future.set(null); + + // Verify actual state update. + assertThat(player.getCurrentMediaItemIndex()).isEqualTo(0); + verify(listener).onPositionDiscontinuity(Player.DISCONTINUITY_REASON_SEEK); + verify(listener).onPositionDiscontinuity(any(), any(), eq(Player.DISCONTINUITY_REASON_SEEK)); + verify(listener).onMediaItemTransition(mediaItem, Player.MEDIA_ITEM_TRANSITION_REASON_SEEK); + verifyNoMoreInteractions(listener); + } + @Test public void seekTo_withoutAvailableCommandForSeekToMediaItem_isNotForwarded() { State state = diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java index 9d4f6f5c87..8a2b6466d9 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java @@ -890,12 +890,15 @@ import java.util.concurrent.TimeoutException; @Player.Command int seekCommand, boolean isRepeatingCurrentItem) { verifyApplicationThread(); + if (mediaItemIndex == C.INDEX_UNSET) { + return; + } checkArgument(mediaItemIndex >= 0); - analyticsCollector.notifySeekStarted(); Timeline timeline = playbackInfo.timeline; if (!timeline.isEmpty() && mediaItemIndex >= timeline.getWindowCount()) { return; } + analyticsCollector.notifySeekStarted(); pendingOperationAcks++; if (isPlayingAd()) { // TODO: Investigate adding support for seeking during ads. This is complicated to do in