loliball/media3
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Forward presumed no-op seeks to handler methods in (Simple)BasePlayer

Browse Source 
Some seek operations are currently filtered in the base classes if
the target index is not explicitly specified and the implicitly
assumed operation doesn't have an obvious target index. (Example:
calling seekToNext() at the last item in a playlist)

This is too opinionated because the actual player implementation
using this base class may be able to handle this seek request (e.g.
by adding new items on the fly or using other logic to select
the most suitable next item).

This can be solved by forwarding all seek requests to the respective
handler methods even if they are a presumed no-op. Also clarify the
Javadoc that the provided index is just an assumption if it wasn't
provided explicitly in the method call.

PiperOrigin-RevId: 624887116
This commit is contained in:
tonihei 2024-04-15 02:36:16 -07:00 committed by Copybara-Service
parent c74603b66b
commit 11257ecacb
7 changed files with 194 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
RELEASENOTES.md
View File

@ -3,6 +3,10 @@
### Unreleased changes
* Common Library:
* Forward presumed no-op seek calls to the protected `BasePlayer.seekTo`
and `SimpleBasePlayer.handleSeek` methods instead of ignoring them. If
you are implementing these methods in a custom player, you may need to
handle these additional calls with `mediaItemIndex == C.INDEX_UNSET`.
* ExoPlayer:
* Add `reset` to `BasePreloadManager` to release all the holding sources
while keep the preload manager instance.

3
libraries/cast/src/main/java/androidx/media3/cast/CastPlayer.java
View File

