diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 02807708b8..a7fc67516a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -7,6 +7,9 @@ implement playback resumption with media button events sent by, for example, a Bluetooth headset ([#167](https://github.com/androidx/media/issues/167)). + * Fix bug where a combined `Timeline` and position update in a + `MediaSession` may cause a `MediaController` to throw an + `IllegalStateException`. * Remove deprecated symbols: * Remove two deprecated `SimpleCache` constructors, use a non-deprecated constructor that takes a `DatabaseProvider` instead for better diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java index f1128e8969..5a4b33be2b 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -2613,6 +2613,13 @@ public void onRenderedFirstFrame() { private void updateSessionPositionInfoIfNeeded(SessionPositionInfo sessionPositionInfo) { if (pendingMaskingSequencedFutureNumbers.isEmpty() && playerInfo.sessionPositionInfo.eventTimeMs < sessionPositionInfo.eventTimeMs) { + if (!MediaUtils.areSessionPositionInfosInSamePeriodOrAd( + sessionPositionInfo, playerInfo.sessionPositionInfo)) { + // MediaSessionImpl before version 1.0.2 has a bug that may send position info updates for + // new periods too early. Ignore these updates to avoid an inconsistent state (see + // [internal b/277301159]). + return; + } playerInfo = playerInfo.copyWithSessionPositionInfo(sessionPositionInfo); } } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java index db50614290..74d660bbf7 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java @@ -807,7 +807,16 @@ private void notifyPeriodicSessionPositionInfoChangesOnHandler() { } } SessionPositionInfo sessionPositionInfo = playerWrapper.createSessionPositionInfoForBundling(); - dispatchOnPeriodicSessionPositionInfoChanged(sessionPositionInfo); + if (!onPlayerInfoChangedHandler.hasPendingPlayerInfoChangedUpdate() + && MediaUtils.areSessionPositionInfosInSamePeriodOrAd( + sessionPositionInfo, playerInfo.sessionPositionInfo)) { + // Send a periodic position info only if a PlayerInfo update is not already already pending + // and the player state is still corresponding to the currently known PlayerInfo. Both + // conditions will soon trigger a new PlayerInfo update with the latest position info anyway + // and we also don't want to send a new position info early if the corresponding Timeline + // update hasn't been sent yet (see [internal b/277301159]). + dispatchOnPeriodicSessionPositionInfoChanged(sessionPositionInfo); + } schedulePeriodicSessionPositionInfoChanges(); } @@ -1362,11 +1371,15 @@ public void handleMessage(Message msg) { } } + public boolean hasPendingPlayerInfoChangedUpdate() { + return hasMessages(MSG_PLAYER_INFO_CHANGED); + } + public void sendPlayerInfoChangedMessage(boolean excludeTimeline, boolean excludeTracks) { this.excludeTimeline = this.excludeTimeline && excludeTimeline; this.excludeTracks = this.excludeTracks && excludeTracks; - if (!onPlayerInfoChangedHandler.hasMessages(MSG_PLAYER_INFO_CHANGED)) { - onPlayerInfoChangedHandler.sendEmptyMessage(MSG_PLAYER_INFO_CHANGED); + if (!hasMessages(MSG_PLAYER_INFO_CHANGED)) { + sendEmptyMessage(MSG_PLAYER_INFO_CHANGED); } } } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java index 0fda511e7c..a15d5fe0c3 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java @@ -1476,6 +1476,19 @@ public static void setMediaItemsWithStartIndexAndPosition( } } + /** + * Returns whether the two provided {@link SessionPositionInfo} describe a position in the same + * period or ad. + */ + public static boolean areSessionPositionInfosInSamePeriodOrAd( + SessionPositionInfo info1, SessionPositionInfo info2) { + // TODO: b/259220235 - Use UIDs instead of mediaItemIndex and periodIndex + return info1.positionInfo.mediaItemIndex == info2.positionInfo.mediaItemIndex + && info1.positionInfo.periodIndex == info2.positionInfo.periodIndex + && info1.positionInfo.adGroupIndex == info2.positionInfo.adGroupIndex + && info1.positionInfo.adIndexInAdGroup == info2.positionInfo.adIndexInAdGroup; + } + private static byte[] convertToByteArray(Bitmap bitmap) throws IOException { try (ByteArrayOutputStream stream = new ByteArrayOutputStream()) { bitmap.compress(Bitmap.CompressFormat.PNG, /* ignored */ 0, stream); diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java index 4a4c01db4f..5405c0f6d7 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerTest.java @@ -34,6 +34,7 @@ import android.content.Context; import android.os.Bundle; import android.os.RemoteException; +import androidx.annotation.Nullable; import androidx.media3.common.AudioAttributes; import androidx.media3.common.C; import androidx.media3.common.Format; @@ -1064,6 +1065,51 @@ public void getBufferedPosition_whilePausedAndNotLoading_isNotUpdatedPeriodicall assertThat(bufferedPositionAfterDelay.get()).isNotEqualTo(testBufferedPosition); } + @Test + public void + getCurrentMediaItemIndex_withPeriodicUpdateOverlappingTimelineChanges_updatesIndexCorrectly() + throws Exception { + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setPlayWhenReady(true) + .setPlaybackState(Player.STATE_READY) + .build(); + remoteSession.setPlayer(playerConfig); + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + ArrayList transitionMediaItemIndices = new ArrayList<>(); + controller.addListener( + new Player.Listener() { + @Override + public void onMediaItemTransition(@Nullable MediaItem mediaItem, int reason) { + transitionMediaItemIndices.add(controller.getCurrentMediaItemIndex()); + } + }); + + // Intentionally trigger update often to ensure there is a likely overlap with Timeline updates. + remoteSession.setSessionPositionUpdateDelayMs(1L); + // Trigger many timeline and position updates that are incompatible with any previous updates. + for (int i = 1; i <= 100; i++) { + remoteSession.getMockPlayer().createAndSetFakeTimeline(/* windowCount= */ i); + remoteSession.getMockPlayer().setCurrentMediaItemIndex(i - 1); + remoteSession + .getMockPlayer() + .notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED); + remoteSession + .getMockPlayer() + .notifyMediaItemTransition( + /* index= */ i - 1, Player.MEDIA_ITEM_TRANSITION_REASON_PLAYLIST_CHANGED); + } + PollingCheck.waitFor(TIMEOUT_MS, () -> transitionMediaItemIndices.size() == 100); + + ImmutableList.Builder expectedMediaItemIndices = ImmutableList.builder(); + for (int i = 0; i < 100; i++) { + expectedMediaItemIndices.add(i); + } + assertThat(transitionMediaItemIndices) + .containsExactlyElementsIn(expectedMediaItemIndices.build()) + .inOrder(); + } + @Test public void getContentBufferedPosition_byDefault_returnsZero() throws Exception { MediaController controller = controllerTestRule.createController(remoteSession.getToken());