Commit 008c8081 by tonihei

Enable release timeout by default and make config non-experimental.

Using a timeout prevents ANRs in cases where the underlying platform
gets blocked forever, so we enable this feature by default.

Issue: #4352
PiperOrigin-RevId: 335642485
parent 53f50f7c
...@@ -12,6 +12,8 @@ ...@@ -12,6 +12,8 @@
([#4463](https://github.com/google/ExoPlayer/issues/4463)). ([#4463](https://github.com/google/ExoPlayer/issues/4463)).
* Add a getter and callback for static metadata to the player * Add a getter and callback for static metadata to the player
([#7266](https://github.com/google/ExoPlayer/issues/7266)). ([#7266](https://github.com/google/ExoPlayer/issues/7266)).
* Timeout on release to prevent ANRs if the underlying platform call
is stuck ([#4352](https://github.com/google/ExoPlayer/issues/4352)).
* Track selection: * Track selection:
* Add option to specify multiple preferred audio or text languages. * Add option to specify multiple preferred audio or text languages.
* Data sources: * Data sources:
......
...@@ -134,6 +134,12 @@ import java.util.List; ...@@ -134,6 +134,12 @@ import java.util.List;
public interface ExoPlayer extends Player { public interface ExoPlayer extends Player {
/** /**
* The default timeout for calls to {@link #release} and {@link #setForegroundMode}, in
* milliseconds.
*/
long DEFAULT_RELEASE_TIMEOUT_MS = 500;
/**
* A builder for {@link ExoPlayer} instances. * A builder for {@link ExoPlayer} instances.
* *
* <p>See {@link #Builder(Context, Renderer...)} for the list of default values. * <p>See {@link #Builder(Context, Renderer...)} for the list of default values.
...@@ -152,9 +158,9 @@ public interface ExoPlayer extends Player { ...@@ -152,9 +158,9 @@ public interface ExoPlayer extends Player {
private boolean useLazyPreparation; private boolean useLazyPreparation;
private SeekParameters seekParameters; private SeekParameters seekParameters;
private boolean pauseAtEndOfMediaItems; private boolean pauseAtEndOfMediaItems;
private long releaseTimeoutMs;
private boolean buildCalled; private boolean buildCalled;
private long releaseTimeoutMs;
private boolean throwWhenStuckBuffering; private boolean throwWhenStuckBuffering;
/** /**
...@@ -173,6 +179,7 @@ public interface ExoPlayer extends Player { ...@@ -173,6 +179,7 @@ public interface ExoPlayer extends Player {
* <li>{@link AnalyticsCollector}: {@link AnalyticsCollector} with {@link Clock#DEFAULT} * <li>{@link AnalyticsCollector}: {@link AnalyticsCollector} with {@link Clock#DEFAULT}
* <li>{@code useLazyPreparation}: {@code true} * <li>{@code useLazyPreparation}: {@code true}
* <li>{@link SeekParameters}: {@link SeekParameters#DEFAULT} * <li>{@link SeekParameters}: {@link SeekParameters#DEFAULT}
* <li>{@code releaseTimeoutMs}: {@link ExoPlayer#DEFAULT_RELEASE_TIMEOUT_MS}
* <li>{@code pauseAtEndOfMediaItems}: {@code false} * <li>{@code pauseAtEndOfMediaItems}: {@code false}
* <li>{@link Clock}: {@link Clock#DEFAULT} * <li>{@link Clock}: {@link Clock#DEFAULT}
* </ul> * </ul>
...@@ -218,20 +225,7 @@ public interface ExoPlayer extends Player { ...@@ -218,20 +225,7 @@ public interface ExoPlayer extends Player {
seekParameters = SeekParameters.DEFAULT; seekParameters = SeekParameters.DEFAULT;
clock = Clock.DEFAULT; clock = Clock.DEFAULT;
throwWhenStuckBuffering = true; throwWhenStuckBuffering = true;
} releaseTimeoutMs = DEFAULT_RELEASE_TIMEOUT_MS;
/**
* Set a limit on the time a call to {@link ExoPlayer#release()} can spend. If a call to {@link
* ExoPlayer#release()} takes more than {@code timeoutMs} milliseconds to complete, the player
* will raise an error via {@link Player.EventListener#onPlayerError}.
*
* <p>This method is experimental, and will be renamed or removed in a future release.
*
* @param timeoutMs The time limit in milliseconds, or 0 for no limit.
*/
public Builder experimentalSetReleaseTimeoutMs(long timeoutMs) {
releaseTimeoutMs = timeoutMs;
return this;
} }
/** /**
...@@ -357,6 +351,23 @@ public interface ExoPlayer extends Player { ...@@ -357,6 +351,23 @@ public interface ExoPlayer extends Player {
} }
/** /**
* Sets a timeout for calls to {@link #release} and {@link #setForegroundMode}.
*
* <p>If a call to {@link #release} or {@link #setForegroundMode} takes more than {@code
* timeoutMs} to complete, the player will report an error via {@link
* Player.EventListener#onPlayerError}.
*
* @param releaseTimeoutMs The release timeout, in milliseconds.
* @return This builder.
* @throws IllegalStateException If {@link #build()} has already been called.
*/
public Builder setReleaseTimeoutMs(long releaseTimeoutMs) {
Assertions.checkState(!buildCalled);
this.releaseTimeoutMs = releaseTimeoutMs;
return this;
}
/**
* Sets whether to pause playback at the end of each media item. * Sets whether to pause playback at the end of each media item.
* *
* <p>This means the player will pause at the end of each window in the current {@link * <p>This means the player will pause at the end of each window in the current {@link
...@@ -407,13 +418,11 @@ public interface ExoPlayer extends Player { ...@@ -407,13 +418,11 @@ public interface ExoPlayer extends Player {
analyticsCollector, analyticsCollector,
useLazyPreparation, useLazyPreparation,
seekParameters, seekParameters,
releaseTimeoutMs,
pauseAtEndOfMediaItems, pauseAtEndOfMediaItems,
clock, clock,
looper); looper);
if (releaseTimeoutMs > 0) {
player.experimentalSetReleaseTimeoutMs(releaseTimeoutMs);
}
if (!throwWhenStuckBuffering) { if (!throwWhenStuckBuffering) {
player.experimentalDisableThrowWhenStuckBuffering(); player.experimentalDisableThrowWhenStuckBuffering();
} }
......
...@@ -256,6 +256,7 @@ public final class ExoPlayerFactory { ...@@ -256,6 +256,7 @@ public final class ExoPlayerFactory {
/* analyticsCollector= */ null, /* analyticsCollector= */ null,
/* useLazyPreparation= */ true, /* useLazyPreparation= */ true,
SeekParameters.DEFAULT, SeekParameters.DEFAULT,
ExoPlayer.DEFAULT_RELEASE_TIMEOUT_MS,
/* pauseAtEndOfMediaItems= */ false, /* pauseAtEndOfMediaItems= */ false,
Clock.DEFAULT, Clock.DEFAULT,
applicationLooper); applicationLooper);
......
...@@ -117,6 +117,7 @@ import java.util.concurrent.TimeoutException; ...@@ -117,6 +117,7 @@ import java.util.concurrent.TimeoutException;
* loads and other initial preparation steps happen immediately. If true, these initial * loads and other initial preparation steps happen immediately. If true, these initial
* preparations are triggered only when the player starts buffering the media. * preparations are triggered only when the player starts buffering the media.
* @param seekParameters The {@link SeekParameters}. * @param seekParameters The {@link SeekParameters}.
* @param releaseTimeoutMs The timeout for calls to {@link #release()} in milliseconds.
* @param pauseAtEndOfMediaItems Whether to pause playback at the end of each media item. * @param pauseAtEndOfMediaItems Whether to pause playback at the end of each media item.
* @param clock The {@link Clock}. * @param clock The {@link Clock}.
* @param applicationLooper The {@link Looper} that must be used for all calls to the player and * @param applicationLooper The {@link Looper} that must be used for all calls to the player and
...@@ -132,6 +133,7 @@ import java.util.concurrent.TimeoutException; ...@@ -132,6 +133,7 @@ import java.util.concurrent.TimeoutException;
@Nullable AnalyticsCollector analyticsCollector, @Nullable AnalyticsCollector analyticsCollector,
boolean useLazyPreparation, boolean useLazyPreparation,
SeekParameters seekParameters, SeekParameters seekParameters,
long releaseTimeoutMs,
boolean pauseAtEndOfMediaItems, boolean pauseAtEndOfMediaItems,
Clock clock, Clock clock,
Looper applicationLooper) { Looper applicationLooper) {
...@@ -180,6 +182,7 @@ import java.util.concurrent.TimeoutException; ...@@ -180,6 +182,7 @@ import java.util.concurrent.TimeoutException;
shuffleModeEnabled, shuffleModeEnabled,
analyticsCollector, analyticsCollector,
seekParameters, seekParameters,
releaseTimeoutMs,
pauseAtEndOfMediaItems, pauseAtEndOfMediaItems,
applicationLooper, applicationLooper,
clock, clock,
...@@ -188,20 +191,6 @@ import java.util.concurrent.TimeoutException; ...@@ -188,20 +191,6 @@ import java.util.concurrent.TimeoutException;
} }
/** /**
* Set a limit on the time a call to {@link #release()} can spend. If a call to {@link #release()}
* takes more than {@code timeoutMs} milliseconds to complete, the player 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
* only be called before the player is used.
*
* @param timeoutMs The time limit in milliseconds, or 0 for no limit.
*/
public void experimentalSetReleaseTimeoutMs(long timeoutMs) {
internalPlayer.experimentalSetReleaseTimeoutMs(timeoutMs);
}
/**
* Configures the player to not throw when it detects it's stuck buffering. * Configures the player to not 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 * <p>This method is experimental, and will be renamed or removed in a future release. It should
......
...@@ -178,6 +178,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -178,6 +178,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
private final PlaybackInfoUpdateListener playbackInfoUpdateListener; private final PlaybackInfoUpdateListener playbackInfoUpdateListener;
private final MediaPeriodQueue queue; private final MediaPeriodQueue queue;
private final MediaSourceList mediaSourceList; private final MediaSourceList mediaSourceList;
private final long releaseTimeoutMs;
@SuppressWarnings("unused") @SuppressWarnings("unused")
private SeekParameters seekParameters; private SeekParameters seekParameters;
...@@ -202,7 +203,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -202,7 +203,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
private boolean deliverPendingMessageAtStartPositionRequired; private boolean deliverPendingMessageAtStartPositionRequired;
@Nullable private ExoPlaybackException pendingRecoverableError; @Nullable private ExoPlaybackException pendingRecoverableError;
private long releaseTimeoutMs;
private boolean throwWhenStuckBuffering; private boolean throwWhenStuckBuffering;
public ExoPlayerImplInternal( public ExoPlayerImplInternal(
...@@ -215,6 +215,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -215,6 +215,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
boolean shuffleModeEnabled, boolean shuffleModeEnabled,
@Nullable AnalyticsCollector analyticsCollector, @Nullable AnalyticsCollector analyticsCollector,
SeekParameters seekParameters, SeekParameters seekParameters,
long releaseTimeoutMs,
boolean pauseAtEndOfWindow, boolean pauseAtEndOfWindow,
Looper applicationLooper, Looper applicationLooper,
Clock clock, Clock clock,
...@@ -228,6 +229,7 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -228,6 +229,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
this.repeatMode = repeatMode; this.repeatMode = repeatMode;
this.shuffleModeEnabled = shuffleModeEnabled; this.shuffleModeEnabled = shuffleModeEnabled;
this.seekParameters = seekParameters; this.seekParameters = seekParameters;
this.releaseTimeoutMs = releaseTimeoutMs;
this.pauseAtEndOfWindow = pauseAtEndOfWindow; this.pauseAtEndOfWindow = pauseAtEndOfWindow;
this.clock = clock; this.clock = clock;
...@@ -262,10 +264,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -262,10 +264,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
handler = clock.createHandler(playbackLooper, this); handler = clock.createHandler(playbackLooper, this);
} }
public void experimentalSetReleaseTimeoutMs(long releaseTimeoutMs) {
this.releaseTimeoutMs = releaseTimeoutMs;
}
public void experimentalDisableThrowWhenStuckBuffering() { public void experimentalDisableThrowWhenStuckBuffering() {
throwWhenStuckBuffering = false; throwWhenStuckBuffering = false;
} }
...@@ -374,6 +372,12 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -374,6 +372,12 @@ import java.util.concurrent.atomic.AtomicBoolean;
handler.obtainMessage(MSG_SEND_MESSAGE, message).sendToTarget(); handler.obtainMessage(MSG_SEND_MESSAGE, message).sendToTarget();
} }
/**
* Sets the foreground mode.
*
* @param foregroundMode Whether foreground mode should be enabled.
* @return Whether the operations succeeded. If false, the operation timed out.
*/
public synchronized boolean setForegroundMode(boolean foregroundMode) { public synchronized boolean setForegroundMode(boolean foregroundMode) {
if (released || !internalPlaybackThread.isAlive()) { if (released || !internalPlaybackThread.isAlive()) {
return true; return true;
...@@ -386,26 +390,22 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -386,26 +390,22 @@ import java.util.concurrent.atomic.AtomicBoolean;
handler handler
.obtainMessage(MSG_SET_FOREGROUND_MODE, /* foregroundMode */ 0, 0, processedFlag) .obtainMessage(MSG_SET_FOREGROUND_MODE, /* foregroundMode */ 0, 0, processedFlag)
.sendToTarget(); .sendToTarget();
if (releaseTimeoutMs > 0) { waitUninterruptibly(/* condition= */ processedFlag::get, releaseTimeoutMs);
waitUninterruptibly(/* condition= */ processedFlag::get, releaseTimeoutMs);
} else {
waitUninterruptibly(/* condition= */ processedFlag::get);
}
return processedFlag.get(); return processedFlag.get();
} }
} }
/**
* Releases the player.
*
* @return Whether the release succeeded. If false, the release timed out.
*/
public synchronized boolean release() { public synchronized boolean release() {
if (released || !internalPlaybackThread.isAlive()) { if (released || !internalPlaybackThread.isAlive()) {
return true; return true;
} }
handler.sendEmptyMessage(MSG_RELEASE); handler.sendEmptyMessage(MSG_RELEASE);
if (releaseTimeoutMs > 0) { waitUninterruptibly(/* condition= */ () -> released, releaseTimeoutMs);
waitUninterruptibly(/* condition= */ () -> released, releaseTimeoutMs);
} else {
waitUninterruptibly(/* condition= */ () -> released);
}
return released; return released;
} }
...@@ -607,29 +607,6 @@ import java.util.concurrent.atomic.AtomicBoolean; ...@@ -607,29 +607,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
/** /**
* Blocks the current thread until a condition becomes true.
*
* <p>If the current thread is interrupted while waiting for the condition to become true, this
* method will restore the interrupt <b>after</b> the condition became true.
*
* @param condition The condition.
*/
private synchronized void waitUninterruptibly(Supplier<Boolean> condition) {
boolean wasInterrupted = false;
while (!condition.get()) {
try {
wait();
} catch (InterruptedException e) {
wasInterrupted = true;
}
}
if (wasInterrupted) {
// Restore the interrupted status.
Thread.currentThread().interrupt();
}
}
/**
* Blocks the current thread until a condition becomes true or the specified amount of time has * Blocks the current thread until a condition becomes true or the specified amount of time has
* elapsed. * elapsed.
* *
......
...@@ -110,6 +110,7 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -110,6 +110,7 @@ public class SimpleExoPlayer extends BasePlayer
@Renderer.VideoScalingMode private int videoScalingMode; @Renderer.VideoScalingMode private int videoScalingMode;
private boolean useLazyPreparation; private boolean useLazyPreparation;
private SeekParameters seekParameters; private SeekParameters seekParameters;
private long releaseTimeoutMs;
private boolean pauseAtEndOfMediaItems; private boolean pauseAtEndOfMediaItems;
private boolean throwWhenStuckBuffering; private boolean throwWhenStuckBuffering;
private boolean buildCalled; private boolean buildCalled;
...@@ -143,6 +144,7 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -143,6 +144,7 @@ public class SimpleExoPlayer extends BasePlayer
* <li>{@link Renderer.VideoScalingMode}: {@link Renderer#VIDEO_SCALING_MODE_DEFAULT} * <li>{@link Renderer.VideoScalingMode}: {@link Renderer#VIDEO_SCALING_MODE_DEFAULT}
* <li>{@code useLazyPreparation}: {@code true} * <li>{@code useLazyPreparation}: {@code true}
* <li>{@link SeekParameters}: {@link SeekParameters#DEFAULT} * <li>{@link SeekParameters}: {@link SeekParameters#DEFAULT}
* <li>{@code releaseTimeoutMs}: {@link ExoPlayer#DEFAULT_RELEASE_TIMEOUT_MS}
* <li>{@code pauseAtEndOfMediaItems}: {@code false} * <li>{@code pauseAtEndOfMediaItems}: {@code false}
* <li>{@link Clock}: {@link Clock#DEFAULT} * <li>{@link Clock}: {@link Clock#DEFAULT}
* </ul> * </ul>
...@@ -240,6 +242,7 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -240,6 +242,7 @@ public class SimpleExoPlayer extends BasePlayer
seekParameters = SeekParameters.DEFAULT; seekParameters = SeekParameters.DEFAULT;
clock = Clock.DEFAULT; clock = Clock.DEFAULT;
throwWhenStuckBuffering = true; throwWhenStuckBuffering = true;
releaseTimeoutMs = ExoPlayer.DEFAULT_RELEASE_TIMEOUT_MS;
} }
/** /**
...@@ -457,6 +460,23 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -457,6 +460,23 @@ public class SimpleExoPlayer extends BasePlayer
} }
/** /**
* Sets a timeout for calls to {@link #release} and {@link #setForegroundMode}.
*
* <p>If a call to {@link #release} or {@link #setForegroundMode} takes more than {@code
* timeoutMs} to complete, the player will report an error via {@link
* Player.EventListener#onPlayerError}.
*
* @param releaseTimeoutMs The release timeout, in milliseconds.
* @return This builder.
* @throws IllegalStateException If {@link #build()} has already been called.
*/
public Builder setReleaseTimeoutMs(long releaseTimeoutMs) {
Assertions.checkState(!buildCalled);
this.releaseTimeoutMs = releaseTimeoutMs;
return this;
}
/**
* Sets whether to pause playback at the end of each media item. * Sets whether to pause playback at the end of each media item.
* *
* <p>This means the player will pause at the end of each window in the current {@link * <p>This means the player will pause at the end of each window in the current {@link
...@@ -631,6 +651,7 @@ public class SimpleExoPlayer extends BasePlayer ...@@ -631,6 +651,7 @@ public class SimpleExoPlayer extends BasePlayer
analyticsCollector, analyticsCollector,
builder.useLazyPreparation, builder.useLazyPreparation,
builder.seekParameters, builder.seekParameters,
builder.releaseTimeoutMs,
builder.pauseAtEndOfMediaItems, builder.pauseAtEndOfMediaItems,
builder.clock, builder.clock,
builder.looper); builder.looper);
......
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