From 77ba0292adf0199b3bf5b6e433f63478c6d96c9e Mon Sep 17 00:00:00 2001 From: tonihei Date: Wed, 27 Sep 2023 10:21:04 -0700 Subject: [PATCH] Add position interpolation to MediaControllerImplLegacy Without this, the position won't udpate until the session sends a new playback state. PiperOrigin-RevId: 568889286 --- RELEASENOTES.md | 2 + .../session/MediaControllerImplBase.java | 42 +++------ .../session/MediaControllerImplLegacy.java | 92 +++++++++---------- .../androidx/media3/session/MediaUtils.java | 47 ++++++++++ .../androidx/media3/session/PlayerInfo.java | 3 +- ...aControllerWithMediaSessionCompatTest.java | 54 +++++++++++ 6 files changed, 162 insertions(+), 78 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 53f37796c0..ca1e418b2f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -76,6 +76,8 @@ controller is always the media notification controller and apps can easily recognize calls coming from the notification in the same way on all supported API levels. + * Fix bug where `MediaController.getCurrentPosition()` is not advancing + when connected to a legacy `MediaSessionCompat`. * UI: * Downloads: * OkHttp Extension: 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 376ece00f1..2ba418400b 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -583,7 +583,12 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Override public long getCurrentPosition() { - maybeUpdateCurrentPositionMs(); + currentPositionMs = + MediaUtils.getUpdatedCurrentPositionMs( + playerInfo, + currentPositionMs, + lastSetPlayWhenReadyCalledTimeMs, + getInstance().getTimeDiffMs()); return currentPositionMs; } @@ -2191,7 +2196,12 @@ import org.checkerframework.checker.nullness.qual.NonNull; } // Update position and then stop estimating until a new positionInfo arrives from the player. - maybeUpdateCurrentPositionMs(); + currentPositionMs = + MediaUtils.getUpdatedCurrentPositionMs( + this.playerInfo, + currentPositionMs, + lastSetPlayWhenReadyCalledTimeMs, + getInstance().getTimeDiffMs()); lastSetPlayWhenReadyCalledTimeMs = SystemClock.elapsedRealtime(); PlayerInfo newPlayerInfo = this.playerInfo.copyWithPlayWhenReady( @@ -2991,34 +3001,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; return playerInfo; } - private void maybeUpdateCurrentPositionMs() { - boolean receivedUpdatedPositionInfo = - lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs; - if (!playerInfo.isPlaying) { - if (receivedUpdatedPositionInfo || currentPositionMs == C.TIME_UNSET) { - currentPositionMs = playerInfo.sessionPositionInfo.positionInfo.positionMs; - } - return; - } - - if (!receivedUpdatedPositionInfo && currentPositionMs != C.TIME_UNSET) { - // Need an updated current position in order to make a new position estimation - return; - } - - long elapsedTimeMs = - (getInstance().getTimeDiffMs() != C.TIME_UNSET) - ? getInstance().getTimeDiffMs() - : SystemClock.elapsedRealtime() - playerInfo.sessionPositionInfo.eventTimeMs; - long estimatedPositionMs = - playerInfo.sessionPositionInfo.positionInfo.positionMs - + (long) (elapsedTimeMs * playerInfo.playbackParameters.speed); - if (playerInfo.sessionPositionInfo.durationMs != C.TIME_UNSET) { - estimatedPositionMs = min(estimatedPositionMs, playerInfo.sessionPositionInfo.durationMs); - } - currentPositionMs = estimatedPositionMs; - } - private static Period getPeriodWithNewWindowIndex( Timeline timeline, int periodIndex, int windowIndex) { Period period = new Period(); diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java index 7b889bd618..ae62eb1ae0 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplLegacy.java @@ -107,6 +107,8 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; private LegacyPlayerInfo legacyPlayerInfo; private LegacyPlayerInfo pendingLegacyPlayerInfo; private ControllerInfo controllerInfo; + private long currentPositionMs; + private long lastSetPlayWhenReadyCalledTimeMs; public MediaControllerImplLegacy( Context context, @@ -130,6 +132,8 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; controllerCompatCallback = new ControllerCompatCallback(applicationLooper); this.token = token; this.bitmapLoader = bitmapLoader; + currentPositionMs = C.TIME_UNSET; + lastSetPlayWhenReadyCalledTimeMs = C.TIME_UNSET; } /* package */ MediaController getInstance() { @@ -227,50 +231,12 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; @Override public void play() { - if (controllerInfo.playerInfo.playWhenReady) { - return; - } - ControllerInfo maskedControllerInfo = - new ControllerInfo( - controllerInfo.playerInfo.copyWithPlayWhenReady( - /* playWhenReady= */ true, - Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST, - Player.PLAYBACK_SUPPRESSION_REASON_NONE), - controllerInfo.availableSessionCommands, - controllerInfo.availablePlayerCommands, - controllerInfo.customLayout); - updateStateMaskedControllerInfo( - maskedControllerInfo, - /* discontinuityReason= */ null, - /* mediaItemTransitionReason= */ null); - - if (isPrepared() && hasMedia()) { - controllerCompat.getTransportControls().play(); - } + setPlayWhenReady(true); } @Override public void pause() { - if (!controllerInfo.playerInfo.playWhenReady) { - return; - } - ControllerInfo maskedControllerInfo = - new ControllerInfo( - controllerInfo.playerInfo.copyWithPlayWhenReady( - /* playWhenReady= */ false, - Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST, - Player.PLAYBACK_SUPPRESSION_REASON_NONE), - controllerInfo.availableSessionCommands, - controllerInfo.availablePlayerCommands, - controllerInfo.customLayout); - updateStateMaskedControllerInfo( - maskedControllerInfo, - /* discontinuityReason= */ null, - /* mediaItemTransitionReason= */ null); - - if (isPrepared() && hasMedia()) { - controllerCompat.getTransportControls().pause(); - } + setPlayWhenReady(false); } @Override @@ -459,7 +425,13 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; @Override public long getCurrentPosition() { - return controllerInfo.playerInfo.sessionPositionInfo.positionInfo.positionMs; + currentPositionMs = + MediaUtils.getUpdatedCurrentPositionMs( + controllerInfo.playerInfo, + currentPositionMs, + lastSetPlayWhenReadyCalledTimeMs, + getInstance().getTimeDiffMs()); + return currentPositionMs; } @Override @@ -758,6 +730,7 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; if (newCurrentMediaItemIndex == C.INDEX_UNSET) { newCurrentMediaItemIndex = Util.constrainValue(fromIndex, /* min= */ 0, newQueueTimeline.getWindowCount() - 1); + // TODO: b/302114474 - This also needs to reset the current position. Log.w( TAG, "Currently playing item is removed. Assumes item at " @@ -1213,10 +1186,37 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; @Override public void setPlayWhenReady(boolean playWhenReady) { - if (playWhenReady) { - play(); - } else { - pause(); + if (controllerInfo.playerInfo.playWhenReady == playWhenReady) { + return; + } + // Update position and then stop estimating until a new positionInfo arrives from the session. + currentPositionMs = + MediaUtils.getUpdatedCurrentPositionMs( + controllerInfo.playerInfo, + currentPositionMs, + lastSetPlayWhenReadyCalledTimeMs, + getInstance().getTimeDiffMs()); + lastSetPlayWhenReadyCalledTimeMs = SystemClock.elapsedRealtime(); + ControllerInfo maskedControllerInfo = + new ControllerInfo( + controllerInfo.playerInfo.copyWithPlayWhenReady( + playWhenReady, + Player.PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST, + Player.PLAYBACK_SUPPRESSION_REASON_NONE), + controllerInfo.availableSessionCommands, + controllerInfo.availablePlayerCommands, + controllerInfo.customLayout); + updateStateMaskedControllerInfo( + maskedControllerInfo, + /* discontinuityReason= */ null, + /* mediaItemTransitionReason= */ null); + + if (isPrepared() && hasMedia()) { + if (playWhenReady) { + controllerCompat.getTransportControls().play(); + } else { + controllerCompat.getTransportControls().pause(); + } } } @@ -2233,7 +2233,7 @@ import org.checkerframework.checker.initialization.qual.UnderInitialization; new SessionPositionInfo( /* positionInfo= */ positionInfo, /* isPlayingAd= */ isPlayingAd, - /* eventTimeMs= */ C.TIME_UNSET, + /* eventTimeMs= */ SystemClock.elapsedRealtime(), /* durationMs= */ durationMs, /* bufferedPositionMs= */ bufferedPositionMs, /* bufferedPercentage= */ bufferedPercentage, 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 3959820cbc..df42396a3e 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java @@ -48,6 +48,7 @@ import static androidx.media3.common.util.Util.castNonNull; import static androidx.media3.common.util.Util.constrainValue; import static androidx.media3.session.MediaConstants.EXTRA_KEY_ROOT_CHILDREN_BROWSABLE_ONLY; import static java.lang.Math.max; +import static java.lang.Math.min; import static java.util.concurrent.TimeUnit.MILLISECONDS; import android.annotation.SuppressLint; @@ -1500,6 +1501,52 @@ import java.util.concurrent.TimeoutException; && info1.positionInfo.adIndexInAdGroup == info2.positionInfo.adIndexInAdGroup; } + /** + * Returns updated value for a media controller position estimate. + * + * @param playerInfo The current {@link PlayerInfo}. + * @param currentPositionMs The current known position estimate in milliseconds, or {@link + * C#TIME_UNSET} if still unknown. + * @param lastSetPlayWhenReadyCalledTimeMs The {@link SystemClock#elapsedRealtime()} when the + * controller was last used to call {@link MediaController#setPlayWhenReady}, or {@link + * C#TIME_UNSET} if it was never called. + * @param timeDiffMs A time difference override since the last {@link PlayerInfo} update. Should + * be {@link C#TIME_UNSET} except for testing. + * @return The updated position estimate in milliseconds. + */ + public static long getUpdatedCurrentPositionMs( + PlayerInfo playerInfo, + long currentPositionMs, + long lastSetPlayWhenReadyCalledTimeMs, + long timeDiffMs) { + boolean receivedUpdatedPositionInfo = + lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs; + if (!playerInfo.isPlaying) { + if (receivedUpdatedPositionInfo || currentPositionMs == C.TIME_UNSET) { + return playerInfo.sessionPositionInfo.positionInfo.positionMs; + } else { + return currentPositionMs; + } + } + + if (!receivedUpdatedPositionInfo && currentPositionMs != C.TIME_UNSET) { + // Need an updated current position in order to make a new position estimation + return currentPositionMs; + } + + long elapsedTimeMs = + timeDiffMs != C.TIME_UNSET + ? timeDiffMs + : SystemClock.elapsedRealtime() - playerInfo.sessionPositionInfo.eventTimeMs; + long estimatedPositionMs = + playerInfo.sessionPositionInfo.positionInfo.positionMs + + (long) (elapsedTimeMs * playerInfo.playbackParameters.speed); + if (playerInfo.sessionPositionInfo.durationMs != C.TIME_UNSET) { + estimatedPositionMs = min(estimatedPositionMs, playerInfo.sessionPositionInfo.durationMs); + } + return estimatedPositionMs; + } + 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/session/src/main/java/androidx/media3/session/PlayerInfo.java b/libraries/session/src/main/java/androidx/media3/session/PlayerInfo.java index bc80edcf35..00b02e3e08 100644 --- a/libraries/session/src/main/java/androidx/media3/session/PlayerInfo.java +++ b/libraries/session/src/main/java/androidx/media3/session/PlayerInfo.java @@ -23,7 +23,6 @@ import static androidx.media3.common.Player.STATE_IDLE; import static androidx.media3.common.Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED; import android.os.Bundle; -import android.os.SystemClock; import androidx.annotation.CheckResult; import androidx.annotation.FloatRange; import androidx.annotation.Nullable; @@ -630,7 +629,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; sessionPositionInfo.positionInfo.adGroupIndex, sessionPositionInfo.positionInfo.adIndexInAdGroup), sessionPositionInfo.isPlayingAd, - /* eventTimeMs= */ SystemClock.elapsedRealtime(), + sessionPositionInfo.eventTimeMs, sessionPositionInfo.durationMs, sessionPositionInfo.bufferedPositionMs, sessionPositionInfo.bufferedPercentage, diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerWithMediaSessionCompatTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerWithMediaSessionCompatTest.java index 8b582a8363..c1ace7555d 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerWithMediaSessionCompatTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerWithMediaSessionCompatTest.java @@ -48,6 +48,7 @@ import android.graphics.Bitmap; import android.media.AudioManager; import android.net.Uri; import android.os.Bundle; +import android.os.SystemClock; import android.support.v4.media.MediaDescriptionCompat; import android.support.v4.media.MediaMetadataCompat; import android.support.v4.media.session.MediaSessionCompat; @@ -1833,6 +1834,59 @@ public class MediaControllerWithMediaSessionCompatTest { assertThat(currentPositionMs).isEqualTo(testDurationMs); } + @Test + public void getCurrentPosition_withDelayWhileNotPlaying_doesNotAdvance() throws Exception { + session.setPlaybackState( + new PlaybackStateCompat.Builder() + .setState( + PlaybackStateCompat.STATE_PAUSED, /* position= */ 500, /* playbackSpeed= */ 2.0f) + .build()); + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + + long currentPositionMs = + threadTestRule + .getHandler() + .postAndSync( + () -> { + Thread.sleep(100); + return controller.getCurrentPosition(); + }); + + assertThat(currentPositionMs).isEqualTo(500); + } + + @Test + public void getCurrentPosition_withTimeDiffWhilePlaying_advancesWithTimeDiff() throws Exception { + long timeBeforeSetPlaybackState = SystemClock.elapsedRealtime(); + session.setPlaybackState( + new PlaybackStateCompat.Builder() + .setState( + PlaybackStateCompat.STATE_PLAYING, /* position= */ 500, /* playbackSpeed= */ 2.0f) + .build()); + MediaController controller = controllerTestRule.createController(session.getSessionToken()); + long timeAfterControllerCreated = SystemClock.elapsedRealtime(); + + AtomicLong timeBeforeGetCurrentPosition = new AtomicLong(); + AtomicLong timeAfterGetCurrentPosition = new AtomicLong(); + AtomicLong currentPositionMs = new AtomicLong(); + threadTestRule + .getHandler() + .postAndSync( + () -> { + Thread.sleep(100); + timeBeforeGetCurrentPosition.set(SystemClock.elapsedRealtime()); + currentPositionMs.set(controller.getCurrentPosition()); + timeAfterGetCurrentPosition.set(SystemClock.elapsedRealtime()); + }); + + long minTimeElapsedMs = timeBeforeGetCurrentPosition.get() - timeAfterControllerCreated; + long maxTimeElapsedMs = timeAfterGetCurrentPosition.get() - timeBeforeSetPlaybackState; + long minExpectedPositionMs = 500 + minTimeElapsedMs * 2; + long maxExpectedPositionMs = 500 + maxTimeElapsedMs * 2; + assertThat(currentPositionMs.get()) + .isIn(Range.closed(minExpectedPositionMs, maxExpectedPositionMs)); + } + @Test public void getContentPosition_byDefault_returnsZero() throws Exception { MediaController controller = controllerTestRule.createController(session.getSessionToken());