From 49215950416e931d0decac36ad1724768a86df9e Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 1 Oct 2018 07:50:26 -0700 Subject: [PATCH] Fix prepare position of DeferredMediaPeriods for windows with non-zero offset. If we prepare a deferred media period before the actual timeline is available, we either prepare with position zero (= the default) or with a non-zero initial seek position. So far, the zero (default) position got replaced by the actual default position (including any potential non-zero window offset) when the timeline became known. However, a non-zero initial seek position was not corrected by the non-zero window offset. This is fixed by this change. Related to that, we always assumed that the deferred media period will the first period in the actual timeline. This is not true if we prepare with an offset (either because of an initial seek position or because of a default window position). So, we also determine the actual first period when the timeline becomes known. Issue:#4873 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=215213030 --- RELEASENOTES.md | 6 ++ .../source/ConcatenatingMediaSource.java | 89 +++++++++++++---- .../source/DeferredMediaPeriod.java | 30 +++--- .../android/exoplayer2/ExoPlayerTest.java | 98 ++++++++++++++++++- .../analytics/AnalyticsCollectorTest.java | 2 +- .../exoplayer2/testutil/ActionSchedule.java | 11 ++- 6 files changed, 195 insertions(+), 41 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index dd7c9c95a9..a74af7fb59 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,5 +1,11 @@ # Release notes # +### 2.9.1 ### + +* Fix an issue with blind seeking to windows with non-zero offset in a + `ConcatenatingMediaSource` + ([#4873](https://github.com/google/ExoPlayer/issues/4873)). + ### 2.9.0 ### * Turn on Java 8 compiler support for the ExoPlayer library. Apps may need to 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 e0b7da8506..7418e84449 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 @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.source; import android.os.Handler; import android.support.annotation.NonNull; import android.support.annotation.Nullable; +import android.util.Pair; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.ExoPlayer; @@ -65,6 +66,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource periodPosition = + timeline.getPeriodPosition(window, period, /* windowIndex= */ 0, windowStartPositionUs); + Object periodUid = periodPosition.first; + long periodPositionUs = periodPosition.second; + mediaSourceHolder.timeline = DeferredTimeline.createWithRealTimeline(timeline, periodUid); + if (deferredMediaPeriod != null) { + deferredMediaPeriod.overridePreparePositionUs(periodPositionUs); MediaPeriodId idInSource = deferredMediaPeriod.id.copyWithPeriodUid( getChildPeriodUid(mediaSourceHolder, deferredMediaPeriod.id.periodUid)); deferredMediaPeriod.createPeriod(idInSource); } - mediaSourceHolder.isPrepared = true; } + mediaSourceHolder.isPrepared = true; scheduleListenerNotification(/* actionOnCompletion= */ null); } @@ -897,18 +932,32 @@ public class ConcatenatingMediaSource extends CompositeMediaSource 0 - ? timeline.getUidOfPeriod(0) - : replacedId); + /** + * Returns a copy with an updated timeline. This keeps the existing period replacement. + * + * @param timeline The new timeline. + */ + public DeferredTimeline cloneWithUpdatedTimeline(Timeline timeline) { + return new DeferredTimeline(timeline, replacedId); } + /** Returns wrapped timeline. */ public Timeline getTimeline() { return timeline; } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java index 5f84731b8d..26c25a749e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java @@ -79,23 +79,19 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb this.listener = listener; } + /** Returns the position at which the deferred media period was prepared, in microseconds. */ + public long getPreparePositionUs() { + return preparePositionUs; + } + /** - * Sets the default prepare position at which to prepare the media period. This value is only used - * if the call to {@link MediaPeriod#prepare(Callback, long)} is being deferred and the call was - * made with a (presumably default) prepare position of 0. + * Overrides the default prepare position at which to prepare the media period. This value is only + * used if the call to {@link MediaPeriod#prepare(Callback, long)} is being deferred. * - *

Note that this will override an intentional seek to zero in the corresponding non-seekable - * timeline window. This is unlikely to be a problem as a non-zero default position usually only - * occurs for live playbacks and seeking to zero in a live window would cause - * BehindLiveWindowExceptions anyway. - * - * @param defaultPreparePositionUs The actual default prepare position, in microseconds. + * @param defaultPreparePositionUs The default prepare position to use, in microseconds. */ - public void setDefaultPreparePositionUs(long defaultPreparePositionUs) { - if (preparePositionUs == 0 && defaultPreparePositionUs != 0) { - preparePositionOverrideUs = defaultPreparePositionUs; - preparePositionUs = defaultPreparePositionUs; - } + public void overridePreparePositionUs(long defaultPreparePositionUs) { + preparePositionOverrideUs = defaultPreparePositionUs; } /** @@ -108,6 +104,10 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb public void createPeriod(MediaPeriodId id) { mediaPeriod = mediaSource.createPeriod(id, allocator); if (callback != null) { + long preparePositionUs = + preparePositionOverrideUs != C.TIME_UNSET + ? preparePositionOverrideUs + : this.preparePositionUs; mediaPeriod.prepare(this, preparePositionUs); } } @@ -157,7 +157,7 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb @Override public long selectTracks(TrackSelection[] selections, boolean[] mayRetainStreamFlags, SampleStream[] streams, boolean[] streamResetFlags, long positionUs) { - if (preparePositionOverrideUs != C.TIME_UNSET && positionUs == 0) { + if (preparePositionOverrideUs != C.TIME_UNSET && positionUs == preparePositionUs) { positionUs = preparePositionOverrideUs; preparePositionOverrideUs = C.TIME_UNSET; } 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 8414be1588..407e9a3827 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 @@ -60,6 +60,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import org.junit.Before; import org.junit.Test; @@ -1346,7 +1347,7 @@ public final class ExoPlayerTest { () -> concatenatingMediaSource.addMediaSources( Arrays.asList(mediaSource, mediaSource))) - .waitForTimelineChanged(null) + .waitForTimelineChanged() .executeRunnable( new PlayerRunnable() { @Override @@ -2192,14 +2193,14 @@ public final class ExoPlayerTest { startPositionUs + expectedDurationUs); Clock clock = new AutoAdvancingFakeClock(); AtomicReference playerReference = new AtomicReference<>(); - AtomicReference positionAtDiscontinuityMs = new AtomicReference<>(); - AtomicReference clockAtStartMs = new AtomicReference<>(); - AtomicReference clockAtDiscontinuityMs = new AtomicReference<>(); + AtomicLong positionAtDiscontinuityMs = new AtomicLong(C.TIME_UNSET); + AtomicLong clockAtStartMs = new AtomicLong(C.TIME_UNSET); + AtomicLong clockAtDiscontinuityMs = new AtomicLong(C.TIME_UNSET); EventListener eventListener = new EventListener() { @Override public void onPlayerStateChanged(boolean playWhenReady, int playbackState) { - if (playbackState == Player.STATE_READY && clockAtStartMs.get() == null) { + if (playbackState == Player.STATE_READY && clockAtStartMs.get() == C.TIME_UNSET) { clockAtStartMs.set(clock.elapsedRealtime()); } } @@ -2446,6 +2447,93 @@ public final class ExoPlayerTest { .blockUntilEnded(TIMEOUT_MS); } + @Test + public void seekToUnpreparedWindowWithNonZeroOffsetInConcatenationStartsAtCorrectPosition() + throws Exception { + Timeline timeline = new FakeTimeline(/* windowCount= */ 1); + FakeMediaSource mediaSource = new FakeMediaSource(/* timeline= */ null, /* manifest= */ null); + MediaSource clippedMediaSource = + new ClippingMediaSource( + mediaSource, + /* startPositionUs= */ 3 * C.MICROS_PER_SECOND, + /* endPositionUs= */ C.TIME_END_OF_SOURCE); + MediaSource concatenatedMediaSource = new ConcatenatingMediaSource(clippedMediaSource); + AtomicLong positionWhenReady = new AtomicLong(); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("seekToUnpreparedWindowWithNonZeroOffsetInConcatenation") + .pause() + .waitForPlaybackState(Player.STATE_BUFFERING) + .seek(/* positionMs= */ 10) + .waitForTimelineChanged() + .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline, /* newManifest= */ null)) + .waitForTimelineChanged() + .waitForPlaybackState(Player.STATE_READY) + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + positionWhenReady.set(player.getContentPosition()); + } + }) + .play() + .build(); + new Builder() + .setMediaSource(concatenatedMediaSource) + .setActionSchedule(actionSchedule) + .build(context) + .start() + .blockUntilEnded(TIMEOUT_MS); + + assertThat(positionWhenReady.get()).isEqualTo(10); + } + + @Test + public void seekToUnpreparedWindowWithMultiplePeriodsInConcatenationStartsAtCorrectPeriod() + throws Exception { + long periodDurationMs = 5000; + Timeline timeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* periodCount =*/ 2, + /* id= */ new Object(), + /* isSeekable= */ true, + /* isDynamic= */ false, + /* durationUs= */ 2 * periodDurationMs * 1000)); + FakeMediaSource mediaSource = new FakeMediaSource(/* timeline= */ null, /* manifest= */ null); + MediaSource concatenatedMediaSource = new ConcatenatingMediaSource(mediaSource); + AtomicInteger periodIndexWhenReady = new AtomicInteger(); + AtomicLong positionWhenReady = new AtomicLong(); + ActionSchedule actionSchedule = + new ActionSchedule.Builder("seekToUnpreparedWindowWithMultiplePeriodsInConcatenation") + .pause() + .waitForPlaybackState(Player.STATE_BUFFERING) + // Seek 10ms into the second period. + .seek(/* positionMs= */ periodDurationMs + 10) + .waitForTimelineChanged() + .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline, /* newManifest= */ null)) + .waitForTimelineChanged() + .waitForPlaybackState(Player.STATE_READY) + .executeRunnable( + new PlayerRunnable() { + @Override + public void run(SimpleExoPlayer player) { + periodIndexWhenReady.set(player.getCurrentPeriodIndex()); + positionWhenReady.set(player.getContentPosition()); + } + }) + .play() + .build(); + new Builder() + .setMediaSource(concatenatedMediaSource) + .setActionSchedule(actionSchedule) + .build(context) + .start() + .blockUntilEnded(TIMEOUT_MS); + + assertThat(periodIndexWhenReady.get()).isEqualTo(1); + assertThat(positionWhenReady.get()).isEqualTo(periodDurationMs + 10); + } + // Internal methods. private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) { diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java index e0feae6f49..3649685f3e 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java @@ -585,7 +585,7 @@ public final class AnalyticsCollectorTest { () -> concatenatedMediaSource.moveMediaSource( /* currentIndex= */ 0, /* newIndex= */ 1)) - .waitForTimelineChanged(/* expectedTimeline= */ null) + .waitForTimelineChanged() .play() .build(); TestAnalyticsListener listener = runAnalyticsTest(concatenatedMediaSource, actionSchedule); diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java index 6e37d7d070..54d97fb905 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/ActionSchedule.java @@ -375,6 +375,15 @@ public final class ActionSchedule { return apply(new SendMessages(tag, target, windowIndex, positionMs, deleteAfterDelivery)); } + /** + * Schedules a delay until any timeline change. + * + * @return The builder, for convenience. + */ + public Builder waitForTimelineChanged() { + return apply(new WaitForTimelineChanged(tag, /* expectedTimeline= */ null)); + } + /** * Schedules a delay until the timeline changed to a specified expected timeline. * @@ -382,7 +391,7 @@ public final class ActionSchedule { * change. * @return The builder, for convenience. */ - public Builder waitForTimelineChanged(@Nullable Timeline expectedTimeline) { + public Builder waitForTimelineChanged(Timeline expectedTimeline) { return apply(new WaitForTimelineChanged(tag, expectedTimeline)); }