From 4437de4818c7a52ef17e0f80cfbfb8afb8571495 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 5 Feb 2018 06:48:48 -0800 Subject: [PATCH] Fix potential media source release before media period release. 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 --- .../DynamicConcatenatingMediaSourceTest.java | 12 +++++++++++ .../DynamicConcatenatingMediaSource.java | 21 +++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSourceTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSourceTest.java index 5198cde72ef..1a1c07fd61e 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSourceTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSourceTest.java @@ -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++) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSource.java index a638992501a..7ed8af59541 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSource.java @@ -56,7 +56,7 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource< // Accessed on the playback thread. private final List mediaSourceHolders; private final MediaSourceHolder query; - private final Map mediaSourceByMediaPeriod; + private final Map mediaSourceByMediaPeriod; private final List deferredMediaPeriods; private ExoPlayer player; @@ -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); } } @@ -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) { @@ -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,