Skip to content

Commit

Permalink
Fix event reporting for merging and looping media sources.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tonihei authored and ojw28 committed Aug 1, 2018
1 parent a237ae1 commit 656c217
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
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.
Expand All @@ -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;

Expand All @@ -58,6 +62,8 @@ public LoopingMediaSource(MediaSource childSource, int loopCount) {
Assertions.checkArgument(loopCount > 0);
this.childSource = childSource;
this.loopCount = loopCount;
childMediaPeriodIdToMediaPeriodId = new HashMap<>();
mediaPeriodToChildMediaPeriodId = new HashMap<>();
}

@Override
Expand All @@ -71,15 +77,23 @@ public void prepareSourceInternal(

@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
Expand All @@ -99,6 +113,14 @@ protected void onChildSourceInfoRefreshed(
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,9 @@ void prepareSource(

/**
* 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ protected void onChildSourceInfoRefreshed(
}
}

@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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void setUp() throws Exception {
}

@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);
Expand All @@ -67,7 +67,7 @@ public void testSingleLoop() throws IOException {
}

@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);
Expand All @@ -88,7 +88,7 @@ public void testMultiLoop() throws IOException {
}

@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);
Expand Down Expand Up @@ -117,6 +117,21 @@ public void testEmptyTimelineLoop() throws IOException {
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.
*/
Expand All @@ -133,4 +148,21 @@ private static Timeline getLoopingTimeline(Timeline timeline, int loopCount) thr
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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,27 @@ public void testMergingTimelinesWithDifferentPeriodCounts() throws IOException {
}
}

@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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,17 +295,25 @@ private void assertPrepareAndReleasePeriod(MediaPeriodId mediaPeriodId)
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);
}

/**
Expand Down

0 comments on commit 656c217

Please sign in to comment.