Commit 1495b9a4 by olly Committed by Oliver Woodman

Fix stuck playback when media has uneven track end times

* Always assume a renderer is ready if it's read to the end of
  its current stream and there's a subsequent period already
  prepared. This prevents getting stuck when a non-clock renderer
  has a short stream.
* Switch to the standalone clock if the renderer providing the
  media clock has read to the end of its current stream, is no
  longer ready, and there's a subsequent period already prepared.
  This prevents getting stuck when a clock renderer has a short
  stream.
* Remove unnecessary clock synchronization logic (since it would
  need to be made more complicated as a result of this change).
* Don't update the playing period holder when playWhenReady is
  false. This avoids the position jumping to the start of the
  next period when seeking to the very end of the current period
  whilst paused (we still end up showing the first frame of video
  from the next period, but fixing that will have to wait).

Github: #1874

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=170717481
parent 7e7fea40
...@@ -556,11 +556,17 @@ import java.io.IOException; ...@@ -556,11 +556,17 @@ import java.io.IOException;
if (periodPositionUs != C.TIME_UNSET) { if (periodPositionUs != C.TIME_UNSET) {
resetRendererPosition(periodPositionUs); resetRendererPosition(periodPositionUs);
} else { } else {
if (rendererMediaClockSource != null && !rendererMediaClockSource.isEnded()) { // Use the standalone clock if there's no renderer clock, or if the providing renderer has
// ended or needs the next sample stream to reenter the ready state. The latter case uses the
// standalone clock to avoid getting stuck if tracks in the current period have uneven
// durations. See: https://github.com/google/ExoPlayer/issues/1874.
if (rendererMediaClockSource == null || rendererMediaClockSource.isEnded()
|| (!rendererMediaClockSource.isReady()
&& rendererWaitingForNextStream(rendererMediaClockSource))) {
rendererPositionUs = standaloneMediaClock.getPositionUs();
} else {
rendererPositionUs = rendererMediaClock.getPositionUs(); rendererPositionUs = rendererMediaClock.getPositionUs();
standaloneMediaClock.setPositionUs(rendererPositionUs); standaloneMediaClock.setPositionUs(rendererPositionUs);
} else {
rendererPositionUs = standaloneMediaClock.getPositionUs();
} }
periodPositionUs = playingPeriodHolder.toPeriodTime(rendererPositionUs); periodPositionUs = playingPeriodHolder.toPeriodTime(rendererPositionUs);
} }
...@@ -597,9 +603,12 @@ import java.io.IOException; ...@@ -597,9 +603,12 @@ import java.io.IOException;
// invocation of this method. // invocation of this method.
renderer.render(rendererPositionUs, elapsedRealtimeUs); renderer.render(rendererPositionUs, elapsedRealtimeUs);
allRenderersEnded = allRenderersEnded && renderer.isEnded(); allRenderersEnded = allRenderersEnded && renderer.isEnded();
// Determine whether the renderer is ready (or ended). If it's not, throw an error that's // Determine whether the renderer is ready (or ended). We override to assume the renderer is
// preventing the renderer from making progress, if such an error exists. // ready if it needs the next sample stream. This is necessary to avoid getting stuck if
boolean rendererReadyOrEnded = renderer.isReady() || renderer.isEnded(); // tracks in the current period have uneven durations. See:
// https://github.com/google/ExoPlayer/issues/1874
boolean rendererReadyOrEnded = renderer.isReady() || renderer.isEnded()
|| rendererWaitingForNextStream(renderer);
if (!rendererReadyOrEnded) { if (!rendererReadyOrEnded) {
renderer.maybeThrowStreamError(); renderer.maybeThrowStreamError();
} }
...@@ -617,7 +626,7 @@ import java.io.IOException; ...@@ -617,7 +626,7 @@ import java.io.IOException;
// TODO: Make LoadControl, period transition position projection, adaptive track selection // TODO: Make LoadControl, period transition position projection, adaptive track selection
// and potentially any time-related code in renderers take into account the playback speed. // and potentially any time-related code in renderers take into account the playback speed.
this.playbackParameters = playbackParameters; this.playbackParameters = playbackParameters;
standaloneMediaClock.synchronize(rendererMediaClock); standaloneMediaClock.setPlaybackParameters(playbackParameters);
eventHandler.obtainMessage(MSG_PLAYBACK_PARAMETERS_CHANGED, playbackParameters) eventHandler.obtainMessage(MSG_PLAYBACK_PARAMETERS_CHANGED, playbackParameters)
.sendToTarget(); .sendToTarget();
} }
...@@ -813,9 +822,10 @@ import java.io.IOException; ...@@ -813,9 +822,10 @@ import java.io.IOException;
} }
private void setPlaybackParametersInternal(PlaybackParameters playbackParameters) { private void setPlaybackParametersInternal(PlaybackParameters playbackParameters) {
playbackParameters = rendererMediaClock != null if (rendererMediaClock != null) {
? rendererMediaClock.setPlaybackParameters(playbackParameters) playbackParameters = rendererMediaClock.setPlaybackParameters(playbackParameters);
: standaloneMediaClock.setPlaybackParameters(playbackParameters); }
standaloneMediaClock.setPlaybackParameters(playbackParameters);
this.playbackParameters = playbackParameters; this.playbackParameters = playbackParameters;
eventHandler.obtainMessage(MSG_PLAYBACK_PARAMETERS_CHANGED, playbackParameters).sendToTarget(); eventHandler.obtainMessage(MSG_PLAYBACK_PARAMETERS_CHANGED, playbackParameters).sendToTarget();
} }
...@@ -945,12 +955,6 @@ import java.io.IOException; ...@@ -945,12 +955,6 @@ import java.io.IOException;
if (sampleStream != renderer.getStream()) { if (sampleStream != renderer.getStream()) {
// We need to disable the renderer. // We need to disable the renderer.
if (renderer == rendererMediaClockSource) { if (renderer == rendererMediaClockSource) {
// The renderer is providing the media clock.
if (sampleStream == null) {
// The renderer won't be re-enabled. Sync standaloneMediaClock so that it can take
// over timing responsibilities.
standaloneMediaClock.synchronize(rendererMediaClock);
}
rendererMediaClock = null; rendererMediaClock = null;
rendererMediaClockSource = null; rendererMediaClockSource = null;
} }
...@@ -1300,8 +1304,8 @@ import java.io.IOException; ...@@ -1300,8 +1304,8 @@ import java.io.IOException;
return; return;
} }
// Update the playing and reading periods. // Advance the playing period if necessary.
while (playingPeriodHolder != readingPeriodHolder while (playWhenReady && playingPeriodHolder != readingPeriodHolder
&& rendererPositionUs >= playingPeriodHolder.next.rendererPositionOffsetUs) { && rendererPositionUs >= playingPeriodHolder.next.rendererPositionOffsetUs) {
// All enabled renderers' streams have been read to the end, and the playback position reached // All enabled renderers' streams have been read to the end, and the playback position reached
// the end of the playing period, so advance playback to the next period. // the end of the playing period, so advance playback to the next period.
...@@ -1327,16 +1331,22 @@ import java.io.IOException; ...@@ -1327,16 +1331,22 @@ import java.io.IOException;
return; return;
} }
// Advance the reading period if necessary.
if (readingPeriodHolder.next == null || !readingPeriodHolder.next.prepared) {
// We don't have a successor to advance the reading period to.
return;
}
for (int i = 0; i < renderers.length; i++) { for (int i = 0; i < renderers.length; i++) {
Renderer renderer = renderers[i]; Renderer renderer = renderers[i];
SampleStream sampleStream = readingPeriodHolder.sampleStreams[i]; SampleStream sampleStream = readingPeriodHolder.sampleStreams[i];
if (renderer.getStream() != sampleStream if (renderer.getStream() != sampleStream
|| (sampleStream != null && !renderer.hasReadStreamToEnd())) { || (sampleStream != null && !renderer.hasReadStreamToEnd())) {
// The current reading period is still being read by at least one renderer.
return; return;
} }
} }
if (readingPeriodHolder.next != null && readingPeriodHolder.next.prepared) {
TrackSelectorResult oldTrackSelectorResult = readingPeriodHolder.trackSelectorResult; TrackSelectorResult oldTrackSelectorResult = readingPeriodHolder.trackSelectorResult;
readingPeriodHolder = readingPeriodHolder.next; readingPeriodHolder = readingPeriodHolder.next;
TrackSelectorResult newTrackSelectorResult = readingPeriodHolder.trackSelectorResult; TrackSelectorResult newTrackSelectorResult = readingPeriodHolder.trackSelectorResult;
...@@ -1379,7 +1389,6 @@ import java.io.IOException; ...@@ -1379,7 +1389,6 @@ import java.io.IOException;
} }
} }
} }
}
private void maybeUpdateLoadingPeriod() throws IOException { private void maybeUpdateLoadingPeriod() throws IOException {
MediaPeriodInfo info; MediaPeriodInfo info;
...@@ -1478,8 +1487,6 @@ import java.io.IOException; ...@@ -1478,8 +1487,6 @@ import java.io.IOException;
// needed to play the next period, or because we need to re-enable it as its current stream // needed to play the next period, or because we need to re-enable it as its current stream
// is final and it's not reading ahead. // is final and it's not reading ahead.
if (renderer == rendererMediaClockSource) { if (renderer == rendererMediaClockSource) {
// Sync standaloneMediaClock so that it can take over timing responsibilities.
standaloneMediaClock.synchronize(rendererMediaClock);
rendererMediaClock = null; rendererMediaClock = null;
rendererMediaClockSource = null; rendererMediaClockSource = null;
} }
...@@ -1539,6 +1546,11 @@ import java.io.IOException; ...@@ -1539,6 +1546,11 @@ import java.io.IOException;
} }
} }
private boolean rendererWaitingForNextStream(Renderer renderer) {
return readingPeriodHolder.next != null && readingPeriodHolder.next.prepared
&& renderer.hasReadStreamToEnd();
}
@NonNull @NonNull
private static Format[] getFormats(TrackSelection newSelection) { private static Format[] getFormats(TrackSelection newSelection) {
// Build an array of formats contained by the selection. // Build an array of formats contained by the selection.
......
...@@ -69,16 +69,6 @@ public final class StandaloneMediaClock implements MediaClock { ...@@ -69,16 +69,6 @@ public final class StandaloneMediaClock implements MediaClock {
} }
} }
/**
* Synchronizes this clock with the current state of {@code clock}.
*
* @param clock The clock with which to synchronize.
*/
public void synchronize(MediaClock clock) {
setPositionUs(clock.getPositionUs());
playbackParameters = clock.getPlaybackParameters();
}
@Override @Override
public long getPositionUs() { public long getPositionUs() {
long positionUs = baseUs; long positionUs = baseUs;
......
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