Merge "InCallTonePlayer concurrency improvements." into tm-dev
diff --git a/src/com/android/server/telecom/InCallTonePlayer.java b/src/com/android/server/telecom/InCallTonePlayer.java
index 5a514ce..7c98d9c 100644
--- a/src/com/android/server/telecom/InCallTonePlayer.java
+++ b/src/com/android/server/telecom/InCallTonePlayer.java
@@ -33,6 +33,7 @@
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Play a call-related tone (ringback, busy signal, etc.) either through ToneGenerator, or using a
@@ -186,7 +187,7 @@
* when we need focus and when it can be release. This should only be manipulated from the main
* thread.
*/
- private static int sTonesPlaying = 0;
+ private static AtomicInteger sTonesPlaying = new AtomicInteger(0);
private final CallAudioManager mCallAudioManager;
private final CallAudioRoutePeripheralAdapter mCallAudioRoutePeripheralAdapter;
@@ -213,6 +214,12 @@
private final AudioManagerAdapter mAudioManagerAdapter;
/**
+ * Latch used for awaiting on playback, which may be interrupted if the tone is stopped from
+ * outside the playback.
+ */
+ private final CountDownLatch mPlaybackLatch = new CountDownLatch(1);
+
+ /**
* Initializes the tone player. Private; use the {@link Factory} to create tone players.
*
* @param toneId ID of the tone to play, see TONE_* constants.
@@ -388,26 +395,21 @@
}
Log.i(this, "playToneGeneratorTone: toneType=%d", toneType);
- // TODO: Certain CDMA tones need to check the ringer-volume state before
- // playing. See CallNotifier.InCallTonePlayer.
- // TODO: Some tones play through the end of a call so we need to inform
- // CallAudioManager that we want focus the same way that Ringer does.
-
- synchronized (this) {
- if (mState != STATE_STOPPED) {
- mState = STATE_ON;
- toneGenerator.startTone(toneType);
- try {
- Log.v(this, "Starting tone %d...waiting for %d ms.", mToneId,
- toneLengthMillis + TIMEOUT_BUFFER_MILLIS);
- wait(toneLengthMillis + TIMEOUT_BUFFER_MILLIS);
- } catch (InterruptedException e) {
- Log.w(this, "wait interrupted", e);
- }
+ mState = STATE_ON;
+ toneGenerator.startTone(toneType);
+ try {
+ Log.v(this, "Starting tone %d...waiting for %d ms.", mToneId,
+ toneLengthMillis + TIMEOUT_BUFFER_MILLIS);
+ if (mPlaybackLatch.await(toneLengthMillis + TIMEOUT_BUFFER_MILLIS,
+ TimeUnit.MILLISECONDS)) {
+ Log.i(this, "playToneGeneratorTone: tone playback stopped.");
}
+ } catch (InterruptedException e) {
+ Log.w(this, "playToneGeneratorTone: wait interrupted", e);
}
- mState = STATE_OFF;
+ // Redundant; don't want anyone re-using at this point.
+ mState = STATE_STOPPED;
} finally {
if (toneGenerator != null) {
toneGenerator.release();
@@ -421,43 +423,40 @@
* @param toneResourceId The resource ID of the tone to play.
*/
private void playMediaTone(int stream, int toneResourceId) {
- synchronized (this) {
- if (mState != STATE_STOPPED) {
- mState = STATE_ON;
+ mState = STATE_ON;
+ Log.i(this, "playMediaTone: toneResourceId=%d", toneResourceId);
+ AudioAttributes attributes = new AudioAttributes.Builder()
+ .setUsage(AudioAttributes.USAGE_VOICE_COMMUNICATION)
+ .setContentType(AudioAttributes.CONTENT_TYPE_SPEECH)
+ .setLegacyStreamType(stream)
+ .build();
+ mToneMediaPlayer = mMediaPlayerFactory.get(toneResourceId, attributes);
+ mToneMediaPlayer.setLooping(false);
+ int durationMillis = mToneMediaPlayer.getDuration();
+ mToneMediaPlayer.setOnCompletionListener(new MediaPlayer.OnCompletionListener() {
+ @Override
+ public void onCompletion(MediaPlayer mp) {
+ Log.i(InCallTonePlayer.this, "playMediaTone: toneResourceId=%d completed.",
+ toneResourceId);
+ mPlaybackLatch.countDown();
}
- Log.i(this, "playMediaTone: toneResourceId=%d", toneResourceId);
- AudioAttributes attributes = new AudioAttributes.Builder()
- .setUsage(AudioAttributes.USAGE_VOICE_COMMUNICATION)
- .setContentType(AudioAttributes.CONTENT_TYPE_SPEECH)
- .setLegacyStreamType(stream)
- .build();
- mToneMediaPlayer = mMediaPlayerFactory.get(toneResourceId, attributes);
- mToneMediaPlayer.setLooping(false);
- int durationMillis = mToneMediaPlayer.getDuration();
- final CountDownLatch toneLatch = new CountDownLatch(1);
- mToneMediaPlayer.setOnCompletionListener(new MediaPlayer.OnCompletionListener() {
- @Override
- public void onCompletion(MediaPlayer mp) {
- Log.i(InCallTonePlayer.this, "playMediaTone: toneResourceId=%d completed.",
- toneResourceId);
- synchronized (InCallTonePlayer.this) {
- mState = STATE_OFF;
- }
- mToneMediaPlayer.release();
- mToneMediaPlayer = null;
- toneLatch.countDown();
- }
- });
- mToneMediaPlayer.start();
- try {
- // Wait for the tone to stop playing; timeout at 2x the length of the file just to
- // be on the safe side.
- toneLatch.await(durationMillis * 2, TimeUnit.MILLISECONDS);
- } catch (InterruptedException ie) {
- Log.e(this, ie, "playMediaTone: tone playback interrupted.");
- }
- }
+ });
+ try {
+ mToneMediaPlayer.start();
+ // Wait for the tone to stop playing; timeout at 2x the length of the file just to
+ // be on the safe side. Playback can also be stopped via stopTone().
+ if (mPlaybackLatch.await(durationMillis * 2, TimeUnit.MILLISECONDS)) {
+ Log.i(this, "playMediaTone: tone playback stopped.");
+ }
+ } catch (InterruptedException ie) {
+ Log.e(this, ie, "playMediaTone: tone playback interrupted.");
+ } finally {
+ // Redundant; don't want anyone re-using at this point.
+ mState = STATE_STOPPED;
+ mToneMediaPlayer.release();
+ mToneMediaPlayer = null;
+ }
}
@VisibleForTesting
@@ -468,8 +467,12 @@
return false;
}
- sTonesPlaying++;
- if (sTonesPlaying == 1) {
+ // Tone already done; don't allow re-used
+ if (mState == STATE_STOPPED) {
+ return false;
+ }
+
+ if (sTonesPlaying.incrementAndGet() == 1) {
mCallAudioManager.setIsTonePlaying(true);
}
@@ -494,18 +497,17 @@
*/
@VisibleForTesting
public void stopTone() {
- synchronized (this) {
- if (mState == STATE_ON) {
- Log.d(this, "Stopping the tone %d.", mToneId);
- notify();
- }
- mState = STATE_STOPPED;
+ if (mState == STATE_ON) {
+ Log.i(this, "stopTone: Stopping the tone %d.", mToneId);
+ // Notify the playback to end early.
+ mPlaybackLatch.countDown();
}
+ mState = STATE_STOPPED;
}
@VisibleForTesting
public void cleanup() {
- sTonesPlaying = 0;
+ sTonesPlaying.set(0);
}
private void cleanUpTonePlayer() {
@@ -514,12 +516,11 @@
mMainThreadHandler.post(new Runnable("ICTP.cUTP", mLock) {
@Override
public void loggedRun() {
- if (sTonesPlaying == 0) {
- Log.wtf(InCallTonePlayer.this,
- "cleanUpTonePlayer(): Over-releasing focus for tone player.");
- } else if (--sTonesPlaying == 0) {
+ int newToneCount = sTonesPlaying.updateAndGet( t -> Math.min(0, t--));
+
+ if (newToneCount == 0) {
Log.i(InCallTonePlayer.this,
- "cleanUpTonePlayer(): tonesPlaying=%d, tone completed", sTonesPlaying);
+ "cleanUpTonePlayer(): tonesPlaying=%d, tone completed", newToneCount);
if (mCallAudioManager != null) {
mCallAudioManager.setIsTonePlaying(false);
} else {
@@ -528,7 +529,7 @@
}
} else {
Log.i(InCallTonePlayer.this,
- "cleanUpTonePlayer(): tonesPlaying=%d; still playing", sTonesPlaying);
+ "cleanUpTonePlayer(): tonesPlaying=%d; still playing", newToneCount);
}
}
}.prepare());
diff --git a/tests/src/com/android/server/telecom/tests/InCallTonePlayerTest.java b/tests/src/com/android/server/telecom/tests/InCallTonePlayerTest.java
index cb0497a..2651e8f 100644
--- a/tests/src/com/android/server/telecom/tests/InCallTonePlayerTest.java
+++ b/tests/src/com/android/server/telecom/tests/InCallTonePlayerTest.java
@@ -87,7 +87,7 @@
@Override
public int getDuration() {
- return 0;
+ return 1000;
}
};
@@ -134,7 +134,41 @@
verify(mMediaPlayerFactory, never()).get(anyInt(), any());
}
- @FlakyTest
+ @SmallTest
+ @Test
+ public void testInterruptMediaTone() {
+ when(mAudioManagerAdapter.isVolumeOverZero()).thenReturn(true);
+ assertTrue(mInCallTonePlayer.startTone());
+ // Verify we did play a tone.
+ verify(mMediaPlayerFactory, timeout(5000)).get(anyInt(), any());
+ verify(mCallAudioManager).setIsTonePlaying(eq(true));
+
+ mInCallTonePlayer.stopTone();
+ // Timeouts due to threads!
+ verify(mCallAudioManager, timeout(5000)).setIsTonePlaying(eq(false));
+
+ // Correctness check: ensure we can't start the tone again.
+ assertFalse(mInCallTonePlayer.startTone());
+ }
+
+ @SmallTest
+ @Test
+ public void testInterruptToneGenerator() {
+ mInCallTonePlayer = mFactory.createPlayer(InCallTonePlayer.TONE_RING_BACK);
+ when(mAudioManagerAdapter.isVolumeOverZero()).thenReturn(true);
+ assertTrue(mInCallTonePlayer.startTone());
+ verify(mToneGenerator, timeout(5000)).startTone(anyInt());
+ verify(mCallAudioManager).setIsTonePlaying(eq(true));
+
+ mInCallTonePlayer.stopTone();
+ // Timeouts due to threads!
+ verify(mCallAudioManager, timeout(5000)).setIsTonePlaying(eq(false));
+ verify(mToneGenerator, timeout(5000)).release();
+
+ // Correctness check: ensure we can't start the tone again.
+ assertFalse(mInCallTonePlayer.startTone());
+ }
+
@SmallTest
@Test
public void testEndCallToneWhenNotSilenced() {
@@ -143,6 +177,6 @@
// Verify we did play a tone.
verify(mMediaPlayerFactory, timeout(5000)).get(anyInt(), any());
- verify(mCallAudioManager).setIsTonePlaying(eq(true));
+ verify(mCallAudioManager, timeout(5000)).setIsTonePlaying(eq(true));
}
}