@ -417,6 +417,9 @@ public final class CastPlayer extends BasePlayer {
long positionMs,
@Player.Command int seekCommand,
boolean isRepeatingCurrentItem) {
if (mediaItemIndex == C.INDEX_UNSET) {
return;
}
checkArgument(mediaItemIndex >= 0);
if (!currentTimeline.isEmpty() && mediaItemIndex >= currentTimeline.getWindowCount()) {
return;

24
libraries/common/src/main/java/androidx/media3/common/BasePlayer.java
View File

@ -197,12 +197,15 @@ public abstract class BasePlayer implements Player {
public final void seekToPrevious() {
Timeline timeline = getCurrentTimeline();
if (timeline.isEmpty() || isPlayingAd()) {
ignoreSeek(Player.COMMAND_SEEK_TO_PREVIOUS);
return;
}
boolean hasPreviousMediaItem = hasPreviousMediaItem();
if (isCurrentMediaItemLive() && !isCurrentMediaItemSeekable()) {
if (hasPreviousMediaItem) {
seekToPreviousMediaItemInternal(Player.COMMAND_SEEK_TO_PREVIOUS);
} else {
ignoreSeek(Player.COMMAND_SEEK_TO_PREVIOUS);
}
} else if (hasPreviousMediaItem && getCurrentPosition() <= getMaxSeekToPreviousPosition()) {
seekToPreviousMediaItemInternal(Player.COMMAND_SEEK_TO_PREVIOUS);
@ -261,12 +264,15 @@ public abstract class BasePlayer implements Player {
public final void seekToNext() {
Timeline timeline = getCurrentTimeline();
if (timeline.isEmpty() || isPlayingAd()) {
ignoreSeek(Player.COMMAND_SEEK_TO_NEXT);
return;
}
if (hasNextMediaItem()) {
seekToNextMediaItemInternal(Player.COMMAND_SEEK_TO_NEXT);
} else if (isCurrentMediaItemLive() && isCurrentMediaItemDynamic()) {
seekToDefaultPositionInternal(getCurrentMediaItemIndex(), Player.COMMAND_SEEK_TO_NEXT);
} else {
ignoreSeek(Player.COMMAND_SEEK_TO_NEXT);
}
}
@ -287,9 +293,13 @@ public abstract class BasePlayer implements Player {
/**
* Seeks to a position in the specified {@link MediaItem}.
*
* @param mediaItemIndex The index of the {@link MediaItem}.
* @param mediaItemIndex The index of the {@link MediaItem}. If the original seek operation did
* not directly specify an index, this is the most likely implied index based on the available
* player state. If the implied action is to do nothing, this will be {@link C#INDEX_UNSET}.
* @param positionMs The seek position in the specified {@link MediaItem} in milliseconds, or
* {@link C#TIME_UNSET} to seek to the media item's default position.
* {@link C#TIME_UNSET} to seek to the media item's default position. If the original seek
* operation did not directly specify a position, this is the most likely implied position
* based on the available player state.
* @param seekCommand The {@link Player.Command} used to trigger the seek.
* @param isRepeatingCurrentItem Whether this seeks repeats the current item.
*/
@ -459,6 +469,14 @@ public abstract class BasePlayer implements Player {
return repeatMode == REPEAT_MODE_ONE ? REPEAT_MODE_OFF : repeatMode;
}
private void ignoreSeek(@Player.Command int seekCommand) {
seekTo(
/* mediaItemIndex= */ C.INDEX_UNSET,
/* positionMs= */ C.TIME_UNSET,
seekCommand,
/* isRepeatingCurrentItem= */ false);
}
private void seekToCurrentItem(long positionMs, @Player.Command int seekCommand) {
seekTo(
getCurrentMediaItemIndex(), positionMs, seekCommand, /* isRepeatingCurrentItem= */ false);
@ -485,6 +503,7 @@ public abstract class BasePlayer implements Player {
private void seekToNextMediaItemInternal(@Player.Command int seekCommand) {
int nextMediaItemIndex = getNextMediaItemIndex();
if (nextMediaItemIndex == C.INDEX_UNSET) {
ignoreSeek(seekCommand);
return;
}
if (nextMediaItemIndex == getCurrentMediaItemIndex()) {
@ -497,6 +516,7 @@ public abstract class BasePlayer implements Player {
private void seekToPreviousMediaItemInternal(@Player.Command int seekCommand) {
int previousMediaItemIndex = getPreviousMediaItemIndex();
if (previousMediaItemIndex == C.INDEX_UNSET) {
ignoreSeek(seekCommand);
return;
}
if (previousMediaItemIndex == getCurrentMediaItemIndex()) {

51
libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java
View File

@ -2352,19 +2352,24 @@ public abstract class SimpleBasePlayer extends BasePlayer {
@Player.Command int seekCommand,
boolean isRepeatingCurrentItem) {
verifyApplicationThreadAndInitState();
checkArgument(mediaItemIndex >= 0);
checkArgument(mediaItemIndex == C.INDEX_UNSET || mediaItemIndex >= 0);
// Use a local copy to ensure the lambda below uses the current state value.
State state = this.state;
if (!shouldHandleCommand(seekCommand)
|| isPlayingAd()
|| (!state.playlist.isEmpty() && mediaItemIndex >= state.playlist.size())) {
if (!shouldHandleCommand(seekCommand)) {
return;
}
boolean ignoreSeekForPlaceholderState =
mediaItemIndex == C.INDEX_UNSET
|| isPlayingAd()
|| (!state.playlist.isEmpty() && mediaItemIndex >= state.playlist.size());
updateStateForPendingOperation(
/* pendingOperation= */ handleSeek(mediaItemIndex, positionMs, seekCommand),
/* placeholderStateSupplier= */ () ->
getStateWithNewPlaylistAndPosition(state, state.playlist, mediaItemIndex, positionMs),
/* seeked= */ true,
ignoreSeekForPlaceholderState
? state
: getStateWithNewPlaylistAndPosition(
state, state.playlist, mediaItemIndex, positionMs),
/* forceSeekDiscontinuity= */ !ignoreSeekForPlaceholderState,
isRepeatingCurrentItem);
}
@ -2921,7 +2926,7 @@ public abstract class SimpleBasePlayer extends BasePlayer {
return;
}
updateStateAndInformListeners(
getState(), /* seeked= */ false, /* isRepeatingCurrentItem= */ false);
getState(), /* forceSeekDiscontinuity= */ false, /* isRepeatingCurrentItem= */ false);
}
/**
@ -3344,10 +3349,13 @@ public abstract class SimpleBasePlayer extends BasePlayer {
* <p>Will only be called if the appropriate {@link Player.Command}, for example {@link
* Player#COMMAND_SEEK_TO_MEDIA_ITEM} or {@link Player#COMMAND_SEEK_TO_NEXT}, is available.
*
* @param mediaItemIndex The media item index to seek to. The index is in the range 0 &lt;= {@code
* mediaItemIndex} &lt; {@code mediaItems.size()}.
* @param mediaItemIndex The media item index to seek to. If the original seek operation did not
* directly specify an index, this is the most likely implied index based on the available
* player state. If the implied action is to do nothing, this will be {@link C#INDEX_UNSET}.
* @param positionMs The position in milliseconds to start playback from, or {@link C#TIME_UNSET}
* to start at the default position in the media item.
* to start at the default position in the media item. If the original seek operation did not
* directly specify a position, this is the most likely implied position based on the
* available player state.
* @param seekCommand The {@link Player.Command} used to trigger the seek.
* @return A {@link ListenableFuture} indicating the completion of all immediate {@link State}
* changes caused by this call.
@ -3385,7 +3393,7 @@ public abstract class SimpleBasePlayer extends BasePlayer {
@SuppressWarnings("deprecation") // Calling deprecated listener methods.
@RequiresNonNull("state")
private void updateStateAndInformListeners(
State newState, boolean seeked, boolean isRepeatingCurrentItem) {
State newState, boolean forceSeekDiscontinuity, boolean isRepeatingCurrentItem) {
State previousState = state;
// Assign new state immediately such that all getters return the right values, but use a
// snapshot of the previous and new state so that listener invocations are triggered correctly.
@ -3407,7 +3415,8 @@ public abstract class SimpleBasePlayer extends BasePlayer {
MediaMetadata previousMediaMetadata = getMediaMetadataInternal(previousState);
MediaMetadata newMediaMetadata = getMediaMetadataInternal(newState);
int positionDiscontinuityReason =
getPositionDiscontinuityReason(previousState, newState, seeked, window, period);
getPositionDiscontinuityReason(
previousState, newState, forceSeekDiscontinuity, window, period);
boolean timelineChanged = !previousState.timeline.equals(newState.timeline);
int mediaItemTransitionReason =
getMediaItemTransitionReason(
@ -3618,7 +3627,7 @@ public abstract class SimpleBasePlayer extends BasePlayer {
updateStateForPendingOperation(
pendingOperation,
placeholderStateSupplier,
/* seeked= */ false,
/* forceSeekDiscontinuity= */ false,
/* isRepeatingCurrentItem= */ false);
}
@ -3626,22 +3635,26 @@ public abstract class SimpleBasePlayer extends BasePlayer {
private void updateStateForPendingOperation(
ListenableFuture<?> pendingOperation,
Supplier<State> placeholderStateSupplier,
boolean seeked,
boolean forceSeekDiscontinuity,
boolean isRepeatingCurrentItem) {
if (pendingOperation.isDone() && pendingOperations.isEmpty()) {
updateStateAndInformListeners(getState(), seeked, isRepeatingCurrentItem);
updateStateAndInformListeners(getState(), forceSeekDiscontinuity, isRepeatingCurrentItem);
} else {
pendingOperations.add(pendingOperation);
State suggestedPlaceholderState = placeholderStateSupplier.get();
updateStateAndInformListeners(
getPlaceholderState(suggestedPlaceholderState), seeked, isRepeatingCurrentItem);
getPlaceholderState(suggestedPlaceholderState),
forceSeekDiscontinuity,
isRepeatingCurrentItem);
pendingOperation.addListener(
() -> {
castNonNull(state); // Already checked by method @RequiresNonNull pre-condition.
pendingOperations.remove(pendingOperation);
if (pendingOperations.isEmpty() && !released) {
updateStateAndInformListeners(
getState(), /* seeked= */ false, /* isRepeatingCurrentItem= */ false);
getState(),
/* forceSeekDiscontinuity= */ false,
/* isRepeatingCurrentItem= */ false);
}
},
this::postOrRunOnApplicationHandler);
@ -3740,14 +3753,14 @@ public abstract class SimpleBasePlayer extends BasePlayer {
private static int getPositionDiscontinuityReason(
State previousState,
State newState,
boolean seeked,
boolean forceSeekDiscontinuity,
Timeline.Window window,
Timeline.Period period) {
if (newState.hasPositionDiscontinuity) {
// We were asked to report a discontinuity.
return newState.positionDiscontinuityReason;
}
if (seeked) {
if (forceSeekDiscontinuity) {
return Player.DISCONTINUITY_REASON_SEEK;
}
if (previousState.playlist.isEmpty()) {

73
libraries/common/src/test/java/androidx/media3/common/BasePlayerTest.java
View File

@ -119,6 +119,32 @@ public class BasePlayerTest {
/* isRepeatingCurrentItem= */ false);
}
@Test
public void seekToNext_withoutNextItem_forwardsCallWithUnsetMediaItemIndex() {
BasePlayer player =
spy(
new TestBasePlayer() {
@Override
public Timeline getCurrentTimeline() {
return new FakeTimeline(/* windowCount= */ 3);
}
@Override
public int getCurrentMediaItemIndex() {
return 2;
}
});
player.seekToNext();
verify(player)
.seekTo(
/* mediaItemIndex= */ C.INDEX_UNSET,
/* positionMs= */ C.TIME_UNSET,
Player.COMMAND_SEEK_TO_NEXT,
/* isRepeatingCurrentItem= */ false);
}
@Test
public void seekToNextMediaItem_usesCommandSeekToNextMediaItem() {
BasePlayer player =
@ -140,6 +166,32 @@ public class BasePlayerTest {
/* isRepeatingCurrentItem= */ false);
}
@Test
public void seekToNextMediaItem_withoutNextItem_forwardsCallWithUnsetMediaItemIndex() {
BasePlayer player =
spy(
new TestBasePlayer() {
@Override
public Timeline getCurrentTimeline() {
return new FakeTimeline(/* windowCount= */ 3);
}
@Override
public int getCurrentMediaItemIndex() {
return 2;
}
});
player.seekToNextMediaItem();
verify(player)
.seekTo(
/* mediaItemIndex= */ C.INDEX_UNSET,
/* positionMs= */ C.TIME_UNSET,
Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM,
/* isRepeatingCurrentItem= */ false);
}
@Test
public void seekForward_usesCommandSeekForward() {
BasePlayer player =
@ -223,6 +275,27 @@ public class BasePlayerTest {
/* isRepeatingCurrentItem= */ false);
}
@Test
public void seekToPreviousMediaItem_withoutPreviousItem_forwardsCallWithUnsetMediaItemIndex() {
BasePlayer player =
spy(
new TestBasePlayer() {
@Override
public int getCurrentMediaItemIndex() {
return 0;
}
});
player.seekToPreviousMediaItem();
verify(player)
.seekTo(
/* mediaItemIndex= */ C.INDEX_UNSET,
/* positionMs= */ C.TIME_UNSET,
Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM,
/* isRepeatingCurrentItem= */ false);
}
@Test
public void seekBack_usesCommandSeekBack() {
BasePlayer player =

56
libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java
View File

@ -8013,6 +8013,62 @@ public class SimpleBasePlayerTest {
verifyNoMoreInteractions(listener);
}
@SuppressWarnings("deprecation") // Verifying deprecated listener calls.
@Test
public void seekTo_asyncHandlingForNoImpliedActionSeeks_usesCurrentStateAsPlaceholderState() {
MediaItem mediaItem = new MediaItem.Builder().setMediaId("2").build();
State state =
new State.Builder()
.setAvailableCommands(new Commands.Builder().addAllCommands().build())
.setPlaylist(
ImmutableList.of(
new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ 1)
.setMediaItem(mediaItem)
.build(),
new SimpleBasePlayer.MediaItemData.Builder(/* uid= */ 2).build()))
.setCurrentMediaItemIndex(1)
.build();
// Change updated state to see a difference to the placeholder state.
State updatedState =
state
.buildUpon()
.setCurrentMediaItemIndex(0)
.setPositionDiscontinuity(
Player.DISCONTINUITY_REASON_SEEK, /* discontinuityPositionMs= */ 0)
.build();
SettableFuture<?> future = SettableFuture.create();
SimpleBasePlayer player =
new SimpleBasePlayer(Looper.myLooper()) {
@Override
protected State getState() {
return future.isDone() ? updatedState : state;
}
@Override
protected ListenableFuture<?> handleSeek(
int mediaItemIndex, long positionMs, @Player.Command int seekCommand) {
return future;
}
};
Listener listener = mock(Listener.class);
player.addListener(listener);
player.seekToNext();
// Verify placeholder state is the same as before with no listener updates.
assertThat(player.getCurrentMediaItemIndex()).isEqualTo(1);
verifyNoMoreInteractions(listener);
future.set(null);
// Verify actual state update.
assertThat(player.getCurrentMediaItemIndex()).isEqualTo(0);
verify(listener).onPositionDiscontinuity(Player.DISCONTINUITY_REASON_SEEK);
verify(listener).onPositionDiscontinuity(any(), any(), eq(Player.DISCONTINUITY_REASON_SEEK));
verify(listener).onMediaItemTransition(mediaItem, Player.MEDIA_ITEM_TRANSITION_REASON_SEEK);
verifyNoMoreInteractions(listener);
}
@Test
public void seekTo_withoutAvailableCommandForSeekToMediaItem_isNotForwarded() {
State state =

5
libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImpl.java
View File

@ -890,12 +890,15 @@ import java.util.concurrent.TimeoutException;
@Player.Command int seekCommand,
boolean isRepeatingCurrentItem) {
verifyApplicationThread();
if (mediaItemIndex == C.INDEX_UNSET) {
return;
}
checkArgument(mediaItemIndex >= 0);
analyticsCollector.notifySeekStarted();
Timeline timeline = playbackInfo.timeline;
if (!timeline.isEmpty() && mediaItemIndex >= timeline.getWindowCount()) {
return;
}
analyticsCollector.notifySeekStarted();
pendingOperationAcks++;
if (isPlayingAd()) {
// TODO: Investigate adding support for seeking during ads. This is complicated to do in