From 9336b95bf4355c4b1e43f95879d9af45cf90cd19 Mon Sep 17 00:00:00 2001 From: michaelkatz Date: Mon, 7 Nov 2022 11:36:36 +0000 Subject: [PATCH] Changed MediaController to return last estimated position while paused The method getCurrentPosition() may return a lesser position during pause than the previous retrieved value due to ipc call delay in playerInfo update. Users see track position jump backwards at pause. Fixed to return last estimated position while paused if have not received updated playerInfo. Code is deduped to point getContentPosition() to getCurrentPosition() when !isPlayingAd. PiperOrigin-RevId: 486617341 --- .../session/MediaControllerImplBase.java | 55 +++++++--------- .../session/MediaControllerListenerTest.java | 8 +-- .../media3/session/MediaControllerTest.java | 64 +++++++++++++++++++ 3 files changed, 90 insertions(+), 37 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 b810600900..41e1f5a97a 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java @@ -125,7 +125,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Nullable private TextureView videoTextureView; private Size surfaceSize; @Nullable private IMediaSession iSession; - private long lastReturnedContentPositionMs; + private long lastReturnedCurrentPositionMs; private long lastSetPlayWhenReadyCalledTimeMs; public MediaControllerImplBase( @@ -168,7 +168,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; ? null : new SessionServiceConnection(connectionHints); flushCommandQueueHandler = new FlushCommandQueueHandler(applicationLooper); - lastReturnedContentPositionMs = C.TIME_UNSET; + lastReturnedCurrentPositionMs = C.TIME_UNSET; lastSetPlayWhenReadyCalledTimeMs = C.TIME_UNSET; } @@ -592,9 +592,20 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Override public long getCurrentPosition() { + boolean receivedUpdatedPositionInfo = + lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs; if (!playerInfo.isPlaying) { - return playerInfo.sessionPositionInfo.positionInfo.positionMs; + 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() @@ -602,9 +613,11 @@ import org.checkerframework.checker.nullness.qual.NonNull; long estimatedPositionMs = playerInfo.sessionPositionInfo.positionInfo.positionMs + (long) (elapsedTimeMs * playerInfo.playbackParameters.speed); - return playerInfo.sessionPositionInfo.durationMs == C.TIME_UNSET - ? estimatedPositionMs - : Math.min(estimatedPositionMs, playerInfo.sessionPositionInfo.durationMs); + if (playerInfo.sessionPositionInfo.durationMs != C.TIME_UNSET) { + estimatedPositionMs = min(estimatedPositionMs, playerInfo.sessionPositionInfo.durationMs); + } + lastReturnedCurrentPositionMs = estimatedPositionMs; + return lastReturnedCurrentPositionMs; } @Override @@ -634,34 +647,10 @@ import org.checkerframework.checker.nullness.qual.NonNull; @Override public long getContentPosition() { - boolean receivedUpdatedPositionInfo = - lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs; - if (!playerInfo.isPlaying || playerInfo.sessionPositionInfo.isPlayingAd) { - if (receivedUpdatedPositionInfo || lastReturnedContentPositionMs == C.TIME_UNSET) { - lastReturnedContentPositionMs = - playerInfo.sessionPositionInfo.positionInfo.contentPositionMs; - } - return lastReturnedContentPositionMs; + if (!playerInfo.sessionPositionInfo.isPlayingAd) { + return getCurrentPosition(); } - - if (!receivedUpdatedPositionInfo && lastReturnedContentPositionMs != C.TIME_UNSET) { - // We need an updated content position to make a new position estimation. - return lastReturnedContentPositionMs; - } - - long elapsedTimeMs = - (getInstance().getTimeDiffMs() != C.TIME_UNSET) - ? getInstance().getTimeDiffMs() - : SystemClock.elapsedRealtime() - playerInfo.sessionPositionInfo.eventTimeMs; - long estimatedPositionMs = - playerInfo.sessionPositionInfo.positionInfo.contentPositionMs - + (long) (elapsedTimeMs * playerInfo.playbackParameters.speed); - if (playerInfo.sessionPositionInfo.contentDurationMs != C.TIME_UNSET) { - estimatedPositionMs = - Math.min(estimatedPositionMs, playerInfo.sessionPositionInfo.contentDurationMs); - } - lastReturnedContentPositionMs = estimatedPositionMs; - return lastReturnedContentPositionMs; + return playerInfo.sessionPositionInfo.positionInfo.contentPositionMs; } @Override diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java index 5a4c2e5055..36a598f988 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerListenerTest.java @@ -723,7 +723,7 @@ public class MediaControllerListenerTest { PlaybackParameters testPlaybackParameters = new PlaybackParameters(/* speed= */ 3.2f, /* pitch= */ 2.1f); long testCurrentPositionMs = 11; - long testContentPositionMs = 33; + long testContentPositionMs = testCurrentPositionMs; // Not playing an ad long testBufferedPositionMs = 100; int testBufferedPercentage = 50; long testTotalBufferedDurationMs = 120; @@ -1595,7 +1595,7 @@ public class MediaControllerListenerTest { @Player.PlaybackSuppressionReason int testReason = Player.PLAYBACK_SUPPRESSION_REASON_TRANSIENT_AUDIO_FOCUS_LOSS; long testCurrentPositionMs = 11; - long testContentPositionMs = 33; + long testContentPositionMs = testCurrentPositionMs; // Not playing an ad long testBufferedPositionMs = 100; int testBufferedPercentage = 50; long testTotalBufferedDurationMs = 120; @@ -1726,7 +1726,7 @@ public class MediaControllerListenerTest { public void onPlaybackStateChanged_updatesGetters() throws Exception { @Player.State int testPlaybackState = Player.EVENT_PLAYER_ERROR; long testCurrentPositionMs = 11; - long testContentPositionMs = 33; + long testContentPositionMs = testCurrentPositionMs; // Not playing an ad long testBufferedPositionMs = 100; int testBufferedPercentage = 50; long testTotalBufferedDurationMs = 120; @@ -1857,7 +1857,7 @@ public class MediaControllerListenerTest { public void onIsPlayingChanged_updatesGetters() throws Exception { boolean testIsPlaying = true; long testCurrentPositionMs = 11; - long testContentPositionMs = 33; + long testContentPositionMs = testCurrentPositionMs; // Not playing an ad long testBufferedPositionMs = 100; int testBufferedPercentage = 50; long testTotalBufferedDurationMs = 120; 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 af98cea8ef..046eede07a 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 @@ -918,6 +918,70 @@ public class MediaControllerTest { assertThat(contentPositionMs).isEqualTo(testContentPosition); } + @Test + public void getContentPosition_whenPlayingMainContent_returnsCurrentPosition() throws Exception { + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setCurrentPosition(100L) + .setContentPosition(100L) // Same as current position b/c not playing an ad + .setDuration(10_000L) + .setIsPlaying(true) + .setIsPlayingAd(false) + .setPlaybackParameters(new PlaybackParameters(/* speed= */ 2.0f)) + .build(); + remoteSession.setPlayer(playerConfig); + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + + long currentPositionMs = + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setTimeDiffMs(50L); + return controller.getCurrentPosition(); + }); + long contentPositionMs = + threadTestRule.getHandler().postAndSync(controller::getContentPosition); + + // expectedPositionMs = initCurrentPositionMs + deltaTime*playbackSpeed + // 200L = 100L + 50L*2.0f + assertThat(contentPositionMs).isEqualTo(200L); + assertThat(currentPositionMs).isEqualTo(200L); + } + + @Test + public void getContentPosition_whenPlayingAd_returnsContentPosition() throws Exception { + Bundle playerConfig = + new RemoteMediaSession.MockPlayerConfigBuilder() + .setCurrentPosition(10L) + .setContentPosition(50L) + .setDuration(10_000L) + .setIsPlaying(true) + .setIsPlayingAd(true) + .setCurrentAdGroupIndex(0) + .setCurrentAdIndexInAdGroup(0) + .setPlaybackParameters(new PlaybackParameters(/* speed= */ 2.0f)) + .build(); + remoteSession.setPlayer(playerConfig); + MediaController controller = controllerTestRule.createController(remoteSession.getToken()); + + long currentPositionMs = + threadTestRule + .getHandler() + .postAndSync( + () -> { + controller.setTimeDiffMs(50L); + return controller.getCurrentPosition(); + }); + long contentPositionMs = + threadTestRule.getHandler().postAndSync(controller::getContentPosition); + + // expectedCurrentPositionMs = initCurrentPositionMs + deltaTime*playbackSpeed + // 110L = 10L + 50L*2.0f + assertThat(currentPositionMs).isEqualTo(110L); + assertThat(contentPositionMs).isEqualTo(50L); + } + @Test public void getBufferedPosition_withPeriodicUpdate_updatedWithoutCallback() throws Exception { long testBufferedPosition = 999L;