Skip to content

Commit

Permalink
Untangle PlayerInfo/PlaybackInfo updates
Browse files Browse the repository at this point in the history 
The methods in ExoPlayerImpl and MediaControllerImplBase that determine
the new PlayerInfo/PlaybackInfo currently have a hard-to-reason-about
setup where the method generating the new info accesses other methods
that rely on the existing class field instead of working with the
passed in PlayerInfo/PlaybackInfo. This prevents reuse of the util
methods (e.g. for replaceMediaItems) because they access potentially
stale state.

This change untangles these methods a bit by making the util methods
either static or at least ensure that they don't rely on existing
class fields of PlayerInfo/PlaybackInfo. Overall, the change is a
complete no-op.

#minor-release

PiperOrigin-RevId: 534036633
(cherry picked from commit f898a7f)
  • Loading branch information
@tonihei @tof-tof
tonihei authored and tof-tof committed May 26, 2023
1 parent 9f6969b commit a80b167
Showing 1 changed file with 65 additions and 44 deletions.
109 changes: 65 additions & 44 deletions library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,16 +648,7 @@ public void addMediaSources(int index, List<MediaSource> mediaSources) {
setMediaSources(mediaSources, /* resetPosition= */ maskingWindowIndex == C.INDEX_UNSET);
return;
}
Timeline oldTimeline = getCurrentTimeline();
pendingOperationAcks++;
List<MediaSourceList.MediaSourceHolder> holders = addMediaSourceHolders(index, mediaSources);
Timeline newTimeline = createMaskingTimeline();
PlaybackInfo newPlaybackInfo =
maskTimelineAndPosition(
playbackInfo,
newTimeline,
getPeriodPositionUsAfterTimelineChanged(oldTimeline, newTimeline));
internalPlayer.addMediaSources(index, holders, shuffleOrder);
PlaybackInfo newPlaybackInfo = addMediaSourcesInternal(playbackInfo, index, mediaSources);
updatePlaybackInfo(
newPlaybackInfo,
/* timelineChangeReason= */ TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED,
Expand All @@ -680,7 +671,7 @@ public void removeMediaItems(int fromIndex, int toIndex) {
// Do nothing.
return;
}
PlaybackInfo newPlaybackInfo = removeMediaItemsInternal(fromIndex, toIndex);
PlaybackInfo newPlaybackInfo = removeMediaItemsInternal(playbackInfo, fromIndex, toIndex);
boolean positionDiscontinuity =
!newPlaybackInfo.periodId.periodUid.equals(playbackInfo.periodId.periodUid);
updatePlaybackInfo(
Expand Down Expand Up @@ -714,7 +705,11 @@ public void moveMediaItems(int fromIndex, int toIndex, int newFromIndex) {
maskTimelineAndPosition(
playbackInfo,
newTimeline,
getPeriodPositionUsAfterTimelineChanged(oldTimeline, newTimeline));
getPeriodPositionUsAfterTimelineChanged(
oldTimeline,
newTimeline,
getCurrentWindowIndexInternal(playbackInfo),
getContentPositionInternal(playbackInfo)));
internalPlayer.moveMediaSources(fromIndex, toIndex, newFromIndex, shuffleOrder);
updatePlaybackInfo(
newPlaybackInfo,
Expand Down Expand Up @@ -1046,7 +1041,7 @@ public int getCurrentPeriodIndex() {
@Override
public int getCurrentMediaItemIndex() {
verifyApplicationThread();
int currentWindowIndex = getCurrentWindowIndexInternal();
int currentWindowIndex = getCurrentWindowIndexInternal(playbackInfo);
return currentWindowIndex == C.INDEX_UNSET ? 0 : currentWindowIndex;
}

Expand Down Expand Up @@ -1106,17 +1101,7 @@ public int getCurrentAdIndexInAdGroup() {
@Override
public long getContentPosition() {
verifyApplicationThread();
if (isPlayingAd()) {
playbackInfo.timeline.getPeriodByUid(playbackInfo.periodId.periodUid, period);
return playbackInfo.requestedContentPositionUs == C.TIME_UNSET
? playbackInfo
.timeline
.getWindow(getCurrentMediaItemIndex(), window)
.getDefaultPositionMs()
: period.getPositionInWindowMs() + Util.usToMs(playbackInfo.requestedContentPositionUs);
} else {
return getCurrentPosition();
}
return getContentPositionInternal(playbackInfo);
}

@Override
Expand Down Expand Up @@ -1842,13 +1827,25 @@ private void stopInternal(@Nullable ExoPlaybackException error) {
/* repeatCurrentMediaItem= */ false);
}

private int getCurrentWindowIndexInternal() {
private int getCurrentWindowIndexInternal(PlaybackInfo playbackInfo) {
if (playbackInfo.timeline.isEmpty()) {
return maskingWindowIndex;
} else {
return playbackInfo.timeline.getPeriodByUid(playbackInfo.periodId.periodUid, period)
.windowIndex;
}
return playbackInfo.timeline.getPeriodByUid(playbackInfo.periodId.periodUid, period)
.windowIndex;
}

private long getContentPositionInternal(PlaybackInfo playbackInfo) {
if (playbackInfo.periodId.isAd()) {
playbackInfo.timeline.getPeriodByUid(playbackInfo.periodId.periodUid, period);
return playbackInfo.requestedContentPositionUs == C.TIME_UNSET
? playbackInfo
.timeline
.getWindow(getCurrentWindowIndexInternal(playbackInfo), window)
.getDefaultPositionMs()
: period.getPositionInWindowMs() + Util.usToMs(playbackInfo.requestedContentPositionUs);
}
return Util.usToMs(getCurrentPositionUsInternal(playbackInfo));
}

private long getCurrentPositionUsInternal(PlaybackInfo playbackInfo) {
Expand All @@ -1863,10 +1860,9 @@ private long getCurrentPositionUsInternal(PlaybackInfo playbackInfo) {

if (playbackInfo.periodId.isAd()) {
return positionUs;
} else {
return periodPositionUsToWindowPositionUs(
playbackInfo.timeline, playbackInfo.periodId, positionUs);
}
return periodPositionUsToWindowPositionUs(
playbackInfo.timeline, playbackInfo.periodId, positionUs);
}

private List<MediaSource> createMediaSources(List<MediaItem> mediaItems) {
Expand Down Expand Up @@ -2265,7 +2261,7 @@ private void setMediaSourcesInternal(
int startWindowIndex,
long startPositionMs,
boolean resetToDefaultPosition) {
int currentWindowIndex = getCurrentWindowIndexInternal();
int currentWindowIndex = getCurrentWindowIndexInternal(playbackInfo);
long currentPositionMs = getCurrentPosition();
pendingOperationAcks++;
if (!mediaSourceHolderSnapshots.isEmpty()) {
Expand Down Expand Up @@ -2336,9 +2332,30 @@ private List<MediaSourceList.MediaSourceHolder> addMediaSourceHolders(
return holders;
}

private PlaybackInfo removeMediaItemsInternal(int fromIndex, int toIndex) {
int currentIndex = getCurrentMediaItemIndex();
Timeline oldTimeline = getCurrentTimeline();
private PlaybackInfo addMediaSourcesInternal(
PlaybackInfo playbackInfo, int index, List<MediaSource> mediaSources) {
Timeline oldTimeline = playbackInfo.timeline;
pendingOperationAcks++;
List<MediaSourceList.MediaSourceHolder> holders = addMediaSourceHolders(index, mediaSources);
Timeline newTimeline = createMaskingTimeline();
PlaybackInfo newPlaybackInfo =
maskTimelineAndPosition(
playbackInfo,
newTimeline,
getPeriodPositionUsAfterTimelineChanged(
oldTimeline,
newTimeline,
getCurrentWindowIndexInternal(playbackInfo),
getContentPositionInternal(playbackInfo)));
internalPlayer.addMediaSources(index, holders, shuffleOrder);
return newPlaybackInfo;
}

private PlaybackInfo removeMediaItemsInternal(
PlaybackInfo playbackInfo, int fromIndex, int toIndex) {
int currentIndex = getCurrentWindowIndexInternal(playbackInfo);
long contentPositionMs = getContentPositionInternal(playbackInfo);
Timeline oldTimeline = playbackInfo.timeline;
int currentMediaSourceCount = mediaSourceHolderSnapshots.size();
pendingOperationAcks++;
removeMediaSourceHolders(fromIndex, /* toIndexExclusive= */ toIndex);
Expand All @@ -2347,7 +2364,8 @@ private PlaybackInfo removeMediaItemsInternal(int fromIndex, int toIndex) {
maskTimelineAndPosition(
playbackInfo,
newTimeline,
getPeriodPositionUsAfterTimelineChanged(oldTimeline, newTimeline));
getPeriodPositionUsAfterTimelineChanged(
oldTimeline, newTimeline, currentIndex, contentPositionMs));
// Player transitions to STATE_ENDED if the current index is part of the removed tail.
final boolean transitionsToEnded =
newPlaybackInfo.playbackState != STATE_IDLE
Expand Down Expand Up @@ -2376,7 +2394,9 @@ private Timeline createMaskingTimeline() {
private PlaybackInfo maskTimelineAndPosition(
PlaybackInfo playbackInfo, Timeline timeline, @Nullable Pair<Object, Long> periodPositionUs) {
checkArgument(timeline.isEmpty() || periodPositionUs != null);
// Get the old timeline and position before updating playbackInfo.
Timeline oldTimeline = playbackInfo.timeline;
long oldContentPositionMs = getContentPositionInternal(playbackInfo);
// Mask the timeline.
playbackInfo = playbackInfo.copyWithTimeline(timeline);

Expand Down Expand Up @@ -2404,7 +2424,7 @@ private PlaybackInfo maskTimelineAndPosition(
MediaPeriodId newPeriodId =
playingPeriodChanged ? new MediaPeriodId(periodPositionUs.first) : playbackInfo.periodId;
long newContentPositionUs = periodPositionUs.second;
long oldContentPositionUs = Util.msToUs(getContentPosition());
long oldContentPositionUs = Util.msToUs(oldContentPositionMs);
if (!oldTimeline.isEmpty()) {
oldContentPositionUs -=
oldTimeline.getPeriodByUid(oldPeriodUid, period).getPositionInWindowUs();
Expand Down Expand Up @@ -2480,20 +2500,21 @@ private PlaybackInfo maskTimelineAndPosition(

@Nullable
private Pair<Object, Long> getPeriodPositionUsAfterTimelineChanged(
Timeline oldTimeline, Timeline newTimeline) {
long currentPositionMs = getContentPosition();
Timeline oldTimeline,
Timeline newTimeline,
int currentWindowIndexInternal,
long contentPositionMs) {
if (oldTimeline.isEmpty() || newTimeline.isEmpty()) {
boolean isCleared = !oldTimeline.isEmpty() && newTimeline.isEmpty();
return maskWindowPositionMsOrGetPeriodPositionUs(
newTimeline,
isCleared ? C.INDEX_UNSET : getCurrentWindowIndexInternal(),
isCleared ? C.TIME_UNSET : currentPositionMs);
isCleared ? C.INDEX_UNSET : currentWindowIndexInternal,
isCleared ? C.TIME_UNSET : contentPositionMs);
}
int currentMediaItemIndex = getCurrentMediaItemIndex();
@Nullable
Pair<Object, Long> oldPeriodPositionUs =
oldTimeline.getPeriodPositionUs(
window, period, currentMediaItemIndex, Util.msToUs(currentPositionMs));
window, period, currentWindowIndexInternal, Util.msToUs(contentPositionMs));
Object periodUid = castNonNull(oldPeriodPositionUs).first;
if (newTimeline.getIndexOfPeriod(periodUid) != C.INDEX_UNSET) {
// The old period position is still available in the new timeline.
Expand Down Expand Up @@ -2545,7 +2566,7 @@ private long periodPositionUsToWindowPositionUs(
}

private PlayerMessage createMessageInternal(Target target) {
int currentWindowIndex = getCurrentWindowIndexInternal();
int currentWindowIndex = getCurrentWindowIndexInternal(playbackInfo);
return new PlayerMessage(
internalPlayer,
target,
Expand Down

0 comments on commit a80b167

Please sign in to comment.