Commit 67a2bb3d by tonihei Committed by Oliver Woodman

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
parent 502fae7c
......@@ -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)),
......
......@@ -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(
......
......@@ -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;
......@@ -450,6 +451,13 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo
}
@Override
@SuppressWarnings("MissingSuperCall")
public void maybeThrowSourceInfoRefreshError() throws IOException {
// Do nothing. Source info refresh errors of the individual sources will be thrown when calling
// DeferredMediaPeriod.maybeThrowPrepareError.
}
@Override
public final MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
Object mediaSourceHolderUid = getMediaSourceHolderUid(id.periodUid);
MediaSourceHolder holder = Assertions.checkNotNull(mediaSourceByUid.get(mediaSourceHolderUid));
......
......@@ -2296,6 +2296,129 @@ public final class ExoPlayerTest {
assertThat(trackGroupsList.get(2).get(0).getFormat(0)).isEqualTo(Builder.AUDIO_FORMAT);
}
@Test
public void testSecondMediaSourceInPlaylistOnlyThrowsWhenPreviousPeriodIsFullyRead()
throws Exception {
Timeline fakeTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* isSeekable= */ true,
/* isDynamic= */ false,
/* durationUs= */ 10 * C.MICROS_PER_SECOND));
MediaSource workingMediaSource =
new FakeMediaSource(fakeTimeline, /* manifest= */ null, Builder.VIDEO_FORMAT);
MediaSource failingMediaSource =
new FakeMediaSource(/* timeline= */ null, /* manifest= */ null, Builder.VIDEO_FORMAT) {
@Override
public void maybeThrowSourceInfoRefreshError() throws IOException {
throw new IOException();
}
};
ConcatenatingMediaSource concatenatingMediaSource =
new ConcatenatingMediaSource(workingMediaSource, failingMediaSource);
FakeRenderer renderer = new FakeRenderer(Builder.VIDEO_FORMAT);
ExoPlayerTestRunner testRunner =
new Builder()
.setMediaSource(concatenatingMediaSource)
.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
testDynamicallyAddedSecondMediaSourceInPlaylistOnlyThrowsWhenPreviousPeriodIsFullyRead()
throws Exception {
Timeline fakeTimeline =
new FakeTimeline(
new TimelineWindowDefinition(
/* isSeekable= */ true,
/* isDynamic= */ false,
/* durationUs= */ 10 * C.MICROS_PER_SECOND));
MediaSource workingMediaSource =
new FakeMediaSource(fakeTimeline, /* manifest= */ null, Builder.VIDEO_FORMAT);
MediaSource failingMediaSource =
new FakeMediaSource(/* timeline= */ null, /* manifest= */ null, Builder.VIDEO_FORMAT) {
@Override
public void maybeThrowSourceInfoRefreshError() throws IOException {
throw new IOException();
}
};
ConcatenatingMediaSource concatenatingMediaSource =
new ConcatenatingMediaSource(workingMediaSource);
ActionSchedule actionSchedule =
new ActionSchedule.Builder("testFailingSecondMediaSourceInPlaylistOnlyThrowsLater")
.pause()
.waitForPlaybackState(Player.STATE_READY)
.executeRunnable(() -> 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<Boolean> 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) {
......
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