Fix event reporting for merging and looping media sources.

The looping media source doesn't convert the media period id to the externally
visible media period id. And the merging media source reports media period
creations multiple times which will break listeners assuming a media period
with a specific id will only be created once.

Also amend the doc for MediaSource.createPeriod to reflect that media periods
created in parallel do not actually have the same id.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=206327241
This commit is contained in:
tonihei 2018-07-27 09:25:27 -07:00 committed by Oliver Woodman
parent a237ae1810
commit 656c2172dd
6 changed files with 104 additions and 16 deletions

View file

@ -24,6 +24,8 @@ import com.google.android.exoplayer2.source.ShuffleOrder.UnshuffledShuffleOrder;
import com.google.android.exoplayer2.upstream.Allocator;
import com.google.android.exoplayer2.upstream.TransferListener;
import com.google.android.exoplayer2.util.Assertions;
import java.util.HashMap;
import java.util.Map;
/**
* Loops a {@link MediaSource} a specified number of times.
@ -35,6 +37,8 @@ public final class LoopingMediaSource extends CompositeMediaSource<Void> {
private final MediaSource childSource;
private final int loopCount;
private final Map<MediaPeriodId, MediaPeriodId> childMediaPeriodIdToMediaPeriodId;
private final Map<MediaPeriod, MediaPeriodId> mediaPeriodToChildMediaPeriodId;
private int childPeriodCount;
@ -58,6 +62,8 @@ public final class LoopingMediaSource extends CompositeMediaSource<Void> {
Assertions.checkArgument(loopCount > 0);
this.childSource = childSource;
this.loopCount = loopCount;
childMediaPeriodIdToMediaPeriodId = new HashMap<>();
mediaPeriodToChildMediaPeriodId = new HashMap<>();
}
@Override
@ -71,15 +77,23 @@ public final class LoopingMediaSource extends CompositeMediaSource<Void> {
@Override
public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) {
return loopCount != Integer.MAX_VALUE
? childSource.createPeriod(id.copyWithPeriodIndex(id.periodIndex % childPeriodCount),
allocator)
: childSource.createPeriod(id, allocator);
if (loopCount == Integer.MAX_VALUE) {
return childSource.createPeriod(id, allocator);
}
MediaPeriodId childMediaPeriodId = id.copyWithPeriodIndex(id.periodIndex % childPeriodCount);
childMediaPeriodIdToMediaPeriodId.put(childMediaPeriodId, id);
MediaPeriod mediaPeriod = childSource.createPeriod(childMediaPeriodId, allocator);
mediaPeriodToChildMediaPeriodId.put(mediaPeriod, childMediaPeriodId);
return mediaPeriod;
}
@Override
public void releasePeriod(MediaPeriod mediaPeriod) {
childSource.releasePeriod(mediaPeriod);
MediaPeriodId childMediaPeriodId = mediaPeriodToChildMediaPeriodId.remove(mediaPeriod);
if (childMediaPeriodId != null) {
childMediaPeriodIdToMediaPeriodId.remove(childMediaPeriodId);
}
}
@Override
@ -99,6 +113,14 @@ public final class LoopingMediaSource extends CompositeMediaSource<Void> {
refreshSourceInfo(loopingTimeline, manifest);
}
@Override
protected @Nullable MediaPeriodId getMediaPeriodIdForChildMediaPeriodId(
Void id, MediaPeriodId mediaPeriodId) {
return loopCount != Integer.MAX_VALUE
? childMediaPeriodIdToMediaPeriodId.get(mediaPeriodId)
: mediaPeriodId;
}
private static final class LoopingTimeline extends AbstractConcatenatedTimeline {
private final Timeline childTimeline;

View file

@ -267,10 +267,9 @@ public interface MediaSource {
/**
* Returns a new {@link MediaPeriod} identified by {@code periodId}. This method may be called
* multiple times with the same period identifier without an intervening call to
* {@link #releasePeriod(MediaPeriod)}.
* <p>
* Should not be called directly from application code.
* multiple times without an intervening call to {@link #releasePeriod(MediaPeriod)}.
*
* <p>Should not be called directly from application code.
*
* @param id The identifier of the period.
* @param allocator An {@link Allocator} from which to obtain media buffer allocations.

View file

@ -163,6 +163,12 @@ public final class MergingMediaSource extends CompositeMediaSource<Integer> {
}
}
@Override
protected @Nullable MediaPeriodId getMediaPeriodIdForChildMediaPeriodId(
Integer id, MediaPeriodId mediaPeriodId) {
return id == 0 ? mediaPeriodId : null;
}
private IllegalMergeException checkTimelineMerges(Timeline timeline) {
if (periodCount == PERIOD_COUNT_UNSET) {
periodCount = timeline.getPeriodCount();

View file

@ -48,7 +48,7 @@ public class LoopingMediaSourceTest {
}
@Test
public void testSingleLoop() throws IOException {
public void testSingleLoopTimeline() throws IOException {
Timeline timeline = getLoopingTimeline(multiWindowTimeline, 1);
TimelineAsserts.assertWindowTags(timeline, 111, 222, 333);
TimelineAsserts.assertPeriodCounts(timeline, 1, 1, 1);
@ -67,7 +67,7 @@ public class LoopingMediaSourceTest {
}
@Test
public void testMultiLoop() throws IOException {
public void testMultiLoopTimeline() throws IOException {
Timeline timeline = getLoopingTimeline(multiWindowTimeline, 3);
TimelineAsserts.assertWindowTags(timeline, 111, 222, 333, 111, 222, 333, 111, 222, 333);
TimelineAsserts.assertPeriodCounts(timeline, 1, 1, 1, 1, 1, 1, 1, 1, 1);
@ -88,7 +88,7 @@ public class LoopingMediaSourceTest {
}
@Test
public void testInfiniteLoop() throws IOException {
public void testInfiniteLoopTimeline() throws IOException {
Timeline timeline = getLoopingTimeline(multiWindowTimeline, Integer.MAX_VALUE);
TimelineAsserts.assertWindowTags(timeline, 111, 222, 333);
TimelineAsserts.assertPeriodCounts(timeline, 1, 1, 1);
@ -117,6 +117,21 @@ public class LoopingMediaSourceTest {
TimelineAsserts.assertEmpty(timeline);
}
@Test
public void testSingleLoopPeriodCreation() throws Exception {
testMediaPeriodCreation(multiWindowTimeline, /* loopCount= */ 1);
}
@Test
public void testMultiLoopPeriodCreation() throws Exception {
testMediaPeriodCreation(multiWindowTimeline, /* loopCount= */ 3);
}
@Test
public void testInfiniteLoopPeriodCreation() throws Exception {
testMediaPeriodCreation(multiWindowTimeline, /* loopCount= */ Integer.MAX_VALUE);
}
/**
* Wraps the specified timeline in a {@link LoopingMediaSource} and returns the looping timeline.
*/
@ -133,4 +148,21 @@ public class LoopingMediaSourceTest {
testRunner.release();
}
}
/**
* Wraps the specified timeline in a {@link LoopingMediaSource} and asserts that all periods of
* the looping timeline can be created and prepared.
*/
private static void testMediaPeriodCreation(Timeline timeline, int loopCount) throws Exception {
FakeMediaSource fakeMediaSource = new FakeMediaSource(timeline, null);
LoopingMediaSource mediaSource = new LoopingMediaSource(fakeMediaSource, loopCount);
MediaSourceTestRunner testRunner = new MediaSourceTestRunner(mediaSource, null);
try {
testRunner.prepareSource();
testRunner.assertPrepareAndReleaseAllPeriods();
testRunner.releaseSource();
} finally {
testRunner.release();
}
}
}

