From c9245c61dedd891cc659f293cb68c9dbcfccd7da Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 5 Feb 2020 12:12:28 +0000 Subject: [PATCH] Fix order of timeline and prepare callback in MaskingMediaSoure Once we receive an update from a masked source, we first start the preparation of an already pending period, and only then notify the player of the new timeline. If the period prepares immediately inline, the MediaPeriod.onPrepared callback arrives before the onPlaylistUpdateRequested call in the Player. THis is the wrong order and causes issues when the player tries to lookup information in the timeline that doesn't exist yet. This change fixes preroll playbacks before live streams. PiperOrigin-RevId: 293340031 --- .../exoplayer2/source/MaskingMediaSource.java | 8 ++- .../android/exoplayer2/ExoPlayerTest.java | 55 ++++++++++++++++++- .../exoplayer2/testutil/FakeTimeline.java | 8 ++- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java index 1049d266f9..e28c1652f2 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java @@ -141,6 +141,7 @@ public final class MaskingMediaSource extends CompositeMediaSource { @Override protected synchronized void onChildSourceInfoRefreshed( Void id, MediaSource mediaSource, Timeline newTimeline) { + @Nullable MediaPeriodId idForMaskingPeriodPreparation = null; if (isPrepared) { timeline = timeline.cloneWithUpdatedTimeline(newTimeline); } else if (newTimeline.isEmpty()) { @@ -183,14 +184,17 @@ public final class MaskingMediaSource extends CompositeMediaSource { if (unpreparedMaskingMediaPeriod != null) { MaskingMediaPeriod maskingPeriod = unpreparedMaskingMediaPeriod; maskingPeriod.overridePreparePositionUs(periodPositionUs); - MediaPeriodId idInSource = + idForMaskingPeriodPreparation = maskingPeriod.id.copyWithPeriodUid(getInternalPeriodUid(maskingPeriod.id.periodUid)); - maskingPeriod.createPeriod(idInSource); } } hasRealTimeline = true; isPrepared = true; refreshSourceInfo(this.timeline); + if (idForMaskingPeriodPreparation != null) { + Assertions.checkNotNull(unpreparedMaskingMediaPeriod) + .createPeriod(idForMaskingPeriodPreparation); + } } @Nullable diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index ca772f85e2..602cff4391 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -2988,7 +2988,7 @@ public final class ExoPlayerTest { /* isDynamic= */ false, /* durationUs= */ 10_000_000, adPlaybackState)); - final FakeMediaSource fakeMediaSource = new FakeMediaSource(fakeTimeline); + FakeMediaSource fakeMediaSource = new FakeMediaSource(/* timeline= */ null); AtomicReference playerReference = new AtomicReference<>(); AtomicLong contentStartPositionMs = new AtomicLong(C.TIME_UNSET); EventListener eventListener = @@ -3011,6 +3011,59 @@ public final class ExoPlayerTest { } }) .seek(/* positionMs= */ 5_000) + .waitForPlaybackState(Player.STATE_BUFFERING) + .executeRunnable(() -> fakeMediaSource.setNewSourceInfo(fakeTimeline)) + .build(); + new ExoPlayerTestRunner.Builder() + .setMediaSources(fakeMediaSource) + .setActionSchedule(actionSchedule) + .build(context) + .start() + .blockUntilEnded(TIMEOUT_MS); + + assertThat(contentStartPositionMs.get()).isAtLeast(5_000L); + } + + @Test + public void contentWithoutInitialSeekStartsAtDefaultPositionAfterPrerollAd() throws Exception { + AdPlaybackState adPlaybackState = + FakeTimeline.createAdPlaybackState(/* adsPerAdGroup= */ 3, /* adGroupTimesUs...= */ 0); + Timeline fakeTimeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* periodCount= */ 1, + /* id= */ 0, + /* isSeekable= */ true, + /* isDynamic= */ false, + /* isLive= */ false, + /* isPlaceholder= */ false, + /* durationUs= */ 10_000_000, + /* defaultPositionUs= */ 5_000_000, + adPlaybackState)); + FakeMediaSource fakeMediaSource = new FakeMediaSource(/* timeline= */ null); + AtomicReference playerReference = new AtomicReference<>(); + AtomicLong contentStartPositionMs = new AtomicLong(C.TIME_UNSET); + EventListener eventListener = + new EventListener() { + @Override + public void onPositionDiscontinuity(@DiscontinuityReason int reason) { + if (reason == Player.DISCONTINUITY_REASON_AD_INSERTION) { + contentStartPositionMs.set(playerReference.get().getContentPosition()); + } + } + }; + ActionSchedule actionSchedule = + new ActionSchedule.Builder("contentWithoutInitialSeekStartsAtDefaultPositionAfterPrerollAd") + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + playerReference.set(player); + player.addListener(eventListener); + } + }) + .waitForPlaybackState(Player.STATE_BUFFERING) + .executeRunnable(() -> fakeMediaSource.setNewSourceInfo(fakeTimeline)) .build(); new ExoPlayerTestRunner.Builder() .setMediaSources(fakeMediaSource) 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 5b882dd9a5..628d7b191d 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 @@ -42,6 +42,7 @@ public final class FakeTimeline extends Timeline { public final boolean isLive; public final boolean isPlaceholder; public final long durationUs; + public final long defaultPositionUs; public final AdPlaybackState adPlaybackState; /** @@ -59,6 +60,7 @@ public final class FakeTimeline extends Timeline { /* isLive= */ false, /* isPlaceholder= */ true, /* durationUs= */ C.TIME_UNSET, + /* defaultPositionUs= */ 0, AdPlaybackState.NONE); } @@ -126,6 +128,7 @@ public final class FakeTimeline extends Timeline { /* isLive= */ isDynamic, /* isPlaceholder= */ false, durationUs, + /* defaultPositionUs= */ 0, adPlaybackState); } @@ -140,6 +143,7 @@ public final class FakeTimeline extends Timeline { * @param isLive Whether the window is live. * @param isPlaceholder Whether the window is a placeholder. * @param durationUs The duration of the window in microseconds. + * @param defaultPositionUs The default position of the window in microseconds. * @param adPlaybackState The ad playback state. */ public TimelineWindowDefinition( @@ -150,6 +154,7 @@ public final class FakeTimeline extends Timeline { boolean isLive, boolean isPlaceholder, long durationUs, + long defaultPositionUs, AdPlaybackState adPlaybackState) { this.periodCount = periodCount; this.id = id; @@ -158,6 +163,7 @@ public final class FakeTimeline extends Timeline { this.isLive = isLive; this.isPlaceholder = isPlaceholder; this.durationUs = durationUs; + this.defaultPositionUs = defaultPositionUs; this.adPlaybackState = adPlaybackState; } } @@ -252,7 +258,7 @@ public final class FakeTimeline extends Timeline { windowDefinition.isSeekable, windowDefinition.isDynamic, windowDefinition.isLive, - /* defaultPositionUs= */ 0, + windowDefinition.defaultPositionUs, windowDefinition.durationUs, periodOffsets[windowIndex], periodOffsets[windowIndex + 1] - 1,