diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 3648082282..b01aafc4c7 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -7,6 +7,9 @@ it easier to handle updates in other classes ([#2128](https://github.com/androidx/media/issues/2128)). * ExoPlayer: + * Fix a bug where `ExoPlayer.isLoading()` remains `true` while it has + transitioned to `STATE_IDLE` or `STATE_ENDED` + ([#2133](https://github.com/androidx/media/issues/2133)). * Transformer: * Track Selection: * Extractors: 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 e364456933..53906fb795 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java @@ -531,8 +531,8 @@ import java.util.concurrent.CopyOnWriteArraySet; } PlaybackInfo playbackInfo = this.playbackInfo.copyWithPlaybackError(null); playbackInfo = - playbackInfo.copyWithPlaybackState( - playbackInfo.timeline.isEmpty() ? STATE_ENDED : STATE_BUFFERING); + maskPlaybackState( + playbackInfo, playbackInfo.timeline.isEmpty() ? STATE_ENDED : STATE_BUFFERING); // Trigger internal prepare first before updating the playback info and notifying external // listeners to ensure that new operations issued in the listener notifications reach the // player after this prepare. The internal player can't change the playback info immediately @@ -905,7 +905,7 @@ import java.util.concurrent.CopyOnWriteArraySet; PlaybackInfo newPlaybackInfo = playbackInfo; if (playbackInfo.playbackState == Player.STATE_READY || (playbackInfo.playbackState == Player.STATE_ENDED && !timeline.isEmpty())) { - newPlaybackInfo = playbackInfo.copyWithPlaybackState(Player.STATE_BUFFERING); + newPlaybackInfo = maskPlaybackState(playbackInfo, Player.STATE_BUFFERING); } int oldMaskingMediaItemIndex = getCurrentMediaItemIndex(); newPlaybackInfo = @@ -1051,7 +1051,7 @@ import java.util.concurrent.CopyOnWriteArraySet; if (playbackInfo.sleepingForOffload) { playbackInfo = playbackInfo.copyWithEstimatedPosition(); } - playbackInfo = playbackInfo.copyWithPlaybackState(Player.STATE_IDLE); + playbackInfo = maskPlaybackState(playbackInfo, Player.STATE_IDLE); playbackInfo = playbackInfo.copyWithLoadingMediaPeriodId(playbackInfo.periodId); playbackInfo.bufferedPositionUs = playbackInfo.positionUs; playbackInfo.totalBufferedDurationUs = 0; @@ -1882,7 +1882,7 @@ import java.util.concurrent.CopyOnWriteArraySet; this.playbackInfo.copyWithLoadingMediaPeriodId(this.playbackInfo.periodId); playbackInfo.bufferedPositionUs = playbackInfo.positionUs; playbackInfo.totalBufferedDurationUs = 0; - playbackInfo = playbackInfo.copyWithPlaybackState(Player.STATE_IDLE); + playbackInfo = maskPlaybackState(playbackInfo, Player.STATE_IDLE); if (error != null) { playbackInfo = playbackInfo.copyWithPlaybackError(error); } @@ -2360,7 +2360,7 @@ import java.util.concurrent.CopyOnWriteArraySet; maskingPlaybackState = STATE_BUFFERING; } } - newPlaybackInfo = newPlaybackInfo.copyWithPlaybackState(maskingPlaybackState); + newPlaybackInfo = maskPlaybackState(newPlaybackInfo, maskingPlaybackState); internalPlayer.setMediaSources( holders, startWindowIndex, Util.msToUs(startPositionMs), shuffleOrder); boolean positionDiscontinuity = @@ -2434,7 +2434,7 @@ import java.util.concurrent.CopyOnWriteArraySet; && toIndex == currentMediaSourceCount && currentIndex >= newPlaybackInfo.timeline.getWindowCount(); if (transitionsToEnded) { - newPlaybackInfo = newPlaybackInfo.copyWithPlaybackState(STATE_ENDED); + newPlaybackInfo = maskPlaybackState(newPlaybackInfo, STATE_ENDED); } internalPlayer.removeMediaSources(fromIndex, toIndex, shuffleOrder); return newPlaybackInfo; @@ -2558,6 +2558,14 @@ import java.util.concurrent.CopyOnWriteArraySet; return playbackInfo; } + private static PlaybackInfo maskPlaybackState(PlaybackInfo playbackInfo, int playbackState) { + playbackInfo = playbackInfo.copyWithPlaybackState(playbackState); + if (playbackState == STATE_IDLE || playbackState == STATE_ENDED) { + playbackInfo = playbackInfo.copyWithIsLoading(false); + } + return playbackInfo; + } + @Nullable private Pair getPeriodPositionUsAfterTimelineChanged( Timeline oldTimeline, 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 a6f58e0bc1..abca9360cf 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerTest.java @@ -1891,6 +1891,20 @@ public class ExoPlayerTest { assertThat(totalBufferedDuration[2]).isEqualTo(0); } + @Test + public void stopWhileLoading_correctMaskingIsLoading() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + player.setMediaSource(new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1))); + player.prepare(); + advance(player).untilLoadingIs(true); + + assertThat(player.isLoading()).isTrue(); + player.stop(); + assertThat(player.isLoading()).isFalse(); + + player.release(); + } + @Test public void stop_releasesMediaSource() throws Exception { Timeline timeline = new FakeTimeline(); @@ -2089,6 +2103,18 @@ public class ExoPlayerTest { assertThat(totalBufferedDuration[2]).isEqualTo(0); } + @Test + public void releaseWhileLoading_correctMaskingIsLoading() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + player.setMediaSource(new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1))); + player.prepare(); + advance(player).untilLoadingIs(true); + + assertThat(player.isLoading()).isTrue(); + player.release(); + assertThat(player.isLoading()).isFalse(); + } + @Test public void settingNewStartPositionPossibleAfterStopAndClearMediaItems() throws Exception { Timeline timeline = new FakeTimeline(); @@ -8810,6 +8836,29 @@ public class ExoPlayerTest { Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE); } + @Test + public void setMediaSourcesWhileLoading_noSeekEmpty_correctMaskingIsLoading() throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + player.setMediaSource(new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1))); + player.prepare(); + advance(player).untilLoadingIs(true); + + assertThat(player.isLoading()).isTrue(); + // Set a media item with empty timeline. + FakeMediaSource fakeMediaSource = + new FakeMediaSource(Timeline.EMPTY) { + @Override + @Nullable + public Timeline getInitialTimeline() { + return getTimeline(); + } + }; + player.setMediaSource(fakeMediaSource, /* resetPosition= */ false); + assertThat(player.isLoading()).isFalse(); + + player.release(); + } + @Test public void addMediaSources_whenEmptyInitialSeek_correctPeriodMasking() throws Exception { final long[] positions = new long[2]; @@ -9269,6 +9318,27 @@ public class ExoPlayerTest { assertThat(bufferedPositions[2]).isEqualTo(0); } + @Test + public void removeMediaItemsWhileLoading_currentItemRemovedThatIsTheLast_correctMaskingIsLoading() + throws Exception { + ExoPlayer player = new TestExoPlayerBuilder(context).build(); + MediaSource firstMediaSource = new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1)); + MediaSource secondMediaSource = new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1)); + MediaSource thirdMediaSource = new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1)); + player.setMediaSources( + ImmutableList.of(firstMediaSource, secondMediaSource, thirdMediaSource), + /* startMediaItemIndex= */ 2, + /* startPositionMs= */ C.TIME_UNSET); + player.prepare(); + advance(player).untilLoadingIs(true); + + assertThat(player.isLoading()).isTrue(); + player.removeMediaItem(/* index= */ 2); + assertThat(player.isLoading()).isFalse(); + + player.release(); + } + @Test public void removeMediaItems_removeTailWithCurrentWindow_whenIdle_finishesPlayback() throws Exception {