Skip to content

Commit

Permalink
Fix issue with stale createPeriod events in ConcatenatingMediaSource.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tonihei authored and ojw28 committed Sep 27, 2018
1 parent 66c9d41 commit 150e54b
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 1 deletion.
2 changes: 2 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 150e54b

Please sign in to comment.