Skip to content

Commit

Permalink
Fix various period preparation and source info refresh error throwing…
Browse files Browse the repository at this point in the history 
… 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
  • Loading branch information
@tonihei @ojw28
tonihei authored and ojw28 committed Sep 12, 2018
1 parent 502fae7 commit 67a2bb3
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 2 deletions.
2 changes: 2 additions & 0 deletions RELEASENOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
23 changes: 21 additions & 2 deletions library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,19 @@ private boolean isTimelineReady() {
&& (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();
Expand Down Expand Up @@ -1435,7 +1448,7 @@ private void updatePeriods() throws ExoPlaybackException, IOException {
}

// 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;
}
Expand All @@ -1450,6 +1463,12 @@ private void updatePeriods() throws ExoPlaybackException, IOException {
}
}

if (!readingPeriodHolder.next.prepared) {
// The successor is not prepared yet.
maybeThrowPeriodPrepareError();
return;
}

TrackSelectorResult oldTrackSelectorResult = readingPeriodHolder.trackSelectorResult;
readingPeriodHolder = queue.advanceReadingPeriod();
TrackSelectorResult newTrackSelectorResult = readingPeriodHolder.trackSelectorResult;
Expand Down Expand Up @@ -1498,7 +1517,7 @@ private void maybeUpdateLoadingPeriod() throws IOException {
if (queue.shouldLoadNextMediaPeriod()) {
MediaPeriodInfo info = queue.getNextMediaPeriodInfo(rendererPositionUs, playbackInfo);
if (info == null) {
mediaSource.maybeThrowSourceInfoRefreshError();
maybeThrowSourceInfoRefreshError();
} else {
MediaPeriod mediaPeriod =
queue.enqueueNextMediaPeriod(
Expand Down
8 changes: 8 additions & 0 deletions ...ary/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
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;
Expand Down Expand Up @@ -449,6 +450,13 @@ public final synchronized void prepareSourceInternal(
}
}

@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);
Expand Down
123 changes: 123 additions & 0 deletions library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2296,6 +2296,129 @@ public void onTracksChanged(
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) {
Expand Down

0 comments on commit 67a2bb3

Please sign in to comment.