Skip to content

SDK / exoplayer

Go to a project
Commit 4cf614c6 by tonihei Committed by Ian Baker
Options

Ensure tracks in PlaybackInfo are always in sync with position. 

Currently both are updated by separate setters. If an exception is thrown between
the two setters, the state may not be consistent.

Avoid this problem by always setting them together.

PiperOrigin-RevId: 290293600
parent ff89170b
Inline Side-by-side
Showing with 117 additions and 45 deletions
library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java
...@@ -802,7 +802,9 @@ import java.util.concurrent.TimeoutException; ...@@ -802,7 +802,9 @@ import java.util.concurrent.TimeoutException;
playbackInfo.periodId, playbackInfo.periodId,
/* positionUs= */ 0, /* positionUs= */ 0,
playbackInfo.contentPositionUs, playbackInfo.contentPositionUs,
playbackInfo.totalBufferedDurationUs); playbackInfo.totalBufferedDurationUs,
playbackInfo.trackGroups,
playbackInfo.trackSelectorResult);
} }
if (!this.playbackInfo.timeline.isEmpty() && playbackInfo.timeline.isEmpty()) { if (!this.playbackInfo.timeline.isEmpty() && playbackInfo.timeline.isEmpty()) {
// Update the masking variables, which are used when the timeline becomes empty. // Update the masking variables, which are used when the timeline becomes empty.
......
library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
...@@ -22,6 +22,7 @@ import android.os.Message; ...@@ -22,6 +22,7 @@ import android.os.Message;
import android.os.Process; import android.os.Process;
import android.os.SystemClock; import android.os.SystemClock;
import android.util.Pair; import android.util.Pair;
import androidx.annotation.CheckResult;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import com.google.android.exoplayer2.DefaultMediaClock.PlaybackParameterListener; import com.google.android.exoplayer2.DefaultMediaClock.PlaybackParameterListener;
import com.google.android.exoplayer2.Player.DiscontinuityReason; import com.google.android.exoplayer2.Player.DiscontinuityReason;
...@@ -999,10 +1000,8 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -999,10 +1000,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
resetRendererPosition(periodPositionUs); resetRendererPosition(periodPositionUs);
maybeContinueLoading(); maybeContinueLoading();
} else { } else {
queue.clear(/* keepFrontPeriodUid= */ true);
// New period has not been prepared. // New period has not been prepared.
playbackInfo = queue.clear(/* keepFrontPeriodUid= */ true);
playbackInfo.copyWithTrackInfo(TrackGroupArray.EMPTY, emptyTrackSelectorResult);
resetRendererPosition(periodPositionUs); resetRendererPosition(periodPositionUs);
} }
...@@ -1141,12 +1140,16 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1141,12 +1140,16 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
MediaPeriodId mediaPeriodId = playbackInfo.periodId; MediaPeriodId mediaPeriodId = playbackInfo.periodId;
long contentPositionUs = playbackInfo.contentPositionUs; long contentPositionUs = playbackInfo.contentPositionUs;
boolean resetTrackInfo = clearPlaylist;
if (resetPosition) { if (resetPosition) {
mediaPeriodId = mediaPeriodId =
timeline.isEmpty() timeline.isEmpty()
? playbackInfo.getDummyPeriodForEmptyTimeline() ? playbackInfo.getDummyPeriodForEmptyTimeline()
: getDummyFirstMediaPeriodForAds(); : getDummyFirstMediaPeriodForAds();
contentPositionUs = C.TIME_UNSET; contentPositionUs = C.TIME_UNSET;
if (!mediaPeriodId.equals(playbackInfo.periodId)) {
resetTrackInfo = true;
}
} }
// Set the start position to TIME_UNSET so that a subsequent seek to 0 isn't ignored. // Set the start position to TIME_UNSET so that a subsequent seek to 0 isn't ignored.
long startPositionUs = resetPosition ? C.TIME_UNSET : playbackInfo.positionUs; long startPositionUs = resetPosition ? C.TIME_UNSET : playbackInfo.positionUs;
...@@ -1159,8 +1162,8 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1159,8 +1162,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
playbackInfo.playbackState, playbackInfo.playbackState,
resetError ? null : playbackInfo.playbackError, resetError ? null : playbackInfo.playbackError,
/* isLoading= */ false, /* isLoading= */ false,
clearPlaylist ? TrackGroupArray.EMPTY : playbackInfo.trackGroups, resetTrackInfo ? TrackGroupArray.EMPTY : playbackInfo.trackGroups,
clearPlaylist ? emptyTrackSelectorResult : playbackInfo.trackSelectorResult, resetTrackInfo ? emptyTrackSelectorResult : playbackInfo.trackSelectorResult,
mediaPeriodId, mediaPeriodId,
startPositionUs, startPositionUs,
/* totalBufferedDurationUs= */ 0, /* totalBufferedDurationUs= */ 0,
...@@ -1390,11 +1393,11 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1390,11 +1393,11 @@ import java.util.concurrent.atomic.AtomicBoolean;
long periodPositionUs = long periodPositionUs =
playingPeriodHolder.applyTrackSelection( playingPeriodHolder.applyTrackSelection(
newTrackSelectorResult, playbackInfo.positionUs, recreateStreams, streamResetFlags); newTrackSelectorResult, playbackInfo.positionUs, recreateStreams, streamResetFlags);
playbackInfo =
copyWithNewPosition(
playbackInfo.periodId, periodPositionUs, playbackInfo.contentPositionUs);
if (playbackInfo.playbackState != Player.STATE_ENDED if (playbackInfo.playbackState != Player.STATE_ENDED
&& periodPositionUs != playbackInfo.positionUs) { && periodPositionUs != playbackInfo.positionUs) {
playbackInfo =
copyWithNewPosition(
playbackInfo.periodId, periodPositionUs, playbackInfo.contentPositionUs);
playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL); playbackInfoUpdate.setPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL);
resetRendererPosition(periodPositionUs); resetRendererPosition(periodPositionUs);
} }
...@@ -1418,9 +1421,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1418,9 +1421,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
} }
} }
playbackInfo =
playbackInfo.copyWithTrackInfo(
playingPeriodHolder.getTrackGroups(), playingPeriodHolder.getTrackSelectorResult());
enableRenderers(rendererWasEnabledFlags, enabledRendererCount); enableRenderers(rendererWasEnabledFlags, enabledRendererCount);
} else { } else {
// Release and re-prepare/buffer periods after the one whose selection changed. // Release and re-prepare/buffer periods after the one whose selection changed.
...@@ -1883,7 +1883,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1883,7 +1883,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
boolean[] rendererWasEnabledFlags = new boolean[renderers.length]; boolean[] rendererWasEnabledFlags = new boolean[renderers.length];
disablePlayingPeriodRenderersForTransition(rendererWasEnabledFlags); disablePlayingPeriodRenderersForTransition(rendererWasEnabledFlags);
MediaPeriodHolder newPlayingPeriodHolder = queue.advancePlayingPeriod(); MediaPeriodHolder newPlayingPeriodHolder = queue.advancePlayingPeriod();
enablePlayingPeriodRenderers(rendererWasEnabledFlags);
playbackInfo = playbackInfo =
copyWithNewPosition( copyWithNewPosition(
newPlayingPeriodHolder.info.id, newPlayingPeriodHolder.info.id,
...@@ -1894,6 +1893,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1894,6 +1893,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
? Player.DISCONTINUITY_REASON_PERIOD_TRANSITION ? Player.DISCONTINUITY_REASON_PERIOD_TRANSITION
: Player.DISCONTINUITY_REASON_AD_INSERTION; : Player.DISCONTINUITY_REASON_AD_INSERTION;
playbackInfoUpdate.setPositionDiscontinuity(discontinuityReason); playbackInfoUpdate.setPositionDiscontinuity(discontinuityReason);
enablePlayingPeriodRenderers(rendererWasEnabledFlags);
updatePlaybackPositions(); updatePlaybackPositions();
advancedPlayingPeriod = true; advancedPlayingPeriod = true;
} }
...@@ -1957,6 +1957,9 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1957,6 +1957,9 @@ import java.util.concurrent.atomic.AtomicBoolean;
// This is the first prepared period, so update the position and the renderers. // This is the first prepared period, so update the position and the renderers.
resetRendererPosition(loadingPeriodHolder.info.startPositionUs); resetRendererPosition(loadingPeriodHolder.info.startPositionUs);
enablePlayingPeriodRenderers(); enablePlayingPeriodRenderers();
playbackInfo =
copyWithNewPosition(
playbackInfo.periodId, playbackInfo.positionUs, playbackInfo.contentPositionUs);
} }
maybeContinueLoading(); maybeContinueLoading();
} }
...@@ -2024,11 +2027,35 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -2024,11 +2027,35 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
} }
@CheckResult
private PlaybackInfo copyWithNewPosition( private PlaybackInfo copyWithNewPosition(
MediaPeriodId mediaPeriodId, long positionUs, long contentPositionUs) { MediaPeriodId mediaPeriodId, long positionUs, long contentPositionUs) {
deliverPendingMessageAtStartPositionRequired = true; deliverPendingMessageAtStartPositionRequired =
positionUs != playbackInfo.positionUs || !mediaPeriodId.equals(playbackInfo.periodId);
TrackGroupArray trackGroupArray = playbackInfo.trackGroups;
TrackSelectorResult trackSelectorResult = playbackInfo.trackSelectorResult;
if (playlist.isPrepared()) {
@Nullable MediaPeriodHolder playingPeriodHolder = queue.getPlayingPeriod();
trackGroupArray =
playingPeriodHolder == null
? TrackGroupArray.EMPTY
: playingPeriodHolder.getTrackGroups();
trackSelectorResult =
playingPeriodHolder == null
? emptyTrackSelectorResult
: playingPeriodHolder.getTrackSelectorResult();
} else if (!mediaPeriodId.equals(playbackInfo.periodId)) {
// Reset previously kept track info if unprepared and the period changes.
trackGroupArray = TrackGroupArray.EMPTY;
trackSelectorResult = emptyTrackSelectorResult;
}
return playbackInfo.copyWithNewPosition( return playbackInfo.copyWithNewPosition(
mediaPeriodId, positionUs, contentPositionUs, getTotalBufferedDurationUs()); mediaPeriodId,
positionUs,
contentPositionUs,
getTotalBufferedDurationUs(),
trackGroupArray,
trackSelectorResult);
} }
private void disablePlayingPeriodRenderersForTransition(boolean[] outRendererWasEnabledFlags) private void disablePlayingPeriodRenderersForTransition(boolean[] outRendererWasEnabledFlags)
...@@ -2058,9 +2085,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -2058,9 +2085,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
private void enablePlayingPeriodRenderers(boolean[] rendererWasEnabledFlags) private void enablePlayingPeriodRenderers(boolean[] rendererWasEnabledFlags)
throws ExoPlaybackException { throws ExoPlaybackException {
MediaPeriodHolder playingPeriodHolder = Assertions.checkNotNull(queue.getPlayingPeriod()); MediaPeriodHolder playingPeriodHolder = Assertions.checkNotNull(queue.getPlayingPeriod());
playbackInfo =
playbackInfo.copyWithTrackInfo(
playingPeriodHolder.getTrackGroups(), playingPeriodHolder.getTrackSelectorResult());
int enabledRendererCount = 0; int enabledRendererCount = 0;
for (int i = 0; i < renderers.length; i++) { for (int i = 0; i < renderers.length; i++) {
if (playingPeriodHolder.getTrackSelectorResult().isRendererEnabled(i)) { if (playingPeriodHolder.getTrackSelectorResult().isRendererEnabled(i)) {
......
library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java
...@@ -191,6 +191,9 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; ...@@ -191,6 +191,9 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
* @param contentPositionUs New content position. See {@link #contentPositionUs}. Value is ignored * @param contentPositionUs New content position. See {@link #contentPositionUs}. Value is ignored
* if {@code periodId.isAd()} is true. * if {@code periodId.isAd()} is true.
* @param totalBufferedDurationUs New buffered duration. See {@link #totalBufferedDurationUs}. * @param totalBufferedDurationUs New buffered duration. See {@link #totalBufferedDurationUs}.
* @param trackGroups The track groups for the new position. See {@link #trackGroups}.
* @param trackSelectorResult The track selector result for the new position. See {@link
* #trackSelectorResult}.
* @return Copied playback info with new playing position. * @return Copied playback info with new playing position.
*/ */
@CheckResult @CheckResult
...@@ -198,7 +201,9 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; ...@@ -198,7 +201,9 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
MediaPeriodId periodId, MediaPeriodId periodId,
long positionUs, long positionUs,
long contentPositionUs, long contentPositionUs,
long totalBufferedDurationUs) { long totalBufferedDurationUs,
TrackGroupArray trackGroups,
TrackSelectorResult trackSelectorResult) {
return new PlaybackInfo( return new PlaybackInfo(
timeline, timeline,
periodId, periodId,
...@@ -312,32 +317,6 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult; ...@@ -312,32 +317,6 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
} }
/** /**
* Copies playback info with new track information.
*
* @param trackGroups New track groups. See {@link #trackGroups}.
* @param trackSelectorResult New track selector result. See {@link #trackSelectorResult}.
* @return Copied playback info with new track information.
*/
@CheckResult
public PlaybackInfo copyWithTrackInfo(
TrackGroupArray trackGroups, TrackSelectorResult trackSelectorResult) {
return new PlaybackInfo(
timeline,
periodId,
startPositionUs,
contentPositionUs,
playbackState,
playbackError,
isLoading,
trackGroups,
trackSelectorResult,
loadingMediaPeriodId,
bufferedPositionUs,
totalBufferedDurationUs,
positionUs);
}
/**
* Copies playback info with new loading media period. * Copies playback info with new loading media period.
* *
* @param loadingMediaPeriodId New loading media period id. See {@link #loadingMediaPeriodId}. * @param loadingMediaPeriodId New loading media period id. See {@link #loadingMediaPeriodId}.
......
library/core/src/test/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 static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.robolectric.Shadows.shadowOf; import static org.robolectric.Shadows.shadowOf;
...@@ -5591,6 +5592,72 @@ public final class ExoPlayerTest { ...@@ -5591,6 +5592,72 @@ public final class ExoPlayerTest {
assertArrayEquals(new int[] {1, 0}, currentWindowIndices); assertArrayEquals(new int[] {1, 0}, currentWindowIndices);
} }
@Test
public void errorThrownDuringPeriodTransition_keepsConsistentPlayerState() throws Exception {
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 period transition.
throw createRendererException(new IllegalStateException(), Builder.AUDIO_FORMAT);
}
};
AtomicReference<TrackGroupArray> trackGroupsAfterError = new AtomicReference<>();
AtomicReference<TrackSelectionArray> trackSelectionsAfterError = new AtomicReference<>();
AtomicInteger windowIndexAfterError = new AtomicInteger();
ActionSchedule actionSchedule =
new ActionSchedule.Builder("errorThrownDuringPeriodTransition_keepsConsistentPlayerState")
.executeRunnable(
new PlayerRunnable() {
@Override
public void run(SimpleExoPlayer player) {
player.addAnalyticsListener(
new AnalyticsListener() {
@Override
public void onPlayerError(
EventTime eventTime, ExoPlaybackException error) {
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)
.play()
.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(windowIndexAfterError.get()).isEqualTo(1);
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. // Internal methods.
private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) { private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {
......
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