Skip to content

Commit

Permalink
Allow to seek a voice message before playing it
Browse files Browse the repository at this point in the history
Previously the only way to start interacting with a voice message was to play it.
This change makes it possible to start interacting also with a seek.
  • Loading branch information
Marco Romano committed Nov 14, 2023
1 parent 2c25e69 commit d97bc3c
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
package io.element.android.features.messages.impl.voicemessages.timeline

import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.libraries.mediaplayer.api.MediaPlayer
import io.element.android.libraries.di.RoomScope
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.media.MediaSource
import io.element.android.libraries.mediaplayer.api.MediaPlayer
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.map
Expand Down Expand Up @@ -75,11 +75,14 @@ interface VoiceMessagePlayer {
fun pause()

/**
* Seek to a specific position.
* Seek to a specific position acquiring control of the
* underlying [MediaPlayer] if needed.
*
* Will suspend whilst the media file is being downloaded.
*
* @param positionMs The position in milliseconds.
*/
fun seekTo(positionMs: Long)
suspend fun seekTo(positionMs: Long): Result<Unit>

data class State(
/**
Expand Down Expand Up @@ -145,22 +148,8 @@ class DefaultVoiceMessagePlayer(
)
}.distinctUntilChanged()

override suspend fun play(): Result<Unit> = if (inControl()) {
override suspend fun play(): Result<Unit> = acquireControl {
mediaPlayer.play()
Result.success(Unit)
} else {
if (eventId != null) {
repo.getMediaFile().mapCatching { mediaFile ->
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"))
}
}

override fun pause() {
Expand All @@ -169,10 +158,8 @@ class DefaultVoiceMessagePlayer(
}
}

override fun seekTo(positionMs: Long) {
ifInControl {
mediaPlayer.seekTo(positionMs)
}
override suspend fun seekTo(positionMs: Long): Result<Unit> = acquireControl {
mediaPlayer.seekTo(positionMs)
}

private fun String?.isMyTrack(): Boolean = if (eventId == null) false else this == eventId.value
Expand All @@ -182,4 +169,21 @@ class DefaultVoiceMessagePlayer(
}

private fun inControl(): Boolean = mediaPlayer.state.value.mediaId.isMyTrack()

private suspend inline fun acquireControl(onReady: (state: MediaPlayer.State) -> Unit): Result<Unit> = if (inControl()) {
onReady(mediaPlayer.state.value)
Result.success(Unit)
} else {
if (eventId != null) {
repo.getMediaFile().mapCatching { mediaFile ->
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.
).let(onReady)
}
} else {
Result.failure(IllegalStateException("Cannot acquireControl on a voice message with no eventId"))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,18 @@ class VoiceMessagePresenter @AssistedInject constructor(
}
}
is VoiceMessageEvents.Seek -> {
player.seekTo((event.percentage * content.duration.toMillis()).toLong())
scope.launch {
play.runUpdatingState(
errorTransform = {
analyticsService.trackError(
VoiceMessageException.PlayMessageError("Error while trying to seek voice message", it)
)
it
},
) {
player.seekTo((event.percentage * content.duration.toMillis()).toLong())
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,39 @@ class DefaultVoiceMessagePlayerTest {
}
}

@Test
fun `download and seek to works`() = runTest {
val player = createDefaultVoiceMessagePlayer()
player.state.test {
skipItems(1) // skip initial state.
Truth.assertThat(player.seekTo(2000).isSuccess).isTrue()
player.seekTo(2000)
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(false)
Truth.assertThat(it.isMyMedia).isEqualTo(true)
Truth.assertThat(it.currentPosition).isEqualTo(2000)
}
}
}

@Test
fun `download and seek to fails`() = runTest {
val player = createDefaultVoiceMessagePlayer(
voiceMessageMediaRepo = FakeVoiceMessageMediaRepo().apply {
shouldFail = true
},
)
player.state.test {
skipItems(1) // skip initial state.
Truth.assertThat(player.seekTo(2000).isFailure).isTrue()
}
}

@Test
fun `seek to works`() = runTest {
val player = createDefaultVoiceMessagePlayer()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ class VoiceMessagePresenterTest {
Truth.assertThat(it.time).isEqualTo("0:02")
}
analyticsService.trackedErrors.first().also {
Truth.assertThat(it).isInstanceOf(VoiceMessageException.PlayMessageError::class.java)
Truth.assertThat(it).apply {
isInstanceOf(VoiceMessageException.PlayMessageError::class.java)
hasMessageThat().isEqualTo("Error while trying to play voice message")
}
}
}
}
Expand Down Expand Up @@ -167,6 +170,83 @@ class VoiceMessagePresenterTest {
}
}

@Test
fun `seeking downloads and seeks`() = runTest {
val presenter = createVoiceMessagePresenter(
content = aTimelineItemVoiceContent(durationMs = 10_000),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem().also {
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play)
Truth.assertThat(it.progress).isEqualTo(0f)
Truth.assertThat(it.time).isEqualTo("0:10")
}

initialState.eventSink(VoiceMessageEvents.Seek(0.5f))

awaitItem().also {
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading)
Truth.assertThat(it.progress).isEqualTo(0f)
Truth.assertThat(it.time).isEqualTo("0:10")
}
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.Downloading)
Truth.assertThat(it.progress).isEqualTo(0.5f)
Truth.assertThat(it.time).isEqualTo("0:05")
}
awaitItem().also {
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play)
Truth.assertThat(it.progress).isEqualTo(0.5f)
Truth.assertThat(it.time).isEqualTo("0:05")
}
}
}

@Test
fun `seeking downloads and fails`() = runTest {
val analyticsService = FakeAnalyticsService()
val presenter = createVoiceMessagePresenter(
voiceMessageMediaRepo = FakeVoiceMessageMediaRepo().apply { shouldFail = true },
analyticsService = analyticsService,
content = aTimelineItemVoiceContent(durationMs = 10_000),
)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem().also {
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play)
Truth.assertThat(it.progress).isEqualTo(0f)
Truth.assertThat(it.time).isEqualTo("0:10")
}

initialState.eventSink(VoiceMessageEvents.Seek(0.5f))

awaitItem().also {
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading)
Truth.assertThat(it.progress).isEqualTo(0f)
Truth.assertThat(it.time).isEqualTo("0:10")
}
awaitItem().also {
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Retry)
Truth.assertThat(it.progress).isEqualTo(0f)
Truth.assertThat(it.time).isEqualTo("0:10")
}
analyticsService.trackedErrors.first().also {
Truth.assertThat(it).apply {
isInstanceOf(VoiceMessageException.PlayMessageError::class.java)
hasMessageThat().isEqualTo("Error while trying to seek voice message")
}
}
}
}

@Test
fun `seeking seeks`() = runTest {
val presenter = createVoiceMessagePresenter(
Expand Down

0 comments on commit d97bc3c

Please sign in to comment.