Merge pull request #125 from spokestack/jz-mediaplayer-churn

fix: tighten task submission in TTS player

Author: space-pope

src/main/java/io/spokestack/spokestack/tts/SpokestackTTSOutput.java

@@ -12,16 +12,12 @@
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.ExoPlaybackException;
 import com.google.android.exoplayer2.ExoPlayer;
-import com.google.android.exoplayer2.PlaybackParameters;
 import com.google.android.exoplayer2.Player;
 import com.google.android.exoplayer2.SimpleExoPlayer;
-import com.google.android.exoplayer2.Timeline;
 import com.google.android.exoplayer2.audio.AudioAttributes;
 import com.google.android.exoplayer2.source.ConcatenatingMediaSource;
 import com.google.android.exoplayer2.source.MediaSource;
 import com.google.android.exoplayer2.source.ProgressiveMediaSource;
-import com.google.android.exoplayer2.source.TrackGroupArray;
-import com.google.android.exoplayer2.trackselection.TrackSelectionArray;
 import com.google.android.exoplayer2.upstream.DataSource;
 import com.google.android.exoplayer2.upstream.DefaultHttpDataSourceFactory;
 import com.google.android.exoplayer2.util.Util;
@@ -135,15 +131,18 @@ PlayerState getPlayerState() {
      * Establish the media player and allocate its internal resources.
      */
     public void prepare() {
-        this.taskHandler.run(() -> {
+        this.taskHandler.run(this::inlinePrepare);
+    }
+
+    void inlinePrepare() {
+        if (this.mediaPlayer == null) {
             ExoPlayer player = this.playerFactory.createPlayer(
                   this.usage,
                   this.contentType,
                   this.appContext);
             player.addListener(this);
             this.mediaPlayer = player;
-            this.mediaPlayer.prepare(mediaSource);
-        });
+        }
     }
 
     @Override
@@ -159,30 +158,24 @@ public void close() {
 
     @Override
     public void audioReceived(AudioResponse response) {
-        if (this.mediaPlayer == null) {
-            prepare();
-        }
-
         this.taskHandler.run(() -> {
+            inlinePrepare();
+
             Uri audioUri = response.getAudioUri();
             MediaSource newTrack = createMediaSource(audioUri);
 
-            if (mediaPlayer.isPlaying()) {
-                mediaSource.addMediaSource(newTrack);
-            } else {
-                mediaSource.clear();
-                mediaSource.addMediaSource(newTrack);
-                mediaPlayer.prepare(mediaSource);
-            }
+            this.mediaSource.addMediaSource(newTrack);
+            this.mediaPlayer.prepare(this.mediaSource);
             this.playerState = new PlayerState(
                   true,
                   this.playerState.shouldPlay,
                   this.playerState.curPosition,
                   this.playerState.window
             );
+
+            inlinePlay();
         });
 
-        playContent();
     }
 
     @Override
@@ -236,7 +229,12 @@ public void onIsPlayingChanged(boolean isPlaying) {
     }
 
     private void resetPlayerState() {
+        this.mediaSource.clear();
         this.playerState = new PlayerState(false, false, 0, 0);
+        if (this.mediaPlayer != null) {
+            this.mediaPlayer.release();
+            this.mediaPlayer = null;
+        }
     }
 
     @Override
@@ -266,20 +264,20 @@ public void onAudioFocusChange(int focusChange) {
      * Start or resume playback of any TTS responses.
      */
     public void playContent() {
-        this.taskHandler.run(() -> {
-            if (!playerState.hasContent) {
-                return;
-            }
+        this.taskHandler.run(this::inlinePlay);
+    }
 
-            if (mediaPlayer == null) {
-                prepare();
-            }
+    void inlinePlay() {
+        if (!playerState.hasContent) {
+            return;
+        }
 
-            // only play if focus is granted
-            if (requestFocus() == AudioManager.AUDIOFOCUS_REQUEST_GRANTED) {
-                mediaPlayer.setPlayWhenReady(true);
-            }
-        });
+        inlinePrepare();
+
+        // only play if focus is granted
+        if (requestFocus() == AudioManager.AUDIOFOCUS_REQUEST_GRANTED) {
+            mediaPlayer.setPlayWhenReady(true);
+        }
     }
 
     int requestFocus() {
@@ -371,46 +369,4 @@ ExoPlayer createPlayer(int usage, int contentType,
             return player;
         }
     }
-
-    // similarly, implementing these listener methods maintains backwards
-    // compatibility for ExoPlayer
-
-    @Override public void onTimelineChanged(@NotNull Timeline timeline,
-                                            int reason) { }
-
-    @Override
-    // it's deprecated, but it's still a default method, so we have to
-    // implement it for older versions of Android
-    @SuppressWarnings("deprecation")
-    public void onTimelineChanged(@NotNull Timeline timeline,
-                                  @Nullable Object manifest, int reason) { }
-
-    @Override
-    public void onTracksChanged(@NotNull TrackGroupArray trackGroups,
-                                @NotNull TrackSelectionArray trackSelections) {
-
-    }
-
-    @Override public void onLoadingChanged(boolean isLoading) { }
-
-    @Override
-    public void onPlaybackSuppressionReasonChanged(
-          int playbackSuppressionReason) { }
-
-    @Override
-    public void onPlayerStateChanged(boolean playWhenReady, int playbackState) {
-    }
-
-    @Override public void onRepeatModeChanged(int repeatMode) { }
-
-    @Override public void onShuffleModeEnabledChanged(
-          boolean shuffleModeEnabled) { }
-
-    @Override public void onPositionDiscontinuity(int reason) { }
-
-    @Override
-    public void onPlaybackParametersChanged(
-          @NotNull PlaybackParameters playbackParameters) { }
-
-    @Override public void onSeekProcessed() { }
 }

src/main/java/io/spokestack/spokestack/util/TaskHandler.java

@@ -14,7 +14,7 @@ public final class TaskHandler {
     /**
      * Initialize a new task handler.
      *
-     * @param runOnMainThread {@code true} if tasks submitted to this hanlder
+     * @param runOnMainThread {@code true} if tasks submitted to this handler
      *                        should run on the application's main {@code
      *                        Looper}.
      */

src/test/java/io/spokestack/spokestack/tts/SpokestackTTSOutputTest.java

@@ -102,10 +102,10 @@ public void testCallbacks() {
         // audioReceived ensures that the player gets set up
         mediaPlayer = ttsOutput.getMediaPlayer();
         assertNotNull(mediaPlayer);
-        verify(ttsOutput, times(1)).playContent();
+        verify(ttsOutput, times(1)).inlinePlay();
         verify(ttsOutput, times(1)).createMediaSource(Uri.EMPTY);
         verify(ttsOutput, times(1)).requestFocus();
-        verify(mediaPlayer, times(2)).prepare(any());
+        verify(mediaPlayer, times(1)).prepare(any());
         verify(mediaPlayer, times(1)).setPlayWhenReady(true);
     }
 
@@ -119,22 +119,26 @@ public void testPlayerStateChange() {
 
         TestExoPlayer mediaPlayer = (TestExoPlayer) ttsOutput.getMediaPlayer();
         assertNotNull(mediaPlayer);
-        // this state change should do nothing
-        ttsOutput.onPlayerStateChanged(false, Player.STATE_BUFFERING);
+        // buffering doesn't send PLAYBACK_STOPPED events
+        mediaPlayer.setPlaybackState(Player.STATE_BUFFERING);
+        ttsOutput.onIsPlayingChanged(false);
         assertTrue(ttsOutput.getPlayerState().hasContent);
+        assertTrue(listener.events.isEmpty());
 
+        // simulate playing audio from a non-Spokestack source
         ttsOutput.stopPlayback();
+        ttsOutput.onIsPlayingChanged(true);
+        ttsOutput.onIsPlayingChanged(false);
         assertFalse(ttsOutput.getPlayerState().shouldPlay);
         assertFalse(ttsOutput.getPlayerState().hasContent);
-
-        ttsOutput.onPlayerStateChanged(false, Player.STATE_ENDED);
-        assertFalse(ttsOutput.getPlayerState().hasContent);
-        // no playback_complete event if the player didn't have content from
-        // the TTS system
+        // no event for non-Spokestack audio
         assertTrue(listener.events.isEmpty());
 
-        // now give it audio to play and check again
+        // give it Spokestack audio to play
         ttsOutput.audioReceived(new AudioResponse(Uri.EMPTY));
+        // the player has to be recreated after `stopPlayback()` and receiving
+        // new audio
+        mediaPlayer = (TestExoPlayer) ttsOutput.getMediaPlayer();
         ttsOutput.onIsPlayingChanged(true);
         ttsOutput.onIsPlayingChanged(false);
         assertEquals(2, listener.events.size());