Skip to content

Commit

Permalink
Fix potential media source release before media period release.
Browse files Browse the repository at this point in the history
This could happen when a media source is removed from a
DynamicConcatenatingMediaSource and one of its media periods is still active.
This media period is only removed by the player after the player received
a timeline update and thus we shouldn't release the removed child source
as long as it has active media periods.

Issue:#3796

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=184522836
  • Loading branch information
tonihei authored and andrewlewis committed Feb 8, 2018
1 parent aa63ad3 commit 4437de4
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,18 @@ public void testPeriodCreationWithAds() throws IOException, InterruptedException
mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(1, 0, 0));
}

public void testRemoveChildSourceWithActiveMediaPeriod() throws IOException {
FakeMediaSource childSource = createFakeMediaSource();
mediaSource.addMediaSource(childSource);
testRunner.prepareSource();
MediaPeriod mediaPeriod = testRunner.createPeriod(new MediaPeriodId(/* periodIndex= */ 0));
mediaSource.removeMediaSource(/* index= */ 0);
testRunner.assertTimelineChangeBlocking();
testRunner.releasePeriod(mediaPeriod);
childSource.assertReleased();
testRunner.releaseSource();
}

private static FakeMediaSource[] createMediaSources(int count) {
FakeMediaSource[] sources = new FakeMediaSource[count];
for (int i = 0; i < count; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource<
// Accessed on the playback thread.
private final List<MediaSourceHolder> mediaSourceHolders;
private final MediaSourceHolder query;
private final Map<MediaPeriod, MediaSource> mediaSourceByMediaPeriod;
private final Map<MediaPeriod, MediaSourceHolder> mediaSourceByMediaPeriod;
private final List<DeferredMediaPeriod> deferredMediaPeriods;

private ExoPlayer player;
Expand Down Expand Up @@ -355,19 +355,23 @@ public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
} else {
mediaPeriod = holder.mediaSource.createPeriod(idInSource, allocator);
}
mediaSourceByMediaPeriod.put(mediaPeriod, holder.mediaSource);
mediaSourceByMediaPeriod.put(mediaPeriod, holder);
holder.activeMediaPeriods++;
return mediaPeriod;
}

@Override
public void releasePeriod(MediaPeriod mediaPeriod) {
MediaSource mediaSource = mediaSourceByMediaPeriod.get(mediaPeriod);
mediaSourceByMediaPeriod.remove(mediaPeriod);
MediaSourceHolder holder = mediaSourceByMediaPeriod.remove(mediaPeriod);
if (mediaPeriod instanceof DeferredMediaPeriod) {
deferredMediaPeriods.remove(mediaPeriod);
((DeferredMediaPeriod) mediaPeriod).releasePeriod();
} else {
mediaSource.releasePeriod(mediaPeriod);
holder.mediaSource.releasePeriod(mediaPeriod);
}
holder.activeMediaPeriods--;
if (holder.activeMediaPeriods == 0 && holder.isRemoved) {
releaseChildSource(holder);
}
}

Expand Down Expand Up @@ -520,7 +524,10 @@ private void removeMediaSourceInternal(int index) {
/* childIndexUpdate= */ -1,
-oldTimeline.getWindowCount(),
-oldTimeline.getPeriodCount());
releaseChildSource(holder);
holder.isRemoved = true;
if (holder.activeMediaPeriods == 0) {
releaseChildSource(holder);
}
}

private void moveMediaSourceInternal(int currentIndex, int newIndex) {
Expand Down Expand Up @@ -573,6 +580,8 @@ private int findMediaSourceHolderByPeriodIndex(int periodIndex) {
public int firstWindowIndexInChild;
public int firstPeriodIndexInChild;
public boolean isPrepared;
public boolean isRemoved;
public int activeMediaPeriods;

public MediaSourceHolder(
MediaSource mediaSource,
Expand Down

0 comments on commit 4437de4

Please sign in to comment.