From b865259e6304da7d16e044888971c454451d6154 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 14 Nov 2017 03:01:58 -0800 Subject: [PATCH] Forward ad group and ad index when creating period from concatanted media sources. Also added tests which verify the intended behaviour. GitHub:#3452 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=175656478 --- .../source/ConcatenatingMediaSourceTest.java | 26 ++++++++ .../DynamicConcatenatingMediaSourceTest.java | 63 +++++++++++-------- .../source/ConcatenatingMediaSource.java | 4 +- .../DynamicConcatenatingMediaSource.java | 3 +- .../exoplayer2/source/LoopingMediaSource.java | 3 +- .../exoplayer2/testutil/FakeMediaSource.java | 21 +++++-- .../exoplayer2/testutil/FakeTimeline.java | 35 ++++++++++- .../exoplayer2/testutil/TimelineAsserts.java | 61 +++++++++++++++++- 8 files changed, 180 insertions(+), 36 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java index 53111e83ac..6f6556225e 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.source; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Player; import com.google.android.exoplayer2.Timeline; +import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; import com.google.android.exoplayer2.testutil.FakeMediaSource; import com.google.android.exoplayer2.testutil.FakeShuffleOrder; import com.google.android.exoplayer2.testutil.FakeTimeline; @@ -196,6 +197,31 @@ public final class ConcatenatingMediaSourceTest extends TestCase { } } + public void testPeriodCreationWithAds() throws InterruptedException { + // Create media source with ad child source. + Timeline timelineContentOnly = new FakeTimeline( + new TimelineWindowDefinition(2, 111, true, false, 10 * C.MICROS_PER_SECOND)); + Timeline timelineWithAds = new FakeTimeline( + new TimelineWindowDefinition(2, 222, true, false, 10 * C.MICROS_PER_SECOND, 1, 1)); + FakeMediaSource mediaSourceContentOnly = new FakeMediaSource(timelineContentOnly, null); + FakeMediaSource mediaSourceWithAds = new FakeMediaSource(timelineWithAds, null); + ConcatenatingMediaSource mediaSource = new ConcatenatingMediaSource(mediaSourceContentOnly, + mediaSourceWithAds); + + // Prepare and assert timeline contains ad groups. + Timeline timeline = TestUtil.extractTimelineFromMediaSource(mediaSource); + TimelineAsserts.assertAdGroupCounts(timeline, 0, 0, 1, 1); + + // Create all periods and assert period creation of child media sources has been called. + TimelineAsserts.assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline, 10_000); + mediaSourceContentOnly.assertMediaPeriodCreated(new MediaPeriodId(0)); + mediaSourceContentOnly.assertMediaPeriodCreated(new MediaPeriodId(1)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(0)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(1)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(0, 0, 0)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(1, 0, 0)); + } + /** * Wraps the specified timelines in a {@link ConcatenatingMediaSource} and returns * the concatenated timeline. 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 c0c5252751..e506d0a4b3 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 @@ -154,7 +154,8 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase { assertEquals(0, timeline.getLastWindowIndex(true)); // Assert all periods can be prepared. - assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline.getPeriodCount()); + TimelineAsserts.assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline, + TIMEOUT_MS); // Remove at front of queue. mediaSource.removeMediaSource(0); @@ -205,7 +206,8 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase { TimelineAsserts.assertPreviousWindowIndices(timeline, Player.REPEAT_MODE_OFF, true, 1, 2, C.INDEX_UNSET); - assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline.getPeriodCount()); + TimelineAsserts.assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline, + TIMEOUT_MS); mediaSource.releaseSource(); for (int i = 1; i < 4; i++) { childSources[i].assertReleased(); @@ -239,7 +241,8 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase { TimelineAsserts.assertPeriodCounts(timeline, 1, 9); TimelineAsserts.assertWindowIds(timeline, 111, 999); TimelineAsserts.assertWindowIsDynamic(timeline, false, false); - assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline.getPeriodCount()); + TimelineAsserts.assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline, + TIMEOUT_MS); //Add lazy sources after preparation (and also try to prepare media period from lazy source). mediaSource.addMediaSource(1, lazySources[2]); @@ -335,7 +338,8 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase { assertEquals(2, timeline.getLastWindowIndex(false)); assertEquals(2, timeline.getFirstWindowIndex(true)); assertEquals(0, timeline.getLastWindowIndex(true)); - assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline.getPeriodCount()); + TimelineAsserts.assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline, + TIMEOUT_MS); } public void testIllegalArguments() { @@ -533,6 +537,35 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase { waitForCustomRunnable(); } + public void testPeriodCreationWithAds() throws InterruptedException { + // Create dynamic media source with ad child source. + Timeline timelineContentOnly = new FakeTimeline( + new TimelineWindowDefinition(2, 111, true, false, 10 * C.MICROS_PER_SECOND)); + Timeline timelineWithAds = new FakeTimeline( + new TimelineWindowDefinition(2, 222, true, false, 10 * C.MICROS_PER_SECOND, 1, 1)); + FakeMediaSource mediaSourceContentOnly = new FakeMediaSource(timelineContentOnly, null); + FakeMediaSource mediaSourceWithAds = new FakeMediaSource(timelineWithAds, null); + DynamicConcatenatingMediaSource mediaSource = new DynamicConcatenatingMediaSource(); + mediaSource.addMediaSource(mediaSourceContentOnly); + mediaSource.addMediaSource(mediaSourceWithAds); + assertNull(timeline); + + // Prepare and assert timeline contains ad groups. + prepareAndListenToTimelineUpdates(mediaSource); + waitForTimelineUpdate(); + TimelineAsserts.assertAdGroupCounts(timeline, 0, 0, 1, 1); + + // Create all periods and assert period creation of child media sources has been called. + TimelineAsserts.assertAllPeriodsCanBeCreatedPreparedAndReleased(mediaSource, timeline, + TIMEOUT_MS); + mediaSourceContentOnly.assertMediaPeriodCreated(new MediaPeriodId(0)); + mediaSourceContentOnly.assertMediaPeriodCreated(new MediaPeriodId(1)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(0)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(1)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(0, 0, 0)); + mediaSourceWithAds.assertMediaPeriodCreated(new MediaPeriodId(1, 0, 0)); + } + private DynamicConcatenatingMediaSourceAndHandler setUpDynamicMediaSourceOnHandlerThread() throws InterruptedException { HandlerThread handlerThread = new HandlerThread("TestCustomCallbackExecutionThread"); @@ -616,28 +649,6 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase { return new FakeTimeline(new TimelineWindowDefinition(index + 1, (index + 1) * 111)); } - private static void assertAllPeriodsCanBeCreatedPreparedAndReleased(MediaSource mediaSource, - int periodCount) { - for (int i = 0; i < periodCount; i++) { - MediaPeriod mediaPeriod = mediaSource.createPeriod(new MediaPeriodId(i), null); - assertNotNull(mediaPeriod); - final ConditionVariable mediaPeriodPrepared = new ConditionVariable(); - mediaPeriod.prepare(new Callback() { - @Override - public void onPrepared(MediaPeriod mediaPeriod) { - mediaPeriodPrepared.open(); - } - @Override - public void onContinueLoadingRequested(MediaPeriod source) {} - }, 0); - assertTrue(mediaPeriodPrepared.block(TIMEOUT_MS)); - MediaPeriod secondMediaPeriod = mediaSource.createPeriod(new MediaPeriodId(i), null); - assertNotNull(secondMediaPeriod); - mediaSource.releasePeriod(secondMediaPeriod); - mediaSource.releasePeriod(mediaPeriod); - } - } - private static class DynamicConcatenatingMediaSourceAndHandler { public final DynamicConcatenatingMediaSource mediaSource; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java index fe8f23c4c0..058471f31f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ConcatenatingMediaSource.java @@ -120,8 +120,8 @@ public final class ConcatenatingMediaSource implements MediaSource { @Override public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { int sourceIndex = timeline.getChildIndexByPeriodIndex(id.periodIndex); - MediaPeriodId periodIdInSource = - new MediaPeriodId(id.periodIndex - timeline.getFirstPeriodIndexByChildIndex(sourceIndex)); + MediaPeriodId periodIdInSource = id.copyWithPeriodIndex( + id.periodIndex - timeline.getFirstPeriodIndexByChildIndex(sourceIndex)); MediaPeriod mediaPeriod = mediaSources[sourceIndex].createPeriod(periodIdInSource, allocator); sourceIndexByMediaPeriod.put(mediaPeriod, sourceIndex); return mediaPeriod; 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 6bfa4047a5..e80abad3ef 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 @@ -341,7 +341,8 @@ public final class DynamicConcatenatingMediaSource implements MediaSource, ExoPl public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { int mediaSourceHolderIndex = findMediaSourceHolderByPeriodIndex(id.periodIndex); MediaSourceHolder holder = mediaSourceHolders.get(mediaSourceHolderIndex); - MediaPeriodId idInSource = new MediaPeriodId(id.periodIndex - holder.firstPeriodIndexInChild); + MediaPeriodId idInSource = id.copyWithPeriodIndex( + id.periodIndex - holder.firstPeriodIndexInChild); MediaPeriod mediaPeriod; if (!holder.isPrepared) { mediaPeriod = new DeferredMediaPeriod(holder.mediaSource, idInSource, allocator); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java index 583c1ed68c..984820cc6a 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/LoopingMediaSource.java @@ -80,7 +80,8 @@ public final class LoopingMediaSource implements MediaSource { @Override public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator) { return loopCount != Integer.MAX_VALUE - ? childSource.createPeriod(new MediaPeriodId(id.periodIndex % childPeriodCount), allocator) + ? childSource.createPeriod(id.copyWithPeriodIndex(id.periodIndex % childPeriodCount), + allocator) : childSource.createPeriod(id, allocator); } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java index cef48285ca..1f2524110a 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaSource.java @@ -38,6 +38,7 @@ public class FakeMediaSource implements MediaSource { private final Object manifest; private final TrackGroupArray trackGroupArray; private final ArrayList activeMediaPeriods; + private final ArrayList createdMediaPeriods; private boolean preparedSource; private boolean releasedSource; @@ -58,13 +59,10 @@ public class FakeMediaSource implements MediaSource { this.timeline = timeline; this.manifest = manifest; this.activeMediaPeriods = new ArrayList<>(); + this.createdMediaPeriods = new ArrayList<>(); this.trackGroupArray = trackGroupArray; } - public void assertReleased() { - Assert.assertTrue(releasedSource); - } - @Override public void prepareSource(ExoPlayer player, boolean isTopLevelSource, Listener listener) { Assert.assertFalse(preparedSource); @@ -84,6 +82,7 @@ public class FakeMediaSource implements MediaSource { Assert.assertFalse(releasedSource); FakeMediaPeriod mediaPeriod = createFakeMediaPeriod(id, trackGroupArray, allocator); activeMediaPeriods.add(mediaPeriod); + createdMediaPeriods.add(id); return mediaPeriod; } @@ -104,6 +103,20 @@ public class FakeMediaSource implements MediaSource { releasedSource = true; } + /** + * Assert that the source and all periods have been released. + */ + public void assertReleased() { + Assert.assertTrue(releasedSource); + } + + /** + * Assert that a media period for the given id has been created. + */ + public void assertMediaPeriodCreated(MediaPeriodId mediaPeriodId) { + Assert.assertTrue(createdMediaPeriods.contains(mediaPeriodId)); + } + protected FakeMediaPeriod createFakeMediaPeriod(MediaPeriodId id, TrackGroupArray trackGroupArray, Allocator allocator) { return new FakeMediaPeriod(trackGroupArray); diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeTimeline.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeTimeline.java index 040782264b..2937ee2770 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeTimeline.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeTimeline.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.testutil; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Timeline; import com.google.android.exoplayer2.util.Util; +import java.util.Arrays; /** * Fake {@link Timeline} which can be setup to return custom {@link TimelineWindowDefinition}s. @@ -36,6 +37,8 @@ public final class FakeTimeline extends Timeline { public final boolean isSeekable; public final boolean isDynamic; public final long durationUs; + public final int adGroupsPerPeriodCount; + public final int adsPerAdGroupCount; public TimelineWindowDefinition(int periodCount, Object id) { this(periodCount, id, true, false, WINDOW_DURATION_US); @@ -47,15 +50,24 @@ public final class FakeTimeline extends Timeline { public TimelineWindowDefinition(int periodCount, Object id, boolean isSeekable, boolean isDynamic, long durationUs) { + this(periodCount, id, isSeekable, isDynamic, durationUs, 0, 0); + } + + public TimelineWindowDefinition(int periodCount, Object id, boolean isSeekable, + boolean isDynamic, long durationUs, int adGroupsCountPerPeriod, int adsPerAdGroupCount) { this.periodCount = periodCount; this.id = id; this.isSeekable = isSeekable; this.isDynamic = isDynamic; this.durationUs = durationUs; + this.adGroupsPerPeriodCount = adGroupsCountPerPeriod; + this.adsPerAdGroupCount = adsPerAdGroupCount; } } + private static final long AD_DURATION_US = 10 * C.MICROS_PER_SECOND; + private final TimelineWindowDefinition[] windowDefinitions; private final int[] periodOffsets; @@ -96,7 +108,28 @@ public final class FakeTimeline extends Timeline { Object id = setIds ? windowPeriodIndex : null; Object uid = setIds ? periodIndex : null; long periodDurationUs = windowDefinition.durationUs / windowDefinition.periodCount; - return period.set(id, uid, windowIndex, periodDurationUs, periodDurationUs * windowPeriodIndex); + long positionInWindowUs = periodDurationUs * windowPeriodIndex; + if (windowDefinition.adGroupsPerPeriodCount == 0) { + return period.set(id, uid, windowIndex, periodDurationUs, positionInWindowUs); + } else { + int adGroups = windowDefinition.adGroupsPerPeriodCount; + long[] adGroupTimesUs = new long[adGroups]; + int[] adCounts = new int[adGroups]; + int[] adLoadedAndPlayedCounts = new int[adGroups]; + long[][] adDurationsUs = new long[adGroups][]; + long adResumePositionUs = 0; + long adGroupOffset = adGroups > 1 ? periodDurationUs / (adGroups - 1) : 0; + for (int i = 0; i < adGroups; i++) { + adGroupTimesUs[i] = i * adGroupOffset; + adCounts[i] = windowDefinition.adsPerAdGroupCount; + adLoadedAndPlayedCounts[i] = 0; + adDurationsUs[i] = new long[adCounts[i]]; + Arrays.fill(adDurationsUs[i], AD_DURATION_US); + } + return period.set(id, uid, windowIndex, periodDurationUs, positionInWindowUs, adGroupTimesUs, + adCounts, adLoadedAndPlayedCounts, adLoadedAndPlayedCounts, adDurationsUs, + adResumePositionUs); + } } @Override diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TimelineAsserts.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TimelineAsserts.java index c61aac708c..b1df8f62e1 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TimelineAsserts.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TimelineAsserts.java @@ -16,12 +16,19 @@ package com.google.android.exoplayer2.testutil; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertTrue; +import android.os.ConditionVariable; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Player; import com.google.android.exoplayer2.Timeline; import com.google.android.exoplayer2.Timeline.Period; import com.google.android.exoplayer2.Timeline.Window; +import com.google.android.exoplayer2.source.MediaPeriod; +import com.google.android.exoplayer2.source.MediaPeriod.Callback; +import com.google.android.exoplayer2.source.MediaSource; +import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; /** * Unit test for {@link Timeline}. @@ -60,7 +67,7 @@ public final class TimelineAsserts { } /** - * Asserts that window properties {@link Window}.isDynamic are set correctly.. + * Asserts that window properties {@link Window}.isDynamic are set correctly. */ public static void assertWindowIsDynamic(Timeline timeline, boolean... windowIsDynamic) { Window window = new Window(); @@ -139,5 +146,57 @@ public final class TimelineAsserts { } } + /** + * Asserts that periods' {@link Period#getAdGroupCount()} are set correctly. + */ + public static void assertAdGroupCounts(Timeline timeline, int... expectedAdGroupCounts) { + Period period = new Period(); + for (int i = 0; i < timeline.getPeriodCount(); i++) { + timeline.getPeriod(i, period); + assertEquals(expectedAdGroupCounts[i], period.getAdGroupCount()); + } + } + + /** + * Asserts that all period (including ad periods) can be created from the source, prepared, and + * released without exception and within timeout. + */ + public static void assertAllPeriodsCanBeCreatedPreparedAndReleased(MediaSource mediaSource, + Timeline timeline, long timeoutMs) { + Period period = new Period(); + for (int i = 0; i < timeline.getPeriodCount(); i++) { + assertPeriodCanBeCreatedPreparedAndReleased(mediaSource, new MediaPeriodId(i), timeoutMs); + timeline.getPeriod(i, period); + for (int adGroupIndex = 0; adGroupIndex < period.getAdGroupCount(); adGroupIndex++) { + for (int adIndex = 0; adIndex < period.getAdCountInAdGroup(adGroupIndex); adIndex++) { + assertPeriodCanBeCreatedPreparedAndReleased(mediaSource, + new MediaPeriodId(i, adGroupIndex, adIndex), timeoutMs); + } + } + } + } + + private static void assertPeriodCanBeCreatedPreparedAndReleased(MediaSource mediaSource, + MediaPeriodId mediaPeriodId, long timeoutMs) { + MediaPeriod mediaPeriod = mediaSource.createPeriod(mediaPeriodId, null); + assertNotNull(mediaPeriod); + final ConditionVariable mediaPeriodPrepared = new ConditionVariable(); + mediaPeriod.prepare(new Callback() { + @Override + public void onPrepared(MediaPeriod mediaPeriod) { + mediaPeriodPrepared.open(); + } + @Override + public void onContinueLoadingRequested(MediaPeriod source) {} + }, /* positionUs= */ 0); + assertTrue(mediaPeriodPrepared.block(timeoutMs)); + // MediaSource is supposed to support multiple calls to createPeriod with the same id without an + // intervening call to releasePeriod. + MediaPeriod secondMediaPeriod = mediaSource.createPeriod(mediaPeriodId, null); + assertNotNull(secondMediaPeriod); + mediaSource.releasePeriod(secondMediaPeriod); + mediaSource.releasePeriod(mediaPeriod); + } + }