diff --git a/RELEASENOTES.md b/RELEASENOTES.md index b211d55961..7b7b842dfd 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -12,8 +12,11 @@ ([#4463](https://github.com/google/ExoPlayer/issues/4463)). * Add a getter and callback for static metadata to the player ([#7266](https://github.com/google/ExoPlayer/issues/7266)). - * Timeout on release to prevent ANRs if the underlying platform call + * Time out on release to prevent ANRs if the underlying platform call is stuck ([#4352](https://github.com/google/ExoPlayer/issues/4352)). + * Time out when detaching a surface to prevent ANRs if the underlying + platform call is stuck + ([#5887](https://github.com/google/ExoPlayer/issues/5887)). * Track selection: * Add option to specify multiple preferred audio or text languages. * Data sources: diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlaybackException.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlaybackException.java index d69b747f8d..369235cbda 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlaybackException.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlaybackException.java @@ -83,16 +83,17 @@ public final class ExoPlaybackException extends Exception { /** * The operation which produced the timeout error. One of {@link #TIMEOUT_OPERATION_RELEASE}, - * {@link #TIMEOUT_OPERATION_SET_FOREGROUND_MODE} or {@link #TIMEOUT_OPERATION_UNDEFINED}. Note - * that new operations may be added in the future and error handling should handle unknown - * operation values. + * {@link #TIMEOUT_OPERATION_SET_FOREGROUND_MODE}, {@link #TIMEOUT_OPERATION_DETACH_SURFACE} or + * {@link #TIMEOUT_OPERATION_UNDEFINED}. Note that new operations may be added in the future and + * error handling should handle unknown operation values. */ @Documented @Retention(RetentionPolicy.SOURCE) @IntDef({ TIMEOUT_OPERATION_UNDEFINED, TIMEOUT_OPERATION_RELEASE, - TIMEOUT_OPERATION_SET_FOREGROUND_MODE + TIMEOUT_OPERATION_SET_FOREGROUND_MODE, + TIMEOUT_OPERATION_DETACH_SURFACE }) public @interface TimeoutOperation {} @@ -102,6 +103,8 @@ public final class ExoPlaybackException extends Exception { public static final int TIMEOUT_OPERATION_RELEASE = 1; /** The error occurred in {@link ExoPlayer#setForegroundMode}. */ public static final int TIMEOUT_OPERATION_SET_FOREGROUND_MODE = 2; + /** The error occurred while detaching a surface from the player. */ + public static final int TIMEOUT_OPERATION_DETACH_SURFACE = 3; /** If {@link #type} is {@link #TYPE_RENDERER}, this is the name of the renderer. */ @Nullable public final String rendererName; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index f2e1aff1af..a435684d69 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -656,18 +656,28 @@ import java.util.concurrent.TimeoutException; if (this.foregroundMode != foregroundMode) { this.foregroundMode = foregroundMode; if (!internalPlayer.setForegroundMode(foregroundMode)) { - notifyListeners( - listener -> - listener.onPlayerError( - ExoPlaybackException.createForTimeout( - new TimeoutException("Setting foreground mode timed out."), - ExoPlaybackException.TIMEOUT_OPERATION_SET_FOREGROUND_MODE))); + stop( + /* reset= */ false, + ExoPlaybackException.createForTimeout( + new TimeoutException("Setting foreground mode timed out."), + ExoPlaybackException.TIMEOUT_OPERATION_SET_FOREGROUND_MODE)); } } } @Override public void stop(boolean reset) { + stop(reset, /* error= */ null); + } + + /** + * Stops the player. + * + * @param reset Whether the playlist should be cleared and whether the playback position and + * playback error should be reset. + * @param error An optional {@link ExoPlaybackException} to set. + */ + public void stop(boolean reset, @Nullable ExoPlaybackException error) { PlaybackInfo playbackInfo; if (reset) { playbackInfo = @@ -680,6 +690,9 @@ import java.util.concurrent.TimeoutException; playbackInfo.totalBufferedDurationUs = 0; } playbackInfo = playbackInfo.copyWithPlaybackState(Player.STATE_IDLE); + if (error != null) { + playbackInfo = playbackInfo.copyWithPlaybackError(error); + } pendingOperationAcks++; internalPlayer.stop(); updatePlaybackInfo( diff --git a/library/core/src/main/java/com/google/android/exoplayer2/PlayerMessage.java b/library/core/src/main/java/com/google/android/exoplayer2/PlayerMessage.java index 7e2cb69bc6..6f81a35dd8 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/PlayerMessage.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/PlayerMessage.java @@ -269,6 +269,20 @@ public final class PlayerMessage { return isCanceled; } + /** + * Marks the message as processed. Should only be called by a {@link Sender} and may be called + * multiple times. + * + * @param isDelivered Whether the message has been delivered to its target. The message is + * considered as being delivered when this method has been called with {@code isDelivered} set + * to true at least once. + */ + public synchronized void markAsProcessed(boolean isDelivered) { + this.isDelivered |= isDelivered; + isProcessed = true; + notifyAll(); + } + /** * Blocks until after the message has been delivered or the player is no longer able to deliver * the message. @@ -292,44 +306,30 @@ public final class PlayerMessage { return isDelivered; } - /** - * Marks the message as processed. Should only be called by a {@link Sender} and may be called - * multiple times. - * - * @param isDelivered Whether the message has been delivered to its target. The message is - * considered as being delivered when this method has been called with {@code isDelivered} set - * to true at least once. - */ - public synchronized void markAsProcessed(boolean isDelivered) { - this.isDelivered |= isDelivered; - isProcessed = true; - notifyAll(); - } - /** * Blocks until after the message has been delivered or the player is no longer able to deliver - * the message or the specified waiting time elapses. + * the message or the specified timeout elapsed. * *

Note that this method can't be called if the current thread is the same thread used by the * message handler set with {@link #setHandler(Handler)} as it would cause a deadlock. * - * @param timeoutMs the maximum time to wait in milliseconds. + * @param timeoutMs The timeout in milliseconds. * @return Whether the message was delivered successfully. * @throws IllegalStateException If this method is called before {@link #send()}. * @throws IllegalStateException If this method is called on the same thread used by the message * handler set with {@link #setHandler(Handler)}. - * @throws TimeoutException If the waiting time elapsed and this message has not been delivered - * and the player is still able to deliver the message. + * @throws TimeoutException If the {@code timeoutMs} elapsed and this message has not been + * delivered and the player is still able to deliver the message. * @throws InterruptedException If the current thread is interrupted while waiting for the message * to be delivered. */ - public synchronized boolean experimentalBlockUntilDelivered(long timeoutMs) + public synchronized boolean blockUntilDelivered(long timeoutMs) throws InterruptedException, TimeoutException { - return experimentalBlockUntilDelivered(timeoutMs, Clock.DEFAULT); + return blockUntilDelivered(timeoutMs, Clock.DEFAULT); } @VisibleForTesting() - /* package */ synchronized boolean experimentalBlockUntilDelivered(long timeoutMs, Clock clock) + /* package */ synchronized boolean blockUntilDelivered(long timeoutMs, Clock clock) throws InterruptedException, TimeoutException { Assertions.checkState(isSent); Assertions.checkState(handler.getLooper().getThread() != Thread.currentThread()); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java b/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java index 4d7fa8c67f..7f5384442e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java @@ -67,6 +67,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.TimeoutException; /** * An {@link ExoPlayer} implementation that uses default {@link Renderer} components. Instances can @@ -80,6 +81,9 @@ public class SimpleExoPlayer extends BasePlayer Player.MetadataComponent, Player.DeviceComponent { + /** The default timeout for detaching a surface from the player, in milliseconds. */ + public static final long DEFAULT_DETACH_SURFACE_TIMEOUT_MS = 2_000; + /** @deprecated Use {@link com.google.android.exoplayer2.video.VideoListener}. */ @Deprecated public interface VideoListener extends com.google.android.exoplayer2.video.VideoListener {} @@ -111,6 +115,7 @@ public class SimpleExoPlayer extends BasePlayer private boolean useLazyPreparation; private SeekParameters seekParameters; private long releaseTimeoutMs; + private long detachSurfaceTimeoutMs; private boolean pauseAtEndOfMediaItems; private boolean throwWhenStuckBuffering; private boolean buildCalled; @@ -145,6 +150,7 @@ public class SimpleExoPlayer extends BasePlayer *

  • {@code useLazyPreparation}: {@code true} *
  • {@link SeekParameters}: {@link SeekParameters#DEFAULT} *
  • {@code releaseTimeoutMs}: {@link ExoPlayer#DEFAULT_RELEASE_TIMEOUT_MS} + *
  • {@code detachSurfaceTimeoutMs}: {@link #DEFAULT_DETACH_SURFACE_TIMEOUT_MS} *
  • {@code pauseAtEndOfMediaItems}: {@code false} *
  • {@link Clock}: {@link Clock#DEFAULT} * @@ -243,6 +249,7 @@ public class SimpleExoPlayer extends BasePlayer clock = Clock.DEFAULT; throwWhenStuckBuffering = true; releaseTimeoutMs = ExoPlayer.DEFAULT_RELEASE_TIMEOUT_MS; + detachSurfaceTimeoutMs = DEFAULT_DETACH_SURFACE_TIMEOUT_MS; } /** @@ -476,6 +483,23 @@ public class SimpleExoPlayer extends BasePlayer return this; } + /** + * Sets a timeout for detaching a surface from the player. + * + *

    If detaching a surface or replacing a surface takes more than {@code + * detachSurfaceTimeoutMs} to complete, the player will report an error via {@link + * Player.EventListener#onPlayerError}. + * + * @param detachSurfaceTimeoutMs The timeout for detaching a surface, in milliseconds. + * @return This builder. + * @throws IllegalStateException If {@link #build()} has already been called. + */ + public Builder setDetachSurfaceTimeoutMs(long detachSurfaceTimeoutMs) { + Assertions.checkState(!buildCalled); + this.detachSurfaceTimeoutMs = detachSurfaceTimeoutMs; + return this; + } + /** * Sets whether to pause playback at the end of each media item. * @@ -557,6 +581,7 @@ public class SimpleExoPlayer extends BasePlayer private final StreamVolumeManager streamVolumeManager; private final WakeLockManager wakeLockManager; private final WifiLockManager wifiLockManager; + private final long detachSurfaceTimeoutMs; @Nullable private Format videoFormat; @Nullable private Format audioFormat; @@ -617,6 +642,7 @@ public class SimpleExoPlayer extends BasePlayer audioAttributes = builder.audioAttributes; videoScalingMode = builder.videoScalingMode; skipSilenceEnabled = builder.skipSilenceEnabled; + detachSurfaceTimeoutMs = builder.detachSurfaceTimeoutMs; componentListener = new ComponentListener(); videoListeners = new CopyOnWriteArraySet<>(); audioListeners = new CopyOnWriteArraySet<>(); @@ -2019,10 +2045,16 @@ public class SimpleExoPlayer extends BasePlayer // We're replacing a surface. Block to ensure that it's not accessed after the method returns. try { for (PlayerMessage message : messages) { - message.blockUntilDelivered(); + message.blockUntilDelivered(detachSurfaceTimeoutMs); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); + } catch (TimeoutException e) { + player.stop( + /* reset= */ false, + ExoPlaybackException.createForTimeout( + new TimeoutException("Detaching surface timed out."), + ExoPlaybackException.TIMEOUT_OPERATION_DETACH_SURFACE)); } // If we created the previous surface, we are responsible for releasing it. if (this.ownsSurface) { diff --git a/library/core/src/test/java/com/google/android/exoplayer2/PlayerMessageTest.java b/library/core/src/test/java/com/google/android/exoplayer2/PlayerMessageTest.java index 490cc520fe..70fd5445e1 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/PlayerMessageTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/PlayerMessageTest.java @@ -17,7 +17,7 @@ package com.google.android.exoplayer2; import static com.google.common.truth.Truth.assertThat; import static java.util.concurrent.TimeUnit.SECONDS; -import static org.junit.Assert.fail; +import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; @@ -66,31 +66,27 @@ public class PlayerMessageTest { } @Test - public void experimentalBlockUntilDelivered_timesOut() throws Exception { + public void blockUntilDelivered_timesOut() throws Exception { when(clock.elapsedRealtime()).thenReturn(0L).thenReturn(TIMEOUT_MS * 2); - try { - message.send().experimentalBlockUntilDelivered(TIMEOUT_MS, clock); - fail(); - } catch (TimeoutException expected) { - } + assertThrows( + TimeoutException.class, () -> message.send().blockUntilDelivered(TIMEOUT_MS, clock)); - // Ensure experimentalBlockUntilDelivered() entered the blocking loop + // Ensure blockUntilDelivered() entered the blocking loop. verify(clock, Mockito.times(2)).elapsedRealtime(); } @Test - public void experimentalBlockUntilDelivered_onAlreadyProcessed_succeeds() throws Exception { + public void blockUntilDelivered_onAlreadyProcessed_succeeds() throws Exception { when(clock.elapsedRealtime()).thenReturn(0L); message.send().markAsProcessed(/* isDelivered= */ true); - assertThat(message.experimentalBlockUntilDelivered(TIMEOUT_MS, clock)).isTrue(); + assertThat(message.blockUntilDelivered(TIMEOUT_MS, clock)).isTrue(); } @Test - public void experimentalBlockUntilDelivered_markAsProcessedWhileBlocked_succeeds() - throws Exception { + public void blockUntilDelivered_markAsProcessedWhileBlocked_succeeds() throws Exception { message.send(); // Use a separate Thread to mark the message as processed. @@ -114,8 +110,8 @@ public class PlayerMessageTest { }); try { - assertThat(message.experimentalBlockUntilDelivered(TIMEOUT_MS, clock)).isTrue(); - // Ensure experimentalBlockUntilDelivered() entered the blocking loop. + assertThat(message.blockUntilDelivered(TIMEOUT_MS, clock)).isTrue(); + // Ensure blockUntilDelivered() entered the blocking loop. verify(clock, Mockito.atLeast(2)).elapsedRealtime(); future.get(1, SECONDS); } finally {