Fix a rare crash.

- Crash could be reproduced by seeking to the very end of a voicemail,
  playing the voicemail to see the ui reset to beginning, then
  increasing rate.
- Reason was that during playback, insufficient data has been read to
  read the sample rate and channels, and in that situation the increase
  rate requires constructing the time scaler, which in turn requires
  a valid sample rate and channels.
- Added a test to prove that the bug exists, and to prove the fix which
  is to ignore the rate change call when sample rate and channels not
  known.
- Also added another unit test in the process to verify that it's safe
  to call set rate at any point during the life cycle of the player.

Bug: 5140693
Change-Id: I474e5769f2b72762348534e4d06104af247e8726

2 files changed, 55 insertions, 2 deletions

diff --git a/variablespeed/jni/variablespeed.cc b/variablespeed/jni/variablespeed.cc
index 12684f1..344eca5 100644
--- a/variablespeed/jni/variablespeed.cc
+++ b/variablespeed/jni/variablespeed.cc
@@ -296,9 +296,15 @@ AudioEngine::~AudioEngine() {
 // Regular AudioEngine class methods.
 
 void AudioEngine::SetVariableSpeed(float speed) {
-  // TODO: Add test, prove that this doesn't crash if called before playing.
   // TODO: Mutex for shared time scaler accesses.
-  GetTimeScaler()->set_speed(speed);
+  if (HasSampleRateAndChannels()) {
+    GetTimeScaler()->set_speed(speed);
+  } else {
+    // This is being called at a point where we have not yet processed enough
+    // data to determine the sample rate and number of channels.
+    // Ignore the call.  See http://b/5140693.
+    LOGD("set varaible speed called, sample rate and channels not ready yet");
+  }
 }
 
 void AudioEngine::RequestStart() {
diff --git a/variablespeed/tests/src/com/android/ex/variablespeed/MediaPlayerProxyTestCase.java b/variablespeed/tests/src/com/android/ex/variablespeed/MediaPlayerProxyTestCase.java
index a7c1127..37a0cce 100644
--- a/variablespeed/tests/src/com/android/ex/variablespeed/MediaPlayerProxyTestCase.java
+++ b/variablespeed/tests/src/com/android/ex/variablespeed/MediaPlayerProxyTestCase.java
@@ -463,6 +463,53 @@ public abstract class MediaPlayerProxyTestCase extends InstrumentationTestCase {
         mCompletionListener.awaitOneCallback(10, TimeUnit.SECONDS);
     }
 
+    public void testSeekToEndThenPlayThenRateChangeCrash() throws Exception {
+        // Unit test for this bug: http://b/5140693
+        // This test proves that the bug is fixed.
+        setDataSourceFromContentProvider(mPlayer, "fake_voicemail.mp3", "audio/mp3");
+        mPlayer.prepare();
+        mPlayer.seekTo(mPlayer.getDuration() - 1);
+        mPlayer.setOnCompletionListener(mCompletionListener);
+        mPlayer.start();
+        mCompletionListener.awaitOneCallback(10, TimeUnit.SECONDS);
+        // Prior to the fix, this next line was causing a crash.
+        // The reason behind this was due to our having seeked so close to the end of the file
+        // that insufficient data was being read, and thus we weren't able to yet determine the
+        // sample rate and number of channels, which was causing an assertion failure when trying
+        // to create the time scaler.
+        setVariableSpeedRateIfSupported(1.0f);
+    }
+
+    public void testVariableSpeedRateChangeAtDifferentTimes() throws Exception {
+        // Just check that we can set the rate at any point during playback.
+        setVariableSpeedRateIfSupported(1.05f);
+        setDataSourceFromContentProvider(mPlayer, "fake_voicemail.mp3", "audio/mp3");
+        setVariableSpeedRateIfSupported(1.10f);
+        mPlayer.prepare();
+        setVariableSpeedRateIfSupported(1.15f);
+        mPlayer.seekTo(mPlayer.getDuration() / 2);
+        setVariableSpeedRateIfSupported(1.20f);
+        mPlayer.setOnCompletionListener(mCompletionListener);
+        setVariableSpeedRateIfSupported(1.25f);
+        mPlayer.start();
+        setVariableSpeedRateIfSupported(1.30f);
+        mCompletionListener.awaitOneCallback(10, TimeUnit.SECONDS);
+        setVariableSpeedRateIfSupported(1.35f);
+    }
+
+    /**
+     * If we have a variable speed media player proxy, set the variable speed rate.
+     * <p>
+     * If we don't have a variable speed media player proxy, this method will be a no-op.
+     */
+    private void setVariableSpeedRateIfSupported(float rate) {
+        if (mPlayer instanceof SingleThreadedMediaPlayerProxy) {
+            ((SingleThreadedMediaPlayerProxy) mPlayer).setVariableSpeed(rate);
+        } else if (mPlayer instanceof VariableSpeed) {
+            ((VariableSpeed) mPlayer).setVariableSpeed(rate);
+        }
+    }
+
     /**
      * Gets the {@link Uri} for the test audio content we should play.
      * <p>