diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPlayer.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPlayer.kt index 1d98fd38da..545a604af2 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPlayer.kt +++ b/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() } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPresenter.kt index 313ebfb3f6..bb642226a9 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/composer/VoiceMessageComposerPresenter.kt +++ b/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( - mediaPath = recording.file.path, - mimeType = recording.mimeType, - ) + localCoroutineScope.launch { + player.play( + mediaPath = recording.file.path, + mimeType = recording.mimeType, + ) + } else -> Timber.e("Voice message player event received but no file to play") } VoiceMessagePlayerEvent.Pause -> { diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt index 93b336095f..67297fc3b3 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt +++ b/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")) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/composer/VoiceMessageComposerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/composer/VoiceMessageComposerPresenterTest.kt index 87606d0272..598ceaf227 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/composer/VoiceMessageComposerPresenterTest.kt +++ b/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, diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessagePlayerTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessagePlayerTest.kt index eac9a905bd..78c34cd60b 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessagePlayerTest.kt +++ b/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) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt index 257174ec8d..0097245d27 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt +++ b/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) diff --git a/libraries/mediaplayer/api/src/main/kotlin/io/element/android/libraries/mediaplayer/api/MediaPlayer.kt b/libraries/mediaplayer/api/src/main/kotlin/io/element/android/libraries/mediaplayer/api/MediaPlayer.kt index 9c74e27daa..46c5b40ead 100644 --- a/libraries/mediaplayer/api/src/main/kotlin/io/element/android/libraries/mediaplayer/api/MediaPlayer.kt +++ b/libraries/mediaplayer/api/src/main/kotlin/io/element/android/libraries/mediaplayer/api/MediaPlayer.kt @@ -30,13 +30,15 @@ interface MediaPlayer : AutoCloseable { val state: StateFlow /** - * 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?, ) } diff --git a/libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/MediaPlayerImpl.kt b/libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/MediaPlayerImpl.kt index 0e46ad0fee..420b01acc7 100644 --- a/libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/MediaPlayerImpl.kt +++ b/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 = _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 + } } diff --git a/libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/SimplePlayer.kt b/libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/SimplePlayer.kt index 8ae4325e64..1395c69e57 100644 --- a/libraries/mediaplayer/impl/src/main/kotlin/io/element/android/libraries/mediaplayer/impl/SimplePlayer.kt +++ b/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) }) } diff --git a/libraries/mediaplayer/test/src/main/kotlin/io/element/android/libraries/mediaplayer/test/FakeMediaPlayer.kt b/libraries/mediaplayer/test/src/main/kotlin/io/element/android/libraries/mediaplayer/test/FakeMediaPlayer.kt index c94261f27e..4f7d19df47 100644 --- a/libraries/mediaplayer/test/src/main/kotlin/io/element/android/libraries/mediaplayer/test/FakeMediaPlayer.kt +++ b/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 = _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, ) } }