Ensure timeline and position are updated even if an error occurs
If an exception is thrown between updating the timeline and updating the position in playbackInfo, the state may be inconsistent. Exceptions are expected to be thrown while updating the player state and we should handle such cases in a consistent way. Similar to how we handle the same situation in seekToInternal, the state is updated in a final block such that it gets updated to the latest state even if an error occurs. Moving both the timeline and position update together also ensures they always stay consistent. PiperOrigin-RevId: 290624020
This commit is contained in:
tonihei
Ian Baker
parent
21c76b012c
commit
8a2a527129
@ -1535,69 +1535,59 @@ import java.util.concurrent.atomic.AtomicBoolean;
|
||||
shuffleModeEnabled,
|
||||
window,
|
||||
period);
|
||||
|
||||
playbackInfo = playbackInfo.copyWithTimeline(timeline);
|
||||
resolvePendingMessagePositions();
|
||||
if (!timeline.isEmpty()) {
|
||||
// Retain pending seek position only while the timeline is still empty.
|
||||
pendingInitialSeekPosition = null;
|
||||
}
|
||||
|
||||
MediaPeriodId oldPeriodId = playbackInfo.periodId;
|
||||
long oldContentPositionUs =
|
||||
oldPeriodId.isAd() ? playbackInfo.contentPositionUs : playbackInfo.positionUs;
|
||||
MediaPeriodId newPeriodId = positionUpdate.periodId;
|
||||
long newContentPositionUs = positionUpdate.contentPositionUs;
|
||||
boolean forceBufferingState = positionUpdate.forceBufferingState;
|
||||
long newPositionUs = newPeriodId.isAd() ? 0 : newContentPositionUs;
|
||||
long oldContentPositionUs =
|
||||
playbackInfo.periodId.isAd() ? playbackInfo.contentPositionUs : playbackInfo.positionUs;
|
||||
boolean isPlaybackPositionUnchanged =
|
||||
playbackInfo.periodId.equals(newPeriodId) && oldContentPositionUs == newContentPositionUs;
|
||||
|
||||
if (positionUpdate.endPlayback) {
|
||||
if (playbackInfo.playbackState != Player.STATE_IDLE) {
|
||||
setState(Player.STATE_ENDED);
|
||||
try {
|
||||
if (positionUpdate.endPlayback) {
|
||||
if (playbackInfo.playbackState != Player.STATE_IDLE) {
|
||||
setState(Player.STATE_ENDED);
|
||||
}
|
||||
resetInternal(
|
||||
/* resetRenderers= */ false,
|
||||
/* resetPosition= */ false,
|
||||
/* releasePlaylist= */ false,
|
||||
/* clearPlaylist= */ false,
|
||||
/* resetError= */ true);
|
||||
}
|
||||
playbackInfo = copyWithNewPosition(newPeriodId, newPositionUs, newContentPositionUs);
|
||||
// Reset, but retain the playlist and new position.
|
||||
resetInternal(
|
||||
/* resetRenderers= */ false,
|
||||
/* resetPosition= */ false,
|
||||
/* releasePlaylist= */ false,
|
||||
/* clearPlaylist= */ false,
|
||||
/* resetError= */ true);
|
||||
if (timeline.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (oldPeriodId.equals(newPeriodId) && oldContentPositionUs == newContentPositionUs) {
|
||||
// We can keep the current playing period. Update the rest of the queued periods.
|
||||
if (!queue.updateQueuedPeriods(
|
||||
timeline, rendererPositionUs, getMaxRendererReadPositionUs())) {
|
||||
seekToCurrentPosition(/* sendDiscontinuity= */ false);
|
||||
}
|
||||
} else {
|
||||
// Something changed. Seek to new start position.
|
||||
@Nullable MediaPeriodHolder periodHolder = queue.getPlayingPeriod();
|
||||
if (periodHolder != null) {
|
||||
// Update the new playing media period info if it already exists.
|
||||
while (periodHolder.getNext() != null) {
|
||||
periodHolder = periodHolder.getNext();
|
||||
if (periodHolder.info.id.equals(newPeriodId)) {
|
||||
periodHolder.info = queue.getUpdatedMediaPeriodInfo(timeline, periodHolder.info);
|
||||
if (isPlaybackPositionUnchanged) {
|
||||
// We can keep the current playing period. Update the rest of the queued periods.
|
||||
if (!queue.updateQueuedPeriods(
|
||||
timeline, rendererPositionUs, getMaxRendererReadPositionUs())) {
|
||||
seekToCurrentPosition(/* sendDiscontinuity= */ false);
|
||||
}
|
||||
} else if (!timeline.isEmpty()) {
|
||||
// Something changed. Seek to new start position.
|
||||
@Nullable MediaPeriodHolder periodHolder = queue.getPlayingPeriod();
|
||||
if (periodHolder != null) {
|
||||
// Update the new playing media period info if it already exists.
|
||||
while (periodHolder.getNext() != null) {
|
||||
periodHolder = periodHolder.getNext();
|
||||
if (periodHolder.info.id.equals(newPeriodId)) {
|
||||
periodHolder.info = queue.getUpdatedMediaPeriodInfo(timeline, periodHolder.info);
|
||||
}
|
||||
}
|
||||
}
|
||||
newPositionUs = seekToPeriodPosition(newPeriodId, newPositionUs, forceBufferingState);
|
||||
}
|
||||
if (!newPeriodId.isAd() && newContentPositionUs == C.TIME_UNSET) {
|
||||
// Get the default position for the first new period that is not an ad.
|
||||
int windowIndex = timeline.getPeriodByUid(newPeriodId.periodUid, period).windowIndex;
|
||||
newContentPositionUs = timeline.getWindow(windowIndex, window).getDefaultPositionUs();
|
||||
newPositionUs = newContentPositionUs;
|
||||
} finally {
|
||||
if (!isPlaybackPositionUnchanged) {
|
||||
playbackInfo = copyWithNewPosition(newPeriodId, newPositionUs, newContentPositionUs);
|
||||
}
|
||||
// Actually do the seek.
|
||||
long seekedToPositionUs =
|
||||
seekToPeriodPosition(newPeriodId, newPositionUs, forceBufferingState);
|
||||
playbackInfo = copyWithNewPosition(newPeriodId, seekedToPositionUs, newContentPositionUs);
|
||||
playbackInfo = playbackInfo.copyWithTimeline(timeline);
|
||||
resolvePendingMessagePositions();
|
||||
if (!timeline.isEmpty()) {
|
||||
// Retain pending seek position only while the timeline is still empty.
|
||||
pendingInitialSeekPosition = null;
|
||||
}
|
||||
handleLoadingMediaPeriodChanged(/* loadingTrackSelectionChanged= */ false);
|
||||
}
|
||||
handleLoadingMediaPeriodChanged(/* loadingTrackSelectionChanged= */ false);
|
||||
}
|
||||
|
||||
private long getMaxRendererReadPositionUs() {
|
||||
@ -2178,12 +2168,17 @@ import java.util.concurrent.atomic.AtomicBoolean;
|
||||
// Ensure ad insertion metadata is up to date.
|
||||
long contentPositionForAdResolution = newContentPositionUs;
|
||||
if (contentPositionForAdResolution == C.TIME_UNSET) {
|
||||
// TODO: Fix me. Using a window position as period position is wrong.
|
||||
contentPositionForAdResolution =
|
||||
timeline.getWindow(timeline.getPeriodByUid(newPeriodUid, period).windowIndex, window)
|
||||
.defaultPositionUs;
|
||||
}
|
||||
MediaPeriodId periodIdWithAds =
|
||||
queue.resolveMediaPeriodIdForAds(timeline, newPeriodUid, contentPositionForAdResolution);
|
||||
if (!periodIdWithAds.isAd() && newContentPositionUs == C.TIME_UNSET) {
|
||||
// We are not going to play an ad, so use resolved content position.
|
||||
newContentPositionUs = contentPositionForAdResolution;
|
||||
}
|
||||
boolean oldAndNewPeriodIdAreSame =
|
||||
oldPeriodId.periodUid.equals(newPeriodUid)
|
||||
&& !oldPeriodId.isAd()
|
||||
|
||||
@ -5593,7 +5593,7 @@ public final class ExoPlayerTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void errorThrownDuringPeriodTransition_keepsConsistentPlayerState() throws Exception {
|
||||
public void errorThrownDuringPeriodTransition_keepsConsistentPlayerState() {
|
||||
FakeMediaSource source1 =
|
||||
new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), Builder.VIDEO_FORMAT);
|
||||
FakeMediaSource source2 =
|
||||
@ -5658,6 +5658,75 @@ public final class ExoPlayerTest {
|
||||
assertThat(trackSelectionsAfterError.get().get(1)).isNotNull(); // Audio renderer.
|
||||
}
|
||||
|
||||
@Test
|
||||
public void errorThrownDuringPlaylistUpdate_keepsConsistentPlayerState() {
|
||||
FakeMediaSource source1 =
|
||||
new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), Builder.VIDEO_FORMAT);
|
||||
FakeMediaSource source2 =
|
||||
new FakeMediaSource(new FakeTimeline(/* windowCount= */ 1), Builder.AUDIO_FORMAT);
|
||||
FakeRenderer videoRenderer = new FakeRenderer(Builder.VIDEO_FORMAT);
|
||||
FakeRenderer audioRenderer =
|
||||
new FakeRenderer(Builder.AUDIO_FORMAT) {
|
||||
@Override
|
||||
protected void onEnabled(boolean joining) throws ExoPlaybackException {
|
||||
// Fail when enabling the renderer. This will happen during the playlist update.
|
||||
throw createRendererException(new IllegalStateException(), Builder.AUDIO_FORMAT);
|
||||
}
|
||||
};
|
||||
AtomicReference<Timeline> timelineAfterError = new AtomicReference<>();
|
||||
AtomicReference<TrackGroupArray> trackGroupsAfterError = new AtomicReference<>();
|
||||
AtomicReference<TrackSelectionArray> trackSelectionsAfterError = new AtomicReference<>();
|
||||
AtomicInteger windowIndexAfterError = new AtomicInteger();
|
||||
ActionSchedule actionSchedule =
|
||||
new ActionSchedule.Builder("errorThrownDuringPlaylistUpdate_keepsConsistentPlayerState")
|
||||
.executeRunnable(
|
||||
new PlayerRunnable() {
|
||||
@Override
|
||||
public void run(SimpleExoPlayer player) {
|
||||
player.addAnalyticsListener(
|
||||
new AnalyticsListener() {
|
||||
@Override
|
||||
public void onPlayerError(
|
||||
EventTime eventTime, ExoPlaybackException error) {
|
||||
timelineAfterError.set(player.getCurrentTimeline());
|
||||
trackGroupsAfterError.set(player.getCurrentTrackGroups());
|
||||
trackSelectionsAfterError.set(player.getCurrentTrackSelections());
|
||||
windowIndexAfterError.set(player.getCurrentWindowIndex());
|
||||
}
|
||||
});
|
||||
}
|
||||
})
|
||||
.pause()
|
||||
// Wait until fully buffered so that the new renderer can be enabled immediately.
|
||||
.waitForIsLoading(true)
|
||||
.waitForIsLoading(false)
|
||||
.waitForIsLoading(true)
|
||||
.waitForIsLoading(false)
|
||||
.removeMediaItem(0)
|
||||
.build();
|
||||
ExoPlayerTestRunner testRunner =
|
||||
new Builder()
|
||||
.setMediaSources(source1, source2)
|
||||
.setActionSchedule(actionSchedule)
|
||||
.setRenderers(videoRenderer, audioRenderer)
|
||||
.build(context);
|
||||
|
||||
assertThrows(
|
||||
ExoPlaybackException.class,
|
||||
() ->
|
||||
testRunner
|
||||
.start(/* doPrepare= */ true)
|
||||
.blockUntilActionScheduleFinished(TIMEOUT_MS)
|
||||
.blockUntilEnded(TIMEOUT_MS));
|
||||
|
||||
assertThat(timelineAfterError.get().getWindowCount()).isEqualTo(1);
|
||||
assertThat(windowIndexAfterError.get()).isEqualTo(0);
|
||||
assertThat(trackGroupsAfterError.get().length).isEqualTo(1);
|
||||
assertThat(trackGroupsAfterError.get().get(0).getFormat(0)).isEqualTo(Builder.AUDIO_FORMAT);
|
||||
assertThat(trackSelectionsAfterError.get().get(0)).isNull(); // Video renderer.
|
||||
assertThat(trackSelectionsAfterError.get().get(1)).isNotNull(); // Audio renderer.
|
||||
}
|
||||
|
||||
// Internal methods.
|
||||
|
||||
private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user