Skip to content

Commit

Permalink
Correctly handle dynamic playlist modifications
Browse files Browse the repository at this point in the history 
- Fix bug where we'd try and call replaceStream having already
  notified the renderer that the current stream is final. This
  could occur if a period was added to the end of the playlist.
- If the current period being played is removed and a new period
  to play cannot be resolved, assume we've gone off the end of
  the playlist and transition to the ended state. This allows
  the current source to be re-used (unlike the previous behavior
  of considering it an error). Treat valid seeks that cannot be
  resolved due to concurrent timeline update similarly.
- Add seek sanity check back to ExoPlayerImpl. Meh. It's probably
  best to keep this, since it stops the exposed window index
  being invalid w.r.t the exposed timeline.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=141167151
  • Loading branch information
@ojw28
ojw28 committed Dec 6, 2016
1 parent 9ea8b02 commit 99957bb
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 36 deletions.
9 changes: 7 additions & 2 deletions library/src/main/java/com/google/android/exoplayer2/BaseRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,15 @@ public final boolean hasReadStreamToEnd() {
}

@Override
public final void setCurrentStreamIsFinal() {
public final void setCurrentStreamFinal() {
streamIsFinal = true;
}

@Override
public final boolean isCurrentStreamFinal() {
return streamIsFinal;
}

@Override
public final void maybeThrowStreamError() throws IOException {
stream.maybeThrowError();
Expand Down Expand Up @@ -243,7 +248,7 @@ protected final int getIndex() {

/**
* Reads from the enabled upstream source. If the upstream source has been read to the end then
* {@link C#RESULT_BUFFER_READ} is only returned if {@link #setCurrentStreamIsFinal()} has been
* {@link C#RESULT_BUFFER_READ} is only returned if {@link #setCurrentStreamFinal()} has been
* called. {@link C#RESULT_NOTHING_READ} is returned otherwise.
*
* @see SampleStream#readData(FormatHolder, DecoderInputBuffer)
Expand Down
3 changes: 3 additions & 0 deletions library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ public void seekTo(long positionMs) {

@Override
public void seekTo(int windowIndex, long positionMs) {
if (windowIndex < 0 || (!timeline.isEmpty() && windowIndex >= timeline.getWindowCount())) {
throw new IllegalSeekPositionException(timeline, windowIndex, positionMs);
}
pendingSeekAcks++;
maskingWindowIndex = windowIndex;
if (positionMs == C.TIME_UNSET) {
Expand Down
104 changes: 71 additions & 33 deletions library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ private void setIsLoading(boolean isLoading) {
}

private void prepareInternal(MediaSource mediaSource, boolean resetPosition) {
resetInternal();
resetInternal(true);
loadControl.onPrepared();
if (resetPosition) {
playbackInfo = new PlaybackInfo(0, C.TIME_UNSET);
Expand Down Expand Up @@ -548,9 +548,16 @@ private void seekToInternal(SeekPosition seekPosition) throws ExoPlaybackExcepti

Pair<Integer, Long> periodPosition = resolveSeekPosition(seekPosition);
if (periodPosition == null) {
// TODO: We should probably propagate an error here.
// We failed to resolve the seek position. Stop the player.
stopInternal();
// The seek position was valid for the timeline that it was performed into, but the
// timeline has changed and a suitable seek position could not be resolved in the new one.
playbackInfo = new PlaybackInfo(0, 0);
eventHandler.obtainMessage(MSG_SEEK_ACK, playbackInfo).sendToTarget();
// Set the internal position to (0,TIME_UNSET) so that a subsequent seek to (0,0) isn't
// ignored.
playbackInfo = new PlaybackInfo(0, C.TIME_UNSET);
setState(ExoPlayer.STATE_ENDED);
// Reset, but retain the source so that it can still be used should a seek occur.
resetInternal(false);
return;
}

Expand Down Expand Up @@ -639,13 +646,13 @@ private void resetRendererPosition(long periodPositionUs) throws ExoPlaybackExce
}

private void stopInternal() {
resetInternal();
resetInternal(true);
loadControl.onStopped();
setState(ExoPlayer.STATE_IDLE);
}

private void releaseInternal() {
resetInternal();
resetInternal(true);
loadControl.onReleased();
setState(ExoPlayer.STATE_IDLE);
synchronized (this) {
Expand All @@ -654,7 +661,7 @@ private void releaseInternal() {
}
}

private void resetInternal() {
private void resetInternal(boolean releaseMediaSource) {
handler.removeMessages(MSG_DO_SOME_WORK);
rebuffering = false;
standaloneMediaClock.stop();
Expand All @@ -672,15 +679,17 @@ private void resetInternal() {
enabledRenderers = new Renderer[0];
releasePeriodHoldersFrom(playingPeriodHolder != null ? playingPeriodHolder
: loadingPeriodHolder);
if (mediaSource != null) {
mediaSource.releaseSource();
mediaSource = null;
}
loadingPeriodHolder = null;
readingPeriodHolder = null;
playingPeriodHolder = null;
timeline = null;
setIsLoading(false);
if (releaseMediaSource) {
if (mediaSource != null) {
mediaSource.releaseSource();
mediaSource = null;
}
timeline = null;
}
}

private void sendMessagesInternal(ExoPlayerMessage[] messages) throws ExoPlaybackException {
Expand Down Expand Up @@ -845,18 +854,21 @@ private void handleSourceInfoRefreshed(Pair<Timeline, Object> timelineAndManifes
if (oldTimeline == null) {
if (pendingInitialSeekCount > 0) {
Pair<Integer, Long> periodPosition = resolveSeekPosition(pendingSeekPosition);
processedInitialSeekCount = pendingInitialSeekCount;
pendingInitialSeekCount = 0;
pendingSeekPosition = null;
if (periodPosition == null) {
// We failed to resolve the seek position. Stop the player.
notifySourceInfoRefresh(manifest, 0);
// TODO: We should probably propagate an error here.
stopInternal();
// The seek position was valid for the timeline that it was performed into, but the
// timeline has changed and a suitable seek position could not be resolved in the new one.
handleSourceInfoRefreshEndedPlayback(manifest, processedInitialSeekCount);
return;
}
playbackInfo = new PlaybackInfo(periodPosition.first, periodPosition.second);
processedInitialSeekCount = pendingInitialSeekCount;
pendingInitialSeekCount = 0;
pendingSeekPosition = null;
} else if (playbackInfo.startPositionUs == C.TIME_UNSET) {
if (timeline.isEmpty()) {
handleSourceInfoRefreshEndedPlayback(manifest, processedInitialSeekCount);
return;
}
Pair<Integer, Long> defaultPosition = getPeriodPosition(0, C.TIME_UNSET);
playbackInfo = new PlaybackInfo(defaultPosition.first, defaultPosition.second);
}
Expand All @@ -876,10 +888,8 @@ private void handleSourceInfoRefreshed(Pair<Timeline, Object> timelineAndManifes
// period whose window we can restart from.
int newPeriodIndex = resolveSubsequentPeriod(periodHolder.index, oldTimeline, timeline);
if (newPeriodIndex == C.INDEX_UNSET) {
// We failed to resolve a subsequent period. Stop the player.
notifySourceInfoRefresh(manifest, processedInitialSeekCount);
// TODO: We should probably propagate an error here.
stopInternal();
// We failed to resolve a suitable restart position.
handleSourceInfoRefreshEndedPlayback(manifest, processedInitialSeekCount);
return;
}
// We resolved a subsequent period. Seek to the default position in the corresponding window.
Expand Down Expand Up @@ -949,6 +959,18 @@ private void handleSourceInfoRefreshed(Pair<Timeline, Object> timelineAndManifes
notifySourceInfoRefresh(manifest, processedInitialSeekCount);
}

private void handleSourceInfoRefreshEndedPlayback(Object manifest,
int processedInitialSeekCount) {
// Set the playback position to (0,0) for notifying the eventHandler.
playbackInfo = new PlaybackInfo(0, 0);
notifySourceInfoRefresh(manifest, processedInitialSeekCount);
// Set the internal position to (0,TIME_UNSET) so that a subsequent seek to (0,0) isn't ignored.
playbackInfo = new PlaybackInfo(0, C.TIME_UNSET);
setState(ExoPlayer.STATE_ENDED);
// Reset, but retain the source so that it can still be used should a seek occur.
resetInternal(false);
}

private void notifySourceInfoRefresh(Object manifest, int processedInitialSeekCount) {
eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED,
new SourceInfo(timeline, manifest, playbackInfo, processedInitialSeekCount)).sendToTarget();
Expand Down Expand Up @@ -980,6 +1002,8 @@ private int resolveSubsequentPeriod(int oldPeriodIndex, Timeline oldTimeline,
*
* @param seekPosition The position to resolve.
* @return The resolved position, or null if resolution was not successful.
* @throws IllegalSeekPositionException If the window index of the seek position is outside the
* bounds of the timeline.
*/
private Pair<Integer, Long> resolveSeekPosition(SeekPosition seekPosition) {
Timeline seekTimeline = seekPosition.timeline;
Expand All @@ -989,8 +1013,15 @@ private Pair<Integer, Long> resolveSeekPosition(SeekPosition seekPosition) {
seekTimeline = timeline;
}
// Map the SeekPosition to a position in the corresponding timeline.
Pair<Integer, Long> periodPosition = getPeriodPosition(seekTimeline, seekPosition.windowIndex,
seekPosition.windowPositionUs);
Pair<Integer, Long> periodPosition;
try {
periodPosition = getPeriodPosition(seekTimeline, seekPosition.windowIndex,
seekPosition.windowPositionUs);
} catch (IndexOutOfBoundsException e) {
// The window index of the seek position was outside the bounds of the timeline.
throw new IllegalSeekPositionException(timeline, seekPosition.windowIndex,
seekPosition.windowPositionUs);
}
if (timeline == seekTimeline) {
// Our internal timeline is the seek timeline, so the mapped position is correct.
return periodPosition;
Expand Down Expand Up @@ -1096,9 +1127,12 @@ private void updatePeriods() throws ExoPlaybackException, IOException {
}

if (readingPeriodHolder.isLast) {
// The renderers have their final SampleStreams.
for (Renderer renderer : enabledRenderers) {
renderer.setCurrentStreamIsFinal();
// Defer setting the stream as final until the renderer has actually consumed the whole
// stream in case of playlist changes that cause the stream to be no longer final.
if (renderer.hasReadStreamToEnd()) {
renderer.setCurrentStreamFinal();
}
}
return;
}
Expand All @@ -1108,6 +1142,7 @@ private void updatePeriods() throws ExoPlaybackException, IOException {
return;
}
}

if (readingPeriodHolder.next != null && readingPeriodHolder.next.prepared) {
TrackSelectionArray oldTrackSelections = readingPeriodHolder.trackSelections;
readingPeriodHolder = readingPeriodHolder.next;
Expand All @@ -1117,7 +1152,8 @@ private void updatePeriods() throws ExoPlaybackException, IOException {
TrackSelection oldSelection = oldTrackSelections.get(i);
TrackSelection newSelection = newTrackSelections.get(i);
if (oldSelection != null) {
if (newSelection != null) {
boolean isCurrentStreamFinal = renderer.isCurrentStreamFinal();
if (newSelection != null && !isCurrentStreamFinal) {
// Replace the renderer's SampleStream so the transition to playing the next period can
// be seamless.
Format[] formats = new Format[newSelection.length()];
Expand All @@ -1126,10 +1162,10 @@ private void updatePeriods() throws ExoPlaybackException, IOException {
}
renderer.replaceStream(formats, readingPeriodHolder.sampleStreams[i],
readingPeriodHolder.getRendererOffset());
} else {
} else if (!isCurrentStreamFinal) {
// The renderer will be disabled when transitioning to playing the next period. Mark the
// SampleStream as final to play out any remaining data.
renderer.setCurrentStreamIsFinal();
renderer.setCurrentStreamFinal();
}
}
}
Expand Down Expand Up @@ -1266,10 +1302,12 @@ private void setPlayingPeriodHolder(MediaPeriodHolder periodHolder) throws ExoPl
rendererWasEnabledFlags[i] = renderer.getState() != Renderer.STATE_DISABLED;
TrackSelection newSelection = periodHolder.trackSelections.get(i);
if (newSelection != null) {
// The renderer should be enabled when playing the new period.
enabledRendererCount++;
} else if (rendererWasEnabledFlags[i]) {
// The renderer should be disabled when playing the new period.
}
if (rendererWasEnabledFlags[i] && (newSelection == null || renderer.isCurrentStreamFinal())) {
// The renderer should be disabled before playing the next period, either because it's not
// needed to play the next period, or because we need to disable and re-enable it because
// the renderer thinks that its current stream is final.
if (renderer == rendererMediaClockSource) {
// Sync standaloneMediaClock so that it can take over timing responsibilities.
standaloneMediaClock.setPositionUs(rendererMediaClock.getPositionUs());
Expand Down
48 changes: 48 additions & 0 deletions library/src/main/java/com/google/android/exoplayer2/IllegalSeekPositionException.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright (C) 2016 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.android.exoplayer2;

/**
* Thrown when an attempt is made to seek to a position that does not exist in the player's
* {@link Timeline}.
*/
public final class IllegalSeekPositionException extends IllegalStateException {

/**
* The {@link Timeline} in which the seek was attempted.
*/
public final Timeline timeline;
/**
* The index of the window being seeked to.
*/
public final int windowIndex;
/**
* The seek position in the specified window.
*/
public final long positionMs;

/**
* @param timeline The {@link Timeline} in which the seek was attempted.
* @param windowIndex The index of the window being seeked to.
* @param positionMs The seek position in the specified window.
*/
public IllegalSeekPositionException(Timeline timeline, int windowIndex, long positionMs) {
this.timeline = timeline;
this.windowIndex = windowIndex;
this.positionMs = positionMs;
}

}
8 changes: 7 additions & 1 deletion library/src/main/java/com/google/android/exoplayer2/Renderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,13 @@ void replaceStream(Format[] formats, SampleStream stream, long offsetUs)
* This method may be called when the renderer is in the following states:
* {@link #STATE_ENABLED}, {@link #STATE_STARTED}.
*/
void setCurrentStreamIsFinal();
void setCurrentStreamFinal();

/**
* Returns whether the current {@link SampleStream} will be the final one supplied before the
* renderer is next disabled or reset.
*/
boolean isCurrentStreamFinal();

/**
* Throws an error that's preventing the renderer from reading from its {@link SampleStream}. Does
Expand Down

0 comments on commit 99957bb

Please sign in to comment.