Commit 99667a6d by tonihei Committed by Oliver Woodman

Detect stuck-buffering cases in the player.

This removes a workaround that always continues buffering and instead
detects if the LoadControl returns false even though we don't have
any buffer. If enabled by a flag, this condition throws an exception.

PiperOrigin-RevId: 301584239
parent 2e5444b4
...@@ -20,6 +20,7 @@ import com.google.android.exoplayer2.trackselection.TrackSelectionArray; ...@@ -20,6 +20,7 @@ import com.google.android.exoplayer2.trackselection.TrackSelectionArray;
import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.Allocator;
import com.google.android.exoplayer2.upstream.DefaultAllocator; import com.google.android.exoplayer2.upstream.DefaultAllocator;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Log;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
/** /**
...@@ -383,6 +384,11 @@ public class DefaultLoadControl implements LoadControl { ...@@ -383,6 +384,11 @@ public class DefaultLoadControl implements LoadControl {
minBufferUs = Math.max(minBufferUs, 500_000); minBufferUs = Math.max(minBufferUs, 500_000);
if (bufferedDurationUs < minBufferUs) { if (bufferedDurationUs < minBufferUs) {
isBuffering = prioritizeTimeOverSizeThresholds || !targetBufferSizeReached; isBuffering = prioritizeTimeOverSizeThresholds || !targetBufferSizeReached;
if (!isBuffering && bufferedDurationUs < 500_000) {
Log.w(
"DefaultLoadControl",
"Target buffer size reached with less than 500ms of buffered media data.");
}
} else if (bufferedDurationUs >= maxBufferUs || targetBufferSizeReached) { } else if (bufferedDurationUs >= maxBufferUs || targetBufferSizeReached) {
isBuffering = false; isBuffering = false;
} // Else don't change the buffering state } // Else don't change the buffering state
......
...@@ -149,6 +149,7 @@ public interface ExoPlayer extends Player { ...@@ -149,6 +149,7 @@ public interface ExoPlayer extends Player {
private boolean buildCalled; private boolean buildCalled;
private long releaseTimeoutMs; private long releaseTimeoutMs;
private boolean throwWhenStuckBuffering;
/** /**
* Creates a builder with a list of {@link Renderer Renderers}. * Creates a builder with a list of {@link Renderer Renderers}.
...@@ -228,8 +229,7 @@ public interface ExoPlayer extends Player { ...@@ -228,8 +229,7 @@ public interface ExoPlayer extends Player {
* ExoPlayer#release()} takes more than {@code timeoutMs} milliseconds to complete, the player * ExoPlayer#release()} takes more than {@code timeoutMs} milliseconds to complete, the player
* will raise an error via {@link Player.EventListener#onPlayerError}. * will raise an error via {@link Player.EventListener#onPlayerError}.
* *
* <p>This method is experimental, and will be renamed or removed in a future release. It should * <p>This method is experimental, and will be renamed or removed in a future release.
* only be called before the player is used.
* *
* @param timeoutMs The time limit in milliseconds, or 0 for no limit. * @param timeoutMs The time limit in milliseconds, or 0 for no limit.
*/ */
...@@ -239,6 +239,19 @@ public interface ExoPlayer extends Player { ...@@ -239,6 +239,19 @@ public interface ExoPlayer extends Player {
} }
/** /**
* Sets whether the player should throw when it detects it's stuck buffering.
*
* <p>This method is experimental, and will be renamed or removed in a future release.
*
* @param throwWhenStuckBuffering Whether to throw when the player detects it's stuck buffering.
* @return This builder.
*/
public Builder experimental_setThrowWhenStuckBuffering(boolean throwWhenStuckBuffering) {
this.throwWhenStuckBuffering = throwWhenStuckBuffering;
return this;
}
/**
* Sets the {@link TrackSelector} that will be used by the player. * Sets the {@link TrackSelector} that will be used by the player.
* *
* @param trackSelector A {@link TrackSelector}. * @param trackSelector A {@link TrackSelector}.
...@@ -372,6 +385,9 @@ public interface ExoPlayer extends Player { ...@@ -372,6 +385,9 @@ public interface ExoPlayer extends Player {
if (releaseTimeoutMs > 0) { if (releaseTimeoutMs > 0) {
player.experimental_setReleaseTimeoutMs(releaseTimeoutMs); player.experimental_setReleaseTimeoutMs(releaseTimeoutMs);
} }
if (throwWhenStuckBuffering) {
player.experimental_throwWhenStuckBuffering();
}
return player; return player;
} }
......
...@@ -182,6 +182,16 @@ import java.util.concurrent.TimeoutException; ...@@ -182,6 +182,16 @@ import java.util.concurrent.TimeoutException;
internalPlayer.experimental_setReleaseTimeoutMs(timeoutMs); internalPlayer.experimental_setReleaseTimeoutMs(timeoutMs);
} }
/**
* Configures the player to throw when it detects it's stuck buffering.
*
* <p>This method is experimental, and will be renamed or removed in a future release. It should
* only be called before the player is used.
*/
public void experimental_throwWhenStuckBuffering() {
internalPlayer.experimental_throwWhenStuckBuffering();
}
@Override @Override
@Nullable @Nullable
public AudioComponent getAudioComponent() { public AudioComponent getAudioComponent() {
......
...@@ -135,6 +135,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -135,6 +135,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
private boolean deliverPendingMessageAtStartPositionRequired; private boolean deliverPendingMessageAtStartPositionRequired;
private long releaseTimeoutMs; private long releaseTimeoutMs;
private boolean throwWhenStuckBuffering;
public ExoPlayerImplInternal( public ExoPlayerImplInternal(
Renderer[] renderers, Renderer[] renderers,
...@@ -192,6 +193,10 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -192,6 +193,10 @@ import java.util.concurrent.atomic.AtomicBoolean;
this.releaseTimeoutMs = releaseTimeoutMs; this.releaseTimeoutMs = releaseTimeoutMs;
} }
public void experimental_throwWhenStuckBuffering() {
throwWhenStuckBuffering = true;
}
public void prepare() { public void prepare() {
handler.obtainMessage(MSG_PREPARE).sendToTarget(); handler.obtainMessage(MSG_PREPARE).sendToTarget();
} }
...@@ -877,6 +882,15 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -877,6 +882,15 @@ import java.util.concurrent.atomic.AtomicBoolean;
renderers[i].maybeThrowStreamError(); renderers[i].maybeThrowStreamError();
} }
} }
if (throwWhenStuckBuffering
&& !shouldContinueLoading
&& playbackInfo.totalBufferedDurationUs < 500_000
&& isLoadingPossible()) {
// Throw if the LoadControl prevents loading even if the buffer is empty or almost empty. We
// can't compare against 0 to account for small differences between the renderer position
// and buffered position in the media at the point where playback gets stuck.
throw new IllegalStateException("Playback stuck buffering and not loading");
}
} }
if ((shouldPlayWhenReady() && playbackInfo.playbackState == Player.STATE_READY) if ((shouldPlayWhenReady() && playbackInfo.playbackState == Player.STATE_READY)
...@@ -1948,13 +1962,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -1948,13 +1962,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
long bufferedDurationUs = long bufferedDurationUs =
getTotalBufferedDurationUs(queue.getLoadingPeriod().getNextLoadPositionUs()); getTotalBufferedDurationUs(queue.getLoadingPeriod().getNextLoadPositionUs());
if (bufferedDurationUs < 500_000) {
// Prevent loading from getting stuck even if LoadControl.shouldContinueLoading returns false
// when the buffer is empty or almost empty. We can't compare against 0 to account for small
// differences between the renderer position and buffered position in the media at the point
// where playback gets stuck.
return true;
}
return loadControl.shouldContinueLoading(bufferedDurationUs, mediaClock.getPlaybackSpeed()); return loadControl.shouldContinueLoading(bufferedDurationUs, mediaClock.getPlaybackSpeed());
} }
......
...@@ -3539,9 +3539,11 @@ public final class ExoPlayerTest { ...@@ -3539,9 +3539,11 @@ public final class ExoPlayerTest {
testRunner.blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS); testRunner.blockUntilActionScheduleFinished(TIMEOUT_MS).blockUntilEnded(TIMEOUT_MS);
} }
// Disabled until the flag to throw exceptions for [internal: b/144538905] is enabled by default.
@Ignore
@Test @Test
public void loadControlNeverWantsToLoadOrPlay_playbackDoesNotGetStuck() throws Exception { public void loadControlNeverWantsToLoad_throwsIllegalStateException() throws Exception {
LoadControl neverLoadingOrPlayingLoadControl = LoadControl neverLoadingLoadControl =
new DefaultLoadControl() { new DefaultLoadControl() {
@Override @Override
public boolean shouldContinueLoading(long bufferedDurationUs, float playbackSpeed) { public boolean shouldContinueLoading(long bufferedDurationUs, float playbackSpeed) {
...@@ -3551,7 +3553,7 @@ public final class ExoPlayerTest { ...@@ -3551,7 +3553,7 @@ public final class ExoPlayerTest {
@Override @Override
public boolean shouldStartPlayback( public boolean shouldStartPlayback(
long bufferedDurationUs, float playbackSpeed, boolean rebuffering) { long bufferedDurationUs, float playbackSpeed, boolean rebuffering) {
return false; return true;
} }
}; };
...@@ -3565,13 +3567,18 @@ public final class ExoPlayerTest { ...@@ -3565,13 +3567,18 @@ public final class ExoPlayerTest {
new TrackGroupArray(new TrackGroup(Builder.VIDEO_FORMAT)), new TrackGroupArray(new TrackGroup(Builder.VIDEO_FORMAT)),
new FakeChunkSource.Factory(dataSetFactory, new FakeDataSource.Factory())); new FakeChunkSource.Factory(dataSetFactory, new FakeDataSource.Factory()));
new ExoPlayerTestRunner.Builder() ExoPlaybackException exception =
.setLoadControl(neverLoadingOrPlayingLoadControl) assertThrows(
.setMediaSources(chunkedMediaSource) ExoPlaybackException.class,
.build(context) () ->
.start() new ExoPlayerTestRunner.Builder()
// This throws if playback doesn't finish within timeout. .setLoadControl(neverLoadingLoadControl)
.blockUntilEnded(TIMEOUT_MS); .setMediaSources(chunkedMediaSource)
.build(context)
.start()
.blockUntilEnded(TIMEOUT_MS));
assertThat(exception.type).isEqualTo(ExoPlaybackException.TYPE_UNEXPECTED);
assertThat(exception.getUnexpectedException()).isInstanceOf(IllegalStateException.class);
} }
@Test @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