Commit d7b38201 by tonihei Committed by Oliver Woodman

Make updateQueuedPeriods more readable and fix bug.

1. The method kept track of the current period index to check if the next
period is still in the correct period. This is unneccessary since we no longer
use the period index but the actual uid in MediaPeriodId and mismatches are
already detected by canKeepMediaPeriodHolder.
2. We updated the MediaPeriodIndfo twice: once in getFollowingMediaPeriodInfo
and once in getUpdatedMediaPeriodInfo. That's confusing and difficult to
follow. The only difference is that getUpdatedMediaPeriodInfo keeps the
content position while getFollowingMediaPeriodInfo resets it. This is made more
explicit for readability.
3. The durations compatibility check for all following periods was broken as
it compared the same durations (partly due to the confusion caused by 2.)

PiperOrigin-RevId: 230519295
parent 3a54d744
......@@ -1376,7 +1376,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
}
}
if (!queue.updateQueuedPeriods(playingPeriodId, rendererPositionUs)) {
if (!queue.updateQueuedPeriods(rendererPositionUs)) {
seekToCurrentPosition(/* sendDiscontinuity= */ false);
}
handleLoadingMediaPeriodChanged(/* loadingTrackSelectionChanged= */ false);
......
......@@ -64,15 +64,26 @@ import com.google.android.exoplayer2.util.Util;
this.isFinal = isFinal;
}
/** Returns a copy of this instance with the start position set to the specified value. */
/**
* Returns a copy of this instance with the start position set to the specified value. May return
* the same instance if nothing changed.
*/
public MediaPeriodInfo copyWithStartPositionUs(long startPositionUs) {
return new MediaPeriodInfo(
id,
startPositionUs,
contentPositionUs,
durationUs,
isLastInTimelinePeriod,
isFinal);
return startPositionUs == this.startPositionUs
? this
: new MediaPeriodInfo(
id, startPositionUs, contentPositionUs, durationUs, isLastInTimelinePeriod, isFinal);
}
/**
* Returns a copy of this instance with the content position set to the specified value. May
* return the same instance if nothing changed.
*/
public MediaPeriodInfo copyWithContentPositionUs(long contentPositionUs) {
return contentPositionUs == this.contentPositionUs
? this
: new MediaPeriodInfo(
id, startPositionUs, contentPositionUs, durationUs, isLastInTimelinePeriod, isFinal);
}
@Override
......
......@@ -61,8 +61,8 @@ import com.google.android.exoplayer2.util.Assertions;
}
/**
* Sets the {@link Timeline}. Call {@link #updateQueuedPeriods(MediaPeriodId, long)} to update the
* queued media periods to take into account the new timeline.
* Sets the {@link Timeline}. Call {@link #updateQueuedPeriods(long)} to update the queued media
* periods to take into account the new timeline.
*/
public void setTimeline(Timeline timeline) {
this.timeline = timeline;
......@@ -292,54 +292,43 @@ import com.google.android.exoplayer2.util.Assertions;
* current playback position. The method assumes that the first media period in the queue is still
* consistent with the new timeline.
*
* @param playingPeriodId The current playing media period identifier.
* @param rendererPositionUs The current renderer position in microseconds.
* @return Whether the timeline change has been handled completely.
*/
public boolean updateQueuedPeriods(MediaPeriodId playingPeriodId, long rendererPositionUs) {
public boolean updateQueuedPeriods(long rendererPositionUs) {
// TODO: Merge this into setTimeline so that the queue gets updated as soon as the new timeline
// is set, once all cases handled by ExoPlayerImplInternal.handleSourceInfoRefreshed can be
// handled here.
int periodIndex = timeline.getIndexOfPeriod(playingPeriodId.periodUid);
// The front period is either playing now, or is being loaded and will become the playing
// period.
MediaPeriodHolder previousPeriodHolder = null;
MediaPeriodHolder periodHolder = getFrontPeriod();
while (periodHolder != null) {
MediaPeriodInfo oldPeriodInfo = periodHolder.info;
// Get period info based on new timeline.
MediaPeriodInfo newPeriodInfo;
if (previousPeriodHolder == null) {
long previousDurationUs = periodHolder.info.durationUs;
periodHolder.info = getUpdatedMediaPeriodInfo(periodHolder.info);
if (!canKeepAfterMediaPeriodHolder(periodHolder, previousDurationUs)) {
return !removeAfter(periodHolder);
}
// The id and start position of the first period have already been verified by
// ExoPlayerImplInternal.handleSourceInfoRefreshed. Just update duration, isLastInTimeline
// and isLastInPeriod flags.
newPeriodInfo = getUpdatedMediaPeriodInfo(oldPeriodInfo);
} else {
// Check this period holder still follows the previous one, based on the new timeline.
if (periodIndex == C.INDEX_UNSET
|| !periodHolder.uid.equals(timeline.getUidOfPeriod(periodIndex))) {
// The holder uid is inconsistent with the new timeline.
return !removeAfter(previousPeriodHolder);
}
MediaPeriodInfo periodInfo =
getFollowingMediaPeriodInfo(previousPeriodHolder, rendererPositionUs);
if (periodInfo == null) {
newPeriodInfo = getFollowingMediaPeriodInfo(previousPeriodHolder, rendererPositionUs);
if (newPeriodInfo == null) {
// We've loaded a next media period that is not in the new timeline.
return !removeAfter(previousPeriodHolder);
}
// Update the period holder.
periodHolder.info = getUpdatedMediaPeriodInfo(periodHolder.info);
// Check the media period information matches the new timeline.
if (!canKeepMediaPeriodHolder(periodHolder, periodInfo)) {
if (!canKeepMediaPeriodHolder(oldPeriodInfo, newPeriodInfo)) {
// The new media period has a different id or start position.
return !removeAfter(previousPeriodHolder);
} else if (!canKeepAfterMediaPeriodHolder(periodHolder, periodInfo.durationUs)) {
return !removeAfter(periodHolder);
}
}
if (periodHolder.info.isLastInTimelinePeriod) {
// Move on to the next timeline period index, if there is one.
periodIndex =
timeline.getNextPeriodIndex(
periodIndex, period, window, repeatMode, shuffleModeEnabled);
// Use new period info, but keep old content position.
periodHolder.info = newPeriodInfo.copyWithContentPositionUs(oldPeriodInfo.contentPositionUs);
if (!areDurationsCompatible(oldPeriodInfo.durationUs, newPeriodInfo.durationUs)) {
// The period duration changed. Remove all subsequent periods.
return !removeAfter(periodHolder);
}
previousPeriodHolder = periodHolder;
......@@ -465,22 +454,18 @@ import com.google.android.exoplayer2.util.Assertions;
}
/**
* Returns whether {@code periodHolder} can be kept for playing the media period described by
* {@code info}.
* Returns whether a period described by {@code oldInfo} can be kept for playing the media period
* described by {@code newInfo}.
*/
private boolean canKeepMediaPeriodHolder(MediaPeriodHolder periodHolder, MediaPeriodInfo info) {
MediaPeriodInfo periodHolderInfo = periodHolder.info;
return periodHolderInfo.startPositionUs == info.startPositionUs
&& periodHolderInfo.id.equals(info.id);
private boolean canKeepMediaPeriodHolder(MediaPeriodInfo oldInfo, MediaPeriodInfo newInfo) {
return oldInfo.startPositionUs == newInfo.startPositionUs && oldInfo.id.equals(newInfo.id);
}
/**
* Returns whether periods after {@code periodHolder} can be kept for playing given its previous
* duration.
* Returns whether a duration change of a period is compatible with keeping the following periods.
*/
private boolean canKeepAfterMediaPeriodHolder(
MediaPeriodHolder periodHolder, long previousDurationUs) {
return previousDurationUs == C.TIME_UNSET || previousDurationUs == periodHolder.info.durationUs;
private boolean areDurationsCompatible(long previousDurationUs, long newDurationUs) {
return previousDurationUs == C.TIME_UNSET || previousDurationUs == newDurationUs;
}
/**
......
......@@ -2028,15 +2028,17 @@ public final class ExoPlayerTest {
.blockUntilEnded(TIMEOUT_MS);
testRunner.assertPlayedPeriodIndices(0, 1);
// Assert that the second period was re-created from the new timeline.
assertThat(mediaSource.getCreatedMediaPeriods())
.containsExactly(
new MediaPeriodId(
timeline1.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0),
new MediaPeriodId(
timeline1.getUidOfPeriod(/* periodIndex= */ 1), /* windowSequenceNumber= */ 1),
new MediaPeriodId(
timeline2.getUidOfPeriod(/* periodIndex= */ 1), /* windowSequenceNumber= */ 2))
.inOrder();
assertThat(mediaSource.getCreatedMediaPeriods()).hasSize(3);
assertThat(mediaSource.getCreatedMediaPeriods().get(0).periodUid)
.isEqualTo(timeline1.getUidOfPeriod(/* periodIndex= */ 0));
assertThat(mediaSource.getCreatedMediaPeriods().get(1).periodUid)
.isEqualTo(timeline1.getUidOfPeriod(/* periodIndex= */ 1));
assertThat(mediaSource.getCreatedMediaPeriods().get(2).periodUid)
.isEqualTo(timeline2.getUidOfPeriod(/* periodIndex= */ 1));
assertThat(mediaSource.getCreatedMediaPeriods().get(1).windowSequenceNumber)
.isGreaterThan(mediaSource.getCreatedMediaPeriods().get(0).windowSequenceNumber);
assertThat(mediaSource.getCreatedMediaPeriods().get(2).windowSequenceNumber)
.isGreaterThan(mediaSource.getCreatedMediaPeriods().get(1).windowSequenceNumber);
}
@Test
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment