Skip to content

SDK / exoplayer

Go to a project
Commit d8c71df2 by andrewlewis Committed by Oliver Woodman
Options

On re-preparation, suppress source info refresh until ack'ed 

ExoPlayerImpl.prepare() replaces the timeline with an empty timeline. After this
happens, MSG_SOURCE_INFO_REFRESHED could be handled on the main thread and
could relate to the old source, so the player could expose a stale timeline.

Count pending prepares in ExoPlayerImpl so that source info refreshes can be
suppressed until preparation actually completes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=152496255
parent cf4358b8
Inline Side-by-side
Showing with 109 additions and 23 deletions
library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java
...@@ -17,6 +17,7 @@ package com.google.android.exoplayer2; ...@@ -17,6 +17,7 @@ package com.google.android.exoplayer2;
import android.os.Handler; import android.os.Handler;
import android.os.HandlerThread; import android.os.HandlerThread;
import android.util.Pair;
import com.google.android.exoplayer2.decoder.DecoderInputBuffer; import com.google.android.exoplayer2.decoder.DecoderInputBuffer;
import com.google.android.exoplayer2.source.MediaPeriod; import com.google.android.exoplayer2.source.MediaPeriod;
import com.google.android.exoplayer2.source.MediaSource; import com.google.android.exoplayer2.source.MediaSource;
...@@ -32,6 +33,7 @@ import com.google.android.exoplayer2.util.MediaClock; ...@@ -32,6 +33,7 @@ import com.google.android.exoplayer2.util.MediaClock;
import com.google.android.exoplayer2.util.MimeTypes; import com.google.android.exoplayer2.util.MimeTypes;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.LinkedList;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeoutException;
...@@ -70,8 +72,7 @@ public final class ExoPlayerTest extends TestCase { ...@@ -70,8 +72,7 @@ public final class ExoPlayerTest extends TestCase {
assertEquals(0, renderer.formatReadCount); assertEquals(0, renderer.formatReadCount);
assertEquals(0, renderer.bufferReadCount); assertEquals(0, renderer.bufferReadCount);
assertFalse(renderer.isEnded); assertFalse(renderer.isEnded);
assertEquals(timeline, playerWrapper.timeline); playerWrapper.assertSourceInfosEquals(Pair.create(timeline, null));
assertNull(playerWrapper.manifest);
} }
/** /**
...@@ -89,9 +90,8 @@ public final class ExoPlayerTest extends TestCase { ...@@ -89,9 +90,8 @@ public final class ExoPlayerTest extends TestCase {
assertEquals(1, renderer.formatReadCount); assertEquals(1, renderer.formatReadCount);
assertEquals(1, renderer.bufferReadCount); assertEquals(1, renderer.bufferReadCount);
assertTrue(renderer.isEnded); assertTrue(renderer.isEnded);
assertEquals(timeline, playerWrapper.timeline);
assertEquals(manifest, playerWrapper.manifest);
assertEquals(new TrackGroupArray(new TrackGroup(TEST_VIDEO_FORMAT)), playerWrapper.trackGroups); assertEquals(new TrackGroupArray(new TrackGroup(TEST_VIDEO_FORMAT)), playerWrapper.trackGroups);
playerWrapper.assertSourceInfosEquals(Pair.create(timeline, manifest));
} }
/** /**
...@@ -111,8 +111,7 @@ public final class ExoPlayerTest extends TestCase { ...@@ -111,8 +111,7 @@ public final class ExoPlayerTest extends TestCase {
assertEquals(3, renderer.formatReadCount); assertEquals(3, renderer.formatReadCount);
assertEquals(1, renderer.bufferReadCount); assertEquals(1, renderer.bufferReadCount);
assertTrue(renderer.isEnded); assertTrue(renderer.isEnded);
assertEquals(timeline, playerWrapper.timeline); playerWrapper.assertSourceInfosEquals(Pair.create(timeline, null));
assertNull(playerWrapper.manifest);
} }
/** /**
...@@ -163,8 +162,60 @@ public final class ExoPlayerTest extends TestCase { ...@@ -163,8 +162,60 @@ public final class ExoPlayerTest extends TestCase {
assertEquals(1, audioRenderer.positionResetCount); assertEquals(1, audioRenderer.positionResetCount);
assertTrue(videoRenderer.isEnded); assertTrue(videoRenderer.isEnded);
assertTrue(audioRenderer.isEnded); assertTrue(audioRenderer.isEnded);
assertEquals(timeline, playerWrapper.timeline); playerWrapper.assertSourceInfosEquals(Pair.create(timeline, null));
assertNull(playerWrapper.manifest); }
public void testRepreparationGivesFreshSourceInfo() throws Exception {
PlayerWrapper playerWrapper = new PlayerWrapper();
Timeline timeline = new FakeTimeline(new TimelineWindowDefinition(false, false, 0));
FakeRenderer renderer = new FakeRenderer(TEST_VIDEO_FORMAT);
// Prepare the player with a source with the first manifest and a non-empty timeline
Object firstSourceManifest = new Object();
playerWrapper.setup(new FakeMediaSource(timeline, firstSourceManifest, TEST_VIDEO_FORMAT),
renderer);
playerWrapper.blockUntilSourceInfoRefreshed(TIMEOUT_MS);
// Prepare the player again with a source and a new manifest, which will never be exposed.
final CountDownLatch queuedSourceInfoCountDownLatch = new CountDownLatch(1);
final CountDownLatch completePreparationCountDownLatch = new CountDownLatch(1);
playerWrapper.prepare(new FakeMediaSource(timeline, new Object(), TEST_VIDEO_FORMAT) {
@Override
public void prepareSource(ExoPlayer player, boolean isTopLevelSource, Listener listener) {
super.prepareSource(player, isTopLevelSource, listener);
// We've queued a source info refresh on the playback thread's event queue. Allow the test
// thread to prepare the player with the third source, and block this thread (the playback
// thread) until the test thread's call to prepare() has returned.
queuedSourceInfoCountDownLatch.countDown();
try {
completePreparationCountDownLatch.await();
} catch (InterruptedException e) {
throw new IllegalStateException(e);
}
}
});
// Prepare the player again with a third source.
queuedSourceInfoCountDownLatch.await();
Object thirdSourceManifest = new Object();
playerWrapper.prepare(new FakeMediaSource(timeline, thirdSourceManifest, TEST_VIDEO_FORMAT));
completePreparationCountDownLatch.countDown();
// Wait for playback to complete.
playerWrapper.blockUntilEnded(TIMEOUT_MS);
assertEquals(0, playerWrapper.positionDiscontinuityCount);
assertEquals(1, renderer.formatReadCount);
assertEquals(1, renderer.bufferReadCount);
assertTrue(renderer.isEnded);
assertEquals(new TrackGroupArray(new TrackGroup(TEST_VIDEO_FORMAT)), playerWrapper.trackGroups);
// The first source's preparation completed with a non-empty timeline. When the player was
// re-prepared with the second source, it immediately exposed an empty timeline, but the source
// info refresh from the second source was suppressed as we re-prepared with the third source.
playerWrapper.assertSourceInfosEquals(
Pair.create(timeline, firstSourceManifest),
Pair.create(Timeline.EMPTY, null),
Pair.create(timeline, thirdSourceManifest));
} }
/** /**
...@@ -172,13 +223,13 @@ public final class ExoPlayerTest extends TestCase { ...@@ -172,13 +223,13 @@ public final class ExoPlayerTest extends TestCase {
*/ */
private static final class PlayerWrapper implements ExoPlayer.EventListener { private static final class PlayerWrapper implements ExoPlayer.EventListener {
private final CountDownLatch sourceInfoCountDownLatch;
private final CountDownLatch endedCountDownLatch; private final CountDownLatch endedCountDownLatch;
private final HandlerThread playerThread; private final HandlerThread playerThread;
private final Handler handler; private final Handler handler;
private final LinkedList<Pair<Timeline, Object>> sourceInfos;
private ExoPlayer player; private ExoPlayer player;
private Timeline timeline;
private Object manifest;
private TrackGroupArray trackGroups; private TrackGroupArray trackGroups;
private Exception exception; private Exception exception;
...@@ -186,17 +237,19 @@ public final class ExoPlayerTest extends TestCase { ...@@ -186,17 +237,19 @@ public final class ExoPlayerTest extends TestCase {
private volatile int positionDiscontinuityCount; private volatile int positionDiscontinuityCount;
public PlayerWrapper() { public PlayerWrapper() {
sourceInfoCountDownLatch = new CountDownLatch(1);
endedCountDownLatch = new CountDownLatch(1); endedCountDownLatch = new CountDownLatch(1);
playerThread = new HandlerThread("ExoPlayerTest thread"); playerThread = new HandlerThread("ExoPlayerTest thread");
playerThread.start(); playerThread.start();
handler = new Handler(playerThread.getLooper()); handler = new Handler(playerThread.getLooper());
sourceInfos = new LinkedList<>();
} }
// Called on the test thread. // Called on the test thread.
public void blockUntilEnded(long timeoutMs) throws Exception { public void blockUntilEnded(long timeoutMs) throws Exception {
if (!endedCountDownLatch.await(timeoutMs, TimeUnit.MILLISECONDS)) { if (!endedCountDownLatch.await(timeoutMs, TimeUnit.MILLISECONDS)) {
exception = new TimeoutException("Test playback timed out."); exception = new TimeoutException("Test playback timed out waiting for playback to end.");
} }
release(); release();
// Throw any pending exception (from playback, timing out or releasing). // Throw any pending exception (from playback, timing out or releasing).
...@@ -205,6 +258,12 @@ public final class ExoPlayerTest extends TestCase { ...@@ -205,6 +258,12 @@ public final class ExoPlayerTest extends TestCase {
} }
} }
public void blockUntilSourceInfoRefreshed(long timeoutMs) throws Exception {
if (!sourceInfoCountDownLatch.await(timeoutMs, TimeUnit.MILLISECONDS)) {
throw new TimeoutException("Test playback timed out waiting for source info.");
}
}
public void setup(final MediaSource mediaSource, final Renderer... renderers) { public void setup(final MediaSource mediaSource, final Renderer... renderers) {
handler.post(new Runnable() { handler.post(new Runnable() {
@Override @Override
...@@ -221,6 +280,19 @@ public final class ExoPlayerTest extends TestCase { ...@@ -221,6 +280,19 @@ public final class ExoPlayerTest extends TestCase {
}); });
} }
public void prepare(final MediaSource mediaSource) {
handler.post(new Runnable() {
@Override
public void run() {
try {
player.prepare(mediaSource);
} catch (Exception e) {
handleError(e);
}
}
});
}
public void release() throws InterruptedException { public void release() throws InterruptedException {
handler.post(new Runnable() { handler.post(new Runnable() {
@Override @Override
...@@ -246,6 +318,14 @@ public final class ExoPlayerTest extends TestCase { ...@@ -246,6 +318,14 @@ public final class ExoPlayerTest extends TestCase {
endedCountDownLatch.countDown(); endedCountDownLatch.countDown();
} }
@SafeVarargs
public final void assertSourceInfosEquals(Pair<Timeline, Object>... sourceInfos) {
assertEquals(sourceInfos.length, this.sourceInfos.size());
for (Pair<Timeline, Object> sourceInfo : sourceInfos) {
assertEquals(sourceInfo, this.sourceInfos.remove());
}
}
// ExoPlayer.EventListener implementation. // ExoPlayer.EventListener implementation.
@Override @Override
...@@ -262,8 +342,8 @@ public final class ExoPlayerTest extends TestCase { ...@@ -262,8 +342,8 @@ public final class ExoPlayerTest extends TestCase {
@Override @Override
public void onTimelineChanged(Timeline timeline, Object manifest) { public void onTimelineChanged(Timeline timeline, Object manifest) {
this.timeline = timeline; sourceInfos.add(Pair.create(timeline, manifest));
this.manifest = manifest; sourceInfoCountDownLatch.countDown();
} }
@Override @Override
...@@ -352,7 +432,7 @@ public final class ExoPlayerTest extends TestCase { ...@@ -352,7 +432,7 @@ public final class ExoPlayerTest extends TestCase {
* Fake {@link MediaSource} that provides a given timeline (which must have one period). Creating * Fake {@link MediaSource} that provides a given timeline (which must have one period). Creating
* the period will return a {@link FakeMediaPeriod}. * the period will return a {@link FakeMediaPeriod}.
*/ */
private static final class FakeMediaSource implements MediaSource { private static class FakeMediaSource implements MediaSource {
private final Timeline timeline; private final Timeline timeline;
private final Object manifest; private final Object manifest;
......
library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java
...@@ -53,6 +53,7 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -53,6 +53,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
private boolean playWhenReady; private boolean playWhenReady;
private int playbackState; private int playbackState;
private int pendingSeekAcks; private int pendingSeekAcks;
private int pendingPrepareAcks;
private boolean isLoading; private boolean isLoading;
private Timeline timeline; private Timeline timeline;
private Object manifest; private Object manifest;
...@@ -142,6 +143,7 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -142,6 +143,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
} }
} }
} }
pendingPrepareAcks++;
internalPlayer.prepare(mediaSource, resetPosition); internalPlayer.prepare(mediaSource, resetPosition);
} }
...@@ -310,18 +312,12 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -310,18 +312,12 @@ import java.util.concurrent.CopyOnWriteArraySet;
@Override @Override
public boolean isCurrentWindowDynamic() { public boolean isCurrentWindowDynamic() {
if (timeline.isEmpty()) { return !timeline.isEmpty() && timeline.getWindow(getCurrentWindowIndex(), window).isDynamic;
return false;
}
return timeline.getWindow(getCurrentWindowIndex(), window).isDynamic;
} }
@Override @Override
public boolean isCurrentWindowSeekable() { public boolean isCurrentWindowSeekable() {
if (timeline.isEmpty()) { return !timeline.isEmpty() && timeline.getWindow(getCurrentWindowIndex(), window).isSeekable;
return false;
}
return timeline.getWindow(getCurrentWindowIndex(), window).isSeekable;
} }
@Override @Override
...@@ -357,6 +353,10 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -357,6 +353,10 @@ import java.util.concurrent.CopyOnWriteArraySet;
// Not private so it can be called from an inner class without going through a thunk method. // Not private so it can be called from an inner class without going through a thunk method.
/* package */ void handleEvent(Message msg) { /* package */ void handleEvent(Message msg) {
switch (msg.what) { switch (msg.what) {
case ExoPlayerImplInternal.MSG_PREPARE_ACK: {
pendingPrepareAcks--;
break;
}
case ExoPlayerImplInternal.MSG_STATE_CHANGED: { case ExoPlayerImplInternal.MSG_STATE_CHANGED: {
playbackState = msg.arg1; playbackState = msg.arg1;
for (EventListener listener : listeners) { for (EventListener listener : listeners) {
...@@ -372,6 +372,7 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -372,6 +372,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
break; break;
} }
case ExoPlayerImplInternal.MSG_TRACKS_CHANGED: { case ExoPlayerImplInternal.MSG_TRACKS_CHANGED: {
if (pendingPrepareAcks == 0) {
TrackSelectorResult trackSelectorResult = (TrackSelectorResult) msg.obj; TrackSelectorResult trackSelectorResult = (TrackSelectorResult) msg.obj;
tracksSelected = true; tracksSelected = true;
trackGroups = trackSelectorResult.groups; trackGroups = trackSelectorResult.groups;
...@@ -380,6 +381,7 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -380,6 +381,7 @@ import java.util.concurrent.CopyOnWriteArraySet;
for (EventListener listener : listeners) { for (EventListener listener : listeners) {
listener.onTracksChanged(trackGroups, trackSelections); listener.onTracksChanged(trackGroups, trackSelections);
} }
}
break; break;
} }
case ExoPlayerImplInternal.MSG_SEEK_ACK: { case ExoPlayerImplInternal.MSG_SEEK_ACK: {
...@@ -404,13 +406,15 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -404,13 +406,15 @@ import java.util.concurrent.CopyOnWriteArraySet;
} }
case ExoPlayerImplInternal.MSG_SOURCE_INFO_REFRESHED: { case ExoPlayerImplInternal.MSG_SOURCE_INFO_REFRESHED: {
SourceInfo sourceInfo = (SourceInfo) msg.obj; SourceInfo sourceInfo = (SourceInfo) msg.obj;
pendingSeekAcks -= sourceInfo.seekAcks;
if (pendingPrepareAcks == 0) {
timeline = sourceInfo.timeline; timeline = sourceInfo.timeline;
manifest = sourceInfo.manifest; manifest = sourceInfo.manifest;
playbackInfo = sourceInfo.playbackInfo; playbackInfo = sourceInfo.playbackInfo;
pendingSeekAcks -= sourceInfo.seekAcks;
for (EventListener listener : listeners) { for (EventListener listener : listeners) {
listener.onTimelineChanged(timeline, manifest); listener.onTimelineChanged(timeline, manifest);
} }
}
break; break;
} }
case ExoPlayerImplInternal.MSG_PLAYBACK_PARAMETERS_CHANGED: { case ExoPlayerImplInternal.MSG_PLAYBACK_PARAMETERS_CHANGED: {
......
library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
...@@ -90,6 +90,7 @@ import java.io.IOException; ...@@ -90,6 +90,7 @@ import java.io.IOException;
private static final String TAG = "ExoPlayerImplInternal"; private static final String TAG = "ExoPlayerImplInternal";
// External messages // External messages
public static final int MSG_PREPARE_ACK = 0;
public static final int MSG_STATE_CHANGED = 1; public static final int MSG_STATE_CHANGED = 1;
public static final int MSG_LOADING_CHANGED = 2; public static final int MSG_LOADING_CHANGED = 2;
public static final int MSG_TRACKS_CHANGED = 3; public static final int MSG_TRACKS_CHANGED = 3;
...@@ -383,6 +384,7 @@ import java.io.IOException; ...@@ -383,6 +384,7 @@ import java.io.IOException;
} }
private void prepareInternal(MediaSource mediaSource, boolean resetPosition) { private void prepareInternal(MediaSource mediaSource, boolean resetPosition) {
eventHandler.sendEmptyMessage(MSG_PREPARE_ACK);
resetInternal(true); resetInternal(true);
loadControl.onPrepared(); loadControl.onPrepared();
if (resetPosition) { if (resetPosition) {
......
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