Skip to content

Commit 1bdc58d

Browse files
tianyifcopybara-github
authored andcommitted
Avoid position jumping back when controller replaces the current item
When the controller replaces the current item, the masking position will be changed to the default position of the new item for a short while, before the correct position comes from the session. This will interrupt the current position fetched from the controller when the playback doesn't interrupted by the item replacing. Issue: #951 #minor-release PiperOrigin-RevId: 611417539
1 parent 27d78e6 commit 1bdc58d

File tree

4 files changed

+100
-10
lines changed

4 files changed

+100
-10
lines changed

RELEASENOTES.md

+3
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@
4949
* Muxers:
5050
* IMA extension:
5151
* Session:
52+
* Fix issue where the current position jumps back when the controller
53+
replaces the current item
54+
([#951](https://github.com/androidx/media/issues/951)).
5255
* UI:
5356
* Fallback to include audio track language name if `Locale` cannot
5457
identify a display name

libraries/session/src/main/java/androidx/media3/session/MediaControllerImplBase.java

+82-9
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,9 @@ private void addMediaItemsInternal(int index, List<MediaItem> mediaItems) {
970970
}
971971
// Add media items to the end of the timeline if the index exceeds the window count.
972972
index = min(index, playerInfo.timeline.getWindowCount());
973-
PlayerInfo newPlayerInfo = maskPlaybackInfoForAddedItems(playerInfo, index, mediaItems);
973+
PlayerInfo newPlayerInfo =
974+
maskPlayerInfoForAddedItems(
975+
playerInfo, index, mediaItems, getCurrentPosition(), getContentPosition());
974976
@Nullable
975977
@Player.MediaItemTransitionReason
976978
Integer mediaItemTransitionReason =
@@ -983,8 +985,23 @@ private void addMediaItemsInternal(int index, List<MediaItem> mediaItems) {
983985
/* mediaItemTransitionReason= */ mediaItemTransitionReason);
984986
}
985987

986-
private static PlayerInfo maskPlaybackInfoForAddedItems(
987-
PlayerInfo playerInfo, int index, List<MediaItem> mediaItems) {
988+
/**
989+
* Returns a masking {@link PlayerInfo} for the added {@linkplain MediaItem media items} with the
990+
* provided information.
991+
*
992+
* @param playerInfo The {@link PlayerInfo} that the new masking {@link PlayerInfo} is based on.
993+
* @param index The index at which the {@linkplain MediaItem media items} are added.
994+
* @param mediaItems The {@linkplain MediaItem media items} added.
995+
* @param currentPositionMs The current position in milliseconds.
996+
* @param currentContentPositionMs The current content position in milliseconds.
997+
* @return A masking {@link PlayerInfo}.
998+
*/
999+
private static PlayerInfo maskPlayerInfoForAddedItems(
1000+
PlayerInfo playerInfo,
1001+
int index,
1002+
List<MediaItem> mediaItems,
1003+
long currentPositionMs,
1004+
long currentContentPositionMs) {
9881005
Timeline oldTimeline = playerInfo.timeline;
9891006
List<Window> newWindows = new ArrayList<>();
9901007
List<Period> newPeriods = new ArrayList<>();
@@ -1017,6 +1034,8 @@ private static PlayerInfo maskPlaybackInfoForAddedItems(
10171034
newTimeline,
10181035
newMediaItemIndex,
10191036
newPeriodIndex,
1037+
currentPositionMs,
1038+
currentContentPositionMs,
10201039
Player.DISCONTINUITY_REASON_INTERNAL);
10211040
}
10221041

@@ -1066,7 +1085,14 @@ private void removeMediaItemsInternal(int fromIndex, int toIndex) {
10661085
}
10671086
boolean wasCurrentItemRemoved =
10681087
getCurrentMediaItemIndex() >= fromIndex && getCurrentMediaItemIndex() < toIndex;
1069-
PlayerInfo newPlayerInfo = maskPlayerInfoForRemovedItems(playerInfo, fromIndex, toIndex);
1088+
PlayerInfo newPlayerInfo =
1089+
maskPlayerInfoForRemovedItems(
1090+
playerInfo,
1091+
fromIndex,
1092+
toIndex,
1093+
/* isReplacingItems= */ false,
1094+
getCurrentPosition(),
1095+
getContentPosition());
10701096
boolean didMediaItemTransitionHappen =
10711097
playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex >= fromIndex
10721098
&& playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex < toIndex;
@@ -1082,8 +1108,30 @@ private void removeMediaItemsInternal(int fromIndex, int toIndex) {
10821108
: null);
10831109
}
10841110

1111+
/**
1112+
* Returns a masking {@link PlayerInfo} for the removed {@linkplain MediaItem media items} with
1113+
* the provided information.
1114+
*
1115+
* @param playerInfo The {@link PlayerInfo} that the new masking {@link PlayerInfo} is based on.
1116+
* @param fromIndex The index at which to start removing media items (inclusive).
1117+
* @param toIndex The index of the first item to be kept (exclusive).
1118+
* @param isReplacingItems A boolean indicating whether the media items are removed due to
1119+
* replacing.
1120+
* @param currentPositionMs The current position in milliseconds. This value will be used in the
1121+
* new masking {@link PlayerInfo} if the removal of the media items doesn't affect the current
1122+
* playback position.
1123+
* @param currentContentPositionMs The current content position in milliseconds. This value will
1124+
* be used in the new masking {@link PlayerInfo} if the removal of the media items doesn't
1125+
* affect the current playback position.
1126+
* @return A masking {@link PlayerInfo}.
1127+
*/
10851128
private static PlayerInfo maskPlayerInfoForRemovedItems(
1086-
PlayerInfo playerInfo, int fromIndex, int toIndex) {
1129+
PlayerInfo playerInfo,
1130+
int fromIndex,
1131+
int toIndex,
1132+
boolean isReplacingItems,
1133+
long currentPositionMs,
1134+
long currentContentPositionMs) {
10871135
Timeline oldTimeline = playerInfo.timeline;
10881136
List<Window> newWindows = new ArrayList<>();
10891137
List<Period> newPeriods = new ArrayList<>();
@@ -1141,6 +1189,16 @@ private static PlayerInfo maskPlayerInfoForRemovedItems(
11411189
newPositionInfo,
11421190
SessionPositionInfo.DEFAULT,
11431191
Player.DISCONTINUITY_REASON_REMOVE);
1192+
} else if (isReplacingItems) {
1193+
newPlayerInfo =
1194+
maskTimelineAndPositionInfo(
1195+
playerInfo,
1196+
newTimeline,
1197+
newMediaItemIndex,
1198+
newPeriodIndex,
1199+
currentPositionMs,
1200+
currentContentPositionMs,
1201+
Player.DISCONTINUITY_REASON_REMOVE);
11441202
} else {
11451203
Window newWindow = newTimeline.getWindow(newMediaItemIndex, new Window());
11461204
long defaultPositionMs = newWindow.getDefaultPositionMs();
@@ -1182,6 +1240,8 @@ private static PlayerInfo maskPlayerInfoForRemovedItems(
11821240
newTimeline,
11831241
newMediaItemIndex,
11841242
newPeriodIndex,
1243+
currentPositionMs,
1244+
currentContentPositionMs,
11851245
Player.DISCONTINUITY_REASON_REMOVE);
11861246
}
11871247

@@ -1290,8 +1350,17 @@ private void replaceMediaItemsInternal(int fromIndex, int toIndex, List<MediaIte
12901350
return;
12911351
}
12921352
toIndex = min(toIndex, playlistSize);
1293-
PlayerInfo newPlayerInfo = maskPlaybackInfoForAddedItems(playerInfo, toIndex, mediaItems);
1294-
newPlayerInfo = maskPlayerInfoForRemovedItems(newPlayerInfo, fromIndex, toIndex);
1353+
PlayerInfo newPlayerInfo =
1354+
maskPlayerInfoForAddedItems(
1355+
playerInfo, toIndex, mediaItems, getCurrentPosition(), getContentPosition());
1356+
newPlayerInfo =
1357+
maskPlayerInfoForRemovedItems(
1358+
newPlayerInfo,
1359+
fromIndex,
1360+
toIndex,
1361+
/* isReplacingItems= */ true,
1362+
getCurrentPosition(),
1363+
getContentPosition());
12951364
boolean wasCurrentItemReplaced =
12961365
playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex >= fromIndex
12971366
&& playerInfo.sessionPositionInfo.positionInfo.mediaItemIndex < toIndex;
@@ -2111,6 +2180,8 @@ private void moveMediaItemsInternal(int fromIndex, int toIndex, int newIndex) {
21112180
newTimeline,
21122181
newWindowIndex,
21132182
newPeriodIndex,
2183+
getCurrentPosition(),
2184+
getContentPosition(),
21142185
Player.DISCONTINUITY_REASON_INTERNAL);
21152186

21162187
updatePlayerInfo(
@@ -2972,6 +3043,8 @@ private static PlayerInfo maskTimelineAndPositionInfo(
29723043
Timeline timeline,
29733044
int newMediaItemIndex,
29743045
int newPeriodIndex,
3046+
long newPositionMs,
3047+
long newContentPositionMs,
29753048
int discontinuityReason) {
29763049
PositionInfo newPositionInfo =
29773050
new PositionInfo(
@@ -2980,8 +3053,8 @@ private static PlayerInfo maskTimelineAndPositionInfo(
29803053
timeline.getWindow(newMediaItemIndex, new Window()).mediaItem,
29813054
/* periodUid= */ null,
29823055
newPeriodIndex,
2983-
playerInfo.sessionPositionInfo.positionInfo.positionMs,
2984-
playerInfo.sessionPositionInfo.positionInfo.contentPositionMs,
3056+
newPositionMs,
3057+
newContentPositionMs,
29853058
playerInfo.sessionPositionInfo.positionInfo.adGroupIndex,
29863059
playerInfo.sessionPositionInfo.positionInfo.adIndexInAdGroup);
29873060
return maskTimelineAndPositionInfo(

libraries/session/src/main/java/androidx/media3/session/MediaUtils.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,8 @@ public static long getUpdatedCurrentPositionMs(
244244
long lastSetPlayWhenReadyCalledTimeMs,
245245
long timeDiffMs) {
246246
boolean receivedUpdatedPositionInfo =
247-
lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs;
247+
playerInfo.sessionPositionInfo.equals(SessionPositionInfo.DEFAULT)
248+
|| (lastSetPlayWhenReadyCalledTimeMs < playerInfo.sessionPositionInfo.eventTimeMs);
248249
if (!playerInfo.isPlaying) {
249250
if (receivedUpdatedPositionInfo || currentPositionMs == C.TIME_UNSET) {
250251
return playerInfo.sessionPositionInfo.positionInfo.positionMs;

libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaControllerStateMaskingTest.java

+13
Original file line numberDiff line numberDiff line change
@@ -2348,6 +2348,7 @@ public void removeMediaItems_currentItemRemoved() throws Exception {
23482348
createMediaItems(firstMediaId, secondMediaId, thirdMediaId)))
23492349
.setCurrentMediaItemIndex(initialMediaItemIndex)
23502350
.setCurrentPeriodIndex(initialMediaItemIndex)
2351+
.setCurrentPosition(2000L)
23512352
.build();
23522353
remoteSession.setPlayer(playerConfig);
23532354

@@ -2409,6 +2410,7 @@ public void onEvents(Player player, Player.Events events) {
24092410
/* testLastPeriodIndex= */ testCurrentMediaItemIndex);
24102411
assertThat(newMediaItemRef.get().mediaId).isEqualTo(testCurrentMediaId);
24112412
assertThat(newPositionInfoRef.get().mediaItemIndex).isEqualTo(testCurrentMediaItemIndex);
2413+
assertThat(newPositionInfoRef.get().positionMs).isEqualTo(0L);
24122414
assertThat(getEventsAsList(onEventsRef.get()))
24132415
.containsExactly(
24142416
Player.EVENT_TIMELINE_CHANGED,
@@ -3439,12 +3441,14 @@ public void replaceMediaItems_replacingCurrentItem_correctMasking() throws Excep
34393441
new RemoteMediaSession.MockPlayerConfigBuilder()
34403442
.setTimeline(MediaTestUtils.createTimeline(3))
34413443
.setCurrentMediaItemIndex(1)
3444+
.setCurrentPosition(2000L)
34423445
.build();
34433446
remoteSession.setPlayer(playerConfig);
34443447
MediaController controller = controllerTestRule.createController(remoteSession.getToken());
34453448
CountDownLatch latch = new CountDownLatch(2);
34463449
AtomicReference<Timeline> newTimelineRef = new AtomicReference<>();
34473450
AtomicReference<Player.Events> onEventsRef = new AtomicReference<>();
3451+
AtomicReference<PositionInfo> newPositionInfoRef = new AtomicReference<>();
34483452
Player.Listener listener =
34493453
new Player.Listener() {
34503454
@Override
@@ -3458,6 +3462,14 @@ public void onEvents(Player player, Player.Events events) {
34583462
onEventsRef.set(events);
34593463
latch.countDown();
34603464
}
3465+
3466+
@Override
3467+
public void onPositionDiscontinuity(
3468+
PositionInfo oldPosition,
3469+
PositionInfo newPosition,
3470+
@Player.DiscontinuityReason int reason) {
3471+
newPositionInfoRef.set(newPosition);
3472+
}
34613473
};
34623474
threadTestRule.getHandler().postAndSync(() -> controller.addListener(listener));
34633475
AtomicInteger currentMediaItemIndexRef = new AtomicInteger();
@@ -3478,6 +3490,7 @@ public void onEvents(Player player, Player.Events events) {
34783490
Player.EVENT_TIMELINE_CHANGED,
34793491
Player.EVENT_POSITION_DISCONTINUITY,
34803492
Player.EVENT_MEDIA_ITEM_TRANSITION);
3493+
assertThat(newPositionInfoRef.get().positionMs).isEqualTo(2000L);
34813494
}
34823495

34833496
@Test

0 commit comments

Comments
 (0)