mirror of
https://github.com/androidx/media.git
synced 2025-05-04 14:10:40 +08:00
Ensure onMediaItemTransition is sent for repeats of the same item
Currently, repeating the same item (via seekNext/Previous) implicitly results in a seek to the default position of the current item, which looks exactly the same as a direct seek. As a result, we don't send onMediaItemTransition as we would for every other seekNext/Previous call. This can be fixed by explicitly marking the repeat case in the internal BasePlayer/ExoPlayerImpl methods, so that the callback can be triggered. Issue: google/ExoPlayer#10667 PiperOrigin-RevId: 481951788 (cherry picked from commit 76ce0cc69fa12a9b2592103da97dd45c15bfe4d3)
This commit is contained in:
tonihei
microkatz
parent
66bae6059b
commit
ed9bc9ad5d
@ -21,6 +21,7 @@ import static java.lang.Math.min;
|
||||
import androidx.annotation.Nullable;
|
||||
import com.google.android.exoplayer2.util.Util;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.errorprone.annotations.ForOverride;
|
||||
import java.util.List;
|
||||
|
||||
/** Abstract base {@link Player} which implements common implementation independent methods. */
|
||||
@ -185,7 +186,12 @@ public abstract class BasePlayer implements Player {
|
||||
@Override
|
||||
public final void seekToPreviousMediaItem() {
|
||||
int previousMediaItemIndex = getPreviousMediaItemIndex();
|
||||
if (previousMediaItemIndex != C.INDEX_UNSET) {
|
||||
if (previousMediaItemIndex == C.INDEX_UNSET) {
|
||||
return;
|
||||
}
|
||||
if (previousMediaItemIndex == getCurrentMediaItemIndex()) {
|
||||
repeatCurrentMediaItem();
|
||||
} else {
|
||||
seekToDefaultPosition(previousMediaItemIndex);
|
||||
}
|
||||
}
|
||||
@ -252,7 +258,12 @@ public abstract class BasePlayer implements Player {
|
||||
@Override
|
||||
public final void seekToNextMediaItem() {
|
||||
int nextMediaItemIndex = getNextMediaItemIndex();
|
||||
if (nextMediaItemIndex != C.INDEX_UNSET) {
|
||||
if (nextMediaItemIndex == C.INDEX_UNSET) {
|
||||
return;
|
||||
}
|
||||
if (nextMediaItemIndex == getCurrentMediaItemIndex()) {
|
||||
repeatCurrentMediaItem();
|
||||
} else {
|
||||
seekToDefaultPosition(nextMediaItemIndex);
|
||||
}
|
||||
}
|
||||
@ -424,6 +435,17 @@ public abstract class BasePlayer implements Player {
|
||||
: timeline.getWindow(getCurrentMediaItemIndex(), window).getDurationMs();
|
||||
}
|
||||
|
||||
/**
|
||||
* Repeat the current media item.
|
||||
*
|
||||
* <p>The default implementation seeks to the default position in the current item, which can be
|
||||
* overridden for additional handling.
|
||||
*/
|
||||
@ForOverride
|
||||
protected void repeatCurrentMediaItem() {
|
||||
seekToDefaultPosition();
|
||||
}
|
||||
|
||||
private @RepeatMode int getRepeatModeForNavigation() {
|
||||
@RepeatMode int repeatMode = getRepeatMode();
|
||||
return repeatMode == REPEAT_MODE_ONE ? REPEAT_MODE_OFF : repeatMode;
|
||||
|
||||
@ -531,7 +531,8 @@ import java.util.concurrent.TimeoutException;
|
||||
/* positionDiscontinuity= */ false,
|
||||
/* ignored */ DISCONTINUITY_REASON_INTERNAL,
|
||||
/* ignored */ C.TIME_UNSET,
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -652,7 +653,8 @@ import java.util.concurrent.TimeoutException;
|
||||
/* positionDiscontinuity= */ false,
|
||||
/* ignored */ DISCONTINUITY_REASON_INTERNAL,
|
||||
/* ignored */ C.TIME_UNSET,
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -670,7 +672,8 @@ import java.util.concurrent.TimeoutException;
|
||||
positionDiscontinuity,
|
||||
DISCONTINUITY_REASON_REMOVE,
|
||||
/* discontinuityWindowStartPositionUs= */ getCurrentPositionUsInternal(newPlaybackInfo),
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -700,7 +703,8 @@ import java.util.concurrent.TimeoutException;
|
||||
/* positionDiscontinuity= */ false,
|
||||
/* ignored */ DISCONTINUITY_REASON_INTERNAL,
|
||||
/* ignored */ C.TIME_UNSET,
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -724,7 +728,8 @@ import java.util.concurrent.TimeoutException;
|
||||
/* positionDiscontinuity= */ false,
|
||||
/* ignored */ DISCONTINUITY_REASON_INTERNAL,
|
||||
/* ignored */ C.TIME_UNSET,
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -803,47 +808,17 @@ import java.util.concurrent.TimeoutException;
|
||||
return playbackInfo.isLoading;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void repeatCurrentMediaItem() {
|
||||
verifyApplicationThread();
|
||||
seekToInternal(
|
||||
getCurrentMediaItemIndex(), /* positionMs= */ C.TIME_UNSET, /* repeatMediaItem= */ true);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void seekTo(int mediaItemIndex, long positionMs) {
|
||||
verifyApplicationThread();
|
||||
analyticsCollector.notifySeekStarted();
|
||||
Timeline timeline = playbackInfo.timeline;
|
||||
if (mediaItemIndex < 0
|
||||
|| (!timeline.isEmpty() && mediaItemIndex >= timeline.getWindowCount())) {
|
||||
throw new IllegalSeekPositionException(timeline, mediaItemIndex, positionMs);
|
||||
}
|
||||
pendingOperationAcks++;
|
||||
if (isPlayingAd()) {
|
||||
// TODO: Investigate adding support for seeking during ads. This is complicated to do in
|
||||
// general because the midroll ad preceding the seek destination must be played before the
|
||||
// content position can be played, if a different ad is playing at the moment.
|
||||
Log.w(TAG, "seekTo ignored because an ad is playing");
|
||||
ExoPlayerImplInternal.PlaybackInfoUpdate playbackInfoUpdate =
|
||||
new ExoPlayerImplInternal.PlaybackInfoUpdate(this.playbackInfo);
|
||||
playbackInfoUpdate.incrementPendingOperationAcks(1);
|
||||
playbackInfoUpdateListener.onPlaybackInfoUpdate(playbackInfoUpdate);
|
||||
return;
|
||||
}
|
||||
@Player.State
|
||||
int newPlaybackState =
|
||||
getPlaybackState() == Player.STATE_IDLE ? Player.STATE_IDLE : STATE_BUFFERING;
|
||||
int oldMaskingMediaItemIndex = getCurrentMediaItemIndex();
|
||||
PlaybackInfo newPlaybackInfo = playbackInfo.copyWithPlaybackState(newPlaybackState);
|
||||
newPlaybackInfo =
|
||||
maskTimelineAndPosition(
|
||||
newPlaybackInfo,
|
||||
timeline,
|
||||
maskWindowPositionMsOrGetPeriodPositionUs(timeline, mediaItemIndex, positionMs));
|
||||
internalPlayer.seekTo(timeline, mediaItemIndex, Util.msToUs(positionMs));
|
||||
updatePlaybackInfo(
|
||||
newPlaybackInfo,
|
||||
/* ignored */ TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED,
|
||||
/* ignored */ PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST,
|
||||
/* seekProcessed= */ true,
|
||||
/* positionDiscontinuity= */ true,
|
||||
/* positionDiscontinuityReason= */ DISCONTINUITY_REASON_SEEK,
|
||||
/* discontinuityWindowStartPositionUs= */ getCurrentPositionUsInternal(newPlaybackInfo),
|
||||
oldMaskingMediaItemIndex);
|
||||
seekToInternal(mediaItemIndex, positionMs, /* repeatMediaItem= */ false);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -884,7 +859,8 @@ import java.util.concurrent.TimeoutException;
|
||||
/* positionDiscontinuity= */ false,
|
||||
/* ignored */ DISCONTINUITY_REASON_INTERNAL,
|
||||
/* ignored */ C.TIME_UNSET,
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -1733,7 +1709,8 @@ import java.util.concurrent.TimeoutException;
|
||||
positionDiscontinuity,
|
||||
DISCONTINUITY_REASON_REMOVE,
|
||||
/* discontinuityWindowStartPositionUs= */ getCurrentPositionUsInternal(playbackInfo),
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
private int getCurrentWindowIndexInternal() {
|
||||
@ -1815,7 +1792,8 @@ import java.util.concurrent.TimeoutException;
|
||||
positionDiscontinuity,
|
||||
pendingDiscontinuityReason,
|
||||
discontinuityWindowStartPositionUs,
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
}
|
||||
|
||||
@ -1829,7 +1807,8 @@ import java.util.concurrent.TimeoutException;
|
||||
boolean positionDiscontinuity,
|
||||
@DiscontinuityReason int positionDiscontinuityReason,
|
||||
long discontinuityWindowStartPositionUs,
|
||||
int oldMaskingMediaItemIndex) {
|
||||
int oldMaskingMediaItemIndex,
|
||||
boolean repeatCurrentMediaItem) {
|
||||
|
||||
// Assign playback info immediately such that all getters return the right values, but keep
|
||||
// snapshot of previous and new state so that listener invocations are triggered correctly.
|
||||
@ -1837,13 +1816,15 @@ import java.util.concurrent.TimeoutException;
|
||||
PlaybackInfo newPlaybackInfo = playbackInfo;
|
||||
this.playbackInfo = playbackInfo;
|
||||
|
||||
boolean timelineChanged = !previousPlaybackInfo.timeline.equals(newPlaybackInfo.timeline);
|
||||
Pair<Boolean, Integer> mediaItemTransitionInfo =
|
||||
evaluateMediaItemTransitionReason(
|
||||
newPlaybackInfo,
|
||||
previousPlaybackInfo,
|
||||
positionDiscontinuity,
|
||||
positionDiscontinuityReason,
|
||||
!previousPlaybackInfo.timeline.equals(newPlaybackInfo.timeline));
|
||||
timelineChanged,
|
||||
repeatCurrentMediaItem);
|
||||
boolean mediaItemTransitioned = mediaItemTransitionInfo.first;
|
||||
int mediaItemTransitionReason = mediaItemTransitionInfo.second;
|
||||
MediaMetadata newMediaMetadata = mediaMetadata;
|
||||
@ -1880,7 +1861,7 @@ import java.util.concurrent.TimeoutException;
|
||||
updatePriorityTaskManagerForIsLoadingChange(newPlaybackInfo.isLoading);
|
||||
}
|
||||
|
||||
if (!previousPlaybackInfo.timeline.equals(newPlaybackInfo.timeline)) {
|
||||
if (timelineChanged) {
|
||||
listeners.queueEvent(
|
||||
Player.EVENT_TIMELINE_CHANGED,
|
||||
listener -> listener.onTimelineChanged(newPlaybackInfo.timeline, timelineChangeReason));
|
||||
@ -2083,7 +2064,8 @@ import java.util.concurrent.TimeoutException;
|
||||
PlaybackInfo oldPlaybackInfo,
|
||||
boolean positionDiscontinuity,
|
||||
@DiscontinuityReason int positionDiscontinuityReason,
|
||||
boolean timelineChanged) {
|
||||
boolean timelineChanged,
|
||||
boolean repeatCurrentMediaItem) {
|
||||
|
||||
Timeline oldTimeline = oldPlaybackInfo.timeline;
|
||||
Timeline newTimeline = playbackInfo.timeline;
|
||||
@ -2114,12 +2096,21 @@ import java.util.concurrent.TimeoutException;
|
||||
throw new IllegalStateException();
|
||||
}
|
||||
return new Pair<>(/* isTransitioning */ true, transitionReason);
|
||||
} else if (positionDiscontinuity
|
||||
} else {
|
||||
// Only mark changes within the current item as a transition if we are repeating automatically
|
||||
// or via a seek to next/previous.
|
||||
if (positionDiscontinuity
|
||||
&& positionDiscontinuityReason == DISCONTINUITY_REASON_AUTO_TRANSITION
|
||||
&& oldPlaybackInfo.periodId.windowSequenceNumber
|
||||
< playbackInfo.periodId.windowSequenceNumber) {
|
||||
return new Pair<>(/* isTransitioning */ true, MEDIA_ITEM_TRANSITION_REASON_REPEAT);
|
||||
}
|
||||
if (positionDiscontinuity
|
||||
&& positionDiscontinuityReason == DISCONTINUITY_REASON_SEEK
|
||||
&& repeatCurrentMediaItem) {
|
||||
return new Pair<>(/* isTransitioning */ true, MEDIA_ITEM_TRANSITION_REASON_SEEK);
|
||||
}
|
||||
}
|
||||
return new Pair<>(/* isTransitioning */ false, /* mediaItemTransitionReason */ C.INDEX_UNSET);
|
||||
}
|
||||
|
||||
@ -2189,7 +2180,8 @@ import java.util.concurrent.TimeoutException;
|
||||
/* positionDiscontinuity= */ positionDiscontinuity,
|
||||
DISCONTINUITY_REASON_REMOVE,
|
||||
/* discontinuityWindowStartPositionUs= */ getCurrentPositionUsInternal(newPlaybackInfo),
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
private List<MediaSourceList.MediaSourceHolder> addMediaSourceHolders(
|
||||
@ -2581,7 +2573,8 @@ import java.util.concurrent.TimeoutException;
|
||||
/* positionDiscontinuity= */ false,
|
||||
/* ignored */ DISCONTINUITY_REASON_INTERNAL,
|
||||
/* ignored */ C.TIME_UNSET,
|
||||
/* ignored */ C.INDEX_UNSET);
|
||||
/* ignored */ C.INDEX_UNSET,
|
||||
/* repeatCurrentMediaItem= */ false);
|
||||
}
|
||||
|
||||
private void updateWakeAndWifiLock() {
|
||||
@ -2679,6 +2672,48 @@ import java.util.concurrent.TimeoutException;
|
||||
}
|
||||
}
|
||||
|
||||
private void seekToInternal(int mediaItemIndex, long positionMs, boolean repeatMediaItem) {
|
||||
analyticsCollector.notifySeekStarted();
|
||||
Timeline timeline = playbackInfo.timeline;
|
||||
if (mediaItemIndex < 0
|
||||
|| (!timeline.isEmpty() && mediaItemIndex >= timeline.getWindowCount())) {
|
||||
throw new IllegalSeekPositionException(timeline, mediaItemIndex, positionMs);
|
||||
}
|
||||
pendingOperationAcks++;
|
||||
if (isPlayingAd()) {
|
||||
// TODO: Investigate adding support for seeking during ads. This is complicated to do in
|
||||
// general because the midroll ad preceding the seek destination must be played before the
|
||||
// content position can be played, if a different ad is playing at the moment.
|
||||
Log.w(TAG, "seekTo ignored because an ad is playing");
|
||||
ExoPlayerImplInternal.PlaybackInfoUpdate playbackInfoUpdate =
|
||||
new ExoPlayerImplInternal.PlaybackInfoUpdate(this.playbackInfo);
|
||||
playbackInfoUpdate.incrementPendingOperationAcks(1);
|
||||
playbackInfoUpdateListener.onPlaybackInfoUpdate(playbackInfoUpdate);
|
||||
return;
|
||||
}
|
||||
@Player.State
|
||||
int newPlaybackState =
|
||||
getPlaybackState() == Player.STATE_IDLE ? Player.STATE_IDLE : STATE_BUFFERING;
|
||||
int oldMaskingMediaItemIndex = getCurrentMediaItemIndex();
|
||||
PlaybackInfo newPlaybackInfo = playbackInfo.copyWithPlaybackState(newPlaybackState);
|
||||
newPlaybackInfo =
|
||||
maskTimelineAndPosition(
|
||||
newPlaybackInfo,
|
||||
timeline,
|
||||
maskWindowPositionMsOrGetPeriodPositionUs(timeline, mediaItemIndex, positionMs));
|
||||
internalPlayer.seekTo(timeline, mediaItemIndex, Util.msToUs(positionMs));
|
||||
updatePlaybackInfo(
|
||||
newPlaybackInfo,
|
||||
/* ignored */ TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED,
|
||||
/* ignored */ PLAY_WHEN_READY_CHANGE_REASON_USER_REQUEST,
|
||||
/* seekProcessed= */ true,
|
||||
/* positionDiscontinuity= */ true,
|
||||
/* positionDiscontinuityReason= */ DISCONTINUITY_REASON_SEEK,
|
||||
/* discontinuityWindowStartPositionUs= */ getCurrentPositionUsInternal(newPlaybackInfo),
|
||||
oldMaskingMediaItemIndex,
|
||||
repeatMediaItem);
|
||||
}
|
||||
|
||||
private static DeviceInfo createDeviceInfo(StreamVolumeManager streamVolumeManager) {
|
||||
return new DeviceInfo(
|
||||
DeviceInfo.PLAYBACK_TYPE_LOCAL,
|
||||
|
||||
@ -8811,6 +8811,39 @@ public final class ExoPlayerTest {
|
||||
player.release();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void seekToNextPrevious_singleItemRepeat_notifiesMediaItemTransition() throws Exception {
|
||||
List<MediaItem> reportedMediaItems = new ArrayList<>();
|
||||
List<Integer> reportedTransitionReasons = new ArrayList<>();
|
||||
MediaSource mediaSource = FakeMediaSource.createWithWindowId(/* windowId= */ new Object());
|
||||
ExoPlayer player = new TestExoPlayerBuilder(context).build();
|
||||
player.addListener(
|
||||
new Listener() {
|
||||
@Override
|
||||
public void onMediaItemTransition(@Nullable MediaItem mediaItem, int reason) {
|
||||
reportedMediaItems.add(mediaItem);
|
||||
reportedTransitionReasons.add(reason);
|
||||
}
|
||||
});
|
||||
player.setMediaSource(mediaSource);
|
||||
player.prepare();
|
||||
runUntilPlaybackState(player, Player.STATE_READY);
|
||||
|
||||
player.setRepeatMode(Player.REPEAT_MODE_ALL);
|
||||
player.seekToNextMediaItem();
|
||||
player.seekToPreviousMediaItem();
|
||||
player.release();
|
||||
|
||||
MediaItem expectedMediaItem = mediaSource.getMediaItem();
|
||||
assertThat(reportedMediaItems)
|
||||
.containsExactly(expectedMediaItem, expectedMediaItem, expectedMediaItem);
|
||||
assertThat(reportedTransitionReasons)
|
||||
.containsExactly(
|
||||
Player.MEDIA_ITEM_TRANSITION_REASON_PLAYLIST_CHANGED,
|
||||
Player.MEDIA_ITEM_TRANSITION_REASON_SEEK,
|
||||
Player.MEDIA_ITEM_TRANSITION_REASON_SEEK);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void repeat_notifiesMediaItemTransition() throws Exception {
|
||||
List<MediaItem> reportedMediaItems = new ArrayList<>();
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user