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.
This commit is contained in:
Marco Romano 2023-11-09 15:34:38 +01:00 committed by GitHub
parent b83d8733e2
commit 878417f557
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 110 additions and 28 deletions

View file

@ -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()
}
}

View file

@ -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 -> {

View file

@ -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"))

View file

@ -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,

View file

@ -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)

View file

@ -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)

View file

@ -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?,
)
}

View file

@ -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
}
}

View file

@ -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)
})
}

View file

@ -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,
)
}
}