From 67a2bb3d28d946e0085c1f4760d21e46f71cc3d6 Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 6 Sep 2018 09:26:03 -0700 Subject: [PATCH] Fix various period preparation and source info refresh error throwing issues 1. Currently, we may throw source info refresh errors while the previous media period is still playing. 2. We don't throw if the next period in a playlist fails to prepare and the previous renderers are all disabled. 3. We throw source info refresh errors for playlists before playback reaches the culprit source. This change: 1. Defers the exceptions until all existing media periods have been played. 2. Checks for period preparation exception if the next period is not getting prepared and the renderer time reached the next period. 3. Does no longer throw from ConcatenatingMediaSource.maybeThrowSourceInfo RefreshError. The deferred media periods take care of that for each source individually. Issue:#4661 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=211819436 --- RELEASENOTES.md | 2 + .../exoplayer2/ExoPlayerImplInternal.java | 23 +++- .../source/ConcatenatingMediaSource.java | 8 ++ .../android/exoplayer2/ExoPlayerTest.java | 123 ++++++++++++++++++ 4 files changed, 154 insertions(+), 2 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 3318136a16..0d27b6098f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -114,6 +114,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 errors of upcoming playlist items are thrown too early + ([#4661](https://github.com/google/ExoPlayer/issues/4661)). * IMA extension: * Refine the previous fix for empty ad groups to avoid discarding ad breaks unnecessarily ([#4030](https://github.com/google/ExoPlayer/issues/4030)), diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index f2aeb7321d..a866fc44ec 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -1122,6 +1122,19 @@ import java.util.Collections; && (playingPeriodHolder.next.prepared || playingPeriodHolder.next.info.id.isAd())); } + private void maybeThrowSourceInfoRefreshError() throws IOException { + MediaPeriodHolder loadingPeriodHolder = queue.getLoadingPeriod(); + if (loadingPeriodHolder != null) { + // Defer throwing until we read all available media periods. + for (Renderer renderer : enabledRenderers) { + if (!renderer.hasReadStreamToEnd()) { + return; + } + } + } + mediaSource.maybeThrowSourceInfoRefreshError(); + } + private void maybeThrowPeriodPrepareError() throws IOException { MediaPeriodHolder loadingPeriodHolder = queue.getLoadingPeriod(); MediaPeriodHolder readingPeriodHolder = queue.getReadingPeriod(); @@ -1435,7 +1448,7 @@ import java.util.Collections; } // Advance the reading period if necessary. - if (readingPeriodHolder.next == null || !readingPeriodHolder.next.prepared) { + if (readingPeriodHolder.next == null) { // We don't have a successor to advance the reading period to. return; } @@ -1450,6 +1463,12 @@ import java.util.Collections; } } + if (!readingPeriodHolder.next.prepared) { + // The successor is not prepared yet. + maybeThrowPeriodPrepareError(); + return; + } + TrackSelectorResult oldTrackSelectorResult = readingPeriodHolder.trackSelectorResult; readingPeriodHolder = queue.advanceReadingPeriod(); TrackSelectorResult newTrackSelectorResult = readingPeriodHolder.trackSelectorResult; @@ -1498,7 +1517,7 @@ import java.util.Collections; if (queue.shouldLoadNextMediaPeriod()) { MediaPeriodInfo info = queue.getNextMediaPeriodInfo(rendererPositionUs, playbackInfo); if (info == null) { - mediaSource.maybeThrowSourceInfoRefreshError(); + maybeThrowSourceInfoRefreshError(); } else { MediaPeriod mediaPeriod = queue.enqueueNextMediaPeriod( 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 9850427063..3a474facd3 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 @@ -29,6 +29,7 @@ import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -449,6 +450,13 @@ public class ConcatenatingMediaSource extends CompositeMediaSource concatenatingMediaSource.addMediaSource(failingMediaSource)) + .play() + .build(); + FakeRenderer renderer = new FakeRenderer(Builder.VIDEO_FORMAT); + ExoPlayerTestRunner testRunner = + new Builder() + .setMediaSource(concatenatingMediaSource) + .setActionSchedule(actionSchedule) + .setRenderers(renderer) + .build(context); + try { + testRunner.start().blockUntilEnded(TIMEOUT_MS); + fail(); + } catch (ExoPlaybackException e) { + // Expected exception. + } + assertThat(renderer.sampleBufferReadCount).isAtLeast(1); + assertThat(renderer.hasReadStreamToEnd()).isTrue(); + } + + @Test + public void failingDynamicUpdateOnlyThrowsWhenAvailablePeriodHasBeenFullyRead() throws Exception { + Timeline fakeTimeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* isSeekable= */ true, + /* isDynamic= */ true, + /* durationUs= */ 10 * C.MICROS_PER_SECOND)); + AtomicReference wasReadyOnce = new AtomicReference<>(false); + MediaSource mediaSource = + new FakeMediaSource(fakeTimeline, /* manifest= */ null, Builder.VIDEO_FORMAT) { + @Override + public void maybeThrowSourceInfoRefreshError() throws IOException { + if (wasReadyOnce.get()) { + throw new IOException(); + } + } + }; + ActionSchedule actionSchedule = + new ActionSchedule.Builder("testFailingDynamicMediaSourceInTimelineOnlyThrowsLater") + .pause() + .waitForPlaybackState(Player.STATE_READY) + .executeRunnable(() -> wasReadyOnce.set(true)) + .play() + .build(); + FakeRenderer renderer = new FakeRenderer(Builder.VIDEO_FORMAT); + ExoPlayerTestRunner testRunner = + new Builder() + .setMediaSource(mediaSource) + .setActionSchedule(actionSchedule) + .setRenderers(renderer) + .build(context); + try { + testRunner.start().blockUntilEnded(TIMEOUT_MS); + fail(); + } catch (ExoPlaybackException e) { + // Expected exception. + } + assertThat(renderer.sampleBufferReadCount).isAtLeast(1); + assertThat(renderer.hasReadStreamToEnd()).isTrue(); + } + // Internal methods. private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {