From aa467f52e474cc50ba65be26e1d3c7f65e81d840 Mon Sep 17 00:00:00 2001 From: bachinger Date: Mon, 27 Dec 2021 13:43:21 +0000 Subject: [PATCH] Make sure CastPlayer calls onIsPlaying if required Before this change we checked whether the playback state and playWhenReady have changed when the state from the cast device arrived. If we detected such a change we called the listener callback `onIsPlayingChanged`. However, in the case when `setPlayWhenReady(boolean)` is called on 'CastPlayer', we mask the change in `playWhenReady`, then send the play/pause request to the cast device and when the state from the cast device arrives we never detect a change because we have already masked `playWhenReady`. This change now moves the check for `isPlaying` to the same place where the state and playWhenReady is updated, so we call the `onIsPlayingChanged` callback in either case, when masking or when a changed state from the server arrives. Issue: google/ExoPlayer#9792 PiperOrigin-RevId: 418483509 --- RELEASENOTES.md | 3 ++ .../exoplayer2/ext/cast/CastPlayer.java | 12 +++--- .../exoplayer2/ext/cast/CastPlayerTest.java | 37 ++++++++++++++++--- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 42041d9020..7c1e42c991 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -86,6 +86,9 @@ * RTSP: * Provide a client API to override the `SocketFactory` used for any server connection ([#9606](https://github.com/google/ExoPlayer/pull/9606)). +* Cast extension + * Fix bug that prevented `CastPlayer` from calling `onIsPlayingChanged` + correctly. * Remove deprecated symbols: * Remove `MediaSourceFactory#setDrmSessionManager`, `MediaSourceFactory#setDrmHttpDataSourceFactory`, and diff --git a/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java b/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java index ac8d4ea8c6..fbe5a31a93 100644 --- a/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java +++ b/extensions/cast/src/main/java/com/google/android/exoplayer2/ext/cast/CastPlayer.java @@ -816,13 +816,7 @@ public final class CastPlayer extends BasePlayer { !getCurrentTimeline().isEmpty() ? getCurrentTimeline().getPeriod(oldWindowIndex, period, /* setIds= */ true).uid : null; - boolean wasPlaying = playbackState == Player.STATE_READY && playWhenReady.value; updatePlayerStateAndNotifyIfChanged(/* resultCallback= */ null); - boolean isPlaying = playbackState == Player.STATE_READY && playWhenReady.value; - if (wasPlaying != isPlaying) { - listeners.queueEvent( - Player.EVENT_IS_PLAYING_CHANGED, listener -> listener.onIsPlayingChanged(isPlaying)); - } updateRepeatModeAndNotifyIfChanged(/* resultCallback= */ null); updatePlaybackRateAndNotifyIfChanged(/* resultCallback= */ null); boolean playingPeriodChangedByTimelineChange = updateTimelineAndNotifyIfChanged(); @@ -1216,6 +1210,7 @@ public final class CastPlayer extends BasePlayer { boolean playWhenReady, @Player.PlayWhenReadyChangeReason int playWhenReadyChangeReason, @Player.State int playbackState) { + boolean wasPlaying = this.playbackState == Player.STATE_READY && this.playWhenReady.value; boolean playWhenReadyChanged = this.playWhenReady.value != playWhenReady; boolean playbackStateChanged = this.playbackState != playbackState; if (playWhenReadyChanged || playbackStateChanged) { @@ -1234,6 +1229,11 @@ public final class CastPlayer extends BasePlayer { Player.EVENT_PLAY_WHEN_READY_CHANGED, listener -> listener.onPlayWhenReadyChanged(playWhenReady, playWhenReadyChangeReason)); } + boolean isPlaying = playbackState == Player.STATE_READY && playWhenReady; + if (wasPlaying != isPlaying) { + listeners.queueEvent( + Player.EVENT_IS_PLAYING_CHANGED, listener -> listener.onIsPlayingChanged(isPlaying)); + } } } diff --git a/extensions/cast/src/test/java/com/google/android/exoplayer2/ext/cast/CastPlayerTest.java b/extensions/cast/src/test/java/com/google/android/exoplayer2/ext/cast/CastPlayerTest.java index 917d25aba0..5b0e1a4fff 100644 --- a/extensions/cast/src/test/java/com/google/android/exoplayer2/ext/cast/CastPlayerTest.java +++ b/extensions/cast/src/test/java/com/google/android/exoplayer2/ext/cast/CastPlayerTest.java @@ -52,6 +52,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -138,13 +139,18 @@ public class CastPlayerTest { @SuppressWarnings("deprecation") @Test public void setPlayWhenReady_masksRemoteState() { + when(mockRemoteMediaClient.getPlayerState()).thenReturn(MediaStatus.PLAYER_STATE_PLAYING); + // Trigger initial update to get out of STATE_IDLE to make onIsPlaying() be called. + remoteMediaClientCallback.onStatusUpdated(); + reset(mockListener); when(mockRemoteMediaClient.play()).thenReturn(mockPendingResult); assertThat(castPlayer.getPlayWhenReady()).isFalse(); castPlayer.play(); verify(mockPendingResult).setResultCallback(setResultCallbackArgumentCaptor.capture()); assertThat(castPlayer.getPlayWhenReady()).isTrue(); - verify(mockListener).onPlayerStateChanged(true, Player.STATE_IDLE); + verify(mockListener).onPlayerStateChanged(true, Player.STATE_READY); + verify(mockListener).onIsPlayingChanged(true); verify(mockListener) .onPlayWhenReadyChanged(true, Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST); @@ -163,13 +169,18 @@ public class CastPlayerTest { @SuppressWarnings("deprecation") @Test public void setPlayWhenReadyMasking_updatesUponResultChange() { + when(mockRemoteMediaClient.getPlayerState()).thenReturn(MediaStatus.PLAYER_STATE_PLAYING); + // Trigger initial update to get out of STATE_IDLE to make onIsPlaying() be called. + remoteMediaClientCallback.onStatusUpdated(); + reset(mockListener); when(mockRemoteMediaClient.play()).thenReturn(mockPendingResult); assertThat(castPlayer.getPlayWhenReady()).isFalse(); castPlayer.play(); verify(mockPendingResult).setResultCallback(setResultCallbackArgumentCaptor.capture()); assertThat(castPlayer.getPlayWhenReady()).isTrue(); - verify(mockListener).onPlayerStateChanged(true, Player.STATE_IDLE); + verify(mockListener).onIsPlayingChanged(true); + verify(mockListener).onPlayerStateChanged(true, Player.STATE_READY); verify(mockListener) .onPlayWhenReadyChanged(true, Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST); @@ -177,38 +188,52 @@ public class CastPlayerTest { setResultCallbackArgumentCaptor .getValue() .onResult(mock(RemoteMediaClient.MediaChannelResult.class)); - verify(mockListener).onPlayerStateChanged(false, Player.STATE_IDLE); + verify(mockListener).onPlayerStateChanged(false, Player.STATE_READY); + verify(mockListener).onIsPlayingChanged(false); verify(mockListener).onPlayWhenReadyChanged(false, Player.PLAY_WHEN_READY_CHANGE_REASON_REMOTE); assertThat(castPlayer.getPlayWhenReady()).isFalse(); + verifyNoMoreInteractions(mockListener); } @SuppressWarnings("deprecation") @Test public void setPlayWhenReady_correctChangeReasonOnPause() { + when(mockRemoteMediaClient.getPlayerState()).thenReturn(MediaStatus.PLAYER_STATE_PLAYING); + // Trigger initial update to get out of STATE_IDLE to make onIsPlaying() be called. + remoteMediaClientCallback.onStatusUpdated(); + reset(mockListener); when(mockRemoteMediaClient.play()).thenReturn(mockPendingResult); when(mockRemoteMediaClient.pause()).thenReturn(mockPendingResult); + castPlayer.play(); assertThat(castPlayer.getPlayWhenReady()).isTrue(); - verify(mockListener).onPlayerStateChanged(true, Player.STATE_IDLE); + verify(mockListener).onIsPlayingChanged(true); + verify(mockListener).onPlayerStateChanged(true, Player.STATE_READY); verify(mockListener) .onPlayWhenReadyChanged(true, Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST); castPlayer.pause(); assertThat(castPlayer.getPlayWhenReady()).isFalse(); - verify(mockListener).onPlayerStateChanged(false, Player.STATE_IDLE); + verify(mockListener).onIsPlayingChanged(false); verify(mockListener) .onPlayWhenReadyChanged(false, Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST); + verify(mockListener).onPlayerStateChanged(false, Player.STATE_READY); + verifyNoMoreInteractions(mockListener); } @SuppressWarnings("deprecation") @Test public void playWhenReady_changesOnStatusUpdates() { + when(mockRemoteMediaClient.getPlayerState()).thenReturn(MediaStatus.PLAYER_STATE_PLAYING); assertThat(castPlayer.getPlayWhenReady()).isFalse(); when(mockRemoteMediaClient.isPaused()).thenReturn(false); remoteMediaClientCallback.onStatusUpdated(); - verify(mockListener).onPlayerStateChanged(true, Player.STATE_IDLE); + verify(mockListener).onPlayerStateChanged(true, Player.STATE_READY); + verify(mockListener).onPlaybackStateChanged(Player.STATE_READY); verify(mockListener).onPlayWhenReadyChanged(true, Player.PLAY_WHEN_READY_CHANGE_REASON_REMOTE); assertThat(castPlayer.getPlayWhenReady()).isTrue(); + verify(mockListener).onIsPlayingChanged(true); + verifyNoMoreInteractions(mockListener); } @Test