Skip to content

Commit

Permalink
Rollback of 78c850a
Browse files Browse the repository at this point in the history
*** Original commit ***

Remove set timeout on release() and setSurface()

Removes the experimental methods to set a timeout when
releasing the player and setting the surface.

***

PiperOrigin-RevId: 312647457
  • Loading branch information
christosts authored and tonihei committed May 21, 2020
1 parent 634634f commit 1154e80
Show file tree
Hide file tree
Showing 5 changed files with 286 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ final class Builder {
private SeekParameters seekParameters;
private boolean pauseAtEndOfMediaItems;
private boolean buildCalled;

private long releaseTimeoutMs;
private boolean throwWhenStuckBuffering;

/**
Expand Down Expand Up @@ -213,6 +215,20 @@ public Builder(
clock = Clock.DEFAULT;
}

/**
* 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 experimental_setReleaseTimeoutMs(long timeoutMs) {
releaseTimeoutMs = timeoutMs;
return this;
}

/**
* Sets whether the player should throw when it detects it's stuck buffering.
*
Expand Down Expand Up @@ -389,6 +405,10 @@ public ExoPlayer build() {
pauseAtEndOfMediaItems,
clock,
looper);

if (releaseTimeoutMs > 0) {
player.experimental_setReleaseTimeoutMs(releaseTimeoutMs);
}
if (throwWhenStuckBuffering) {
player.experimental_throwWhenStuckBuffering();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.Collections;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeoutException;

/**
* An {@link ExoPlayer} implementation. Instances can be obtained from {@link ExoPlayer.Builder}.
Expand Down Expand Up @@ -177,6 +178,20 @@ public void handleMessage(Message msg) {
internalPlayerHandler = new Handler(internalPlayer.getPlaybackLooper());
}

/**
* 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 experimental_setReleaseTimeoutMs(long timeoutMs) {
internalPlayer.experimental_setReleaseTimeoutMs(timeoutMs);
}

/**
* Configures the player to throw when it detects it's stuck buffering.
*
Expand Down Expand Up @@ -675,7 +690,13 @@ public void release() {
Log.i(TAG, "Release " + Integer.toHexString(System.identityHashCode(this)) + " ["
+ ExoPlayerLibraryInfo.VERSION_SLASHY + "] [" + Util.DEVICE_DEBUG_INFO + "] ["
+ ExoPlayerLibraryInfo.registeredModules() + "]");
internalPlayer.release();
if (!internalPlayer.release()) {
notifyListeners(
listener ->
listener.onPlayerError(
ExoPlaybackException.createForUnexpected(
new RuntimeException(new TimeoutException("Player release timed out.")))));
}
applicationHandler.removeCallbacksAndMessages(null);
playbackInfo =
getResetPlaybackInfo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@
private long rendererPositionUs;
private int nextPendingMessageIndexHint;
private boolean deliverPendingMessageAtStartPositionRequired;

private long releaseTimeoutMs;
private boolean throwWhenStuckBuffering;

public ExoPlayerImplInternal(
Expand Down Expand Up @@ -189,6 +191,10 @@ public ExoPlayerImplInternal(
}
}

public void experimental_setReleaseTimeoutMs(long releaseTimeoutMs) {
this.releaseTimeoutMs = releaseTimeoutMs;
}

public void experimental_throwWhenStuckBuffering() {
throwWhenStuckBuffering = true;
}
Expand Down Expand Up @@ -316,23 +322,23 @@ public synchronized void setForegroundMode(boolean foregroundMode) {
}
}

public synchronized void release() {
public synchronized boolean release() {
if (released || !internalPlaybackThread.isAlive()) {
return;
return true;
}

handler.sendEmptyMessage(MSG_RELEASE);
boolean wasInterrupted = false;
while (!released) {
try {
wait();
} catch (InterruptedException e) {
wasInterrupted = true;
try {
if (releaseTimeoutMs > 0) {
waitUntilReleased(releaseTimeoutMs);
} else {
waitUntilReleased();
}
}
if (wasInterrupted) {
// Restore the interrupted status.
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}

return released;
}

public Looper getPlaybackLooper() {
Expand Down Expand Up @@ -498,6 +504,63 @@ public boolean handleMessage(Message msg) {

// Private methods.

/**
* Blocks the current thread until {@link #releaseInternal()} is executed on the playback Thread.
*
* <p>If the current thread is interrupted while waiting for {@link #releaseInternal()} to
* complete, this method will delay throwing the {@link InterruptedException} to ensure that the
* underlying resources have been released, and will an {@link InterruptedException} <b>after</b>
* {@link #releaseInternal()} is complete.
*
* @throws {@link InterruptedException} if the current Thread was interrupted while waiting for
* {@link #releaseInternal()} to complete.
*/
private synchronized void waitUntilReleased() throws InterruptedException {
InterruptedException interruptedException = null;
while (!released) {
try {
wait();
} catch (InterruptedException e) {
interruptedException = e;
}
}

if (interruptedException != null) {
throw interruptedException;
}
}

/**
* Blocks the current thread until {@link #releaseInternal()} is performed on the playback Thread
* or the specified amount of time has elapsed.
*
* <p>If the current thread is interrupted while waiting for {@link #releaseInternal()} to
* complete, this method will delay throwing the {@link InterruptedException} to ensure that the
* underlying resources have been released or the operation timed out, and will throw an {@link
* InterruptedException} afterwards.
*
* @param timeoutMs the time in milliseconds to wait for {@link #releaseInternal()} to complete.
* @throws {@link InterruptedException} if the current Thread was interrupted while waiting for
* {@link #releaseInternal()} to complete.
*/
private synchronized void waitUntilReleased(long timeoutMs) throws InterruptedException {
long deadlineMs = clock.elapsedRealtime() + timeoutMs;
long remainingMs = timeoutMs;
InterruptedException interruptedException = null;
while (!released && remainingMs > 0) {
try {
wait(remainingMs);
} catch (InterruptedException e) {
interruptedException = e;
}
remainingMs = deadlineMs - clock.elapsedRealtime();
}

if (interruptedException != null) {
throw interruptedException;
}
}

private void setState(int state) {
if (playbackInfo.playbackState != state) {
playbackInfo = playbackInfo.copyWithPlaybackState(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

import android.os.Handler;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Clock;
import java.util.concurrent.TimeoutException;

/**
* Defines a player message which can be sent with a {@link Sender} and received by a {@link
Expand Down Expand Up @@ -289,6 +292,28 @@ public synchronized boolean blockUntilDelivered() throws InterruptedException {
return isDelivered;
}

/**
* 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.
*
* <p>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.
* @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 InterruptedException If the current thread is interrupted while waiting for the message
* to be delivered.
*/
public synchronized boolean experimental_blockUntilDelivered(long timeoutMs)
throws InterruptedException, TimeoutException {
return experimental_blockUntilDelivered(timeoutMs, Clock.DEFAULT);
}

/**
* Marks the message as processed. Should only be called by a {@link Sender} and may be called
* multiple times.
Expand All @@ -302,4 +327,24 @@ public synchronized void markAsProcessed(boolean isDelivered) {
isProcessed = true;
notifyAll();
}

@VisibleForTesting()
/* package */ synchronized boolean experimental_blockUntilDelivered(long timeoutMs, Clock clock)
throws InterruptedException, TimeoutException {
Assertions.checkState(isSent);
Assertions.checkState(handler.getLooper().getThread() != Thread.currentThread());

long deadlineMs = clock.elapsedRealtime() + timeoutMs;
long remainingMs = timeoutMs;
while (!isProcessed && remainingMs > 0) {
wait(remainingMs);
remainingMs = deadlineMs - clock.elapsedRealtime();
}

if (!isProcessed) {
throw new TimeoutException("Message delivery timed out.");
}

return isDelivered;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright (C) 2019 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.android.exoplayer2;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;

import android.os.Handler;
import android.os.HandlerThread;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.android.exoplayer2.util.Clock;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;

/** Unit test for {@link PlayerMessage}. */
@RunWith(AndroidJUnit4.class)
public class PlayerMessageTest {

private static final long TIMEOUT_MS = 10;

@Mock Clock clock;
private HandlerThread handlerThread;
private PlayerMessage message;

@Before
public void setUp() {
initMocks(this);
PlayerMessage.Sender sender = (message) -> {};
PlayerMessage.Target target = (messageType, payload) -> {};
handlerThread = new HandlerThread("TestHandler");
handlerThread.start();
Handler handler = new Handler(handlerThread.getLooper());
message =
new PlayerMessage(sender, target, Timeline.EMPTY, /* defaultWindowIndex= */ 0, handler);
}

@After
public void tearDown() {
handlerThread.quit();
}

@Test
public void experimental_blockUntilDelivered_timesOut() throws Exception {
when(clock.elapsedRealtime()).thenReturn(0L).thenReturn(TIMEOUT_MS * 2);

try {
message.send().experimental_blockUntilDelivered(TIMEOUT_MS, clock);
fail();
} catch (TimeoutException expected) {
}

// Ensure experimental_blockUntilDelivered() entered the blocking loop
verify(clock, Mockito.times(2)).elapsedRealtime();
}

@Test
public void experimental_blockUntilDelivered_onAlreadyProcessed_succeeds() throws Exception {
when(clock.elapsedRealtime()).thenReturn(0L);

message.send().markAsProcessed(/* isDelivered= */ true);

assertThat(message.experimental_blockUntilDelivered(TIMEOUT_MS, clock)).isTrue();
}

@Test
public void experimental_blockUntilDelivered_markAsProcessedWhileBlocked_succeeds()
throws Exception {
message.send();

// Use a separate Thread to mark the message as processed.
CountDownLatch prepareLatch = new CountDownLatch(1);
ExecutorService executorService = Executors.newSingleThreadExecutor();
Future<Boolean> future =
executorService.submit(
() -> {
prepareLatch.await();
message.markAsProcessed(true);
return true;
});

when(clock.elapsedRealtime())
.thenReturn(0L)
.then(
(invocation) -> {
// Signal the background thread to call PlayerMessage#markAsProcessed.
prepareLatch.countDown();
return TIMEOUT_MS - 1;
});

try {
assertThat(message.experimental_blockUntilDelivered(TIMEOUT_MS, clock)).isTrue();
// Ensure experimental_blockUntilDelivered() entered the blocking loop.
verify(clock, Mockito.atLeast(2)).elapsedRealtime();
future.get(1, TimeUnit.SECONDS);
} finally {
executorService.shutdown();
}
}
}

0 comments on commit 1154e80

Please sign in to comment.