Voice message MediaPlayer: wait until player is ready (#1772)

Change to `MediaPlayer` API to allow waiting for the player to be in a ready state.
This is needed in order to perform some tasks (e.g. read the media duration, seek) after changing the media file.

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPlayer.kt

@@ -42,7 +42,7 @@ class VoiceMessageComposerPlayer @Inject constructor(
         State(
             isPlaying = state.isPlaying,
             currentPosition = state.currentPosition,
-            duration = state.duration,
+            duration = state.duration ?: 0L,
         )
     }.distinctUntilChanged()
 
@@ -52,16 +52,17 @@ class VoiceMessageComposerPlayer @Inject constructor(
      * @param mediaPath The path to the media to be played.
      * @param mimeType The mime type of the media file.
      */
-    fun play(mediaPath: String, mimeType: String) {
+    suspend fun play(mediaPath: String, mimeType: String) {
         if (mediaPath == curPlayingMediaId) {
             mediaPlayer.play()
         } else {
             lastPlayedMediaPath = mediaPath
-            mediaPlayer.acquireControlAndPlay(
+            mediaPlayer.setMedia(
                 uri = mediaPath,
                 mediaId = mediaPath,
                 mimeType = mimeType,
             )
+            mediaPlayer.play()
         }
     }
 

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPresenter.kt

@@ -120,10 +120,12 @@ class VoiceMessageComposerPresenter @Inject constructor(
                 VoiceMessagePlayerEvent.Play ->
                     when (val recording = recorderState) {
                         is VoiceRecorderState.Finished ->
-                            player.play(
+                            localCoroutineScope.launch {
-                                mediaPath = recording.file.path,
+                                player.play(
-                                mimeType = recording.mimeType,
+                                    mediaPath = recording.file.path,
-                            )
+                                    mimeType = recording.mimeType,
+                                )
+                            }
                         else -> Timber.e("Voice message player event received but no file to play")
                     }
                 VoiceMessagePlayerEvent.Pause -> {

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt

@@ -151,11 +151,12 @@ class DefaultVoiceMessagePlayer(
     } else {
         if (eventId != null) {
             repo.getMediaFile().mapCatching { mediaFile ->
-                mediaPlayer.acquireControlAndPlay(
+                mediaPlayer.setMedia(
                     uri = mediaFile.path,
                     mediaId = eventId.value,
                     mimeType = "audio/ogg" // Files in the voice cache have no extension so we need to set the mime type manually.
                 )
+                mediaPlayer.play()
             }
         } else {
             Result.failure(IllegalStateException("Cannot play a voice message with no eventId"))

features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/composer/VoiceMessageComposerPresenterTest.kt

@@ -195,6 +195,7 @@ class VoiceMessageComposerPresenterTest {
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.PressStart))
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.LongPressEnd))
             awaitItem().eventSink(VoiceMessageComposerEvents.PlayerEvent(VoiceMessagePlayerEvent.Play))
+            awaitItem().apply { assertThat(voiceMessageState).isEqualTo(aLoadedState()) }
             val finalState = awaitItem().also {
                 assertThat(it.voiceMessageState).isEqualTo(aPlayingState())
             }
@@ -213,6 +214,7 @@ class VoiceMessageComposerPresenterTest {
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.PressStart))
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.LongPressEnd))
             awaitItem().eventSink(VoiceMessageComposerEvents.PlayerEvent(VoiceMessagePlayerEvent.Play))
+            skipItems(1) // Loaded state
             awaitItem().eventSink(VoiceMessageComposerEvents.PlayerEvent(VoiceMessagePlayerEvent.Pause))
             val finalState = awaitItem().also {
                 assertThat(it.voiceMessageState).isEqualTo(aPausedState())
@@ -250,6 +252,7 @@ class VoiceMessageComposerPresenterTest {
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.PressStart))
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.LongPressEnd))
             awaitItem().eventSink(VoiceMessageComposerEvents.PlayerEvent(VoiceMessagePlayerEvent.Play))
+            skipItems(1) // Loaded state
             awaitItem().eventSink(VoiceMessageComposerEvents.DeleteVoiceMessage)
             awaitItem().apply {
                 assertThat(voiceMessageState).isEqualTo(aPausedState())
@@ -321,6 +324,7 @@ class VoiceMessageComposerPresenterTest {
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.PressStart))
             awaitItem().eventSink(VoiceMessageComposerEvents.RecordButtonEvent(PressEvent.LongPressEnd))
             awaitItem().eventSink(VoiceMessageComposerEvents.PlayerEvent(VoiceMessagePlayerEvent.Play))
+            skipItems(1) // Loaded state
             awaitItem().eventSink(VoiceMessageComposerEvents.SendVoiceMessage)
             assertThat(awaitItem().voiceMessageState).isEqualTo(aPlayingState().toSendingState())
 
@@ -635,6 +639,14 @@ class VoiceMessageComposerPresenterTest {
         waveform = waveform.toImmutableList(),
     )
 
+    private fun aLoadedState() =
+        aPreviewState(
+            isPlaying = false,
+            playbackProgress = 0.0f,
+            showCursor = true,
+            time = 0.seconds,
+        )
+
     private fun aPlayingState() =
         aPreviewState(
             isPlaying = true,

features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessagePlayerTest.kt

@@ -47,6 +47,11 @@ class DefaultVoiceMessagePlayerTest {
         player.state.test {
             skipItems(1) // skip initial state.
             Truth.assertThat(player.play().isSuccess).isTrue()
+            awaitItem().let {
+                Truth.assertThat(it.isPlaying).isEqualTo(false)
+                Truth.assertThat(it.isMyMedia).isEqualTo(true)
+                Truth.assertThat(it.currentPosition).isEqualTo(0)
+            }
             awaitItem().let {
                 Truth.assertThat(it.isPlaying).isEqualTo(true)
                 Truth.assertThat(it.isMyMedia).isEqualTo(true)
@@ -85,7 +90,7 @@ class DefaultVoiceMessagePlayerTest {
         player.state.test {
             skipItems(1) // skip initial state.
             Truth.assertThat(player.play().isSuccess).isTrue()
-            skipItems(1) // skip play state
+            skipItems(2) // skip play states
             player.pause()
             awaitItem().let {
                 Truth.assertThat(it.isPlaying).isEqualTo(false)
@@ -101,7 +106,7 @@ class DefaultVoiceMessagePlayerTest {
         player.state.test {
             skipItems(1) // skip initial state.
             Truth.assertThat(player.play().isSuccess).isTrue()
-            skipItems(1) // skip play state
+            skipItems(2) // skip play states
             player.pause()
             skipItems(1)
             player.play()
@@ -119,7 +124,7 @@ class DefaultVoiceMessagePlayerTest {
         player.state.test {
             skipItems(1) // skip initial state.
             Truth.assertThat(player.play().isSuccess).isTrue()
-            skipItems(1) // skip play state
+            skipItems(2) // skip play states
             player.seekTo(2000)
             awaitItem().let {
                 Truth.assertThat(it.isPlaying).isEqualTo(true)

features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt

@@ -70,6 +70,11 @@ class VoiceMessagePresenterTest {
                 Truth.assertThat(it.progress).isEqualTo(0f)
                 Truth.assertThat(it.time).isEqualTo("0:02")
             }
+            awaitItem().also {
+                Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading)
+                Truth.assertThat(it.progress).isEqualTo(0f)
+                Truth.assertThat(it.time).isEqualTo("0:00")
+            }
             awaitItem().also {
                 Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause)
                 Truth.assertThat(it.progress).isEqualTo(0.5f)
@@ -128,7 +133,7 @@ class VoiceMessagePresenterTest {
             }
 
             initialState.eventSink(VoiceMessageEvents.PlayPause)
-            skipItems(1) // skip downloading state
+            skipItems(2) // skip downloading states
 
             val playingState = awaitItem().also {
                 Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause)
@@ -177,7 +182,7 @@ class VoiceMessagePresenterTest {
 
             initialState.eventSink(VoiceMessageEvents.PlayPause)
 
-            skipItems(1) // skip downloading state
+            skipItems(2) // skip downloading states
 
             awaitItem().also {
                 Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause)

libraries/mediaplayer/api/src/main/kotlin/io/element/android/libraries/mediaplayer/api/MediaPlayer.kt

@@ -30,13 +30,15 @@ interface MediaPlayer : AutoCloseable {
     val state: StateFlow<State>
 
     /**
-     * Acquires control of the player and starts playing the given media.
+     * Initialises the player with a new media item, will suspend until the player is ready.
+     *
+     * @return the ready state of the player.
      */
-    fun acquireControlAndPlay(
+    suspend fun setMedia(
         uri: String,
         mediaId: String,
         mimeType: String,
-    )
+    ): State
 
     /**
      * Plays the current media.
@@ -59,6 +61,10 @@ interface MediaPlayer : AutoCloseable {
     override fun close()
 
     data class State(
+        /**
+         * Whether the player is ready to play.
+         */
+        val isReady: Boolean,
         /**
          * Whether the player is currently playing.
          */
@@ -75,8 +81,8 @@ interface MediaPlayer : AutoCloseable {
          */
         val currentPosition: Long,
         /**
-         * The duration of the current content.
+         * The duration of the current content, if available.
          */
-        val duration: Long,
+        val duration: Long?,
     )
 }

libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/MediaPlayerImpl.kt

@@ -16,6 +16,7 @@
 
 package io.element.android.libraries.mediaplayer.impl
 
+import androidx.media3.common.C
 import androidx.media3.common.MediaItem
 import androidx.media3.common.Player
 import com.squareup.anvil.annotations.ContributesBinding
@@ -24,14 +25,18 @@ import io.element.android.libraries.di.SingleIn
 import io.element.android.libraries.mediaplayer.api.MediaPlayer
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.timeout
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import javax.inject.Inject
+import kotlin.time.Duration.Companion.seconds
 
 /**
  * Default implementation of [MediaPlayer] backed by a [SimplePlayer].
@@ -47,7 +52,7 @@ class MediaPlayerImpl @Inject constructor(
             _state.update {
                 it.copy(
                     currentPosition = player.currentPosition,
-                    duration = player.duration.coerceAtLeast(0),
+                    duration = duration,
                     isPlaying = isPlaying,
                 )
             }
@@ -62,11 +67,21 @@ class MediaPlayerImpl @Inject constructor(
             _state.update {
                 it.copy(
                     currentPosition = player.currentPosition,
-                    duration = player.duration.coerceAtLeast(0),
+                    duration = duration,
                     mediaId = mediaItem?.mediaId,
                 )
             }
         }
+
+        override fun onPlaybackStateChanged(playbackState: Int) {
+            _state.update {
+                it.copy(
+                    isReady = playbackState == Player.STATE_READY,
+                    currentPosition = player.currentPosition,
+                    duration = duration,
+                )
+            }
+        }
     }
 
     init {
@@ -76,11 +91,21 @@ class MediaPlayerImpl @Inject constructor(
     private val scope = CoroutineScope(Job() + Dispatchers.Main)
     private var job: Job? = null
 
-    private val _state = MutableStateFlow(MediaPlayer.State(false, null, 0L, 0L))
+    private val _state = MutableStateFlow(
+        MediaPlayer.State(
+            isReady = false,
+            isPlaying = false,
+            mediaId = null,
+            currentPosition = 0L,
+            duration = 0L
+        )
+    )
 
     override val state: StateFlow<MediaPlayer.State> = _state.asStateFlow()
 
-    override fun acquireControlAndPlay(uri: String, mediaId: String, mimeType: String) {
+    @OptIn(FlowPreview::class)
+    override suspend fun setMedia(uri: String, mediaId: String, mimeType: String): MediaPlayer.State {
+        player.pause() // Must pause here otherwise if the player was playing it would keep on playing the new media item.
         player.clearMediaItems()
         player.setMediaItem(
             MediaItem.Builder()
@@ -90,7 +115,8 @@ class MediaPlayerImpl @Inject constructor(
                 .build()
         )
         player.prepare()
-        player.play()
+        // Will throw TimeoutCancellationException if the player is not ready after 1 second.
+        return state.timeout(1.seconds).first { it.isReady }
     }
 
     override fun play() {
@@ -136,4 +162,9 @@ class MediaPlayerImpl @Inject constructor(
             }
         }
     }
+
+    private val duration: Long?
+        get() = player.duration.let {
+            if (it == C.TIME_UNSET) null else it
+        }
 }

libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/SimplePlayer.kt

@@ -45,6 +45,7 @@ interface SimplePlayer {
     interface Listener {
         fun onIsPlayingChanged(isPlaying: Boolean)
         fun onMediaItemTransition(mediaItem: MediaItem?)
+        fun onPlaybackStateChanged(playbackState: Int)
     }
 }
 
@@ -67,6 +68,7 @@ class SimplePlayerImpl(
         p.addListener(object : Player.Listener {
             override fun onIsPlayingChanged(isPlaying: Boolean) = listener.onIsPlayingChanged(isPlaying)
             override fun onMediaItemTransition(mediaItem: MediaItem?, reason: Int) = listener.onMediaItemTransition(mediaItem)
+            override fun onPlaybackStateChanged(playbackState: Int) = listener.onPlaybackStateChanged(playbackState)
         })
     }
 

libraries/mediaplayer/test/src/main/kotlin/io/element/android/libraries/mediaplayer/test/FakeMediaPlayer.kt

@@ -17,6 +17,7 @@
 package io.element.android.libraries.mediaplayer.test
 
 import io.element.android.libraries.mediaplayer.api.MediaPlayer
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
@@ -31,19 +32,36 @@ class FakeMediaPlayer : MediaPlayer {
         private const val FAKE_PLAYED_DURATION_MS = 1000L
     }
 
-    private val _state = MutableStateFlow(MediaPlayer.State(false, null, 0L, 0L))
+    private val _state = MutableStateFlow(
+        MediaPlayer.State(
+            isReady = false,
+            isPlaying = false,
+            mediaId = null,
+            currentPosition = 0L,
+            duration = 0L
+        )
+    )
 
     override val state: StateFlow<MediaPlayer.State> = _state.asStateFlow()
 
-    override fun acquireControlAndPlay(uri: String, mediaId: String, mimeType: String) {
+    override suspend fun setMedia(uri: String, mediaId: String, mimeType: String): MediaPlayer.State {
         _state.update {
             it.copy(
-                isPlaying = true,
+                isReady = false,
+                isPlaying = false,
                 mediaId = mediaId,
-                currentPosition = it.currentPosition + FAKE_PLAYED_DURATION_MS,
+                currentPosition = 0,
+                duration = null,
+            )
+        }
+        delay(1) // fake delay to simulate prepare() call.
+        _state.update {
+            it.copy(
+                isReady = true,
                 duration = FAKE_TOTAL_DURATION_MS,
             )
         }
+        return _state.value
     }
 
     override fun play() {
@@ -51,7 +69,6 @@ class FakeMediaPlayer : MediaPlayer {
             it.copy(
                 isPlaying = true,
                 currentPosition = it.currentPosition + FAKE_PLAYED_DURATION_MS,
-                duration = FAKE_TOTAL_DURATION_MS,
             )
         }
     }