diff --git a/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java b/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java index a87fe9a437..02ab698a0f 100644 --- a/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java +++ b/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java @@ -43,7 +43,6 @@ import com.google.android.exoplayer2.audio.AudioAttributes; import com.google.android.exoplayer2.source.TrackGroup; import com.google.android.exoplayer2.text.CueGroup; import com.google.android.exoplayer2.trackselection.TrackSelectionParameters; -import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.ListenerSet; import com.google.android.exoplayer2.util.Log; @@ -294,7 +293,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; @@ -304,14 +303,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; } @@ -324,9 +320,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; } @@ -404,6 +401,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/library/common/src/main/java/com/google/android/exoplayer2/Player.java b/library/common/src/main/java/com/google/android/exoplayer2/Player.java index 9058ac9f60..cda2baeb46 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/Player.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/Player.java @@ -1669,7 +1669,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. */ @@ -1678,8 +1679,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. @@ -1689,16 +1692,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); @@ -1879,9 +1884,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); @@ -1896,11 +1900,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/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index 999692917b..ead8179224 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -30,6 +30,7 @@ import static com.google.android.exoplayer2.Renderer.MSG_SET_VIDEO_FRAME_METADAT import static com.google.android.exoplayer2.Renderer.MSG_SET_VIDEO_OUTPUT; import static com.google.android.exoplayer2.Renderer.MSG_SET_VIDEO_OUTPUT_RESOLUTION; import static com.google.android.exoplayer2.Renderer.MSG_SET_VOLUME; +import static com.google.android.exoplayer2.util.Assertions.checkArgument; import static com.google.android.exoplayer2.util.Assertions.checkNotNull; import static com.google.android.exoplayer2.util.Assertions.checkState; import static com.google.android.exoplayer2.util.Util.castNonNull; @@ -83,7 +84,6 @@ import com.google.android.exoplayer2.trackselection.TrackSelectionParameters; import com.google.android.exoplayer2.trackselection.TrackSelector; import com.google.android.exoplayer2.trackselection.TrackSelectorResult; import com.google.android.exoplayer2.upstream.BandwidthMeter; -import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.ConditionVariable; import com.google.android.exoplayer2.util.HandlerWrapper; @@ -613,7 +613,6 @@ import java.util.concurrent.TimeoutException; @Override public void addMediaItems(int index, List mediaItems) { verifyApplicationThread(); - index = min(index, mediaSourceHolderSnapshots.size()); addMediaSources(index, createMediaSources(mediaItems)); } @@ -638,7 +637,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); @@ -664,7 +664,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); @@ -683,14 +689,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 = @@ -819,11 +827,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()) { @@ -2251,8 +2259,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(); @@ -2291,7 +2297,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/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index aa2e6bfc68..c15edcbe37 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -921,31 +921,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