From 150e54b560275646ed53c167ce580a812aa3505f Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 27 Sep 2018 09:33:36 -0700 Subject: [PATCH] Fix issue with stale createPeriod events in ConcatenatingMediaSource. If a source is removed from the playlist, the player may still call createPeriod for a period of the removed source as long as the new timeline hasn't been handled by the player. These events are stale and can be ignored by using a dummy media source. The stale media period will be released when the player handles the updated timeline. Issue:#4871 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=214787090 --- RELEASENOTES.md | 2 + .../source/ConcatenatingMediaSource.java | 39 ++++++++++++++++++- .../android/exoplayer2/ExoPlayerTest.java | 27 +++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 08c61d5c39a..12b4bb48727 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -128,6 +128,8 @@ * Fix bugs reporting events for multi-period media sources ([#4492](https://github.com/google/ExoPlayer/issues/4492) and [#4634](https://github.com/google/ExoPlayer/issues/4634)). +* Fix issue where removing looping media from a playlist throws an exception + ([#4871](https://github.com/google/ExoPlayer/issues/4871). * Fix issue where the preferred audio or text track would not be selected if mapped onto a secondary renderer of the corresponding type ([#4711](http://github.com/google/ExoPlayer/issues/4711)). diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java index 30e764dfe59..e0b7da85063 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java @@ -474,7 +474,12 @@ public void maybeThrowSourceInfoRefreshError() throws IOException { @Override public final MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { Object mediaSourceHolderUid = getMediaSourceHolderUid(id.periodUid); - MediaSourceHolder holder = Assertions.checkNotNull(mediaSourceByUid.get(mediaSourceHolderUid)); + MediaSourceHolder holder = mediaSourceByUid.get(mediaSourceHolderUid); + if (holder == null) { + // Stale event. The media source has already been removed. + holder = new MediaSourceHolder(new DummyMediaSource()); + holder.hasStartedPreparing = true; + } DeferredMediaPeriod mediaPeriod = new DeferredMediaPeriod(holder.mediaSource, id, allocator); mediaSourceByMediaPeriod.put(mediaPeriod, holder); holder.activeMediaPeriods.add(mediaPeriod); @@ -994,5 +999,37 @@ public Object getUidOfPeriod(int periodIndex) { return DeferredTimeline.DUMMY_ID; } } + + /** Dummy media source which does nothing and does not support creating periods. */ + private static final class DummyMediaSource extends BaseMediaSource { + + @Override + protected void prepareSourceInternal( + ExoPlayer player, + boolean isTopLevelSource, + @Nullable TransferListener mediaTransferListener) { + // Do nothing. + } + + @Override + protected void releaseSourceInternal() { + // Do nothing. + } + + @Override + public void maybeThrowSourceInfoRefreshError() throws IOException { + // Do nothing. + } + + @Override + public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { + throw new UnsupportedOperationException(); + } + + @Override + public void releasePeriod(MediaPeriod mediaPeriod) { + // Do nothing. + } + } } 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 d42d3ee9a7c..8414be15882 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 @@ -2419,6 +2419,33 @@ public void maybeThrowSourceInfoRefreshError() throws IOException { assertThat(renderer.hasReadStreamToEnd()).isTrue(); } + @Test + public void removingLoopingLastPeriodFromPlaylistDoesNotThrow() throws Exception { + Timeline timeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* isSeekable= */ true, /* isDynamic= */ true, /* durationUs= */ 100_000)); + MediaSource mediaSource = new FakeMediaSource(timeline, /* manifest= */ null); + ConcatenatingMediaSource concatenatingMediaSource = new ConcatenatingMediaSource(mediaSource); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("removingLoopingLastPeriodFromPlaylistDoesNotThrow") + .pause() + .waitForPlaybackState(Player.STATE_READY) + // Play almost to end to ensure the current period is fully buffered. + .playUntilPosition(/* windowIndex= */ 0, /* positionMs= */ 90) + // Enable repeat mode to trigger the creation of new media periods. + .setRepeatMode(Player.REPEAT_MODE_ALL) + // Remove the media source. + .executeRunnable(concatenatingMediaSource::clear) + .build(); + new Builder() + .setMediaSource(concatenatingMediaSource) + .setActionSchedule(actionSchedule) + .build(context) + .start() + .blockUntilEnded(TIMEOUT_MS); + } + // Internal methods. private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {