Commit 49215950 by tonihei Committed by Oliver Woodman

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
parent 6b562f0a
# Release notes # # 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 ### ### 2.9.0 ###
* Turn on Java 8 compiler support for the ExoPlayer library. Apps may need to * Turn on Java 8 compiler support for the ExoPlayer library. Apps may need to
......
...@@ -18,6 +18,7 @@ package com.google.android.exoplayer2.source; ...@@ -18,6 +18,7 @@ package com.google.android.exoplayer2.source;
import android.os.Handler; import android.os.Handler;
import android.support.annotation.NonNull; import android.support.annotation.NonNull;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import android.util.Pair;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.ExoPlaybackException;
import com.google.android.exoplayer2.ExoPlayer; import com.google.android.exoplayer2.ExoPlayer;
...@@ -65,6 +66,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo ...@@ -65,6 +66,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo
private final boolean isAtomic; private final boolean isAtomic;
private final boolean useLazyPreparation; private final boolean useLazyPreparation;
private final Timeline.Window window; private final Timeline.Window window;
private final Timeline.Period period;
private @Nullable ExoPlayer player; private @Nullable ExoPlayer player;
private @Nullable Handler playerApplicationHandler; private @Nullable Handler playerApplicationHandler;
...@@ -131,6 +133,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo ...@@ -131,6 +133,7 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo
this.isAtomic = isAtomic; this.isAtomic = isAtomic;
this.useLazyPreparation = useLazyPreparation; this.useLazyPreparation = useLazyPreparation;
window = new Timeline.Window(); window = new Timeline.Window();
period = new Timeline.Period();
addMediaSources(Arrays.asList(mediaSources)); addMediaSources(Arrays.asList(mediaSources));
} }
...@@ -684,21 +687,53 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo ...@@ -684,21 +687,53 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo
windowOffsetUpdate, windowOffsetUpdate,
periodOffsetUpdate); periodOffsetUpdate);
} }
mediaSourceHolder.timeline = deferredTimeline.cloneWithNewTimeline(timeline); if (mediaSourceHolder.isPrepared) {
if (!mediaSourceHolder.isPrepared && !timeline.isEmpty()) { mediaSourceHolder.timeline = deferredTimeline.cloneWithUpdatedTimeline(timeline);
timeline.getWindow(/* windowIndex= */ 0, window); } else if (timeline.isEmpty()) {
long defaultPeriodPositionUs = mediaSourceHolder.timeline =
window.getPositionInFirstPeriodUs() + window.getDefaultPositionUs(); DeferredTimeline.createWithRealTimeline(timeline, DeferredTimeline.DUMMY_ID);
for (int i = 0; i < mediaSourceHolder.activeMediaPeriods.size(); i++) { } else {
DeferredMediaPeriod deferredMediaPeriod = mediaSourceHolder.activeMediaPeriods.get(i); // We should have at most one deferred media period for the DummyTimeline because the duration
deferredMediaPeriod.setDefaultPreparePositionUs(defaultPeriodPositionUs); // is unset and we don't load beyond periods with unset duration. We need to figure out how to
// handle the prepare positions of multiple deferred media periods, should that ever change.
Assertions.checkState(mediaSourceHolder.activeMediaPeriods.size() <= 1);
DeferredMediaPeriod deferredMediaPeriod =
mediaSourceHolder.activeMediaPeriods.isEmpty()
? null
: mediaSourceHolder.activeMediaPeriods.get(0);
// Determine first period and the start position.
// This will be:
// 1. The default window start position if no deferred period has been created yet.
// 2. The non-zero prepare position of the deferred period under the assumption that this is
// a non-zero initial seek position in the window.
// 3. The default window start position if the deferred period has a prepare position of zero
// under the assumption that the prepare position of zero was used because it's the
// default position of the DummyTimeline window. Note that this will override an
// intentional seek to zero for a window with a non-zero default position. 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.
long windowStartPositionUs = window.getDefaultPositionUs();
if (deferredMediaPeriod != null) {
long periodPreparePositionUs = deferredMediaPeriod.getPreparePositionUs();
if (periodPreparePositionUs != 0) {
windowStartPositionUs = periodPreparePositionUs;
}
}
Pair<Object, Long> 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 = MediaPeriodId idInSource =
deferredMediaPeriod.id.copyWithPeriodUid( deferredMediaPeriod.id.copyWithPeriodUid(
getChildPeriodUid(mediaSourceHolder, deferredMediaPeriod.id.periodUid)); getChildPeriodUid(mediaSourceHolder, deferredMediaPeriod.id.periodUid));
deferredMediaPeriod.createPeriod(idInSource); deferredMediaPeriod.createPeriod(idInSource);
} }
mediaSourceHolder.isPrepared = true;
} }
mediaSourceHolder.isPrepared = true;
scheduleListenerNotification(/* actionOnCompletion= */ null); scheduleListenerNotification(/* actionOnCompletion= */ null);
} }
...@@ -897,18 +932,32 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo ...@@ -897,18 +932,32 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo
} }
/** /**
* Timeline used as placeholder for an unprepared media source. After preparation, a copy of the * Timeline used as placeholder for an unprepared media source. After preparation, a
* DeferredTimeline is used to keep the originally assigned first period ID. * DeferredTimeline is used to keep the originally assigned dummy period ID.
*/ */
private static final class DeferredTimeline extends ForwardingTimeline { private static final class DeferredTimeline extends ForwardingTimeline {
private static final Object DUMMY_ID = new Object(); private static final Object DUMMY_ID = new Object();
private static final DummyTimeline dummyTimeline = new DummyTimeline(); private static final DummyTimeline DUMMY_TIMELINE = new DummyTimeline();
private final Object replacedId; private final Object replacedId;
/**
* Returns an instance with a real timeline, replacing the provided period ID with the already
* assigned dummy period ID.
*
* @param timeline The real timeline.
* @param firstPeriodUid The period UID in the timeline which will be replaced by the already
* assigned dummy period UID.
*/
public static DeferredTimeline createWithRealTimeline(
Timeline timeline, Object firstPeriodUid) {
return new DeferredTimeline(timeline, firstPeriodUid);
}
/** Creates deferred timeline exposing a {@link DummyTimeline}. */
public DeferredTimeline() { public DeferredTimeline() {
this(dummyTimeline, DUMMY_ID); this(DUMMY_TIMELINE, DUMMY_ID);
} }
private DeferredTimeline(Timeline timeline, Object replacedId) { private DeferredTimeline(Timeline timeline, Object replacedId) {
...@@ -916,14 +965,16 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo ...@@ -916,14 +965,16 @@ public class ConcatenatingMediaSource extends CompositeMediaSource<MediaSourceHo
this.replacedId = replacedId; this.replacedId = replacedId;
} }
public DeferredTimeline cloneWithNewTimeline(Timeline timeline) { /**
return new DeferredTimeline( * Returns a copy with an updated timeline. This keeps the existing period replacement.
timeline, *
replacedId == DUMMY_ID && timeline.getPeriodCount() > 0 * @param timeline The new timeline.
? timeline.getUidOfPeriod(0) */
: replacedId); public DeferredTimeline cloneWithUpdatedTimeline(Timeline timeline) {
return new DeferredTimeline(timeline, replacedId);
} }
/** Returns wrapped timeline. */
public Timeline getTimeline() { public Timeline getTimeline() {
return timeline; return timeline;
} }
......
...@@ -79,23 +79,19 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb ...@@ -79,23 +79,19 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb
this.listener = listener; 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 * Overrides the default prepare position at which to prepare the media period. This value is only
* if the call to {@link MediaPeriod#prepare(Callback, long)} is being deferred and the call was * used if the call to {@link MediaPeriod#prepare(Callback, long)} is being deferred.
* made with a (presumably default) prepare position of 0.
* *
* <p>Note that this will override an intentional seek to zero in the corresponding non-seekable * @param defaultPreparePositionUs The default prepare position to use, in microseconds.
* 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.
*/ */
public void setDefaultPreparePositionUs(long defaultPreparePositionUs) { public void overridePreparePositionUs(long defaultPreparePositionUs) {
if (preparePositionUs == 0 && defaultPreparePositionUs != 0) { preparePositionOverrideUs = defaultPreparePositionUs;
preparePositionOverrideUs = defaultPreparePositionUs;
preparePositionUs = defaultPreparePositionUs;
}
} }
/** /**
...@@ -108,6 +104,10 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb ...@@ -108,6 +104,10 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb
public void createPeriod(MediaPeriodId id) { public void createPeriod(MediaPeriodId id) {
mediaPeriod = mediaSource.createPeriod(id, allocator); mediaPeriod = mediaSource.createPeriod(id, allocator);
if (callback != null) { if (callback != null) {
long preparePositionUs =
preparePositionOverrideUs != C.TIME_UNSET
? preparePositionOverrideUs
: this.preparePositionUs;
mediaPeriod.prepare(this, preparePositionUs); mediaPeriod.prepare(this, preparePositionUs);
} }
} }
...@@ -157,7 +157,7 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb ...@@ -157,7 +157,7 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb
@Override @Override
public long selectTracks(TrackSelection[] selections, boolean[] mayRetainStreamFlags, public long selectTracks(TrackSelection[] selections, boolean[] mayRetainStreamFlags,
SampleStream[] streams, boolean[] streamResetFlags, long positionUs) { SampleStream[] streams, boolean[] streamResetFlags, long positionUs) {
if (preparePositionOverrideUs != C.TIME_UNSET && positionUs == 0) { if (preparePositionOverrideUs != C.TIME_UNSET && positionUs == preparePositionUs) {
positionUs = preparePositionOverrideUs; positionUs = preparePositionOverrideUs;
preparePositionOverrideUs = C.TIME_UNSET; preparePositionOverrideUs = C.TIME_UNSET;
} }
......
...@@ -60,6 +60,7 @@ import java.util.Collections; ...@@ -60,6 +60,7 @@ import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
...@@ -1346,7 +1347,7 @@ public final class ExoPlayerTest { ...@@ -1346,7 +1347,7 @@ public final class ExoPlayerTest {
() -> () ->
concatenatingMediaSource.addMediaSources( concatenatingMediaSource.addMediaSources(
Arrays.asList(mediaSource, mediaSource))) Arrays.asList(mediaSource, mediaSource)))
.waitForTimelineChanged(null) .waitForTimelineChanged()
.executeRunnable( .executeRunnable(
new PlayerRunnable() { new PlayerRunnable() {
@Override @Override
...@@ -2192,14 +2193,14 @@ public final class ExoPlayerTest { ...@@ -2192,14 +2193,14 @@ public final class ExoPlayerTest {
startPositionUs + expectedDurationUs); startPositionUs + expectedDurationUs);
Clock clock = new AutoAdvancingFakeClock(); Clock clock = new AutoAdvancingFakeClock();
AtomicReference<Player> playerReference = new AtomicReference<>(); AtomicReference<Player> playerReference = new AtomicReference<>();
AtomicReference<Long> positionAtDiscontinuityMs = new AtomicReference<>(); AtomicLong positionAtDiscontinuityMs = new AtomicLong(C.TIME_UNSET);
AtomicReference<Long> clockAtStartMs = new AtomicReference<>(); AtomicLong clockAtStartMs = new AtomicLong(C.TIME_UNSET);
AtomicReference<Long> clockAtDiscontinuityMs = new AtomicReference<>(); AtomicLong clockAtDiscontinuityMs = new AtomicLong(C.TIME_UNSET);
EventListener eventListener = EventListener eventListener =
new EventListener() { new EventListener() {
@Override @Override
public void onPlayerStateChanged(boolean playWhenReady, int playbackState) { 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()); clockAtStartMs.set(clock.elapsedRealtime());
} }
} }
...@@ -2446,6 +2447,93 @@ public final class ExoPlayerTest { ...@@ -2446,6 +2447,93 @@ public final class ExoPlayerTest {
.blockUntilEnded(TIMEOUT_MS); .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. // Internal methods.
private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) { private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {
......
...@@ -585,7 +585,7 @@ public final class AnalyticsCollectorTest { ...@@ -585,7 +585,7 @@ public final class AnalyticsCollectorTest {
() -> () ->
concatenatedMediaSource.moveMediaSource( concatenatedMediaSource.moveMediaSource(
/* currentIndex= */ 0, /* newIndex= */ 1)) /* currentIndex= */ 0, /* newIndex= */ 1))
.waitForTimelineChanged(/* expectedTimeline= */ null) .waitForTimelineChanged()
.play() .play()
.build(); .build();
TestAnalyticsListener listener = runAnalyticsTest(concatenatedMediaSource, actionSchedule); TestAnalyticsListener listener = runAnalyticsTest(concatenatedMediaSource, actionSchedule);
......
...@@ -376,13 +376,22 @@ public final class ActionSchedule { ...@@ -376,13 +376,22 @@ public final class ActionSchedule {
} }
/** /**
* 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. * Schedules a delay until the timeline changed to a specified expected timeline.
* *
* @param expectedTimeline The expected timeline to wait for. If null, wait for any timeline * @param expectedTimeline The expected timeline to wait for. If null, wait for any timeline
* change. * change.
* @return The builder, for convenience. * @return The builder, for convenience.
*/ */
public Builder waitForTimelineChanged(@Nullable Timeline expectedTimeline) { public Builder waitForTimelineChanged(Timeline expectedTimeline) {
return apply(new WaitForTimelineChanged(tag, expectedTimeline)); return apply(new WaitForTimelineChanged(tag, expectedTimeline));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment