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 b7c1443816..8d2a0cbde1 100644 --- a/libraries/cast/src/main/java/androidx/media3/cast/CastPlayer.java +++ b/libraries/cast/src/main/java/androidx/media3/cast/CastPlayer.java @@ -44,7 +44,6 @@ import androidx.media3.common.TrackSelectionParameters; import androidx.media3.common.Tracks; import androidx.media3.common.VideoSize; import androidx.media3.common.text.CueGroup; -import androidx.media3.common.util.Assertions; import androidx.media3.common.util.Clock; import androidx.media3.common.util.ListenerSet; import androidx.media3.common.util.Log; @@ -296,7 +295,7 @@ public final class CastPlayer extends BasePlayer { @Override public void addMediaItems(int index, List mediaItems) { - Assertions.checkArgument(index >= 0); + checkArgument(index >= 0); int uid = MediaQueueItem.INVALID_ITEM_ID; if (index < currentTimeline.getWindowCount()) { uid = (int) currentTimeline.getWindow(/* windowIndex= */ index, window).uid; @@ -306,14 +305,11 @@ public final class CastPlayer extends BasePlayer { @Override public void moveMediaItems(int fromIndex, int toIndex, int newIndex) { - Assertions.checkArgument( - fromIndex >= 0 - && fromIndex <= toIndex - && toIndex <= currentTimeline.getWindowCount() - && newIndex >= 0 - && newIndex < currentTimeline.getWindowCount()); - newIndex = min(newIndex, currentTimeline.getWindowCount() - (toIndex - fromIndex)); - if (fromIndex == toIndex || fromIndex == newIndex) { + checkArgument(fromIndex >= 0 && fromIndex <= toIndex && newIndex >= 0); + int playlistSize = currentTimeline.getWindowCount(); + toIndex = min(toIndex, playlistSize); + newIndex = min(newIndex, playlistSize - (toIndex - fromIndex)); + if (fromIndex >= playlistSize || fromIndex == toIndex || fromIndex == newIndex) { // Do nothing. return; } @@ -326,9 +322,10 @@ public final class CastPlayer extends BasePlayer { @Override public void removeMediaItems(int fromIndex, int toIndex) { - Assertions.checkArgument(fromIndex >= 0 && toIndex >= fromIndex); - toIndex = min(toIndex, currentTimeline.getWindowCount()); - if (fromIndex == toIndex) { + checkArgument(fromIndex >= 0 && toIndex >= fromIndex); + int playlistSize = currentTimeline.getWindowCount(); + toIndex = min(toIndex, playlistSize); + if (fromIndex >= playlistSize || fromIndex == toIndex) { // Do nothing. return; } @@ -406,6 +403,10 @@ public final class CastPlayer extends BasePlayer { long positionMs, @Player.Command int seekCommand, boolean isRepeatingCurrentItem) { + checkArgument(mediaItemIndex >= 0); + if (!currentTimeline.isEmpty() && mediaItemIndex >= currentTimeline.getWindowCount()) { + return; + } MediaStatus mediaStatus = getMediaStatus(); // We assume the default position is 0. There is no support for seeking to the default position // in RemoteMediaClient. diff --git a/libraries/common/src/main/java/androidx/media3/common/Player.java b/libraries/common/src/main/java/androidx/media3/common/Player.java index 4019b85613..71a4c07483 100644 --- a/libraries/common/src/main/java/androidx/media3/common/Player.java +++ b/libraries/common/src/main/java/androidx/media3/common/Player.java @@ -1680,7 +1680,8 @@ public interface Player { /** * Moves the media item at the current index to the new index. * - * @param currentIndex The current index of the media item to move. + * @param currentIndex The current index of the media item to move. If the index is larger than + * the size of the playlist, the request is ignored. * @param newIndex The new index of the media item. If the new index is larger than the size of * the playlist the item is moved to the end of the playlist. */ @@ -1689,8 +1690,10 @@ public interface Player { /** * Moves the media item range to the new index. * - * @param fromIndex The start of the range to move. - * @param toIndex The first item not to be included in the range (exclusive). + * @param fromIndex The start of the range to move. If the index is larger than the size of the + * playlist, the request is ignored. + * @param toIndex The first item not to be included in the range (exclusive). If the index is + * larger than the size of the playlist, items up to the end of the playlist are moved. * @param newIndex The new index of the first media item of the range. If the new index is larger * than the size of the remaining playlist after removing the range, the range is moved to the * end of the playlist. @@ -1700,16 +1703,18 @@ public interface Player { /** * Removes the media item at the given index of the playlist. * - * @param index The index at which to remove the media item. + * @param index The index at which to remove the media item. If the index is larger than the size + * of the playlist, the request is ignored. */ void removeMediaItem(int index); /** * Removes a range of media items from the playlist. * - * @param fromIndex The index at which to start removing media items. + * @param fromIndex The index at which to start removing media items. If the index is larger than + * the size of the playlist, the request is ignored. * @param toIndex The index of the first item to be kept (exclusive). If the index is larger than - * the size of the playlist, media items to the end of the playlist are removed. + * the size of the playlist, media items up to the end of the playlist are removed. */ void removeMediaItems(int fromIndex, int toIndex); @@ -1890,9 +1895,8 @@ public interface Player { * For other streams it will typically be the start. * * @param mediaItemIndex The index of the {@link MediaItem} whose associated default position - * should be seeked to. - * @throws IllegalSeekPositionException If the player has a non-empty timeline and the provided - * {@code mediaItemIndex} is not within the bounds of the current timeline. + * should be seeked to. If the index is larger than the size of the playlist, the request is + * ignored. */ void seekToDefaultPosition(int mediaItemIndex); @@ -1907,11 +1911,10 @@ public interface Player { /** * Seeks to a position specified in milliseconds in the specified {@link MediaItem}. * - * @param mediaItemIndex The index of the {@link MediaItem}. + * @param mediaItemIndex The index of the {@link MediaItem}. If the index is larger than the size + * of the playlist, the request is ignored. * @param positionMs The seek position in the specified {@link MediaItem}, or {@link C#TIME_UNSET} * to seek to the media item's default position. - * @throws IllegalSeekPositionException If the player has a non-empty timeline and the provided - * {@code mediaItemIndex} is not within the bounds of the current timeline. */ void seekTo(int mediaItemIndex, long positionMs); 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 e92e122c57..5ec2aa8acd 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java @@ -18,6 +18,7 @@ package androidx.media3.exoplayer; import static androidx.media3.common.C.TRACK_TYPE_AUDIO; import static androidx.media3.common.C.TRACK_TYPE_CAMERA_MOTION; import static androidx.media3.common.C.TRACK_TYPE_VIDEO; +import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Util.castNonNull; @@ -77,7 +78,6 @@ import androidx.media3.common.Tracks; import androidx.media3.common.VideoSize; import androidx.media3.common.text.Cue; import androidx.media3.common.text.CueGroup; -import androidx.media3.common.util.Assertions; import androidx.media3.common.util.Clock; import androidx.media3.common.util.ConditionVariable; import androidx.media3.common.util.HandlerWrapper; @@ -624,7 +624,6 @@ import java.util.concurrent.TimeoutException; @Override public void addMediaItems(int index, List mediaItems) { verifyApplicationThread(); - index = min(index, mediaSourceHolderSnapshots.size()); addMediaSources(index, createMediaSources(mediaItems)); } @@ -649,7 +648,8 @@ import java.util.concurrent.TimeoutException; @Override public void addMediaSources(int index, List mediaSources) { verifyApplicationThread(); - Assertions.checkArgument(index >= 0); + checkArgument(index >= 0); + index = min(index, mediaSourceHolderSnapshots.size()); Timeline oldTimeline = getCurrentTimeline(); pendingOperationAcks++; List holders = addMediaSourceHolders(index, mediaSources); @@ -675,7 +675,13 @@ import java.util.concurrent.TimeoutException; @Override public void removeMediaItems(int fromIndex, int toIndex) { verifyApplicationThread(); - toIndex = min(toIndex, mediaSourceHolderSnapshots.size()); + checkArgument(fromIndex >= 0 && toIndex >= fromIndex); + int playlistSize = mediaSourceHolderSnapshots.size(); + toIndex = min(toIndex, playlistSize); + if (fromIndex >= playlistSize || fromIndex == toIndex) { + // Do nothing. + return; + } PlaybackInfo newPlaybackInfo = removeMediaItemsInternal(fromIndex, toIndex); boolean positionDiscontinuity = !newPlaybackInfo.periodId.periodUid.equals(playbackInfo.periodId.periodUid); @@ -694,14 +700,16 @@ import java.util.concurrent.TimeoutException; @Override public void moveMediaItems(int fromIndex, int toIndex, int newFromIndex) { verifyApplicationThread(); - Assertions.checkArgument( - fromIndex >= 0 - && fromIndex <= toIndex - && toIndex <= mediaSourceHolderSnapshots.size() - && newFromIndex >= 0); + checkArgument(fromIndex >= 0 && fromIndex <= toIndex && newFromIndex >= 0); + int playlistSize = mediaSourceHolderSnapshots.size(); + toIndex = min(toIndex, playlistSize); + newFromIndex = min(newFromIndex, playlistSize - (toIndex - fromIndex)); + if (fromIndex >= playlistSize || fromIndex == toIndex || fromIndex == newFromIndex) { + // Do nothing. + return; + } Timeline oldTimeline = getCurrentTimeline(); pendingOperationAcks++; - newFromIndex = min(newFromIndex, mediaSourceHolderSnapshots.size() - (toIndex - fromIndex)); Util.moveItems(mediaSourceHolderSnapshots, fromIndex, toIndex, newFromIndex); Timeline newTimeline = createMaskingTimeline(); PlaybackInfo newPlaybackInfo = @@ -830,11 +838,11 @@ import java.util.concurrent.TimeoutException; @Player.Command int seekCommand, boolean isRepeatingCurrentItem) { verifyApplicationThread(); + checkArgument(mediaItemIndex >= 0); analyticsCollector.notifySeekStarted(); Timeline timeline = playbackInfo.timeline; - if (mediaItemIndex < 0 - || (!timeline.isEmpty() && mediaItemIndex >= timeline.getWindowCount())) { - throw new IllegalSeekPositionException(timeline, mediaItemIndex, positionMs); + if (!timeline.isEmpty() && mediaItemIndex >= timeline.getWindowCount()) { + return; } pendingOperationAcks++; if (isPlayingAd()) { @@ -2262,8 +2270,6 @@ import java.util.concurrent.TimeoutException; } private PlaybackInfo removeMediaItemsInternal(int fromIndex, int toIndex) { - Assertions.checkArgument( - fromIndex >= 0 && toIndex >= fromIndex && toIndex <= mediaSourceHolderSnapshots.size()); int currentIndex = getCurrentMediaItemIndex(); Timeline oldTimeline = getCurrentTimeline(); int currentMediaSourceCount = mediaSourceHolderSnapshots.size(); @@ -2302,7 +2308,7 @@ import java.util.concurrent.TimeoutException; private PlaybackInfo maskTimelineAndPosition( PlaybackInfo playbackInfo, Timeline timeline, @Nullable Pair periodPositionUs) { - Assertions.checkArgument(timeline.isEmpty() || periodPositionUs != null); + checkArgument(timeline.isEmpty() || periodPositionUs != null); Timeline oldTimeline = playbackInfo.timeline; // Mask the timeline. playbackInfo = playbackInfo.copyWithTimeline(timeline); 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 c392ae5920..6a635af4a7 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -95,7 +95,6 @@ import androidx.media3.common.AdPlaybackState; import androidx.media3.common.AudioAttributes; import androidx.media3.common.C; import androidx.media3.common.Format; -import androidx.media3.common.IllegalSeekPositionException; import androidx.media3.common.MediaItem; import androidx.media3.common.MediaMetadata; import androidx.media3.common.Metadata; @@ -931,31 +930,100 @@ public final class ExoPlayerTest { } @Test - public void illegalSeekPositionDoesThrow() throws Exception { - final IllegalSeekPositionException[] exception = new IllegalSeekPositionException[1]; - ActionSchedule actionSchedule = - new ActionSchedule.Builder(TAG) - .waitForPlaybackState(Player.STATE_BUFFERING) - .executeRunnable( - new PlayerRunnable() { - @Override - public void run(ExoPlayer player) { - try { - player.seekTo(/* mediaItemIndex= */ 100, /* positionMs= */ 0); - } catch (IllegalSeekPositionException e) { - exception[0] = e; - } - } - }) - .waitForPlaybackState(Player.STATE_ENDED) - .build(); - new ExoPlayerTestRunner.Builder(context) - .setActionSchedule(actionSchedule) - .build() - .start() - .blockUntilActionScheduleFinished(TIMEOUT_MS) - .blockUntilEnded(TIMEOUT_MS); - assertThat(exception[0]).isNotNull(); + public void seekTo_indexLargerThanPlaylist_isIgnored() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + player.setMediaItem(MediaItem.fromUri("http://test")); + + player.seekTo(/* mediaItemIndex= */ 1, /* positionMs= */ 1000); + + assertThat(player.getCurrentMediaItemIndex()).isEqualTo(0); + player.release(); + } + + @Test + public void addMediaItems_indexLargerThanPlaylist_addsToEndOfPlaylist() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + player.setMediaItem(MediaItem.fromUri("http://test")); + ImmutableList addedItems = + ImmutableList.of(MediaItem.fromUri("http://new1"), MediaItem.fromUri("http://new2")); + + player.addMediaItems(/* index= */ 5000, addedItems); + + assertThat(player.getMediaItemCount()).isEqualTo(3); + assertThat(player.getMediaItemAt(1)).isEqualTo(addedItems.get(0)); + assertThat(player.getMediaItemAt(2)).isEqualTo(addedItems.get(1)); + player.release(); + } + + @Test + public void removeMediaItems_fromIndexLargerThanPlaylist_isIgnored() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + player.setMediaItems( + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2"))); + + player.removeMediaItems(/* fromIndex= */ 5000, /* toIndex= */ 6000); + + assertThat(player.getMediaItemCount()).isEqualTo(2); + player.release(); + } + + @Test + public void removeMediaItems_toIndexLargerThanPlaylist_removesUpToEndOfPlaylist() + throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + player.setMediaItems( + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2"))); + + player.removeMediaItems(/* fromIndex= */ 1, /* toIndex= */ 6000); + + assertThat(player.getMediaItemCount()).isEqualTo(1); + assertThat(player.getMediaItemAt(0).localConfiguration.uri.toString()) + .isEqualTo("http://item1"); + player.release(); + } + + @Test + public void moveMediaItems_fromIndexLargerThanPlaylist_isIgnored() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + ImmutableList items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + player.setMediaItems(items); + + player.moveMediaItems(/* fromIndex= */ 5000, /* toIndex= */ 6000, /* newIndex= */ 0); + + assertThat(player.getMediaItemAt(0)).isEqualTo(items.get(0)); + assertThat(player.getMediaItemAt(1)).isEqualTo(items.get(1)); + player.release(); + } + + @Test + public void moveMediaItems_toIndexLargerThanPlaylist_movesItemsUpToEndOfPlaylist() + throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + ImmutableList items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + player.setMediaItems(items); + + player.moveMediaItems(/* fromIndex= */ 1, /* toIndex= */ 6000, /* newIndex= */ 0); + + assertThat(player.getMediaItemAt(0)).isEqualTo(items.get(1)); + assertThat(player.getMediaItemAt(1)).isEqualTo(items.get(0)); + player.release(); + } + + @Test + public void moveMediaItems_newIndexLargerThanPlaylist_movesItemsUpToEndOfPlaylist() + throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + ImmutableList items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + player.setMediaItems(items); + + player.moveMediaItems(/* fromIndex= */ 0, /* toIndex= */ 1, /* newIndex= */ 5000); + + assertThat(player.getMediaItemAt(0)).isEqualTo(items.get(1)); + assertThat(player.getMediaItemAt(1)).isEqualTo(items.get(0)); + player.release(); } @Test diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java index 6560cea856..e3a2ca4a33 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -463,6 +463,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_SEEK_TO_MEDIA_ITEM)) { return; } + checkArgument(mediaItemIndex >= 0); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_SEEK_TO_MEDIA_ITEM, @@ -499,6 +500,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_SEEK_TO_MEDIA_ITEM)) { return; } + checkArgument(mediaItemIndex >= 0); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_SEEK_TO_MEDIA_ITEM, @@ -938,6 +940,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_CHANGE_MEDIA_ITEMS)) { return; } + checkArgument(index >= 0); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_CHANGE_MEDIA_ITEMS, @@ -969,6 +972,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_CHANGE_MEDIA_ITEMS)) { return; } + checkArgument(index >= 0); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_CHANGE_MEDIA_ITEMS, @@ -1038,6 +1042,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_CHANGE_MEDIA_ITEMS)) { return; } + checkArgument(index >= 0); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_CHANGE_MEDIA_ITEMS, @@ -1051,6 +1056,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_CHANGE_MEDIA_ITEMS)) { return; } + checkArgument(fromIndex >= 0 && toIndex >= fromIndex); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_CHANGE_MEDIA_ITEMS, @@ -1073,19 +1079,17 @@ import org.checkerframework.checker.nullness.qual.NonNull; } private void removeMediaItemsInternal(int fromIndex, int toIndex) { - int clippedToIndex = min(toIndex, playerInfo.timeline.getWindowCount()); - - checkArgument( - fromIndex >= 0 - && clippedToIndex >= fromIndex - && clippedToIndex <= playerInfo.timeline.getWindowCount()); - Timeline oldTimeline = playerInfo.timeline; + int playlistSize = playerInfo.timeline.getWindowCount(); + toIndex = min(toIndex, playlistSize); + if (fromIndex >= playlistSize || fromIndex == toIndex) { + return; + } List newWindows = new ArrayList<>(); List newPeriods = new ArrayList<>(); for (int i = 0; i < oldTimeline.getWindowCount(); i++) { - if (i < fromIndex || i >= clippedToIndex) { + if (i < fromIndex || i >= toIndex) { newWindows.add(oldTimeline.getWindow(i, new Window())); } } @@ -1097,7 +1101,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; int oldPeriodIndex = playerInfo.sessionPositionInfo.positionInfo.periodIndex; int newPeriodIndex = oldPeriodIndex; boolean currentItemRemoved = - getCurrentMediaItemIndex() >= fromIndex && getCurrentMediaItemIndex() < clippedToIndex; + getCurrentMediaItemIndex() >= fromIndex && getCurrentMediaItemIndex() < toIndex; Window window = new Window(); if (oldTimeline.isEmpty()) { // No masking required. Just forwarding command to session. @@ -1117,17 +1121,17 @@ import org.checkerframework.checker.nullness.qual.NonNull; toIndex); if (oldNextMediaItemIndex == C.INDEX_UNSET) { newMediaItemIndex = newTimeline.getFirstWindowIndex(getShuffleModeEnabled()); - } else if (oldNextMediaItemIndex >= clippedToIndex) { - newMediaItemIndex = oldNextMediaItemIndex - (clippedToIndex - fromIndex); + } else if (oldNextMediaItemIndex >= toIndex) { + newMediaItemIndex = oldNextMediaItemIndex - (toIndex - fromIndex); } else { newMediaItemIndex = oldNextMediaItemIndex; } newPeriodIndex = newTimeline.getWindow(newMediaItemIndex, window).firstPeriodIndex; - } else if (oldMediaItemIndex >= clippedToIndex) { - newMediaItemIndex -= (clippedToIndex - fromIndex); + } else if (oldMediaItemIndex >= toIndex) { + newMediaItemIndex -= (toIndex - fromIndex); newPeriodIndex = getNewPeriodIndexWithoutRemovedPeriods( - oldTimeline, oldPeriodIndex, fromIndex, clippedToIndex); + oldTimeline, oldPeriodIndex, fromIndex, toIndex); } } @@ -1191,8 +1195,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; final boolean transitionsToEnded = newPlayerInfo.playbackState != Player.STATE_IDLE && newPlayerInfo.playbackState != Player.STATE_ENDED - && fromIndex < clippedToIndex - && clippedToIndex == oldTimeline.getWindowCount() + && fromIndex < toIndex + && toIndex == oldTimeline.getWindowCount() && getCurrentMediaItemIndex() >= fromIndex; if (transitionsToEnded) { newPlayerInfo = @@ -1207,7 +1211,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; Player.DISCONTINUITY_REASON_REMOVE, /* mediaItemTransition= */ playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex >= fromIndex - && playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex < clippedToIndex, + && playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex < toIndex, Player.MEDIA_ITEM_TRANSITION_REASON_PLAYLIST_CHANGED); } } @@ -1217,18 +1221,14 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_CHANGE_MEDIA_ITEMS)) { return; } - - checkArgument( - currentIndex >= 0 && currentIndex < playerInfo.timeline.getWindowCount() && newIndex >= 0); + checkArgument(currentIndex >= 0 && newIndex >= 0); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_CHANGE_MEDIA_ITEMS, (iSession, seq) -> iSession.moveMediaItem(controllerStub, seq, currentIndex, newIndex)); - int clippedNewIndex = min(newIndex, playerInfo.timeline.getWindowCount() - 1); - moveMediaItemsInternal( - /* fromIndex= */ currentIndex, /* toIndex= */ currentIndex + 1, clippedNewIndex); + /* fromIndex= */ currentIndex, /* toIndex= */ currentIndex + 1, newIndex); } @Override @@ -1236,22 +1236,14 @@ import org.checkerframework.checker.nullness.qual.NonNull; if (!isPlayerCommandAvailable(Player.COMMAND_CHANGE_MEDIA_ITEMS)) { return; } - - checkArgument( - fromIndex >= 0 - && fromIndex <= toIndex - && toIndex <= playerInfo.timeline.getWindowCount() - && newIndex >= 0); + checkArgument(fromIndex >= 0 && fromIndex <= toIndex && newIndex >= 0); dispatchRemoteSessionTaskWithPlayerCommand( Player.COMMAND_CHANGE_MEDIA_ITEMS, (iSession, seq) -> iSession.moveMediaItems(controllerStub, seq, fromIndex, toIndex, newIndex)); - int clippedNewIndex = - min(newIndex, playerInfo.timeline.getWindowCount() - (toIndex - fromIndex)); - - moveMediaItemsInternal(fromIndex, toIndex, clippedNewIndex); + moveMediaItemsInternal(fromIndex, toIndex, newIndex); } @Override @@ -1899,16 +1891,18 @@ import org.checkerframework.checker.nullness.qual.NonNull; } private void moveMediaItemsInternal(int fromIndex, int toIndex, int newIndex) { - if (fromIndex == 0 && toIndex == playerInfo.timeline.getWindowCount()) { + Timeline oldTimeline = playerInfo.timeline; + int playlistSize = playerInfo.timeline.getWindowCount(); + toIndex = min(toIndex, playlistSize); + newIndex = min(newIndex, playlistSize - (toIndex - fromIndex)); + if (fromIndex >= playlistSize || fromIndex == toIndex || fromIndex == newIndex) { return; } - Timeline oldTimeline = playerInfo.timeline; - List newWindows = new ArrayList<>(); List newPeriods = new ArrayList<>(); - for (int i = 0; i < oldTimeline.getWindowCount(); i++) { + for (int i = 0; i < playlistSize; i++) { newWindows.add(oldTimeline.getWindow(i, new Window())); } Util.moveItems(newWindows, fromIndex, toIndex, newIndex); @@ -1968,11 +1962,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; private void seekToInternal(int windowIndex, long positionMs) { Timeline timeline = playerInfo.timeline; - if (windowIndex < 0 || (!timeline.isEmpty() && windowIndex >= timeline.getWindowCount())) { - throw new IllegalSeekPositionException(timeline, windowIndex, positionMs); - } - - if (isPlayingAd()) { + if ((!timeline.isEmpty() && windowIndex >= timeline.getWindowCount()) || isPlayingAd()) { return; } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java index 2c670fa855..ff8e3c7c9a 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java @@ -15,6 +15,7 @@ */ package androidx.media3.session; +import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; @@ -47,7 +48,6 @@ import androidx.annotation.Nullable; import androidx.media3.common.AudioAttributes; import androidx.media3.common.C; import androidx.media3.common.DeviceInfo; -import androidx.media3.common.IllegalSeekPositionException; import androidx.media3.common.MediaItem; import androidx.media3.common.MediaMetadata; import androidx.media3.common.PlaybackException; @@ -311,13 +311,11 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } private void seekToInternal(int mediaItemIndex, long positionMs) { + checkArgument(mediaItemIndex >= 0); int currentMediaItemIndex = getCurrentMediaItemIndex(); Timeline currentTimeline = controllerInfo.playerInfo.timeline; - if (currentMediaItemIndex != mediaItemIndex - && (mediaItemIndex < 0 || mediaItemIndex >= currentTimeline.getWindowCount())) { - throw new IllegalSeekPositionException(currentTimeline, mediaItemIndex, positionMs); - } - if (isPlayingAd()) { + if ((!currentTimeline.isEmpty() && mediaItemIndex >= currentTimeline.getWindowCount()) + || isPlayingAd()) { return; } int newMediaItemIndex = currentMediaItemIndex; @@ -687,6 +685,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; @Override public void addMediaItems(int index, List mediaItems) { + checkArgument(index >= 0); if (mediaItems.isEmpty()) { return; } @@ -732,9 +731,10 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; @Override public void removeMediaItems(int fromIndex, int toIndex) { + checkArgument(fromIndex >= 0 && toIndex >= fromIndex); int windowCount = getCurrentTimeline().getWindowCount(); toIndex = min(toIndex, windowCount); - if (fromIndex >= toIndex) { + if (fromIndex >= windowCount || fromIndex == toIndex) { return; } @@ -787,15 +787,16 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; @Override public void moveMediaItems(int fromIndex, int toIndex, int newIndex) { + checkArgument(fromIndex >= 0 && fromIndex <= toIndex && newIndex >= 0); QueueTimeline queueTimeline = (QueueTimeline) controllerInfo.playerInfo.timeline; int size = queueTimeline.getWindowCount(); toIndex = min(toIndex, size); - if (fromIndex >= toIndex) { - return; - } int moveItemsSize = toIndex - fromIndex; int lastItemIndexAfterRemove = size - moveItemsSize - 1; - newIndex = min(newIndex, lastItemIndexAfterRemove); + newIndex = min(newIndex, lastItemIndexAfterRemove + 1); + if (fromIndex >= size || fromIndex == toIndex || fromIndex == newIndex) { + return; + } int currentMediaItemIndex = getCurrentMediaItemIndex(); int currentMediaItemIndexAfterRemove = diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java index 7e7f18dc17..e923f21d5a 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java @@ -46,6 +46,7 @@ import androidx.media3.test.session.common.TestUtils; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.LargeTest; +import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -2957,6 +2958,162 @@ public class MediaControllerStateMaskingTest { assertThat(reportedStateChangeToEndedAtSameTimeAsDiscontinuity.get()).isTrue(); } + @Test + public void seekTo_indexLargerThanPlaylist_isIgnored() throws Exception { + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + AtomicInteger mediaItemIndexAfterSeek = new AtomicInteger(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItem(MediaItem.fromUri("http://test")); + + controller.seekTo(/* windowIndex= */ 1, /* positionMs= */ 1000); + + mediaItemIndexAfterSeek.set(controller.getCurrentMediaItemIndex()); + }); + + assertThat(mediaItemIndexAfterSeek.get()).isEqualTo(0); + } + + @Test + public void addMediaItems_indexLargerThanPlaylist_addsToEndOfPlaylist() throws Exception { + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + List addedItems = + ImmutableList.of(MediaItem.fromUri("http://new1"), MediaItem.fromUri("http://new2")); + ArrayList mediaItemsAfterAdd = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItem(MediaItem.fromUri("http://test")); + + controller.addMediaItems(/* index= */ 5000, addedItems); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + mediaItemsAfterAdd.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(mediaItemsAfterAdd).hasSize(3); + assertThat(mediaItemsAfterAdd.get(1)).isEqualTo(addedItems.get(0)); + assertThat(mediaItemsAfterAdd.get(2)).isEqualTo(addedItems.get(1)); + } + + @Test + public void removeMediaItems_fromIndexLargerThanPlaylist_isIgnored() throws Exception { + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + AtomicInteger mediaItemCountAfterRemove = new AtomicInteger(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems( + ImmutableList.of( + MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2"))); + + controller.removeMediaItems(/* fromIndex= */ 5000, /* toIndex= */ 6000); + + mediaItemCountAfterRemove.set(controller.getMediaItemCount()); + }); + + assertThat(mediaItemCountAfterRemove.get()).isEqualTo(2); + } + + @Test + public void removeMediaItems_toIndexLargerThanPlaylist_removesUpToEndOfPlaylist() + throws Exception { + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + AtomicInteger mediaItemCountAfterRemove = new AtomicInteger(); + AtomicReference remainingItemAfterRemove = new AtomicReference<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems( + ImmutableList.of( + MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2"))); + + controller.removeMediaItems(/* fromIndex= */ 1, /* toIndex= */ 6000); + + mediaItemCountAfterRemove.set(controller.getMediaItemCount()); + remainingItemAfterRemove.set(controller.getMediaItemAt(0)); + }); + + assertThat(mediaItemCountAfterRemove.get()).isEqualTo(1); + assertThat(remainingItemAfterRemove.get().localConfiguration.uri.toString()) + .isEqualTo("http://item1"); + } + + @Test + public void moveMediaItems_fromIndexLargerThanPlaylist_isIgnored() throws Exception { + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + List items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + ArrayList itemsAfterMove = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems(items); + + controller.moveMediaItems( + /* fromIndex= */ 5000, /* toIndex= */ 6000, /* newIndex= */ 0); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + itemsAfterMove.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(itemsAfterMove).isEqualTo(items); + } + + @Test + public void moveMediaItems_toIndexLargerThanPlaylist_movesItemsUpToEndOfPlaylist() + throws Exception { + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + List items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + ArrayList itemsAfterMove = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems(items); + + controller.moveMediaItems(/* fromIndex= */ 1, /* toIndex= */ 6000, /* newIndex= */ 0); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + itemsAfterMove.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(itemsAfterMove).containsExactly(items.get(1), items.get(0)).inOrder(); + } + + @Test + public void moveMediaItems_newIndexLargerThanPlaylist_movesItemsUpToEndOfPlaylist() + throws Exception { + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + List items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + ArrayList itemsAfterMove = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems(items); + + controller.moveMediaItems(/* fromIndex= */ 0, /* toIndex= */ 1, /* newIndex= */ 5000); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + itemsAfterMove.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(itemsAfterMove).containsExactly(items.get(1), items.get(0)).inOrder(); + } + private void assertMoveMediaItems( int initialMediaItemCount, int initialMediaItemIndex, diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingWithMediaSessionCompatTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingWithMediaSessionCompatTest.java index e10dd5ae96..4fb1de36af 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingWithMediaSessionCompatTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingWithMediaSessionCompatTest.java @@ -58,6 +58,7 @@ import androidx.media3.test.session.common.MainLooperTestRule; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.LargeTest; +import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -1150,4 +1151,160 @@ public class MediaControllerStateMaskingWithMediaSessionCompatTest { assertThat(currentMediaItemIndexRef.get()).isEqualTo(testCurrentMediaItemIndex); MediaTestUtils.assertTimelineContains(timelineFromGetterRef.get(), testMediaItems); } + + @Test + public void seekTo_indexLargerThanPlaylist_isIgnored() throws Exception { + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + AtomicInteger mediaItemIndexAfterSeek = new AtomicInteger(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItem(MediaItem.fromUri("http://test")); + + controller.seekTo(/* windowIndex= */ 1, /* positionMs= */ 1000); + + mediaItemIndexAfterSeek.set(controller.getCurrentMediaItemIndex()); + }); + + assertThat(mediaItemIndexAfterSeek.get()).isEqualTo(0); + } + + @Test + public void addMediaItems_indexLargerThanPlaylist_addsToEndOfPlaylist() throws Exception { + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + List addedItems = + ImmutableList.of(MediaItem.fromUri("http://new1"), MediaItem.fromUri("http://new2")); + ArrayList mediaItemsAfterAdd = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItem(MediaItem.fromUri("http://test")); + + controller.addMediaItems(/* index= */ 5000, addedItems); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + mediaItemsAfterAdd.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(mediaItemsAfterAdd).hasSize(3); + assertThat(mediaItemsAfterAdd.get(1)).isEqualTo(addedItems.get(0)); + assertThat(mediaItemsAfterAdd.get(2)).isEqualTo(addedItems.get(1)); + } + + @Test + public void removeMediaItems_fromIndexLargerThanPlaylist_isIgnored() throws Exception { + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + AtomicInteger mediaItemCountAfterRemove = new AtomicInteger(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems( + ImmutableList.of( + MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2"))); + + controller.removeMediaItems(/* fromIndex= */ 5000, /* toIndex= */ 6000); + + mediaItemCountAfterRemove.set(controller.getMediaItemCount()); + }); + + assertThat(mediaItemCountAfterRemove.get()).isEqualTo(2); + } + + @Test + public void removeMediaItems_toIndexLargerThanPlaylist_removesUpToEndOfPlaylist() + throws Exception { + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + AtomicInteger mediaItemCountAfterRemove = new AtomicInteger(); + AtomicReference remainingItemAfterRemove = new AtomicReference<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems( + ImmutableList.of( + MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2"))); + + controller.removeMediaItems(/* fromIndex= */ 1, /* toIndex= */ 6000); + + mediaItemCountAfterRemove.set(controller.getMediaItemCount()); + remainingItemAfterRemove.set(controller.getMediaItemAt(0)); + }); + + assertThat(mediaItemCountAfterRemove.get()).isEqualTo(1); + assertThat(remainingItemAfterRemove.get().localConfiguration.uri.toString()) + .isEqualTo("http://item1"); + } + + @Test + public void moveMediaItems_fromIndexLargerThanPlaylist_isIgnored() throws Exception { + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + List items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + ArrayList itemsAfterMove = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems(items); + + controller.moveMediaItems( + /* fromIndex= */ 5000, /* toIndex= */ 6000, /* newIndex= */ 0); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + itemsAfterMove.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(itemsAfterMove).isEqualTo(items); + } + + @Test + public void moveMediaItems_toIndexLargerThanPlaylist_movesItemsUpToEndOfPlaylist() + throws Exception { + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + List items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + ArrayList itemsAfterMove = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems(items); + + controller.moveMediaItems(/* fromIndex= */ 1, /* toIndex= */ 6000, /* newIndex= */ 0); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + itemsAfterMove.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(itemsAfterMove).containsExactly(items.get(1), items.get(0)).inOrder(); + } + + @Test + public void moveMediaItems_newIndexLargerThanPlaylist_movesItemsUpToEndOfPlaylist() + throws Exception { + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + List items = + ImmutableList.of(MediaItem.fromUri("http://item1"), MediaItem.fromUri("http://item2")); + ArrayList itemsAfterMove = new ArrayList<>(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setMediaItems(items); + + controller.moveMediaItems(/* fromIndex= */ 0, /* toIndex= */ 1, /* newIndex= */ 5000); + + for (int i = 0; i < controller.getMediaItemCount(); i++) { + itemsAfterMove.add(controller.getMediaItemAt(i)); + } + }); + + assertThat(itemsAfterMove).containsExactly(items.get(1), items.get(0)).inOrder(); + } }