Skip to content

SDK / exoplayer

Go to a project
Commit 318618d7 by tonihei Committed by Oliver Woodman
Options

Fix seek/prepare/stop acks when exception is thrown. 

1. The player doesn't acknowledge phantom stops when an exception is thrown anymore.
2. It also makes sure it doesn't reset the pendingPrepareCount unless it's actually
immediately acknowledging these prepares.
3. It ensures a seek is acknowledged even though an exception is thrown during seeking.

Added tests (which previously failed) for all three cases.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178876362
parent 6c4bb2cd
Inline Side-by-side
Showing with 157 additions and 68 deletions
library/core/src/androidTest/java/com/google/android/exoplayer2/ExoPlayerTest.java
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
*/ */
package com.google.android.exoplayer2; package com.google.android.exoplayer2;
import com.google.android.exoplayer2.Player.DefaultEventListener;
import com.google.android.exoplayer2.Player.EventListener;
import com.google.android.exoplayer2.source.ConcatenatingMediaSource; import com.google.android.exoplayer2.source.ConcatenatingMediaSource;
import com.google.android.exoplayer2.source.MediaSource; import com.google.android.exoplayer2.source.MediaSource;
import com.google.android.exoplayer2.source.TrackGroup; import com.google.android.exoplayer2.source.TrackGroup;
...@@ -344,6 +346,39 @@ public final class ExoPlayerTest extends TestCase { ...@@ -344,6 +346,39 @@ public final class ExoPlayerTest extends TestCase {
assertEquals(Player.STATE_BUFFERING, (int) playbackStatesWhenSeekProcessed.get(3)); assertEquals(Player.STATE_BUFFERING, (int) playbackStatesWhenSeekProcessed.get(3));
} }
public void testSeekProcessedCalledWithIllegalSeekPosition() throws Exception {
ActionSchedule actionSchedule =
new ActionSchedule.Builder("testSeekProcessedCalledWithIllegalSeekPosition")
.waitForPlaybackState(Player.STATE_BUFFERING)
// Cause an illegal seek exception by seeking to an invalid position while the media
// source is still being prepared and the player doesn't immediately know it will fail.
// Because the media source prepares immediately, the exception will be thrown when the
// player processed the seek.
.seek(/* windowIndex= */ 100, /* positionMs= */ 0)
.waitForPlaybackState(Player.STATE_IDLE)
.build();
final boolean[] onSeekProcessedCalled = new boolean[1];
EventListener listener =
new DefaultEventListener() {
@Override
public void onSeekProcessed() {
onSeekProcessedCalled[0] = true;
}
};
ExoPlayerTestRunner testRunner =
new ExoPlayerTestRunner.Builder()
.setActionSchedule(actionSchedule)
.setEventListener(listener)
.build();
try {
testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS);
fail();
} catch (ExoPlaybackException e) {
// Expected exception.
}
assertTrue(onSeekProcessedCalled[0]);
}
public void testSeekDiscontinuity() throws Exception { public void testSeekDiscontinuity() throws Exception {
FakeTimeline timeline = new FakeTimeline(1); FakeTimeline timeline = new FakeTimeline(1);
ActionSchedule actionSchedule = new ActionSchedule.Builder("testSeekDiscontinuity") ActionSchedule actionSchedule = new ActionSchedule.Builder("testSeekDiscontinuity")
...@@ -808,4 +843,69 @@ public final class ExoPlayerTest extends TestCase { ...@@ -808,4 +843,69 @@ public final class ExoPlayerTest extends TestCase {
testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED); testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED);
testRunner.assertPositionDiscontinuityReasonsEqual(Player.DISCONTINUITY_REASON_SEEK); testRunner.assertPositionDiscontinuityReasonsEqual(Player.DISCONTINUITY_REASON_SEEK);
} }
public void testReprepareAfterPlaybackError() throws Exception {
Timeline timeline = new FakeTimeline(/* windowCount= */ 1);
ActionSchedule actionSchedule =
new ActionSchedule.Builder("testReprepareAfterPlaybackError")
.waitForPlaybackState(Player.STATE_BUFFERING)
// Cause an internal exception by seeking to an invalid position while the media source
// is still being prepared and the player doesn't immediately know it will fail.
.seek(/* windowIndex= */ 100, /* positionMs= */ 0)
.waitForPlaybackState(Player.STATE_IDLE)
.prepareSource(
new FakeMediaSource(timeline, /* manifest= */ null),
/* resetPosition= */ false,
/* resetState= */ false)
.build();
ExoPlayerTestRunner testRunner =
new ExoPlayerTestRunner.Builder()
.setTimeline(timeline)
.setActionSchedule(actionSchedule)
.build();
try {
testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS);
fail();
} catch (ExoPlaybackException e) {
// Expected exception.
}
testRunner.assertTimelinesEqual(timeline, timeline);
testRunner.assertTimelineChangeReasonsEqual(
Player.TIMELINE_CHANGE_REASON_PREPARED, Player.TIMELINE_CHANGE_REASON_PREPARED);
}
public void testPlaybackErrorDuringSourceInfoRefreshStillUpdatesTimeline() throws Exception {
final Timeline timeline = new FakeTimeline(/* windowCount= */ 1);
final FakeMediaSource mediaSource =
new FakeMediaSource(/* timeline= */ null, /* manifest= */ null);
ActionSchedule actionSchedule =
new ActionSchedule.Builder("testPlaybackErrorDuringSourceInfoRefreshStillUpdatesTimeline")
.waitForPlaybackState(Player.STATE_BUFFERING)
// Cause an internal exception by seeking to an invalid position while the media source
// is still being prepared. The error will be thrown while the player handles the new
// source info.
.seek(/* windowIndex= */ 100, /* positionMs= */ 0)
.executeRunnable(
new Runnable() {
@Override
public void run() {
mediaSource.setNewSourceInfo(timeline, /* manifest= */ null);
}
})
.waitForPlaybackState(Player.STATE_IDLE)
.build();
ExoPlayerTestRunner testRunner =
new ExoPlayerTestRunner.Builder()
.setMediaSource(mediaSource)
.setActionSchedule(actionSchedule)
.build();
try {
testRunner.start().blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS);
fail();
} catch (ExoPlaybackException e) {
// Expected exception.
}
testRunner.assertTimelinesEqual(timeline);
testRunner.assertTimelineChangeReasonsEqual(Player.TIMELINE_CHANGE_REASON_PREPARED);
}
} }
library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
...@@ -328,7 +328,7 @@ import java.io.IOException; ...@@ -328,7 +328,7 @@ import java.io.IOException;
setSeekParametersInternal((SeekParameters) msg.obj); setSeekParametersInternal((SeekParameters) msg.obj);
return true; return true;
case MSG_STOP: case MSG_STOP:
stopInternal(/* reset= */ msg.arg1 != 0); stopInternal(/* reset= */ msg.arg1 != 0, /* acknowledgeStop= */ true);
return true; return true;
case MSG_RELEASE: case MSG_RELEASE:
releaseInternal(); releaseInternal();
...@@ -353,19 +353,19 @@ import java.io.IOException; ...@@ -353,19 +353,19 @@ import java.io.IOException;
} }
} catch (ExoPlaybackException e) { } catch (ExoPlaybackException e) {
Log.e(TAG, "Renderer error.", e); Log.e(TAG, "Renderer error.", e);
stopInternal(/* reset= */ false, /* acknowledgeStop= */ false);
eventHandler.obtainMessage(MSG_ERROR, e).sendToTarget(); eventHandler.obtainMessage(MSG_ERROR, e).sendToTarget();
stopInternal(/* reset= */ false);
return true; return true;
} catch (IOException e) { } catch (IOException e) {
Log.e(TAG, "Source error.", e); Log.e(TAG, "Source error.", e);
stopInternal(/* reset= */ false, /* acknowledgeStop= */ false);
eventHandler.obtainMessage(MSG_ERROR, ExoPlaybackException.createForSource(e)).sendToTarget(); eventHandler.obtainMessage(MSG_ERROR, ExoPlaybackException.createForSource(e)).sendToTarget();
stopInternal(/* reset= */ false);
return true; return true;
} catch (RuntimeException e) { } catch (RuntimeException e) {
Log.e(TAG, "Internal runtime error.", e); Log.e(TAG, "Internal runtime error.", e);
stopInternal(/* reset= */ false, /* acknowledgeStop= */ false);
eventHandler.obtainMessage(MSG_ERROR, ExoPlaybackException.createForUnexpected(e)) eventHandler.obtainMessage(MSG_ERROR, ExoPlaybackException.createForUnexpected(e))
.sendToTarget(); .sendToTarget();
stopInternal(/* reset= */ false);
return true; return true;
} }
} }
...@@ -635,49 +635,50 @@ import java.io.IOException; ...@@ -635,49 +635,50 @@ import java.io.IOException;
return; return;
} }
Pair<Integer, Long> periodPosition = resolveSeekPosition(seekPosition);
if (periodPosition == null) {
// The seek position was valid for the timeline that it was performed into, but the
// timeline has changed and a suitable seek position could not be resolved in the new one.
setState(Player.STATE_ENDED);
// Reset, but retain the source so that it can still be used should a seek occur.
resetInternal(
/* releaseMediaSource= */ false, /* resetPosition= */ true, /* resetState= */ false);
eventHandler
.obtainMessage(MSG_SEEK_ACK, /* seekAdjusted */ 1, 0, playbackInfo)
.sendToTarget();
return;
}
boolean seekPositionAdjusted = seekPosition.windowPositionUs == C.TIME_UNSET; boolean seekPositionAdjusted = seekPosition.windowPositionUs == C.TIME_UNSET;
int periodIndex = periodPosition.first;
long periodPositionUs = periodPosition.second;
long contentPositionUs = periodPositionUs;
MediaPeriodId periodId =
mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, periodPositionUs);
if (periodId.isAd()) {
seekPositionAdjusted = true;
periodPositionUs = 0;
}
try { try {
if (periodId.equals(playbackInfo.periodId)) { Pair<Integer, Long> periodPosition = resolveSeekPosition(seekPosition);
long adjustedPeriodPositionUs = periodPositionUs; if (periodPosition == null) {
if (playingPeriodHolder != null) { // The seek position was valid for the timeline that it was performed into, but the
adjustedPeriodPositionUs = // timeline has changed and a suitable seek position could not be resolved in the new one.
playingPeriodHolder.mediaPeriod.getAdjustedSeekPositionUs( setState(Player.STATE_ENDED);
adjustedPeriodPositionUs, SeekParameters.DEFAULT); // Reset, but retain the source so that it can still be used should a seek occur.
} resetInternal(
if ((adjustedPeriodPositionUs / 1000) == (playbackInfo.positionUs / 1000)) { /* releaseMediaSource= */ false, /* resetPosition= */ true, /* resetState= */ false);
// Seek will be performed to the current position. Do nothing. seekPositionAdjusted = true;
periodPositionUs = playbackInfo.positionUs; return;
return; }
int periodIndex = periodPosition.first;
long periodPositionUs = periodPosition.second;
long contentPositionUs = periodPositionUs;
MediaPeriodId periodId =
mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, periodPositionUs);
if (periodId.isAd()) {
seekPositionAdjusted = true;
periodPositionUs = 0;
}
try {
if (periodId.equals(playbackInfo.periodId)) {
long adjustedPeriodPositionUs = periodPositionUs;
if (playingPeriodHolder != null) {
adjustedPeriodPositionUs =
playingPeriodHolder.mediaPeriod.getAdjustedSeekPositionUs(
adjustedPeriodPositionUs, SeekParameters.DEFAULT);
}
if ((adjustedPeriodPositionUs / 1000) == (playbackInfo.positionUs / 1000)) {
// Seek will be performed to the current position. Do nothing.
periodPositionUs = playbackInfo.positionUs;
return;
}
} }
long newPeriodPositionUs = seekToPeriodPosition(periodId, periodPositionUs);
seekPositionAdjusted |= periodPositionUs != newPeriodPositionUs;
periodPositionUs = newPeriodPositionUs;
} finally {
playbackInfo = playbackInfo.fromNewPosition(periodId, periodPositionUs, contentPositionUs);
} }
long newPeriodPositionUs = seekToPeriodPosition(periodId, periodPositionUs);
seekPositionAdjusted |= periodPositionUs != newPeriodPositionUs;
periodPositionUs = newPeriodPositionUs;
} finally { } finally {
playbackInfo = playbackInfo.fromNewPosition(periodId, periodPositionUs, contentPositionUs);
eventHandler.obtainMessage(MSG_SEEK_ACK, seekPositionAdjusted ? 1 : 0, 0, playbackInfo) eventHandler.obtainMessage(MSG_SEEK_ACK, seekPositionAdjusted ? 1 : 0, 0, playbackInfo)
.sendToTarget(); .sendToTarget();
} }
...@@ -775,12 +776,10 @@ import java.io.IOException; ...@@ -775,12 +776,10 @@ import java.io.IOException;
this.seekParameters = seekParameters; this.seekParameters = seekParameters;
} }
private void stopInternal(boolean reset) { private void stopInternal(boolean reset, boolean acknowledgeStop) {
resetInternal( resetInternal(
/* releaseMediaSource= */ true, /* resetPosition= */ reset, /* resetState= */ reset); /* releaseMediaSource= */ true, /* resetPosition= */ reset, /* resetState= */ reset);
int prepareOrStopAcks = pendingPrepareCount + 1; notifySourceInfoRefresh(acknowledgeStop);
pendingPrepareCount = 0;
notifySourceInfoRefresh(prepareOrStopAcks, playbackInfo);
loadControl.onStopped(); loadControl.onStopped();
setState(Player.STATE_IDLE); setState(Player.STATE_IDLE);
} }
...@@ -1011,15 +1010,13 @@ import java.io.IOException; ...@@ -1011,15 +1010,13 @@ import java.io.IOException;
playbackInfo = playbackInfo.copyWithTimeline(timeline, manifest); playbackInfo = playbackInfo.copyWithTimeline(timeline, manifest);
if (oldTimeline == null) { if (oldTimeline == null) {
int processedPrepareAcks = pendingPrepareCount;
pendingPrepareCount = 0;
if (pendingInitialSeekPosition != null) { if (pendingInitialSeekPosition != null) {
Pair<Integer, Long> periodPosition = resolveSeekPosition(pendingInitialSeekPosition); Pair<Integer, Long> periodPosition = resolveSeekPosition(pendingInitialSeekPosition);
pendingInitialSeekPosition = null; pendingInitialSeekPosition = null;
if (periodPosition == null) { if (periodPosition == null) {
// The seek position was valid for the timeline that it was performed into, but the // The seek position was valid for the timeline that it was performed into, but the
// timeline has changed and a suitable seek position could not be resolved in the new one. // timeline has changed and a suitable seek position could not be resolved in the new one.
handleSourceInfoRefreshEndedPlayback(processedPrepareAcks); handleSourceInfoRefreshEndedPlayback();
} else { } else {
int periodIndex = periodPosition.first; int periodIndex = periodPosition.first;
long positionUs = periodPosition.second; long positionUs = periodPosition.second;
...@@ -1027,11 +1024,11 @@ import java.io.IOException; ...@@ -1027,11 +1024,11 @@ import java.io.IOException;
mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, positionUs); mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, positionUs);
playbackInfo = playbackInfo.fromNewPosition(periodId, periodId.isAd() ? 0 : positionUs, playbackInfo = playbackInfo.fromNewPosition(periodId, periodId.isAd() ? 0 : positionUs,
positionUs); positionUs);
notifySourceInfoRefresh(processedPrepareAcks); notifySourceInfoRefresh();
} }
} else if (playbackInfo.startPositionUs == C.TIME_UNSET) { } else if (playbackInfo.startPositionUs == C.TIME_UNSET) {
if (timeline.isEmpty()) { if (timeline.isEmpty()) {
handleSourceInfoRefreshEndedPlayback(processedPrepareAcks); handleSourceInfoRefreshEndedPlayback();
} else { } else {
Pair<Integer, Long> defaultPosition = getPeriodPosition(timeline, Pair<Integer, Long> defaultPosition = getPeriodPosition(timeline,
timeline.getFirstWindowIndex(shuffleModeEnabled), C.TIME_UNSET); timeline.getFirstWindowIndex(shuffleModeEnabled), C.TIME_UNSET);
...@@ -1041,10 +1038,10 @@ import java.io.IOException; ...@@ -1041,10 +1038,10 @@ import java.io.IOException;
startPositionUs); startPositionUs);
playbackInfo = playbackInfo.fromNewPosition(periodId, playbackInfo = playbackInfo.fromNewPosition(periodId,
periodId.isAd() ? 0 : startPositionUs, startPositionUs); periodId.isAd() ? 0 : startPositionUs, startPositionUs);
notifySourceInfoRefresh(processedPrepareAcks); notifySourceInfoRefresh();
} }
} else { } else {
notifySourceInfoRefresh(processedPrepareAcks); notifySourceInfoRefresh();
} }
return; return;
} }
...@@ -1171,26 +1168,20 @@ import java.io.IOException; ...@@ -1171,26 +1168,20 @@ import java.io.IOException;
} }
private void handleSourceInfoRefreshEndedPlayback() { private void handleSourceInfoRefreshEndedPlayback() {
handleSourceInfoRefreshEndedPlayback(0);
}
private void handleSourceInfoRefreshEndedPlayback(int prepareAcks) {
setState(Player.STATE_ENDED); setState(Player.STATE_ENDED);
// Reset, but retain the source so that it can still be used should a seek occur. // Reset, but retain the source so that it can still be used should a seek occur.
resetInternal( resetInternal(
/* releaseMediaSource= */ false, /* resetPosition= */ true, /* resetState= */ false); /* releaseMediaSource= */ false, /* resetPosition= */ true, /* resetState= */ false);
notifySourceInfoRefresh(prepareAcks, playbackInfo); notifySourceInfoRefresh();
} }
private void notifySourceInfoRefresh() { private void notifySourceInfoRefresh() {
notifySourceInfoRefresh(0); notifySourceInfoRefresh(/* acknowledgeStop= */ false);
} }
private void notifySourceInfoRefresh(int prepareOrStopAcks) { private void notifySourceInfoRefresh(boolean acknowledgeStop) {
notifySourceInfoRefresh(prepareOrStopAcks, playbackInfo); int prepareOrStopAcks = pendingPrepareCount + (acknowledgeStop ? 1 : 0);
} pendingPrepareCount = 0;
private void notifySourceInfoRefresh(int prepareOrStopAcks, PlaybackInfo playbackInfo) {
eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, prepareOrStopAcks, 0, playbackInfo) eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, prepareOrStopAcks, 0, playbackInfo)
.sendToTarget(); .sendToTarget();
} }
......
testutils/src/main/java/com/google/android/exoplayer2/testutil/ExoPlayerTestRunner.java
...@@ -478,9 +478,8 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener ...@@ -478,9 +478,8 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener
} }
/** /**
* Blocks the current thread until the action schedule finished. Also returns when an * Blocks the current thread until the action schedule finished. This does not release the test
* {@link ExoPlaybackException} is thrown. This does not release the test runner and the test must * runner and the test must still call {@link #blockUntilEnded(long)}.
* still call {@link #blockUntilEnded(long)}.
* *
* @param timeoutMs The maximum time to wait for the action schedule to finish. * @param timeoutMs The maximum time to wait for the action schedule to finish.
* @return This test runner. * @return This test runner.
...@@ -611,7 +610,6 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener ...@@ -611,7 +610,6 @@ public final class ExoPlayerTestRunner extends Player.DefaultEventListener
while (endedCountDownLatch.getCount() > 0) { while (endedCountDownLatch.getCount() > 0) {
endedCountDownLatch.countDown(); endedCountDownLatch.countDown();
} }
actionScheduleFinishedCountDownLatch.countDown();
} }
// Player.EventListener // Player.EventListener
......
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