Skip to content

Commit 96e4106

Browse files
author
Marco Romano
authored
Allow to seek a voice message before playing it (#1780)
1 parent c3471a1 commit 96e4106

File tree

4 files changed

+152
-24
lines changed

4 files changed

+152
-24
lines changed

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

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
package io.element.android.features.messages.impl.voicemessages.timeline
1818

1919
import com.squareup.anvil.annotations.ContributesBinding
20-
import io.element.android.libraries.mediaplayer.api.MediaPlayer
2120
import io.element.android.libraries.di.RoomScope
2221
import io.element.android.libraries.matrix.api.core.EventId
2322
import io.element.android.libraries.matrix.api.media.MediaSource
23+
import io.element.android.libraries.mediaplayer.api.MediaPlayer
2424
import kotlinx.coroutines.flow.Flow
2525
import kotlinx.coroutines.flow.distinctUntilChanged
2626
import kotlinx.coroutines.flow.map
@@ -75,11 +75,14 @@ interface VoiceMessagePlayer {
7575
fun pause()
7676

7777
/**
78-
* Seek to a specific position.
78+
* Seek to a specific position acquiring control of the
79+
* underlying [MediaPlayer] if needed.
80+
*
81+
* Will suspend whilst the media file is being downloaded.
7982
*
8083
* @param positionMs The position in milliseconds.
8184
*/
82-
fun seekTo(positionMs: Long)
85+
suspend fun seekTo(positionMs: Long): Result<Unit>
8386

8487
data class State(
8588
/**
@@ -145,22 +148,8 @@ class DefaultVoiceMessagePlayer(
145148
)
146149
}.distinctUntilChanged()
147150

148-
override suspend fun play(): Result<Unit> = if (inControl()) {
151+
override suspend fun play(): Result<Unit> = acquireControl {
149152
mediaPlayer.play()
150-
Result.success(Unit)
151-
} else {
152-
if (eventId != null) {
153-
repo.getMediaFile().mapCatching { mediaFile ->
154-
mediaPlayer.setMedia(
155-
uri = mediaFile.path,
156-
mediaId = eventId.value,
157-
mimeType = "audio/ogg", // Files in the voice cache have no extension so we need to set the mime type manually.
158-
)
159-
mediaPlayer.play()
160-
}
161-
} else {
162-
Result.failure(IllegalStateException("Cannot play a voice message with no eventId"))
163-
}
164153
}
165154

166155
override fun pause() {
@@ -169,10 +158,8 @@ class DefaultVoiceMessagePlayer(
169158
}
170159
}
171160

172-
override fun seekTo(positionMs: Long) {
173-
ifInControl {
174-
mediaPlayer.seekTo(positionMs)
175-
}
161+
override suspend fun seekTo(positionMs: Long): Result<Unit> = acquireControl {
162+
mediaPlayer.seekTo(positionMs)
176163
}
177164

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

184171
private fun inControl(): Boolean = mediaPlayer.state.value.mediaId.isMyTrack()
172+
173+
private suspend inline fun acquireControl(onReady: (state: MediaPlayer.State) -> Unit): Result<Unit> = if (inControl()) {
174+
onReady(mediaPlayer.state.value)
175+
Result.success(Unit)
176+
} else {
177+
if (eventId != null) {
178+
repo.getMediaFile().mapCatching { mediaFile ->
179+
mediaPlayer.setMedia(
180+
uri = mediaFile.path,
181+
mediaId = eventId.value,
182+
mimeType = "audio/ogg" // Files in the voice cache have no extension so we need to set the mime type manually.
183+
).let(onReady)
184+
}
185+
} else {
186+
Result.failure(IllegalStateException("Cannot acquireControl on a voice message with no eventId"))
187+
}
188+
}
185189
}

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,18 @@ class VoiceMessagePresenter @AssistedInject constructor(
126126
}
127127
}
128128
is VoiceMessageEvents.Seek -> {
129-
player.seekTo((event.percentage * content.duration.toMillis()).toLong())
129+
scope.launch {
130+
play.runUpdatingState(
131+
errorTransform = {
132+
analyticsService.trackError(
133+
VoiceMessageException.PlayMessageError("Error while trying to seek voice message", it)
134+
)
135+
it
136+
},
137+
) {
138+
player.seekTo((event.percentage * content.duration.toMillis()).toLong())
139+
}
140+
}
130141
}
131142
}
132143
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,39 @@ class DefaultVoiceMessagePlayerTest {
118118
}
119119
}
120120

121+
@Test
122+
fun `download and seek to works`() = runTest {
123+
val player = createDefaultVoiceMessagePlayer()
124+
player.state.test {
125+
skipItems(1) // skip initial state.
126+
Truth.assertThat(player.seekTo(2000).isSuccess).isTrue()
127+
player.seekTo(2000)
128+
awaitItem().let {
129+
Truth.assertThat(it.isPlaying).isEqualTo(false)
130+
Truth.assertThat(it.isMyMedia).isEqualTo(true)
131+
Truth.assertThat(it.currentPosition).isEqualTo(0)
132+
}
133+
awaitItem().let {
134+
Truth.assertThat(it.isPlaying).isEqualTo(false)
135+
Truth.assertThat(it.isMyMedia).isEqualTo(true)
136+
Truth.assertThat(it.currentPosition).isEqualTo(2000)
137+
}
138+
}
139+
}
140+
141+
@Test
142+
fun `download and seek to fails`() = runTest {
143+
val player = createDefaultVoiceMessagePlayer(
144+
voiceMessageMediaRepo = FakeVoiceMessageMediaRepo().apply {
145+
shouldFail = true
146+
},
147+
)
148+
player.state.test {
149+
skipItems(1) // skip initial state.
150+
Truth.assertThat(player.seekTo(2000).isFailure).isTrue()
151+
}
152+
}
153+
121154
@Test
122155
fun `seek to works`() = runTest {
123156
val player = createDefaultVoiceMessagePlayer()

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

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,10 @@ class VoiceMessagePresenterTest {
114114
Truth.assertThat(it.time).isEqualTo("0:02")
115115
}
116116
analyticsService.trackedErrors.first().also {
117-
Truth.assertThat(it).isInstanceOf(VoiceMessageException.PlayMessageError::class.java)
117+
Truth.assertThat(it).apply {
118+
isInstanceOf(VoiceMessageException.PlayMessageError::class.java)
119+
hasMessageThat().isEqualTo("Error while trying to play voice message")
120+
}
118121
}
119122
}
120123
}
@@ -167,6 +170,83 @@ class VoiceMessagePresenterTest {
167170
}
168171
}
169172

173+
@Test
174+
fun `seeking downloads and seeks`() = runTest {
175+
val presenter = createVoiceMessagePresenter(
176+
content = aTimelineItemVoiceContent(durationMs = 10_000),
177+
)
178+
moleculeFlow(RecompositionMode.Immediate) {
179+
presenter.present()
180+
}.test {
181+
val initialState = awaitItem().also {
182+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play)
183+
Truth.assertThat(it.progress).isEqualTo(0f)
184+
Truth.assertThat(it.time).isEqualTo("0:10")
185+
}
186+
187+
initialState.eventSink(VoiceMessageEvents.Seek(0.5f))
188+
189+
awaitItem().also {
190+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading)
191+
Truth.assertThat(it.progress).isEqualTo(0f)
192+
Truth.assertThat(it.time).isEqualTo("0:10")
193+
}
194+
awaitItem().also {
195+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading)
196+
Truth.assertThat(it.progress).isEqualTo(0f)
197+
Truth.assertThat(it.time).isEqualTo("0:00")
198+
}
199+
awaitItem().also {
200+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading)
201+
Truth.assertThat(it.progress).isEqualTo(0.5f)
202+
Truth.assertThat(it.time).isEqualTo("0:05")
203+
}
204+
awaitItem().also {
205+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play)
206+
Truth.assertThat(it.progress).isEqualTo(0.5f)
207+
Truth.assertThat(it.time).isEqualTo("0:05")
208+
}
209+
}
210+
}
211+
212+
@Test
213+
fun `seeking downloads and fails`() = runTest {
214+
val analyticsService = FakeAnalyticsService()
215+
val presenter = createVoiceMessagePresenter(
216+
voiceMessageMediaRepo = FakeVoiceMessageMediaRepo().apply { shouldFail = true },
217+
analyticsService = analyticsService,
218+
content = aTimelineItemVoiceContent(durationMs = 10_000),
219+
)
220+
moleculeFlow(RecompositionMode.Immediate) {
221+
presenter.present()
222+
}.test {
223+
val initialState = awaitItem().also {
224+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play)
225+
Truth.assertThat(it.progress).isEqualTo(0f)
226+
Truth.assertThat(it.time).isEqualTo("0:10")
227+
}
228+
229+
initialState.eventSink(VoiceMessageEvents.Seek(0.5f))
230+
231+
awaitItem().also {
232+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading)
233+
Truth.assertThat(it.progress).isEqualTo(0f)
234+
Truth.assertThat(it.time).isEqualTo("0:10")
235+
}
236+
awaitItem().also {
237+
Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Retry)
238+
Truth.assertThat(it.progress).isEqualTo(0f)
239+
Truth.assertThat(it.time).isEqualTo("0:10")
240+
}
241+
analyticsService.trackedErrors.first().also {
242+
Truth.assertThat(it).apply {
243+
isInstanceOf(VoiceMessageException.PlayMessageError::class.java)
244+
hasMessageThat().isEqualTo("Error while trying to seek voice message")
245+
}
246+
}
247+
}
248+
}
249+
170250
@Test
171251
fun `seeking seeks`() = runTest {
172252
val presenter = createVoiceMessagePresenter(

0 commit comments

Comments
 (0)