From 286977998da55fff2f03a19a1733f4b535c78bd2 Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 31 Jan 2018 04:51:23 -0800 Subject: [PATCH] Fix bug in DynamicConcatenatingMediaSource at dynamic changes of empty timelines. When the dynamic media source contains multiple empty timelines in a row and some of them dynamically change to a non-empty timeline, the window and period indices are not updated correctly because the index of the changed child source is wrong. To fix this bug, the child index is added to the media period holder to have direct access on the current child index to prevent ambiguity. Furthermore, the uid is changed to be the hash code of the MediaSourceHolder not the MediaSource itself to allow adding the same MediaSource twice without violating the unique uid policy. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=183973462 --- .../DynamicConcatenatingMediaSourceTest.java | 26 +++++++++ .../DynamicConcatenatingMediaSource.java | 53 +++++++++++++------ 2 files changed, 63 insertions(+), 16 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 cb831deb57..af4b149c98 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 @@ -344,6 +344,32 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase { testRunner.assertPrepareAndReleaseAllPeriods(); } + public void testDynamicChangeOfEmptyTimelines() throws IOException { + FakeMediaSource[] childSources = + new FakeMediaSource[] { + new FakeMediaSource(Timeline.EMPTY, /* manifest= */ null), + new FakeMediaSource(Timeline.EMPTY, /* manifest= */ null), + new FakeMediaSource(Timeline.EMPTY, /* manifest= */ null), + }; + Timeline nonEmptyTimeline = new FakeTimeline(/* windowCount = */ 1); + + mediaSource.addMediaSources(Arrays.asList(childSources)); + Timeline timeline = testRunner.prepareSource(); + TimelineAsserts.assertEmpty(timeline); + + childSources[0].setNewSourceInfo(nonEmptyTimeline, /* newManifest== */ null); + timeline = testRunner.assertTimelineChangeBlocking(); + TimelineAsserts.assertPeriodCounts(timeline, 1); + + childSources[2].setNewSourceInfo(nonEmptyTimeline, /* newManifest== */ null); + timeline = testRunner.assertTimelineChangeBlocking(); + TimelineAsserts.assertPeriodCounts(timeline, 1, 1); + + childSources[1].setNewSourceInfo(nonEmptyTimeline, /* newManifest== */ null); + timeline = testRunner.assertTimelineChangeBlocking(); + TimelineAsserts.assertPeriodCounts(timeline, 1, 1, 1); + } + public void testIllegalArguments() { MediaSource validSource = new FakeMediaSource(createFakeTimeline(1), null); 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 5928010620..a638992501 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 @@ -453,18 +453,24 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource< private void addMediaSourceInternal(int newIndex, MediaSource newMediaSource) { final MediaSourceHolder newMediaSourceHolder; - Object newUid = System.identityHashCode(newMediaSource); DeferredTimeline newTimeline = new DeferredTimeline(); if (newIndex > 0) { MediaSourceHolder previousHolder = mediaSourceHolders.get(newIndex - 1); - newMediaSourceHolder = new MediaSourceHolder(newMediaSource, newTimeline, - previousHolder.firstWindowIndexInChild + previousHolder.timeline.getWindowCount(), - previousHolder.firstPeriodIndexInChild + previousHolder.timeline.getPeriodCount(), - newUid); + newMediaSourceHolder = + new MediaSourceHolder( + newMediaSource, + newTimeline, + newIndex, + previousHolder.firstWindowIndexInChild + previousHolder.timeline.getWindowCount(), + previousHolder.firstPeriodIndexInChild + previousHolder.timeline.getPeriodCount()); } else { - newMediaSourceHolder = new MediaSourceHolder(newMediaSource, newTimeline, 0, 0, newUid); + newMediaSourceHolder = new MediaSourceHolder(newMediaSource, newTimeline, 0, 0, 0); } - correctOffsets(newIndex, newTimeline.getWindowCount(), newTimeline.getPeriodCount()); + correctOffsets( + newIndex, + /* childIndexUpdate= */ 1, + newTimeline.getWindowCount(), + newTimeline.getPeriodCount()); mediaSourceHolders.add(newIndex, newMediaSourceHolder); prepareChildSource(newMediaSourceHolder, newMediaSourceHolder.mediaSource); } @@ -486,8 +492,11 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource< int windowOffsetUpdate = timeline.getWindowCount() - deferredTimeline.getWindowCount(); int periodOffsetUpdate = timeline.getPeriodCount() - deferredTimeline.getPeriodCount(); if (windowOffsetUpdate != 0 || periodOffsetUpdate != 0) { - int index = findMediaSourceHolderByPeriodIndex(mediaSourceHolder.firstPeriodIndexInChild); - correctOffsets(index + 1, windowOffsetUpdate, periodOffsetUpdate); + correctOffsets( + mediaSourceHolder.childIndex + 1, + /* childIndexUpdate= */ 0, + windowOffsetUpdate, + periodOffsetUpdate); } mediaSourceHolder.timeline = deferredTimeline.cloneWithNewTimeline(timeline); if (!mediaSourceHolder.isPrepared) { @@ -506,7 +515,11 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource< MediaSourceHolder holder = mediaSourceHolders.get(index); mediaSourceHolders.remove(index); Timeline oldTimeline = holder.timeline; - correctOffsets(index, -oldTimeline.getWindowCount(), -oldTimeline.getPeriodCount()); + correctOffsets( + index, + /* childIndexUpdate= */ -1, + -oldTimeline.getWindowCount(), + -oldTimeline.getPeriodCount()); releaseChildSource(holder); } @@ -525,10 +538,12 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource< } } - private void correctOffsets(int startIndex, int windowOffsetUpdate, int periodOffsetUpdate) { + private void correctOffsets( + int startIndex, int childIndexUpdate, int windowOffsetUpdate, int periodOffsetUpdate) { windowCount += windowOffsetUpdate; periodCount += periodOffsetUpdate; for (int i = startIndex; i < mediaSourceHolders.size(); i++) { + mediaSourceHolders.get(i).childIndex += childIndexUpdate; mediaSourceHolders.get(i).firstWindowIndexInChild += windowOffsetUpdate; mediaSourceHolders.get(i).firstPeriodIndexInChild += periodOffsetUpdate; } @@ -551,20 +566,26 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource< /* package */ static final class MediaSourceHolder implements Comparable { public final MediaSource mediaSource; - public final Object uid; + public final int uid; public DeferredTimeline timeline; + public int childIndex; public int firstWindowIndexInChild; public int firstPeriodIndexInChild; public boolean isPrepared; - public MediaSourceHolder(MediaSource mediaSource, DeferredTimeline timeline, int window, - int period, Object uid) { + public MediaSourceHolder( + MediaSource mediaSource, + DeferredTimeline timeline, + int childIndex, + int window, + int period) { this.mediaSource = mediaSource; this.timeline = timeline; + this.childIndex = childIndex; this.firstWindowIndexInChild = window; this.firstPeriodIndexInChild = period; - this.uid = uid; + this.uid = System.identityHashCode(this); } @Override @@ -638,7 +659,7 @@ public final class DynamicConcatenatingMediaSource extends CompositeMediaSource< timelines[index] = mediaSourceHolder.timeline; firstPeriodInChildIndices[index] = mediaSourceHolder.firstPeriodIndexInChild; firstWindowInChildIndices[index] = mediaSourceHolder.firstWindowIndexInChild; - uids[index] = (int) mediaSourceHolder.uid; + uids[index] = mediaSourceHolder.uid; childIndexByUid.put(uids[index], index++); } } -- GitLab