View file

@ -65,6 +65,27 @@ public class MergingMediaSourceTest {
}
}
@Test
public void testMergingMediaSourcePeriodCreation() throws Exception {
FakeMediaSource[] mediaSources = new FakeMediaSource[2];
for (int i = 0; i < mediaSources.length; i++) {
mediaSources[i] =
new FakeMediaSource(new FakeTimeline(/* windowCount= */ 2), /* manifest= */ null);
}
MergingMediaSource mediaSource = new MergingMediaSource(mediaSources);
MediaSourceTestRunner testRunner = new MediaSourceTestRunner(mediaSource, null);
try {
testRunner.prepareSource();
testRunner.assertPrepareAndReleaseAllPeriods();
for (FakeMediaSource element : mediaSources) {
assertThat(element.getCreatedMediaPeriods()).isNotEmpty();
}
testRunner.releaseSource();
} finally {
testRunner.release();
}
}
/**
* Wraps the specified timelines in a {@link MergingMediaSource}, prepares it and checks that it
* forwards the first of the wrapped timelines.

View file

@ -295,17 +295,25 @@ public class MediaSourceTestRunner {
assertThat(lastCreatedMediaPeriod.getAndSet(/* newValue= */ null)).isEqualTo(mediaPeriodId);
CountDownLatch preparedCondition = preparePeriod(mediaPeriod, 0);
assertThat(preparedCondition.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)).isTrue();
// MediaSource is supposed to support multiple calls to createPeriod with the same id without an
// intervening call to releasePeriod.
MediaPeriod secondMediaPeriod = createPeriod(mediaPeriodId);
assertThat(lastCreatedMediaPeriod.getAndSet(/* newValue= */ null)).isEqualTo(mediaPeriodId);
// MediaSource is supposed to support multiple calls to createPeriod without an intervening call
// to releasePeriod.
MediaPeriodId secondMediaPeriodId =
new MediaPeriodId(
mediaPeriodId.periodIndex,
mediaPeriodId.adGroupIndex,
mediaPeriodId.adIndexInAdGroup,
mediaPeriodId.windowSequenceNumber + 1000);
MediaPeriod secondMediaPeriod = createPeriod(secondMediaPeriodId);
assertThat(lastCreatedMediaPeriod.getAndSet(/* newValue= */ null))
.isEqualTo(secondMediaPeriodId);
CountDownLatch secondPreparedCondition = preparePeriod(secondMediaPeriod, 0);
assertThat(secondPreparedCondition.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)).isTrue();
// Release the periods.
releasePeriod(mediaPeriod);
assertThat(lastReleasedMediaPeriod.getAndSet(/* newValue= */ null)).isEqualTo(mediaPeriodId);
releasePeriod(secondMediaPeriod);
assertThat(lastReleasedMediaPeriod.getAndSet(/* newValue= */ null)).isEqualTo(mediaPeriodId);
assertThat(lastReleasedMediaPeriod.getAndSet(/* newValue= */ null))
.isEqualTo(secondMediaPeriodId);
}
/**