From e961c1b5e9bb4a6f63458b1bdcb49e97f415fabf Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 19 Jan 2023 15:00:49 +0000 Subject: [PATCH] Update media controller position before pausing. We stop estimating new position when pausing until we receive a new position from the player. However, this means that we will continue to return a possible stale previous position. Updating the current position before pausing solves this issue. PiperOrigin-RevId: 503153982 --- .../session/MediaControllerImplBase.java | 63 ++++++++++--------- .../media3/session/MediaControllerTest.java | 31 +++++++++ 2 files changed, 65 insertions(+), 29 deletions(-) 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 8cca5c4d87..8f224cb1ee 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -129,7 +129,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Nullable private TextureView videoTextureView; private Size surfaceSize; @Nullable private IMediaSession iSession; - private long lastReturnedCurrentPositionMs; + private long currentPositionMs; private long lastSetPlayWhenReadyCalledTimeMs; @Nullable private PlayerInfo pendingPlayerInfo; @Nullable private BundlingExclusions pendingBundlingExclusions; @@ -175,7 +175,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; ? null : new SessionServiceConnection(connectionHints); flushCommandQueueHandler = new FlushCommandQueueHandler(applicationLooper); - lastReturnedCurrentPositionMs = C.TIME_UNSET; + currentPositionMs = C.TIME_UNSET; lastSetPlayWhenReadyCalledTimeMs = C.TIME_UNSET; } @@ -582,32 +582,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Override public long getCurrentPosition() { - boolean receivedUpdatedPositionInfo = - lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs; - if (!playerInfo.isPlaying) { - if (receivedUpdatedPositionInfo || lastReturnedCurrentPositionMs == C.TIME_UNSET) { - lastReturnedCurrentPositionMs = playerInfo.sessionPositionInfo.positionInfo.positionMs; - } - return lastReturnedCurrentPositionMs; - } - - if (!receivedUpdatedPositionInfo && lastReturnedCurrentPositionMs != C.TIME_UNSET) { - // Need an updated current position in order to make a new position estimation - return lastReturnedCurrentPositionMs; - } - - 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); - } - lastReturnedCurrentPositionMs = estimatedPositionMs; - return lastReturnedCurrentPositionMs; + maybeUpdateCurrentPositionMs(); + return currentPositionMs; } @Override @@ -1966,7 +1942,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; return; } - // Stop estimating content position until a new positionInfo arrives from the player + // Update position and then stop estimating until a new positionInfo arrives from the player. + maybeUpdateCurrentPositionMs(); lastSetPlayWhenReadyCalledTimeMs = SystemClock.elapsedRealtime(); PlayerInfo playerInfo = this.playerInfo.copyWithPlayWhenReady( @@ -2726,6 +2703,34 @@ 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 Period getPeriodWithNewWindowIndex(Timeline timeline, int periodIndex, int windowIndex) { Period period = new Period(); timeline.getPeriod(periodIndex, period); 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 a7b5dbfc6c..0bf910b686 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 @@ -887,6 +887,37 @@ public class MediaControllerTest { assertThat(currentPositionMs).isEqualTo(expectedCurrentPositionMs); } + @Test + public void getCurrentPosition_afterPause_returnsCorrectPosition() throws Exception { + long testCurrentPosition = 100L; + PlaybackParameters testPlaybackParameters = new PlaybackParameters(/* speed= */ 2.0f); + long testTimeDiff = 50L; + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setPlaybackState(Player.STATE_READY) + .setPlayWhenReady(true) + .setCurrentPosition(testCurrentPosition) + .setDuration(10_000L) + .setPlaybackParameters(testPlaybackParameters) + .build(); + remoteSession.setPlayer(playerConfig); + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + + long currentPositionMs = + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setTimeDiffMs(testTimeDiff); + controller.pause(); + return controller.getCurrentPosition(); + }); + + long expectedCurrentPositionMs = + testCurrentPosition + (long) (testTimeDiff * testPlaybackParameters.speed); + assertThat(currentPositionMs).isEqualTo(expectedCurrentPositionMs); + } + @Test public void getContentPosition_whenPlayingAd_doesNotAdvance() throws Exception { long testContentPosition = 100L;