Commit ca2105d5 by tonihei Committed by Oliver Woodman

Wait with throwing stuck buffering error until pending load is finished

We currently check for shouldContinueLoading, which is the intention to load,
not playbackInfo.isLoading, which is the actual loading state, when detecting
stuck buffering issues.

They only differ if the LoadControl said stop loading (based on nextLoadPosition),
but there is still a load in flight (updating bufferedPosition). This may cause
the exception to be thrown in edge cases that are only temporary.

PiperOrigin-RevId: 307022608
parent 15bbd181
...@@ -870,7 +870,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -870,7 +870,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
} }
if (throwWhenStuckBuffering if (throwWhenStuckBuffering
&& !shouldContinueLoading && !playbackInfo.isLoading
&& playbackInfo.totalBufferedDurationUs < 500_000 && playbackInfo.totalBufferedDurationUs < 500_000
&& isLoadingPossible()) { && isLoadingPossible()) {
// Throw if the LoadControl prevents loading even if the buffer is empty or almost empty. We // Throw if the LoadControl prevents loading even if the buffer is empty or almost empty. We
......
...@@ -100,6 +100,7 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -100,6 +100,7 @@ public class SimpleExoPlayer extends BasePlayer
private AnalyticsCollector analyticsCollector; private AnalyticsCollector analyticsCollector;
private Looper looper; private Looper looper;
private boolean useLazyPreparation; private boolean useLazyPreparation;
private boolean throwWhenStuckBuffering;
private boolean buildCalled; private boolean buildCalled;
/** /**
...@@ -295,6 +296,19 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -295,6 +296,19 @@ public class SimpleExoPlayer extends BasePlayer
} }
/** /**
* 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 Clock} that will be used by the player. Should only be set for testing * Sets the {@link Clock} that will be used by the player. Should only be set for testing
* purposes. * purposes.
* *
...@@ -384,6 +398,9 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -384,6 +398,9 @@ public class SimpleExoPlayer extends BasePlayer
builder.useLazyPreparation, builder.useLazyPreparation,
builder.clock, builder.clock,
builder.looper); builder.looper);
if (builder.throwWhenStuckBuffering) {
player.experimental_throwWhenStuckBuffering();
}
} }
/** /**
......
...@@ -67,6 +67,7 @@ import com.google.android.exoplayer2.testutil.FakeTimeline; ...@@ -67,6 +67,7 @@ import com.google.android.exoplayer2.testutil.FakeTimeline;
import com.google.android.exoplayer2.testutil.FakeTimeline.TimelineWindowDefinition; import com.google.android.exoplayer2.testutil.FakeTimeline.TimelineWindowDefinition;
import com.google.android.exoplayer2.testutil.FakeTrackSelection; import com.google.android.exoplayer2.testutil.FakeTrackSelection;
import com.google.android.exoplayer2.testutil.FakeTrackSelector; import com.google.android.exoplayer2.testutil.FakeTrackSelector;
import com.google.android.exoplayer2.testutil.TestExoPlayer;
import com.google.android.exoplayer2.trackselection.TrackSelection; import com.google.android.exoplayer2.trackselection.TrackSelection;
import com.google.android.exoplayer2.trackselection.TrackSelectionArray; import com.google.android.exoplayer2.trackselection.TrackSelectionArray;
import com.google.android.exoplayer2.upstream.Allocation; import com.google.android.exoplayer2.upstream.Allocation;
...@@ -3540,6 +3541,78 @@ public final class ExoPlayerTest { ...@@ -3540,6 +3541,78 @@ public final class ExoPlayerTest {
} }
@Test @Test
public void
nextLoadPositionExceedingLoadControlMaxBuffer_whileCurrentLoadInProgress_doesNotThrowException() {
long maxBufferUs = 2 * C.MICROS_PER_SECOND;
LoadControl loadControlWithMaxBufferUs =
new DefaultLoadControl() {
@Override
public boolean shouldContinueLoading(long bufferedDurationUs, float playbackSpeed) {
return bufferedDurationUs < maxBufferUs;
}
@Override
public boolean shouldStartPlayback(
long bufferedDurationUs, float playbackSpeed, boolean rebuffering) {
return true;
}
};
MediaSource mediaSourceWithLoadInProgress =
new FakeMediaSource(
new FakeTimeline(/* windowCount= */ 1), ExoPlayerTestRunner.VIDEO_FORMAT) {
@Override
protected FakeMediaPeriod createFakeMediaPeriod(
MediaPeriodId id,
TrackGroupArray trackGroupArray,
Allocator allocator,
EventDispatcher eventDispatcher,
@Nullable TransferListener transferListener) {
return new FakeMediaPeriod(trackGroupArray, eventDispatcher) {
@Override
public long getBufferedPositionUs() {
// Pretend not to have buffered data yet.
return 0;
}
@Override
public long getNextLoadPositionUs() {
// Set next load position beyond the maxBufferUs configured in the LoadControl.
return Long.MAX_VALUE;
}
@Override
public boolean isLoading() {
return true;
}
};
}
};
FakeRenderer rendererWaitingForData =
new FakeRenderer(C.TRACK_TYPE_VIDEO) {
@Override
public boolean isReady() {
return false;
}
};
SimpleExoPlayer player =
new TestExoPlayer.Builder(context)
.setRenderers(rendererWaitingForData)
.setLoadControl(loadControlWithMaxBufferUs)
.experimental_setThrowWhenStuckBuffering(true)
.build();
player.setMediaSource(mediaSourceWithLoadInProgress);
player.prepare();
// Wait until the MediaSource is prepared, i.e. returned its timeline, and at least one
// iteration of doSomeWork after this was run.
TestExoPlayer.runUntilTimelineChanged(player, /* expectedTimeline= */ null);
TestExoPlayer.runUntilPendingCommandsAreFullyHandled(player);
assertThat(player.getPlayerError()).isNull();
}
@Test
public void loadControlNeverWantsToPlay_playbackDoesNotGetStuck() throws Exception { public void loadControlNeverWantsToPlay_playbackDoesNotGetStuck() throws Exception {
LoadControl neverLoadingOrPlayingLoadControl = LoadControl neverLoadingOrPlayingLoadControl =
new DefaultLoadControl() { new DefaultLoadControl() {
......
...@@ -36,6 +36,7 @@ import com.google.android.exoplayer2.upstream.DefaultBandwidthMeter; ...@@ -36,6 +36,7 @@ import com.google.android.exoplayer2.upstream.DefaultBandwidthMeter;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.Clock;
import com.google.android.exoplayer2.util.Supplier; import com.google.android.exoplayer2.util.Supplier;
import com.google.android.exoplayer2.util.Util;
import com.google.android.exoplayer2.video.VideoListener; import com.google.android.exoplayer2.video.VideoListener;
import java.lang.reflect.InvocationTargetException; import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method; import java.lang.reflect.Method;
...@@ -78,6 +79,7 @@ public class TestExoPlayer { ...@@ -78,6 +79,7 @@ public class TestExoPlayer {
@Nullable private Renderer[] renderers; @Nullable private Renderer[] renderers;
@Nullable private RenderersFactory renderersFactory; @Nullable private RenderersFactory renderersFactory;
private boolean useLazyPreparation; private boolean useLazyPreparation;
private boolean throwWhenStuckBuffering;
private @MonotonicNonNull Looper looper; private @MonotonicNonNull Looper looper;
public Builder(Context context) { public Builder(Context context) {
...@@ -248,6 +250,19 @@ public class TestExoPlayer { ...@@ -248,6 +250,19 @@ public class TestExoPlayer {
} }
/** /**
* 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;
}
/**
* Builds an {@link SimpleExoPlayer} using the provided values or their defaults. * Builds an {@link SimpleExoPlayer} using the provided values or their defaults.
* *
* @return The built {@link ExoPlayerTestRunner}. * @return The built {@link ExoPlayerTestRunner}.
...@@ -281,6 +296,7 @@ public class TestExoPlayer { ...@@ -281,6 +296,7 @@ public class TestExoPlayer {
.setClock(clock) .setClock(clock)
.setUseLazyPreparation(useLazyPreparation) .setUseLazyPreparation(useLazyPreparation)
.setLooper(looper) .setLooper(looper)
.experimental_setThrowWhenStuckBuffering(throwWhenStuckBuffering)
.build(); .build();
} }
} }
...@@ -436,6 +452,23 @@ public class TestExoPlayer { ...@@ -436,6 +452,23 @@ public class TestExoPlayer {
runUntil(() -> receivedCallback.get()); runUntil(() -> receivedCallback.get());
} }
/**
* Runs tasks of the main {@link Looper} until the {@code player} handled all previously issued
* commands completely on the internal playback thread.
*/
public static void runUntilPendingCommandsAreFullyHandled(SimpleExoPlayer player) {
verifyMainTestThread(player);
// Send message to player that will arrive after all other pending commands. Thus, the message
// execution on the app thread will also happen after all other pending command
// acknowledgements have arrived back on the app thread.
AtomicBoolean receivedMessageCallback = new AtomicBoolean(false);
player
.createMessage((type, data) -> receivedMessageCallback.set(true))
.setHandler(Util.createHandler())
.send();
runUntil(() -> receivedMessageCallback.get());
}
/** Run tasks of the main {@link Looper} until the {@code condition} returns {@code true}. */ /** Run tasks of the main {@link Looper} until the {@code condition} returns {@code true}. */
public static void runUntil(Supplier<Boolean> condition) { public static void runUntil(Supplier<Boolean> condition) {
verifyMainTestThread(); verifyMainTestThread();
......
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