Notify listeners of error changes when masking in MediaControllerImplBase

Currently, the implementation of `MediaControllerImplBase` differs from `ExoPlayerImpl`. The listeners of the former are notified of player error changes only in `onPlayerInfoChanged` and not `updatePlayerInfo` (masking method). Whereas `ExoPlayerImpl` has one unified method - `updatePlaybackInfo` - which sends the events to all the available listeners.

This change fixes the lack of 2 particular callbacks - `onPlayerErrorChanged` and `onPlayerError`, however, there might be more differences. Ideally, there should be a unified method for oldPlayerInfo/newPlayerInfo comparison-update-notify-listeners flow.

Issue: androidx/media#449

#minor-release

PiperOrigin-RevId: 539961618
(cherry picked from commit 4b5a4577905d86ec6e24a92a088a65689c7be901)
This commit is contained in:
jbibik 2023-06-13 15:46:46 +01:00 committed by Tianyi Feng
parent 6615399861
commit 9dec0d308b
2 changed files with 33 additions and 10 deletions

View File

@ -421,13 +421,13 @@ import org.checkerframework.checker.nullness.qual.NonNull;
(iSession, seq) -> iSession.prepare(controllerStub, seq)); (iSession, seq) -> iSession.prepare(controllerStub, seq));
if (playerInfo.playbackState == Player.STATE_IDLE) { if (playerInfo.playbackState == Player.STATE_IDLE) {
PlayerInfo playerInfo = PlayerInfo newPlayerInfo =
this.playerInfo.copyWithPlaybackState( this.playerInfo.copyWithPlaybackState(
this.playerInfo.timeline.isEmpty() ? Player.STATE_ENDED : Player.STATE_BUFFERING, this.playerInfo.timeline.isEmpty() ? Player.STATE_ENDED : Player.STATE_BUFFERING,
/* playerError= */ null); /* playerError= */ null);
updatePlayerInfo( updatePlayerInfo(
playerInfo, newPlayerInfo,
/* ignored */ Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED, /* ignored */ Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED,
/* ignored */ Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST, /* ignored */ Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST,
/* positionDiscontinuity= */ false, /* positionDiscontinuity= */ false,
@ -2152,11 +2152,11 @@ import org.checkerframework.checker.nullness.qual.NonNull;
// Update position and then stop estimating until a new positionInfo arrives from the player. // Update position and then stop estimating until a new positionInfo arrives from the player.
maybeUpdateCurrentPositionMs(); maybeUpdateCurrentPositionMs();
lastSetPlayWhenReadyCalledTimeMs = SystemClock.elapsedRealtime(); lastSetPlayWhenReadyCalledTimeMs = SystemClock.elapsedRealtime();
PlayerInfo playerInfo = PlayerInfo newPlayerInfo =
this.playerInfo.copyWithPlayWhenReady( this.playerInfo.copyWithPlayWhenReady(
playWhenReady, playWhenReadyChangeReason, playbackSuppressionReason); playWhenReady, playWhenReadyChangeReason, playbackSuppressionReason);
updatePlayerInfo( updatePlayerInfo(
playerInfo, newPlayerInfo,
/* ignored */ Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED, /* ignored */ Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED,
playWhenReadyChangeReason, playWhenReadyChangeReason,
/* positionDiscontinuity= */ false, /* positionDiscontinuity= */ false,
@ -2199,6 +2199,21 @@ import org.checkerframework.checker.nullness.qual.NonNull;
/* eventFlag= */ Player.EVENT_TIMELINE_CHANGED, /* eventFlag= */ Player.EVENT_TIMELINE_CHANGED,
listener -> listener.onTimelineChanged(newPlayerInfo.timeline, timelineChangeReason)); listener -> listener.onTimelineChanged(newPlayerInfo.timeline, timelineChangeReason));
} }
PlaybackException oldPlayerError = oldPlayerInfo.playerError;
PlaybackException newPlayerError = newPlayerInfo.playerError;
boolean errorsMatch =
oldPlayerError == newPlayerError
|| (oldPlayerError != null && oldPlayerError.errorInfoEquals(newPlayerError));
if (!errorsMatch) {
listeners.queueEvent(
/* eventFlag= */ Player.EVENT_PLAYER_ERROR,
listener -> listener.onPlayerErrorChanged(newPlayerError));
if (newPlayerError != null) {
listeners.queueEvent(
/* eventFlag= */ Player.EVENT_PLAYER_ERROR,
listener -> listener.onPlayerError(newPlayerError));
}
}
if (oldPlayerInfo.playbackState != newPlayerInfo.playbackState) { if (oldPlayerInfo.playbackState != newPlayerInfo.playbackState) {
listeners.queueEvent( listeners.queueEvent(
/* eventFlag= */ Player.EVENT_PLAYBACK_STATE_CHANGED, /* eventFlag= */ Player.EVENT_PLAYBACK_STATE_CHANGED,

View File

@ -630,9 +630,10 @@ public class MediaControllerStateMaskingTest {
remoteSession.setPlayer(playerConfig); remoteSession.setPlayer(playerConfig);
MediaController controller = controllerTestRule.createController(remoteSession.getToken()); MediaController controller = controllerTestRule.createController(remoteSession.getToken());
CountDownLatch latch = new CountDownLatch(2); CountDownLatch latch = new CountDownLatch(3);
AtomicInteger playbackStateFromCallbackRef = new AtomicInteger(); AtomicInteger playbackStateFromCallbackRef = new AtomicInteger();
AtomicReference<Player.Events> onEventsRef = new AtomicReference<>(); AtomicReference<Player.Events> onEventsRef = new AtomicReference<>();
AtomicReference<PlaybackException> playerErrorFromCallbackRef = new AtomicReference<>();
Player.Listener listener = Player.Listener listener =
new Player.Listener() { new Player.Listener() {
@Override @Override
@ -646,25 +647,32 @@ public class MediaControllerStateMaskingTest {
onEventsRef.set(events); onEventsRef.set(events);
latch.countDown(); latch.countDown();
} }
@Override
public void onPlayerErrorChanged(@Nullable PlaybackException error) {
playerErrorFromCallbackRef.set(error);
latch.countDown();
}
}; };
threadTestRule.getHandler().postAndSync(() -> controller.addListener(listener)); threadTestRule.getHandler().postAndSync(() -> controller.addListener(listener));
AtomicInteger playbackStateFromGetterRef = new AtomicInteger(); AtomicInteger playbackStateFromGetterRef = new AtomicInteger();
AtomicReference<PlaybackException> playerErrorRef = new AtomicReference<>(); AtomicReference<PlaybackException> playerErrorFromGetterRef = new AtomicReference<>();
threadTestRule threadTestRule
.getHandler() .getHandler()
.postAndSync( .postAndSync(
() -> { () -> {
controller.prepare(); controller.prepare();
playbackStateFromGetterRef.set(controller.getPlaybackState()); playbackStateFromGetterRef.set(controller.getPlaybackState());
playerErrorRef.set(controller.getPlayerError()); playerErrorFromGetterRef.set(controller.getPlayerError());
}); });
assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue(); assertThat(latch.await(TIMEOUT_MS, MILLISECONDS)).isTrue();
assertThat(playbackStateFromCallbackRef.get()).isEqualTo(testPlaybackState); assertThat(playbackStateFromCallbackRef.get()).isEqualTo(testPlaybackState);
assertThat(getEventsAsList(onEventsRef.get()))
.containsExactly(Player.EVENT_PLAYBACK_STATE_CHANGED);
assertThat(playbackStateFromGetterRef.get()).isEqualTo(testPlaybackState); assertThat(playbackStateFromGetterRef.get()).isEqualTo(testPlaybackState);
assertThat(playerErrorRef.get()).isNull(); assertThat(playerErrorFromGetterRef.get()).isNull();
assertThat(playerErrorFromCallbackRef.get()).isNull();
assertThat(getEventsAsList(onEventsRef.get()))
.containsExactly(Player.EVENT_PLAYBACK_STATE_CHANGED, Player.EVENT_PLAYER_ERROR);
} }
@Test @Test