Skip to content

Commit

Permalink
Clear one-off events from state as soon as they are triggered.
Browse files Browse the repository at this point in the history
This ensures they are not accidentally triggered again when
the state is rebuilt with a buildUpon method.

PiperOrigin-RevId: 495280711
  • Loading branch information
tonihei authored and icbaker committed Dec 15, 2022
1 parent 254842b commit fa5aaf9
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2891,6 +2891,15 @@ private void updateStateAndInformListeners(State newState) {
// Assign new state immediately such that all getters return the right values, but use a
// snapshot of the previous and new state so that listener invocations are triggered correctly.
this.state = newState;
if (newState.hasPositionDiscontinuity || newState.newlyRenderedFirstFrame) {
// Clear one-time events to avoid signalling them again later.
this.state =
this.state
.buildUpon()
.clearPositionDiscontinuity()
.setNewlyRenderedFirstFrame(false)
.build();
}

boolean playWhenReadyChanged = previousState.playWhenReady != newState.playWhenReady;
boolean playbackStateChanged = previousState.playbackState != newState.playbackState;
Expand All @@ -2917,7 +2926,7 @@ private void updateStateAndInformListeners(State newState) {
PositionInfo positionInfo =
getPositionInfo(
newState,
/* useDiscontinuityPosition= */ state.hasPositionDiscontinuity,
/* useDiscontinuityPosition= */ newState.hasPositionDiscontinuity,
window,
period);
listeners.queueEvent(
Expand All @@ -2931,9 +2940,9 @@ private void updateStateAndInformListeners(State newState) {
if (mediaItemTransitionReason != C.INDEX_UNSET) {
@Nullable
MediaItem mediaItem =
state.timeline.isEmpty()
newState.timeline.isEmpty()
? null
: state.playlist.get(state.currentMediaItemIndex).mediaItem;
: newState.playlist.get(state.currentMediaItemIndex).mediaItem;
listeners.queueEvent(
Player.EVENT_MEDIA_ITEM_TRANSITION,
listener -> listener.onMediaItemTransition(mediaItem, mediaItemTransitionReason));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1704,6 +1704,82 @@ protected State getState() {
verify(listener, never()).onMediaItemTransition(any(), anyInt());
}

@SuppressWarnings("deprecation") // Verifying deprecated listener call.
@Test
public void invalidateStateAndOtherOperation_withDiscontinuity_reportsDiscontinuityOnlyOnce() {
State state =
new State.Builder()
.setAvailableCommands(new Commands.Builder().addAllCommands().build())
.setPlaylist(
ImmutableList.of(new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ 0).build()))
.setPositionDiscontinuity(
Player.DISCONTINUITY_REASON_INTERNAL, /* discontinuityPositionMs= */ 2000)
.build();
SimpleBasePlayer player =
new SimpleBasePlayer(Looper.myLooper()) {
@Override
protected State getState() {
return state;
}

@Override
protected ListenableFuture<?> handlePrepare() {
// We just care about the placeholder state, so return an unfulfilled future.
return SettableFuture.create();
}
};
Listener listener = mock(Listener.class);
player.addListener(listener);

player.invalidateState();
player.prepare();

// Assert listener calls (in particular getting only a single discontinuity).
verify(listener)
.onPositionDiscontinuity(any(), any(), eq(Player.DISCONTINUITY_REASON_INTERNAL));
verify(listener).onPositionDiscontinuity(Player.DISCONTINUITY_REASON_INTERNAL);
verify(listener).onPlaybackStateChanged(Player.STATE_BUFFERING);
verify(listener).onPlayerStateChanged(/* playWhenReady= */ false, Player.STATE_BUFFERING);
verifyNoMoreInteractions(listener);
}

@SuppressWarnings("deprecation") // Verifying deprecated listener call.
@Test
public void
invalidateStateAndOtherOperation_withRenderedFirstFrame_reportsRenderedFirstFrameOnlyOnce() {
State state =
new State.Builder()
.setAvailableCommands(new Commands.Builder().addAllCommands().build())
.setPlaylist(
ImmutableList.of(new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ 0).build()))
.setNewlyRenderedFirstFrame(true)
.build();
SimpleBasePlayer player =
new SimpleBasePlayer(Looper.myLooper()) {
@Override
protected State getState() {
return state;
}

@Override
protected ListenableFuture<?> handlePrepare() {
// We just care about the placeholder state, so return an unfulfilled future.
return SettableFuture.create();
}
};
Listener listener = mock(Listener.class);
player.addListener(listener);

player.invalidateState();
player.prepare();

// Assert listener calls (in particular getting only a single rendered first frame).
verify(listener).onRenderedFirstFrame();
verify(listener).onPlaybackStateChanged(Player.STATE_BUFFERING);
verify(listener).onPlayerStateChanged(/* playWhenReady= */ false, Player.STATE_BUFFERING);
verifyNoMoreInteractions(listener);
}

@Test
public void invalidateState_duringAsyncMethodHandling_isIgnored() {
State state1 =
Expand Down

0 comments on commit fa5aaf9

Please sign in to comment.