Commit 8c6da05f by tonihei Committed by Tofunmi Adigun-Hameed

Ensure behavior of add/setMediaItems is consistent on empty playlist

Adding items to an empty playlist is slightly different from adding
items to a non-empty playlist, because the former usually requires to
handle a change in the current item, position and playback state,
while the latter is not expected to affect the current item, position
or state.

The current ExoPlayer and SimpleBasePlayer code doesn't account for
this difference, leading to inconsistent behavior between
setMediaItem(s) and addMediaItem(s) when called on an empty playlist.

PiperOrigin-RevId: 530549928
parent 6f9a3b24
...@@ -2104,7 +2104,16 @@ public abstract class SimpleBasePlayer extends BasePlayer { ...@@ -2104,7 +2104,16 @@ public abstract class SimpleBasePlayer extends BasePlayer {
placeholderPlaylist.add( placeholderPlaylist.add(
i + correctedIndex, getPlaceholderMediaItemData(mediaItems.get(i))); i + correctedIndex, getPlaceholderMediaItemData(mediaItems.get(i)));
} }
return getStateWithNewPlaylist(state, placeholderPlaylist, period); if (!state.playlist.isEmpty()) {
return getStateWithNewPlaylist(state, placeholderPlaylist, period);
} else {
// Handle initial position update when these are the first items added to the playlist.
return getStateWithNewPlaylistAndPosition(
state,
placeholderPlaylist,
state.currentMediaItemIndex,
state.contentPositionMsSupplier.get());
}
}); });
} }
...@@ -3801,7 +3810,7 @@ public abstract class SimpleBasePlayer extends BasePlayer { ...@@ -3801,7 +3810,7 @@ public abstract class SimpleBasePlayer extends BasePlayer {
State.Builder stateBuilder = oldState.buildUpon(); State.Builder stateBuilder = oldState.buildUpon();
stateBuilder.setPlaylist(newPlaylist); stateBuilder.setPlaylist(newPlaylist);
if (oldState.playbackState != Player.STATE_IDLE) { if (oldState.playbackState != Player.STATE_IDLE) {
if (newPlaylist.isEmpty()) { if (newPlaylist.isEmpty() || (newIndex != C.INDEX_UNSET && newIndex >= newPlaylist.size())) {
stateBuilder.setPlaybackState(Player.STATE_ENDED).setIsLoading(false); stateBuilder.setPlaybackState(Player.STATE_ENDED).setIsLoading(false);
} else { } else {
stateBuilder.setPlaybackState(Player.STATE_BUFFERING); stateBuilder.setPlaybackState(Player.STATE_BUFFERING);
......
...@@ -644,6 +644,12 @@ import java.util.concurrent.TimeoutException; ...@@ -644,6 +644,12 @@ import java.util.concurrent.TimeoutException;
verifyApplicationThread(); verifyApplicationThread();
checkArgument(index >= 0); checkArgument(index >= 0);
index = min(index, mediaSourceHolderSnapshots.size()); index = min(index, mediaSourceHolderSnapshots.size());
if (mediaSourceHolderSnapshots.isEmpty()) {
// Handle initial items in a playlist as a set operation to ensure state changes and initial
// position are updated correctly.
setMediaSources(mediaSources, /* resetPosition= */ maskingWindowIndex == C.INDEX_UNSET);
return;
}
Timeline oldTimeline = getCurrentTimeline(); Timeline oldTimeline = getCurrentTimeline();
pendingOperationAcks++; pendingOperationAcks++;
List<MediaSourceList.MediaSourceHolder> holders = addMediaSourceHolders(index, mediaSources); List<MediaSourceList.MediaSourceHolder> holders = addMediaSourceHolders(index, mediaSources);
...@@ -850,11 +856,12 @@ import java.util.concurrent.TimeoutException; ...@@ -850,11 +856,12 @@ import java.util.concurrent.TimeoutException;
playbackInfoUpdateListener.onPlaybackInfoUpdate(playbackInfoUpdate); playbackInfoUpdateListener.onPlaybackInfoUpdate(playbackInfoUpdate);
return; return;
} }
@Player.State PlaybackInfo newPlaybackInfo = playbackInfo;
int newPlaybackState = if (playbackInfo.playbackState == Player.STATE_READY
getPlaybackState() == Player.STATE_IDLE ? Player.STATE_IDLE : STATE_BUFFERING; || (playbackInfo.playbackState == Player.STATE_ENDED && !timeline.isEmpty())) {
newPlaybackInfo = playbackInfo.copyWithPlaybackState(Player.STATE_BUFFERING);
}
int oldMaskingMediaItemIndex = getCurrentMediaItemIndex(); int oldMaskingMediaItemIndex = getCurrentMediaItemIndex();
PlaybackInfo newPlaybackInfo = playbackInfo.copyWithPlaybackState(newPlaybackState);
newPlaybackInfo = newPlaybackInfo =
maskTimelineAndPosition( maskTimelineAndPosition(
newPlaybackInfo, newPlaybackInfo,
......
...@@ -6095,61 +6095,137 @@ public final class ExoPlayerTest { ...@@ -6095,61 +6095,137 @@ public final class ExoPlayerTest {
} }
@Test @Test
public void modifyPlaylistPrepared_remainsInEnded_needsSeekForBuffering() throws Exception { public void addMediaSource_toEndedPlaylist_remainsInEndedAndNeedsSeekForBuffering()
Timeline timeline = new FakeTimeline(); throws Exception {
FakeMediaSource secondMediaSource = new FakeMediaSource(timeline); ExoPlayer player = new TestExoPlayerBuilder(context).build();
ActionSchedule actionSchedule = player.setMediaSource(new FakeMediaSource());
new ActionSchedule.Builder(TAG) player.prepare();
.waitForTimelineChanged(timeline, Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE) player.play();
.waitForPlaybackState(Player.STATE_BUFFERING)
.waitForPlaybackState(Player.STATE_READY)
.clearMediaItems()
.waitForPlaybackState(Player.STATE_ENDED)
.addMediaSources(secondMediaSource) // add must not transition to buffering
.waitForTimelineChanged()
.clearMediaItems() // clear must remain in ended
.addMediaSources(secondMediaSource) // add again to be able to test the seek
.waitForTimelineChanged()
.seek(/* positionMs= */ 2_000) // seek must transition to buffering
.waitForPlaybackState(Player.STATE_BUFFERING)
.waitForPlaybackState(Player.STATE_READY)
.waitForPlaybackState(Player.STATE_ENDED)
.build();
ExoPlayerTestRunner exoPlayerTestRunner =
new ExoPlayerTestRunner.Builder(context)
.setExpectedPlayerEndedCount(/* expectedPlayerEndedCount= */ 2)
.setTimeline(timeline)
.setActionSchedule(actionSchedule)
.build()
.start()
.blockUntilActionScheduleFinished(TIMEOUT_MS)
.blockUntilEnded(TIMEOUT_MS);
exoPlayerTestRunner.assertPlaybackStatesEqual( runUntilPlaybackState(player, Player.STATE_ENDED);
Player.STATE_BUFFERING, // first buffering player.addMediaSource(new FakeMediaSource());
Player.STATE_READY, int stateAfterAdd = player.getPlaybackState();
Player.STATE_ENDED, // clear playlist runUntilPendingCommandsAreFullyHandled(player);
Player.STATE_BUFFERING, // second buffering after seek int stateAfterAddHandled = player.getPlaybackState();
Player.STATE_READY, player.seekTo(100);
Player.STATE_ENDED); int stateAfterSeek = player.getPlaybackState();
exoPlayerTestRunner.assertTimelinesSame( runUntilPendingCommandsAreFullyHandled(player);
placeholderTimeline, int stateAfterSeekHandled = player.getPlaybackState();
timeline, player.release();
Timeline.EMPTY,
placeholderTimeline, assertThat(stateAfterAdd).isEqualTo(Player.STATE_ENDED);
timeline, assertThat(stateAfterAddHandled).isEqualTo(Player.STATE_ENDED);
Timeline.EMPTY, assertThat(stateAfterSeek).isEqualTo(Player.STATE_BUFFERING);
placeholderTimeline, assertThat(stateAfterSeekHandled).isAnyOf(Player.STATE_BUFFERING, Player.STATE_READY);
timeline); }
exoPlayerTestRunner.assertTimelineChangeReasonsEqual(
Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED /* media item set (masked timeline) */, @Test
Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE /* source prepared */, public void addMediaSource_toEmptyPreparedPlaylist_startsBuffering() throws Exception {
Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED /* playlist cleared */, ExoPlayer player = new TestExoPlayerBuilder(context).build();
Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED /* media items added */, player.prepare();
Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE /* source prepared */, player.play();
Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED /* playlist cleared */,
Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED /* media items added */, int stateAfterPrepare = player.getPlaybackState();
Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE /* source prepared */); player.addMediaSource(new FakeMediaSource());
int stateAfterAdd = player.getPlaybackState();
runUntilPendingCommandsAreFullyHandled(player);
int stateAfterAddHandled = player.getPlaybackState();
runUntilPlaybackState(player, Player.STATE_ENDED);
player.release();
assertThat(stateAfterPrepare).isEqualTo(Player.STATE_ENDED);
assertThat(stateAfterAdd).isEqualTo(Player.STATE_BUFFERING);
assertThat(stateAfterAddHandled).isAnyOf(Player.STATE_BUFFERING, Player.STATE_READY);
}
@Test
public void addMediaSources_afterSeekOnEmptyPreparedPlaylist_startsBufferingAtRequestedPosition()
throws Exception {
ExoPlayer player = new TestExoPlayerBuilder(context).build();
player.prepare();
player.seekTo(/* mediaItemIndex= */ 1, /* positionMs= */ 5000);
int stateAfterSeek = player.getPlaybackState();
int mediaItemIndexAfterSeek = player.getCurrentMediaItemIndex();
long positionAfterSeek = player.getCurrentPosition();
player.addMediaSources(ImmutableList.of(new FakeMediaSource(), new FakeMediaSource()));
int stateAfterAdd = player.getPlaybackState();
int mediaItemIndexAfterAdd = player.getCurrentMediaItemIndex();
long positionAfterAdd = player.getCurrentPosition();
runUntilPendingCommandsAreFullyHandled(player);
int stateAfterAddHandled = player.getPlaybackState();
int mediaItemIndexAfterAddHandled = player.getCurrentMediaItemIndex();
long positionAfterAddHandled = player.getCurrentPosition();
player.play();
runUntilPlaybackState(player, Player.STATE_ENDED);
player.release();
assertThat(stateAfterSeek).isEqualTo(Player.STATE_ENDED);
assertThat(mediaItemIndexAfterSeek).isEqualTo(1);
assertThat(positionAfterSeek).isEqualTo(5000);
assertThat(stateAfterAdd).isEqualTo(Player.STATE_BUFFERING);
assertThat(mediaItemIndexAfterAdd).isEqualTo(1);
assertThat(positionAfterAdd).isEqualTo(5000);
assertThat(stateAfterAddHandled).isAnyOf(Player.STATE_BUFFERING, Player.STATE_READY);
assertThat(mediaItemIndexAfterAddHandled).isEqualTo(1);
assertThat(positionAfterAddHandled).isEqualTo(5000);
}
@Test
public void addMediaSources_afterInvalidSeekOnEmptyPreparedPlaylist_remainsInEndedState()
throws Exception {
ExoPlayer player = new TestExoPlayerBuilder(context).build();
player.prepare();
player.seekTo(/* mediaItemIndex= */ 1, /* positionMs= */ 5000);
int stateAfterSeek = player.getPlaybackState();
int mediaItemIndexAfterSeek = player.getCurrentMediaItemIndex();
long positionAfterSeek = player.getCurrentPosition();
player.addMediaSources(ImmutableList.of(new FakeMediaSource()));
int stateAfterAdd = player.getPlaybackState();
int mediaItemIndexAfterAdd = player.getCurrentMediaItemIndex();
long positionAfterAdd = player.getCurrentPosition();
runUntilPendingCommandsAreFullyHandled(player);
int stateAfterAddHandled = player.getPlaybackState();
int mediaItemIndexAfterAddHandled = player.getCurrentMediaItemIndex();
long positionAfterAddHandled = player.getCurrentPosition();
player.play();
runUntilPlaybackState(player, Player.STATE_ENDED);
player.release();
assertThat(stateAfterSeek).isEqualTo(Player.STATE_ENDED);
assertThat(mediaItemIndexAfterSeek).isEqualTo(1);
assertThat(positionAfterSeek).isEqualTo(5000);
assertThat(stateAfterAdd).isEqualTo(Player.STATE_ENDED);
assertThat(mediaItemIndexAfterAdd).isEqualTo(0);
assertThat(positionAfterAdd).isEqualTo(0);
assertThat(stateAfterAddHandled).isEqualTo(Player.STATE_ENDED);
assertThat(mediaItemIndexAfterAddHandled).isEqualTo(0);
assertThat(positionAfterAddHandled).isEqualTo(0);
}
@Test
public void clearMediaItemsAndSeek_inStateEnded_remainsInEnded() throws Exception {
ExoPlayer player = new TestExoPlayerBuilder(context).build();
player.setMediaSource(new FakeMediaSource());
player.prepare();
player.play();
runUntilPlaybackState(player, Player.STATE_ENDED);
player.clearMediaItems();
int stateAfterClear = player.getPlaybackState();
runUntilPendingCommandsAreFullyHandled(player);
int stateAfterClearHandled = player.getPlaybackState();
player.seekTo(100);
int stateAfterSeek = player.getPlaybackState();
runUntilPendingCommandsAreFullyHandled(player);
int stateAfterSeekHandled = player.getPlaybackState();
player.release();
assertThat(stateAfterClear).isEqualTo(Player.STATE_ENDED);
assertThat(stateAfterClearHandled).isEqualTo(Player.STATE_ENDED);
assertThat(stateAfterSeek).isEqualTo(Player.STATE_ENDED);
assertThat(stateAfterSeekHandled).isEqualTo(Player.STATE_ENDED);
} }
@Test @Test
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment