Commit 99957bba by olly Committed by Oliver Woodman

Correctly handle dynamic playlist modifications

- 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
parent 9ea8b020
...@@ -107,11 +107,16 @@ public abstract class BaseRenderer implements Renderer, RendererCapabilities { ...@@ -107,11 +107,16 @@ public abstract class BaseRenderer implements Renderer, RendererCapabilities {
} }
@Override @Override
public final void setCurrentStreamIsFinal() { public final void setCurrentStreamFinal() {
streamIsFinal = true; streamIsFinal = true;
} }
@Override @Override
public final boolean isCurrentStreamFinal() {
return streamIsFinal;
}
@Override
public final void maybeThrowStreamError() throws IOException { public final void maybeThrowStreamError() throws IOException {
stream.maybeThrowError(); stream.maybeThrowError();
} }
...@@ -243,7 +248,7 @@ public abstract class BaseRenderer implements Renderer, RendererCapabilities { ...@@ -243,7 +248,7 @@ public abstract class BaseRenderer implements Renderer, RendererCapabilities {
/** /**
* Reads from the enabled upstream source. If the upstream source has been read to the end then * 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. * called. {@link C#RESULT_NOTHING_READ} is returned otherwise.
* *
* @see SampleStream#readData(FormatHolder, DecoderInputBuffer) * @see SampleStream#readData(FormatHolder, DecoderInputBuffer)
......
...@@ -179,6 +179,9 @@ import java.util.concurrent.CopyOnWriteArraySet; ...@@ -179,6 +179,9 @@ import java.util.concurrent.CopyOnWriteArraySet;
@Override @Override
public void seekTo(int windowIndex, long positionMs) { public void seekTo(int windowIndex, long positionMs) {
if (windowIndex < 0 || (!timeline.isEmpty() && windowIndex >= timeline.getWindowCount())) {
throw new IllegalSeekPositionException(timeline, windowIndex, positionMs);
}
pendingSeekAcks++; pendingSeekAcks++;
maskingWindowIndex = windowIndex; maskingWindowIndex = windowIndex;
if (positionMs == C.TIME_UNSET) { if (positionMs == C.TIME_UNSET) {
......
...@@ -378,7 +378,7 @@ import java.io.IOException; ...@@ -378,7 +378,7 @@ import java.io.IOException;
} }
private void prepareInternal(MediaSource mediaSource, boolean resetPosition) { private void prepareInternal(MediaSource mediaSource, boolean resetPosition) {
resetInternal(); resetInternal(true);
loadControl.onPrepared(); loadControl.onPrepared();
if (resetPosition) { if (resetPosition) {
playbackInfo = new PlaybackInfo(0, C.TIME_UNSET); playbackInfo = new PlaybackInfo(0, C.TIME_UNSET);
...@@ -548,9 +548,16 @@ import java.io.IOException; ...@@ -548,9 +548,16 @@ import java.io.IOException;
Pair<Integer, Long> periodPosition = resolveSeekPosition(seekPosition); Pair<Integer, Long> periodPosition = resolveSeekPosition(seekPosition);
if (periodPosition == null) { if (periodPosition == null) {
// TODO: We should probably propagate an error here. // The seek position was valid for the timeline that it was performed into, but the
// We failed to resolve the seek position. Stop the player. // timeline has changed and a suitable seek position could not be resolved in the new one.
stopInternal(); 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; return;
} }
...@@ -639,13 +646,13 @@ import java.io.IOException; ...@@ -639,13 +646,13 @@ import java.io.IOException;
} }
private void stopInternal() { private void stopInternal() {
resetInternal(); resetInternal(true);
loadControl.onStopped(); loadControl.onStopped();
setState(ExoPlayer.STATE_IDLE); setState(ExoPlayer.STATE_IDLE);
} }
private void releaseInternal() { private void releaseInternal() {
resetInternal(); resetInternal(true);
loadControl.onReleased(); loadControl.onReleased();
setState(ExoPlayer.STATE_IDLE); setState(ExoPlayer.STATE_IDLE);
synchronized (this) { synchronized (this) {
...@@ -654,7 +661,7 @@ import java.io.IOException; ...@@ -654,7 +661,7 @@ import java.io.IOException;
} }
} }
private void resetInternal() { private void resetInternal(boolean releaseMediaSource) {
handler.removeMessages(MSG_DO_SOME_WORK); handler.removeMessages(MSG_DO_SOME_WORK);
rebuffering = false; rebuffering = false;
standaloneMediaClock.stop(); standaloneMediaClock.stop();
...@@ -672,15 +679,17 @@ import java.io.IOException; ...@@ -672,15 +679,17 @@ import java.io.IOException;
enabledRenderers = new Renderer[0]; enabledRenderers = new Renderer[0];
releasePeriodHoldersFrom(playingPeriodHolder != null ? playingPeriodHolder releasePeriodHoldersFrom(playingPeriodHolder != null ? playingPeriodHolder
: loadingPeriodHolder); : loadingPeriodHolder);
loadingPeriodHolder = null;
readingPeriodHolder = null;
playingPeriodHolder = null;
setIsLoading(false);
if (releaseMediaSource) {
if (mediaSource != null) { if (mediaSource != null) {
mediaSource.releaseSource(); mediaSource.releaseSource();
mediaSource = null; mediaSource = null;
} }
loadingPeriodHolder = null;
readingPeriodHolder = null;
playingPeriodHolder = null;
timeline = null; timeline = null;
setIsLoading(false); }
} }
private void sendMessagesInternal(ExoPlayerMessage[] messages) throws ExoPlaybackException { private void sendMessagesInternal(ExoPlayerMessage[] messages) throws ExoPlaybackException {
...@@ -845,18 +854,21 @@ import java.io.IOException; ...@@ -845,18 +854,21 @@ import java.io.IOException;
if (oldTimeline == null) { if (oldTimeline == null) {
if (pendingInitialSeekCount > 0) { if (pendingInitialSeekCount > 0) {
Pair<Integer, Long> periodPosition = resolveSeekPosition(pendingSeekPosition); Pair<Integer, Long> periodPosition = resolveSeekPosition(pendingSeekPosition);
processedInitialSeekCount = pendingInitialSeekCount;
pendingInitialSeekCount = 0;
pendingSeekPosition = null;
if (periodPosition == null) { if (periodPosition == null) {
// We failed to resolve the seek position. Stop the player. // The seek position was valid for the timeline that it was performed into, but the
notifySourceInfoRefresh(manifest, 0); // timeline has changed and a suitable seek position could not be resolved in the new one.
// TODO: We should probably propagate an error here. handleSourceInfoRefreshEndedPlayback(manifest, processedInitialSeekCount);
stopInternal();
return; return;
} }
playbackInfo = new PlaybackInfo(periodPosition.first, periodPosition.second); playbackInfo = new PlaybackInfo(periodPosition.first, periodPosition.second);
processedInitialSeekCount = pendingInitialSeekCount;
pendingInitialSeekCount = 0;
pendingSeekPosition = null;
} else if (playbackInfo.startPositionUs == C.TIME_UNSET) { } else if (playbackInfo.startPositionUs == C.TIME_UNSET) {
if (timeline.isEmpty()) {
handleSourceInfoRefreshEndedPlayback(manifest, processedInitialSeekCount);
return;
}
Pair<Integer, Long> defaultPosition = getPeriodPosition(0, C.TIME_UNSET); Pair<Integer, Long> defaultPosition = getPeriodPosition(0, C.TIME_UNSET);
playbackInfo = new PlaybackInfo(defaultPosition.first, defaultPosition.second); playbackInfo = new PlaybackInfo(defaultPosition.first, defaultPosition.second);
} }
...@@ -876,10 +888,8 @@ import java.io.IOException; ...@@ -876,10 +888,8 @@ import java.io.IOException;
// period whose window we can restart from. // period whose window we can restart from.
int newPeriodIndex = resolveSubsequentPeriod(periodHolder.index, oldTimeline, timeline); int newPeriodIndex = resolveSubsequentPeriod(periodHolder.index, oldTimeline, timeline);
if (newPeriodIndex == C.INDEX_UNSET) { if (newPeriodIndex == C.INDEX_UNSET) {
// We failed to resolve a subsequent period. Stop the player. // We failed to resolve a suitable restart position.
notifySourceInfoRefresh(manifest, processedInitialSeekCount); handleSourceInfoRefreshEndedPlayback(manifest, processedInitialSeekCount);
// TODO: We should probably propagate an error here.
stopInternal();
return; return;
} }
// We resolved a subsequent period. Seek to the default position in the corresponding window. // We resolved a subsequent period. Seek to the default position in the corresponding window.
...@@ -949,6 +959,18 @@ import java.io.IOException; ...@@ -949,6 +959,18 @@ import java.io.IOException;
notifySourceInfoRefresh(manifest, processedInitialSeekCount); 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) { private void notifySourceInfoRefresh(Object manifest, int processedInitialSeekCount) {
eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED, eventHandler.obtainMessage(MSG_SOURCE_INFO_REFRESHED,
new SourceInfo(timeline, manifest, playbackInfo, processedInitialSeekCount)).sendToTarget(); new SourceInfo(timeline, manifest, playbackInfo, processedInitialSeekCount)).sendToTarget();
...@@ -980,6 +1002,8 @@ import java.io.IOException; ...@@ -980,6 +1002,8 @@ import java.io.IOException;
* *
* @param seekPosition The position to resolve. * @param seekPosition The position to resolve.
* @return The resolved position, or null if resolution was not successful. * @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) { private Pair<Integer, Long> resolveSeekPosition(SeekPosition seekPosition) {
Timeline seekTimeline = seekPosition.timeline; Timeline seekTimeline = seekPosition.timeline;
...@@ -989,8 +1013,15 @@ import java.io.IOException; ...@@ -989,8 +1013,15 @@ import java.io.IOException;
seekTimeline = timeline; seekTimeline = timeline;
} }
// Map the SeekPosition to a position in the corresponding timeline. // Map the SeekPosition to a position in the corresponding timeline.
Pair<Integer, Long> periodPosition = getPeriodPosition(seekTimeline, seekPosition.windowIndex, 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); seekPosition.windowPositionUs);
}
if (timeline == seekTimeline) { if (timeline == seekTimeline) {
// Our internal timeline is the seek timeline, so the mapped position is correct. // Our internal timeline is the seek timeline, so the mapped position is correct.
return periodPosition; return periodPosition;
...@@ -1096,9 +1127,12 @@ import java.io.IOException; ...@@ -1096,9 +1127,12 @@ import java.io.IOException;
} }
if (readingPeriodHolder.isLast) { if (readingPeriodHolder.isLast) {
// The renderers have their final SampleStreams.
for (Renderer renderer : enabledRenderers) { 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; return;
} }
...@@ -1108,6 +1142,7 @@ import java.io.IOException; ...@@ -1108,6 +1142,7 @@ import java.io.IOException;
return; return;
} }
} }
if (readingPeriodHolder.next != null && readingPeriodHolder.next.prepared) { if (readingPeriodHolder.next != null && readingPeriodHolder.next.prepared) {
TrackSelectionArray oldTrackSelections = readingPeriodHolder.trackSelections; TrackSelectionArray oldTrackSelections = readingPeriodHolder.trackSelections;
readingPeriodHolder = readingPeriodHolder.next; readingPeriodHolder = readingPeriodHolder.next;
...@@ -1117,7 +1152,8 @@ import java.io.IOException; ...@@ -1117,7 +1152,8 @@ import java.io.IOException;
TrackSelection oldSelection = oldTrackSelections.get(i); TrackSelection oldSelection = oldTrackSelections.get(i);
TrackSelection newSelection = newTrackSelections.get(i); TrackSelection newSelection = newTrackSelections.get(i);
if (oldSelection != null) { 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 // Replace the renderer's SampleStream so the transition to playing the next period can
// be seamless. // be seamless.
Format[] formats = new Format[newSelection.length()]; Format[] formats = new Format[newSelection.length()];
...@@ -1126,10 +1162,10 @@ import java.io.IOException; ...@@ -1126,10 +1162,10 @@ import java.io.IOException;
} }
renderer.replaceStream(formats, readingPeriodHolder.sampleStreams[i], renderer.replaceStream(formats, readingPeriodHolder.sampleStreams[i],
readingPeriodHolder.getRendererOffset()); readingPeriodHolder.getRendererOffset());
} else { } else if (!isCurrentStreamFinal) {
// The renderer will be disabled when transitioning to playing the next period. Mark the // The renderer will be disabled when transitioning to playing the next period. Mark the
// SampleStream as final to play out any remaining data. // SampleStream as final to play out any remaining data.
renderer.setCurrentStreamIsFinal(); renderer.setCurrentStreamFinal();
} }
} }
} }
...@@ -1266,10 +1302,12 @@ import java.io.IOException; ...@@ -1266,10 +1302,12 @@ import java.io.IOException;
rendererWasEnabledFlags[i] = renderer.getState() != Renderer.STATE_DISABLED; rendererWasEnabledFlags[i] = renderer.getState() != Renderer.STATE_DISABLED;
TrackSelection newSelection = periodHolder.trackSelections.get(i); TrackSelection newSelection = periodHolder.trackSelections.get(i);
if (newSelection != null) { if (newSelection != null) {
// The renderer should be enabled when playing the new period.
enabledRendererCount++; 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) { if (renderer == rendererMediaClockSource) {
// Sync standaloneMediaClock so that it can take over timing responsibilities. // Sync standaloneMediaClock so that it can take over timing responsibilities.
standaloneMediaClock.setPositionUs(rendererMediaClock.getPositionUs()); standaloneMediaClock.setPositionUs(rendererMediaClock.getPositionUs());
......
/*
* 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;
}
}
...@@ -149,7 +149,13 @@ public interface Renderer extends ExoPlayerComponent { ...@@ -149,7 +149,13 @@ public interface Renderer extends ExoPlayerComponent {
* This method may be called when the renderer is in the following states: * This method may be called when the renderer is in the following states:
* {@link #STATE_ENABLED}, {@link #STATE_STARTED}. * {@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 * Throws an error that's preventing the renderer from reading from its {@link SampleStream}. Does
......
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