From f8852856c6d60c6b9245d47b2393df5ed08dabc5 Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Tue, 10 Jan 2023 10:34:45 +0100 Subject: [PATCH 1/6] Convert state enum to sealed interface --- .../MessageVoiceBroadcastListeningItem.kt | 20 +++--- .../listening/VoiceBroadcastPlayer.kt | 12 ++-- .../listening/VoiceBroadcastPlayerImpl.kt | 62 +++++++++---------- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index b788d79214..d21e6771d9 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -63,10 +63,10 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem playPauseButton.setOnClickListener { if (player.currentVoiceBroadcast == voiceBroadcast) { when (player.playingState) { - VoiceBroadcastPlayer.State.PLAYING, - VoiceBroadcastPlayer.State.BUFFERING -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) - VoiceBroadcastPlayer.State.PAUSED, - VoiceBroadcastPlayer.State.IDLE -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) + VoiceBroadcastPlayer.State.Playing, + VoiceBroadcastPlayer.State.Buffering -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) + VoiceBroadcastPlayer.State.Paused, + VoiceBroadcastPlayer.State.Idle -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) } } else { callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) @@ -100,17 +100,17 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem private fun renderPlayingState(holder: Holder, state: VoiceBroadcastPlayer.State) { with(holder) { - bufferingView.isVisible = state == VoiceBroadcastPlayer.State.BUFFERING - voiceBroadcastMetadata.isVisible = state != VoiceBroadcastPlayer.State.BUFFERING + bufferingView.isVisible = state == VoiceBroadcastPlayer.State.Buffering + voiceBroadcastMetadata.isVisible = state != VoiceBroadcastPlayer.State.Buffering when (state) { - VoiceBroadcastPlayer.State.PLAYING, - VoiceBroadcastPlayer.State.BUFFERING -> { + VoiceBroadcastPlayer.State.Playing, + VoiceBroadcastPlayer.State.Buffering -> { playPauseButton.setImageResource(R.drawable.ic_play_pause_pause) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_pause_voice_broadcast) } - VoiceBroadcastPlayer.State.IDLE, - VoiceBroadcastPlayer.State.PAUSED -> { + VoiceBroadcastPlayer.State.Idle, + VoiceBroadcastPlayer.State.Paused -> { playPauseButton.setImageResource(R.drawable.ic_play_pause_play) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_play_voice_broadcast) } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt index 0de88e9992..5e5c320d43 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt @@ -26,7 +26,7 @@ interface VoiceBroadcastPlayer { val currentVoiceBroadcast: VoiceBroadcast? /** - * The current playing [State], [State.IDLE] by default. + * The current playing [State], [State.Idle] by default. */ val playingState: State @@ -68,11 +68,11 @@ interface VoiceBroadcastPlayer { /** * Player states. */ - enum class State { - PLAYING, - PAUSED, - BUFFERING, - IDLE + sealed interface State { + object Playing : State + object Paused : State + object Buffering : State + object Idle : State } /** diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 9cb894bb58..f00d657682 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -79,7 +79,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } - override var playingState = State.IDLE + override var playingState: State = State.Idle @MainThread set(value) { if (field != value) { @@ -96,7 +96,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( val hasChanged = currentVoiceBroadcast != voiceBroadcast when { hasChanged -> startPlayback(voiceBroadcast) - playingState == State.PAUSED -> resumePlayback() + playingState == State.Paused -> resumePlayback() else -> Unit } } @@ -107,7 +107,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override fun stop() { // Update state - playingState = State.IDLE + playingState = State.Idle // Stop and release media players stopPlayer() @@ -129,7 +129,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( listeners[voiceBroadcast.voiceBroadcastId]?.add(listener) ?: run { listeners[voiceBroadcast.voiceBroadcastId] = CopyOnWriteArrayList().apply { add(listener) } } - listener.onPlayingStateChanged(if (voiceBroadcast == currentVoiceBroadcast) playingState else State.IDLE) + listener.onPlayingStateChanged(if (voiceBroadcast == currentVoiceBroadcast) playingState else State.Idle) listener.onLiveModeChanged(voiceBroadcast == currentVoiceBroadcast) } @@ -139,11 +139,11 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun startPlayback(voiceBroadcast: VoiceBroadcast) { // Stop listening previous voice broadcast if any - if (playingState != State.IDLE) stop() + if (playingState != State.Idle) stop() currentVoiceBroadcast = voiceBroadcast - playingState = State.BUFFERING + playingState = State.Buffering observeVoiceBroadcastStateEvent(voiceBroadcast) } @@ -175,13 +175,13 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun onPlaylistUpdated() { when (playingState) { - State.PLAYING, - State.PAUSED -> { + State.Playing, + State.Paused -> { if (nextMediaPlayer == null && !isPreparingNextPlayer) { prepareNextMediaPlayer() } } - State.BUFFERING -> { + State.Buffering -> { val nextItem = if (isLiveListening && playlist.currentSequence == null) { // live listening, jump to the last item if playback has not started playlist.lastOrNull() @@ -193,7 +193,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( startPlayback(nextItem.startTime) } } - State.IDLE -> Unit // Should not happen + State.Idle -> Unit // Should not happen } } @@ -213,7 +213,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( if (sequencePosition > 0) { mp.seekTo(sequencePosition) } - playingState = State.PLAYING + playingState = State.Playing prepareNextMediaPlayer() } } catch (failure: Throwable) { @@ -224,7 +224,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } private fun pausePlayback() { - playingState = State.PAUSED // This will trigger a playing state update and save the current position + playingState = State.Paused // This will trigger a playing state update and save the current position if (currentMediaPlayer != null) { currentMediaPlayer?.pause() } else { @@ -234,7 +234,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun resumePlayback() { if (currentMediaPlayer != null) { - playingState = State.PLAYING + playingState = State.Playing currentMediaPlayer?.start() } else { val savedPosition = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPlaybackTime(it) } ?: 0 @@ -247,11 +247,11 @@ class VoiceBroadcastPlayerImpl @Inject constructor( voiceBroadcast != currentVoiceBroadcast -> { playbackTracker.updatePausedAtPlaybackTime(voiceBroadcast.voiceBroadcastId, positionMillis, positionMillis.toFloat() / duration) } - playingState == State.PLAYING || playingState == State.BUFFERING -> { + playingState == State.Playing || playingState == State.Buffering -> { updateLiveListeningMode(positionMillis) startPlayback(positionMillis) } - playingState == State.IDLE || playingState == State.PAUSED -> { + playingState == State.Idle || playingState == State.Paused -> { stopPlayer() playbackTracker.updatePausedAtPlaybackTime(voiceBroadcast.voiceBroadcastId, positionMillis, positionMillis.toFloat() / duration) } @@ -267,15 +267,15 @@ class VoiceBroadcastPlayerImpl @Inject constructor( isPreparingNextPlayer = false nextMediaPlayer = mp when (playingState) { - State.PLAYING, - State.PAUSED -> { + State.Playing, + State.Paused -> { currentMediaPlayer?.setNextMediaPlayer(mp) } - State.BUFFERING -> { + State.Buffering -> { mp.start() onNextMediaPlayerStarted(mp) } - State.IDLE -> stopPlayer() + State.Idle -> stopPlayer() } } } @@ -327,10 +327,10 @@ class VoiceBroadcastPlayerImpl @Inject constructor( currentVoiceBroadcast?.voiceBroadcastId?.let { voiceBroadcastId -> // Start or stop playback ticker when (playingState) { - State.PLAYING -> playbackTicker.startPlaybackTicker(voiceBroadcastId) - State.PAUSED, - State.BUFFERING, - State.IDLE -> playbackTicker.stopPlaybackTicker(voiceBroadcastId) + State.Playing -> playbackTicker.startPlaybackTicker(voiceBroadcastId) + State.Paused, + State.Buffering, + State.Idle -> playbackTicker.stopPlaybackTicker(voiceBroadcastId) } // Notify state change to all the listeners attached to the current voice broadcast id listeners[voiceBroadcastId]?.forEach { listener -> listener.onPlayingStateChanged(playingState) } @@ -348,7 +348,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( // the current voice broadcast is not live (ended) mostRecentVoiceBroadcastEvent?.isLive != true -> false // the player is stopped or paused - playingState == State.IDLE || playingState == State.PAUSED -> false + playingState == State.Idle || playingState == State.Paused -> false seekPosition != null -> { val seekDirection = seekPosition.compareTo(getCurrentPlaybackPosition() ?: 0) val newSequence = playlist.findByPosition(seekPosition)?.sequence @@ -374,13 +374,13 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun onLiveListeningChanged(isLiveListening: Boolean) { // Live has ended and last chunk has been reached, we can stop the playback - if (!isLiveListening && playingState == State.BUFFERING && playlist.currentSequence == mostRecentVoiceBroadcastEvent?.content?.lastChunkSequence) { + if (!isLiveListening && playingState == State.Buffering && playlist.currentSequence == mostRecentVoiceBroadcastEvent?.content?.lastChunkSequence) { stop() } } private fun onNextMediaPlayerStarted(mp: MediaPlayer) { - playingState = State.PLAYING + playingState = State.Playing playlist.currentSequence = playlist.currentSequence?.inc() currentMediaPlayer = mp nextMediaPlayer = null @@ -427,7 +427,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( stop() } else { // Enter in buffering mode and release current media player - playingState = State.BUFFERING + playingState = State.Buffering currentMediaPlayer?.release() currentMediaPlayer = null } @@ -462,18 +462,18 @@ class VoiceBroadcastPlayerImpl @Inject constructor( val playbackTime = getCurrentPlaybackPosition() val percentage = getCurrentPlaybackPercentage() when (playingState) { - State.PLAYING -> { + State.Playing -> { if (playbackTime != null && percentage != null) { playbackTracker.updatePlayingAtPlaybackTime(id, playbackTime, percentage) } } - State.PAUSED, - State.BUFFERING -> { + State.Paused, + State.Buffering -> { if (playbackTime != null && percentage != null) { playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) } } - State.IDLE -> { + State.Idle -> { if (playbackTime == null || percentage == null || (playlist.duration - playbackTime) < 50) { playbackTracker.stopPlayback(id) } else { From 2d24eb1273d0aa72e9ea2e5f758484763628af34 Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Tue, 10 Jan 2023 14:00:46 +0100 Subject: [PATCH 2/6] Handle playback error --- changelog.d/7829.bugfix | 1 + .../src/main/res/values/strings.xml | 1 + .../vector/app/core/error/ErrorFormatter.kt | 2 + .../voice/VoiceMessageRecorderView.kt | 1 + .../factory/VoiceBroadcastItemFactory.kt | 3 + .../helper/AudioMessagePlaybackTracker.kt | 30 ++++++--- .../item/AbsMessageVoiceBroadcastItem.kt | 3 + .../detail/timeline/item/MessageAudioItem.kt | 1 + .../MessageVoiceBroadcastListeningItem.kt | 35 ++++++++--- .../detail/timeline/item/MessageVoiceItem.kt | 1 + .../voicebroadcast/VoiceBroadcastFailure.kt | 11 ++++ .../listening/VoiceBroadcastPlayer.kt | 2 + .../listening/VoiceBroadcastPlayerImpl.kt | 63 +++++++++++-------- .../res/drawable/ic_voice_broadcast_error.xml | 10 +++ ...e_event_voice_broadcast_listening_stub.xml | 23 +++++++ 15 files changed, 144 insertions(+), 43 deletions(-) create mode 100644 changelog.d/7829.bugfix create mode 100644 vector/src/main/res/drawable/ic_voice_broadcast_error.xml diff --git a/changelog.d/7829.bugfix b/changelog.d/7829.bugfix new file mode 100644 index 0000000000..705f7310f0 --- /dev/null +++ b/changelog.d/7829.bugfix @@ -0,0 +1 @@ +Handle exceptions when listening a voice broadcast diff --git a/library/ui-strings/src/main/res/values/strings.xml b/library/ui-strings/src/main/res/values/strings.xml index d9f94ba27b..ce84ef61ad 100644 --- a/library/ui-strings/src/main/res/values/strings.xml +++ b/library/ui-strings/src/main/res/values/strings.xml @@ -3121,6 +3121,7 @@ You don’t have the required permissions to start a voice broadcast in this room. Contact a room administrator to upgrade your permissions. Someone else is already recording a voice broadcast. Wait for their voice broadcast to end to start a new one. You are already recording a voice broadcast. Please end your current voice broadcast to start a new one. + Unable to play this voice broadcast. %1$s left Stop live broadcasting? diff --git a/vector/src/main/java/im/vector/app/core/error/ErrorFormatter.kt b/vector/src/main/java/im/vector/app/core/error/ErrorFormatter.kt index 380c80775b..78aaa058e9 100644 --- a/vector/src/main/java/im/vector/app/core/error/ErrorFormatter.kt +++ b/vector/src/main/java/im/vector/app/core/error/ErrorFormatter.kt @@ -157,6 +157,8 @@ class DefaultErrorFormatter @Inject constructor( RecordingError.BlockedBySomeoneElse -> stringProvider.getString(R.string.error_voice_broadcast_blocked_by_someone_else_message) RecordingError.NoPermission -> stringProvider.getString(R.string.error_voice_broadcast_permission_denied_message) RecordingError.UserAlreadyBroadcasting -> stringProvider.getString(R.string.error_voice_broadcast_already_in_progress_message) + is VoiceBroadcastFailure.ListeningError.UnableToPlay, + is VoiceBroadcastFailure.ListeningError.DownloadError -> stringProvider.getString(R.string.error_voice_broadcast_unable_to_play) } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt index a7b926f29a..b5c4b4a537 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/composer/voice/VoiceMessageRecorderView.kt @@ -229,6 +229,7 @@ class VoiceMessageRecorderView @JvmOverloads constructor( voiceMessageViews.renderPlaying(state) } is AudioMessagePlaybackTracker.Listener.State.Paused, + is AudioMessagePlaybackTracker.Listener.State.Error, is AudioMessagePlaybackTracker.Listener.State.Idle -> { voiceMessageViews.renderIdle() } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt index cc3a015120..ad8e163633 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt @@ -15,6 +15,7 @@ */ package im.vector.app.features.home.room.detail.timeline.factory +import im.vector.app.core.error.ErrorFormatter import im.vector.app.core.resources.ColorProvider import im.vector.app.core.resources.DrawableProvider import im.vector.app.features.displayname.getBestName @@ -45,6 +46,7 @@ class VoiceBroadcastItemFactory @Inject constructor( private val avatarSizeProvider: AvatarSizeProvider, private val colorProvider: ColorProvider, private val drawableProvider: DrawableProvider, + private val errorFormatter: ErrorFormatter, private val voiceBroadcastRecorder: VoiceBroadcastRecorder?, private val voiceBroadcastPlayer: VoiceBroadcastPlayer, private val playbackTracker: AudioMessagePlaybackTracker, @@ -82,6 +84,7 @@ class VoiceBroadcastItemFactory @Inject constructor( roomItem = session.getRoom(params.event.roomId)?.roomSummary()?.toMatrixItem(), colorProvider = colorProvider, drawableProvider = drawableProvider, + errorFormatter = errorFormatter, ) return if (isRecording) { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt index c34cbbc74a..c598a99af7 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/helper/AudioMessagePlaybackTracker.kt @@ -50,8 +50,11 @@ class AudioMessagePlaybackTracker @Inject constructor() { listeners.remove(id) } - fun pauseAllPlaybacks() { - listeners.keys.forEach(::pausePlayback) + fun unregisterListeners() { + listeners.forEach { + it.value.onUpdate(Listener.State.Idle) + } + listeners.clear() } /** @@ -84,6 +87,10 @@ class AudioMessagePlaybackTracker @Inject constructor() { } } + fun pauseAllPlaybacks() { + listeners.keys.forEach(::pausePlayback) + } + fun pausePlayback(id: String) { val state = getPlaybackState(id) if (state is Listener.State.Playing) { @@ -94,7 +101,14 @@ class AudioMessagePlaybackTracker @Inject constructor() { } fun stopPlayback(id: String) { - setState(id, Listener.State.Idle) + val state = getPlaybackState(id) + if (state !is Listener.State.Error) { + setState(id, Listener.State.Idle) + } + } + + fun onError(id: String, error: Throwable) { + setState(id, Listener.State.Error(error)) } fun updatePlayingAtPlaybackTime(id: String, time: Int, percentage: Float) { @@ -116,6 +130,7 @@ class AudioMessagePlaybackTracker @Inject constructor() { is Listener.State.Playing -> state.playbackTime is Listener.State.Paused -> state.playbackTime is Listener.State.Recording, + is Listener.State.Error, Listener.State.Idle, null -> null } @@ -126,18 +141,12 @@ class AudioMessagePlaybackTracker @Inject constructor() { is Listener.State.Playing -> state.percentage is Listener.State.Paused -> state.percentage is Listener.State.Recording, + is Listener.State.Error, Listener.State.Idle, null -> null } } - fun unregisterListeners() { - listeners.forEach { - it.value.onUpdate(Listener.State.Idle) - } - listeners.clear() - } - companion object { const val RECORDING_ID = "RECORDING_ID" } @@ -148,6 +157,7 @@ class AudioMessagePlaybackTracker @Inject constructor() { sealed class State { object Idle : State() + data class Error(val failure: Throwable) : State() data class Playing(val playbackTime: Int, val percentage: Float) : State() data class Paused(val playbackTime: Int, val percentage: Float) : State() data class Recording(val amplitudeList: List) : State() diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt index c6b90cdabe..7cde978e42 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageVoiceBroadcastItem.kt @@ -22,6 +22,7 @@ import androidx.annotation.IdRes import androidx.core.view.isVisible import com.airbnb.epoxy.EpoxyAttribute import im.vector.app.R +import im.vector.app.core.error.ErrorFormatter import im.vector.app.core.extensions.tintBackground import im.vector.app.core.resources.ColorProvider import im.vector.app.core.resources.DrawableProvider @@ -48,6 +49,7 @@ abstract class AbsMessageVoiceBroadcastItem() { private fun renderStateBasedOnAudioPlayback(holder: Holder) { audioMessagePlaybackTracker.track(attributes.informationData.eventId) { state -> when (state) { + is AudioMessagePlaybackTracker.Listener.State.Error, is AudioMessagePlaybackTracker.Listener.State.Idle -> renderIdleState(holder) is AudioMessagePlaybackTracker.Listener.State.Playing -> renderPlayingState(holder, state) is AudioMessagePlaybackTracker.Listener.State.Paused -> renderPausedState(holder, state) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt index d21e6771d9..0aa2aaad3b 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceBroadcastListeningItem.kt @@ -20,11 +20,13 @@ import android.text.format.DateUtils import android.widget.ImageButton import android.widget.SeekBar import android.widget.TextView +import androidx.constraintlayout.widget.Group import androidx.core.view.isInvisible import androidx.core.view.isVisible import com.airbnb.epoxy.EpoxyModelClass import im.vector.app.R import im.vector.app.core.epoxy.onClick +import im.vector.app.core.extensions.setTextOrHide import im.vector.app.features.home.room.detail.RoomDetailAction.VoiceBroadcastAction import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker.Listener.State import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer @@ -54,6 +56,16 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } } player.addListener(voiceBroadcast, playerListener) + + playbackTracker.track(voiceBroadcast.voiceBroadcastId) { playbackState -> + renderBackwardForwardButtons(holder, playbackState) + renderPlaybackError(holder, playbackState) + renderLiveIndicator(holder) + if (!isUserSeeking) { + holder.seekBar.progress = playbackTracker.getPlaybackTime(voiceBroadcast.voiceBroadcastId) ?: 0 + } + } + bindSeekBar(holder) bindButtons(holder) } @@ -66,6 +78,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem VoiceBroadcastPlayer.State.Playing, VoiceBroadcastPlayer.State.Buffering -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.Pause) VoiceBroadcastPlayer.State.Paused, + is VoiceBroadcastPlayer.State.Error, VoiceBroadcastPlayer.State.Idle -> callback?.onTimelineItemAction(VoiceBroadcastAction.Listening.PlayOrResume(voiceBroadcast)) } } else { @@ -109,6 +122,7 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem playPauseButton.setImageResource(R.drawable.ic_play_pause_pause) playPauseButton.contentDescription = view.resources.getString(R.string.a11y_pause_voice_broadcast) } + is VoiceBroadcastPlayer.State.Error, VoiceBroadcastPlayer.State.Idle, VoiceBroadcastPlayer.State.Paused -> { playPauseButton.setImageResource(R.drawable.ic_play_pause_play) @@ -120,6 +134,18 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } } + private fun renderPlaybackError(holder: Holder, playbackState: State) { + with(holder) { + if (playbackState is State.Error) { + controlsGroup.isVisible = false + errorView.setTextOrHide(errorFormatter.toHumanReadable(playbackState.failure)) + } else { + errorView.isVisible = false + controlsGroup.isVisible = true + } + } + } + private fun bindSeekBar(holder: Holder) { with(holder) { remainingTimeView.text = formatRemainingTime(duration) @@ -141,13 +167,6 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem } }) } - playbackTracker.track(voiceBroadcast.voiceBroadcastId) { playbackState -> - renderBackwardForwardButtons(holder, playbackState) - renderLiveIndicator(holder) - if (!isUserSeeking) { - holder.seekBar.progress = playbackTracker.getPlaybackTime(voiceBroadcast.voiceBroadcastId) ?: 0 - } - } } private fun renderBackwardForwardButtons(holder: Holder, playbackState: State) { @@ -187,6 +206,8 @@ abstract class MessageVoiceBroadcastListeningItem : AbsMessageVoiceBroadcastItem val broadcasterNameMetadata by bind(R.id.broadcasterNameMetadata) val voiceBroadcastMetadata by bind(R.id.voiceBroadcastMetadata) val listenersCountMetadata by bind(R.id.listenersCountMetadata) + val errorView by bind(R.id.errorView) + val controlsGroup by bind(R.id.controlsGroup) } companion object { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt index d3f320db7d..a8e215b4a9 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageVoiceItem.kt @@ -124,6 +124,7 @@ abstract class MessageVoiceItem : AbsMessageItem() { audioMessagePlaybackTracker.track(attributes.informationData.eventId) { state -> when (state) { + is AudioMessagePlaybackTracker.Listener.State.Error, is AudioMessagePlaybackTracker.Listener.State.Idle -> renderIdleState(holder, waveformColorIdle, waveformColorPlayed) is AudioMessagePlaybackTracker.Listener.State.Playing -> renderPlayingState(holder, state, waveformColorIdle, waveformColorPlayed) is AudioMessagePlaybackTracker.Listener.State.Paused -> renderPausedState(holder, state, waveformColorIdle, waveformColorPlayed) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastFailure.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastFailure.kt index 76b50c78ab..75863dc042 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastFailure.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/VoiceBroadcastFailure.kt @@ -16,10 +16,21 @@ package im.vector.app.features.voicebroadcast +import android.media.MediaPlayer + sealed class VoiceBroadcastFailure : Throwable() { sealed class RecordingError : VoiceBroadcastFailure() { object NoPermission : RecordingError() object BlockedBySomeoneElse : RecordingError() object UserAlreadyBroadcasting : RecordingError() } + + sealed class ListeningError : VoiceBroadcastFailure() { + /** + * @property what the type of error that has occurred, see [MediaPlayer.OnErrorListener.onError]. + * @property extra an extra code, specific to the error, see [MediaPlayer.OnErrorListener.onError]. + */ + data class UnableToPlay(val what: Int, val extra: Int) : ListeningError() + data class DownloadError(override val cause: Throwable?) : ListeningError() + } } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt index 5e5c320d43..ad0ecf69b4 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayer.kt @@ -16,6 +16,7 @@ package im.vector.app.features.voicebroadcast.listening +import im.vector.app.features.voicebroadcast.VoiceBroadcastFailure import im.vector.app.features.voicebroadcast.model.VoiceBroadcast interface VoiceBroadcastPlayer { @@ -72,6 +73,7 @@ interface VoiceBroadcastPlayer { object Playing : State object Paused : State object Buffering : State + data class Error(val failure: VoiceBroadcastFailure.ListeningError) : State object Idle : State } diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index f00d657682..538b2f8da4 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -24,7 +24,7 @@ import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.extensions.onFirst import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.session.coroutineScope -import im.vector.app.features.voice.VoiceFailure +import im.vector.app.features.voicebroadcast.VoiceBroadcastFailure import im.vector.app.features.voicebroadcast.isLive import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.Listener import im.vector.app.features.voicebroadcast.listening.VoiceBroadcastPlayer.State @@ -193,6 +193,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( startPlayback(nextItem.startTime) } } + is State.Error -> Unit State.Idle -> Unit // Should not happen } } @@ -205,20 +206,15 @@ class VoiceBroadcastPlayerImpl @Inject constructor( val sequence = playlistItem.sequence ?: run { Timber.w("## Voice Broadcast | Playlist item has no sequence"); return } val sequencePosition = position - playlistItem.startTime sessionScope.launch { - try { - prepareMediaPlayer(content) { mp -> - currentMediaPlayer = mp - playlist.currentSequence = sequence - mp.start() - if (sequencePosition > 0) { - mp.seekTo(sequencePosition) - } - playingState = State.Playing - prepareNextMediaPlayer() + prepareMediaPlayer(content) { mp -> + currentMediaPlayer = mp + playlist.currentSequence = sequence + mp.start() + if (sequencePosition > 0) { + mp.seekTo(sequencePosition) } - } catch (failure: Throwable) { - Timber.e(failure, "## Voice Broadcast | Unable to start playback: $failure") - throw VoiceFailure.UnableToPlay(failure) + playingState = State.Playing + prepareNextMediaPlayer() } } } @@ -275,6 +271,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( mp.start() onNextMediaPlayerStarted(mp) } + is State.Error, State.Idle -> stopPlayer() } } @@ -288,11 +285,12 @@ class VoiceBroadcastPlayerImpl @Inject constructor( session.fileService().downloadFile(messageAudioContent) } catch (failure: Throwable) { Timber.e(failure, "Voice Broadcast | Download has failed: $failure") - throw VoiceFailure.UnableToPlay(failure) + throw VoiceBroadcastFailure.ListeningError.DownloadError(failure) } return audioFile.inputStream().use { fis -> MediaPlayer().apply { + setOnErrorListener(mediaPlayerListener) setAudioAttributes( AudioAttributes.Builder() // Do not use CONTENT_TYPE_SPEECH / USAGE_VOICE_COMMUNICATION because we want to play loud here @@ -302,10 +300,9 @@ class VoiceBroadcastPlayerImpl @Inject constructor( ) setDataSource(fis.fd) setOnInfoListener(mediaPlayerListener) - setOnErrorListener(mediaPlayerListener) setOnPreparedListener(onPreparedListener) setOnCompletionListener(mediaPlayerListener) - prepare() + prepareAsync() } } } @@ -330,8 +327,15 @@ class VoiceBroadcastPlayerImpl @Inject constructor( State.Playing -> playbackTicker.startPlaybackTicker(voiceBroadcastId) State.Paused, State.Buffering, + is State.Error, State.Idle -> playbackTicker.stopPlaybackTicker(voiceBroadcastId) } + + // Notify playback tracker about error + if (playingState is State.Error) { + playbackTracker.onError(voiceBroadcastId, playingState.failure) + } + // Notify state change to all the listeners attached to the current voice broadcast id listeners[voiceBroadcastId]?.forEach { listener -> listener.onPlayingStateChanged(playingState) } } @@ -374,7 +378,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun onLiveListeningChanged(isLiveListening: Boolean) { // Live has ended and last chunk has been reached, we can stop the playback - if (!isLiveListening && playingState == State.Buffering && playlist.currentSequence == mostRecentVoiceBroadcastEvent?.content?.lastChunkSequence) { + val hasReachedLastChunk = playlist.currentSequence == mostRecentVoiceBroadcastEvent?.content?.lastChunkSequence + if (!isLiveListening && playingState == State.Buffering && hasReachedLastChunk) { stop() } } @@ -389,16 +394,16 @@ class VoiceBroadcastPlayerImpl @Inject constructor( private fun getCurrentPlaybackPosition(): Int? { val voiceBroadcastId = currentVoiceBroadcast?.voiceBroadcastId ?: return null - val computedPosition = currentMediaPlayer?.currentPosition?.let { playlist.currentItem?.startTime?.plus(it) } + val computedPosition = tryOrNull { currentMediaPlayer?.currentPosition }?.let { playlist.currentItem?.startTime?.plus(it) } val savedPosition = playbackTracker.getPlaybackTime(voiceBroadcastId) return computedPosition ?: savedPosition } private fun getCurrentPlaybackPercentage(): Float? { val playlistPosition = playlist.currentItem?.startTime - val computedPosition = currentMediaPlayer?.currentPosition?.let { playlistPosition?.plus(it) } ?: playlistPosition - val duration = playlist.duration.takeIf { it > 0 } - val computedPercentage = if (computedPosition != null && duration != null) computedPosition.toFloat() / duration else null + val computedPosition = tryOrNull { currentMediaPlayer?.currentPosition }?.let { playlistPosition?.plus(it) } ?: playlistPosition + val duration = playlist.duration + val computedPercentage = if (computedPosition != null && duration > 0) computedPosition.toFloat() / duration else null val savedPercentage = currentVoiceBroadcast?.voiceBroadcastId?.let { playbackTracker.getPercentage(it) } return computedPercentage ?: savedPercentage } @@ -416,6 +421,10 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } override fun onCompletion(mp: MediaPlayer) { + // Release media player as soon as it completed + mp.release() + currentMediaPlayer = null + // Next media player is already attached to this player and will start playing automatically if (nextMediaPlayer != null) return @@ -426,15 +435,16 @@ class VoiceBroadcastPlayerImpl @Inject constructor( // We'll not receive new chunks anymore so we can stop the live listening stop() } else { - // Enter in buffering mode and release current media player playingState = State.Buffering - currentMediaPlayer?.release() - currentMediaPlayer = null + prepareNextMediaPlayer() } } override fun onError(mp: MediaPlayer, what: Int, extra: Int): Boolean { - stop() + Timber.d("## Voice Broadcast | onError: what=$what, extra=$extra") + if (playingState == State.Buffering || tryOrNull { currentMediaPlayer?.isPlaying } != true) { + playingState = State.Error(VoiceBroadcastFailure.ListeningError.UnableToPlay(what, extra)) + } return true } } @@ -480,6 +490,7 @@ class VoiceBroadcastPlayerImpl @Inject constructor( playbackTracker.updatePausedAtPlaybackTime(id, playbackTime, percentage) } } + is State.Error -> Unit } } } diff --git a/vector/src/main/res/drawable/ic_voice_broadcast_error.xml b/vector/src/main/res/drawable/ic_voice_broadcast_error.xml new file mode 100644 index 0000000000..6cbd4592cb --- /dev/null +++ b/vector/src/main/res/drawable/ic_voice_broadcast_error.xml @@ -0,0 +1,10 @@ + + + diff --git a/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml b/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml index 760293ee64..deec85e2ed 100644 --- a/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml +++ b/vector/src/main/res/layout/item_timeline_event_voice_broadcast_listening_stub.xml @@ -176,4 +176,27 @@ tools:ignore="NegativeMargin" tools:text="-0:12" /> + + + + From 3663f225905aabdfaa73550224cf210d6616fa96 Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Tue, 10 Jan 2023 15:16:25 +0100 Subject: [PATCH 3/6] Handle download error during playback --- .../listening/VoiceBroadcastPlayerImpl.kt | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 538b2f8da4..ab4b6c2269 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -206,15 +206,19 @@ class VoiceBroadcastPlayerImpl @Inject constructor( val sequence = playlistItem.sequence ?: run { Timber.w("## Voice Broadcast | Playlist item has no sequence"); return } val sequencePosition = position - playlistItem.startTime sessionScope.launch { - prepareMediaPlayer(content) { mp -> - currentMediaPlayer = mp - playlist.currentSequence = sequence - mp.start() - if (sequencePosition > 0) { - mp.seekTo(sequencePosition) + try { + prepareMediaPlayer(content) { mp -> + currentMediaPlayer = mp + playlist.currentSequence = sequence + mp.start() + if (sequencePosition > 0) { + mp.seekTo(sequencePosition) + } + playingState = State.Playing + prepareNextMediaPlayer() } - playingState = State.Playing - prepareNextMediaPlayer() + } catch (failure: VoiceBroadcastFailure.ListeningError.DownloadError) { + playingState = State.Error(failure) } } } @@ -259,20 +263,27 @@ class VoiceBroadcastPlayerImpl @Inject constructor( if (nextItem != null) { isPreparingNextPlayer = true sessionScope.launch { - prepareMediaPlayer(nextItem.audioEvent.content) { mp -> + try { + prepareMediaPlayer(nextItem.audioEvent.content) { mp -> + isPreparingNextPlayer = false + nextMediaPlayer = mp + when (playingState) { + State.Playing, + State.Paused -> { + currentMediaPlayer?.setNextMediaPlayer(mp) + } + State.Buffering -> { + mp.start() + onNextMediaPlayerStarted(mp) + } + is State.Error, + State.Idle -> stopPlayer() + } + } + } catch (failure: VoiceBroadcastFailure.ListeningError.DownloadError) { isPreparingNextPlayer = false - nextMediaPlayer = mp - when (playingState) { - State.Playing, - State.Paused -> { - currentMediaPlayer?.setNextMediaPlayer(mp) - } - State.Buffering -> { - mp.start() - onNextMediaPlayerStarted(mp) - } - is State.Error, - State.Idle -> stopPlayer() + if (playingState == State.Buffering || tryOrNull { currentMediaPlayer?.isPlaying } != true) { + playingState = State.Error(failure) } } } From 9d3b5c5bbb7c0d2a457a11fa6a20214b3e788293 Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Tue, 10 Jan 2023 16:00:19 +0100 Subject: [PATCH 4/6] Fix no display name for some voice broadcast recorder name --- .../room/detail/timeline/factory/VoiceBroadcastItemFactory.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt index ad8e163633..3439fb1f57 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/VoiceBroadcastItemFactory.kt @@ -18,7 +18,6 @@ package im.vector.app.features.home.room.detail.timeline.factory import im.vector.app.core.error.ErrorFormatter import im.vector.app.core.resources.ColorProvider import im.vector.app.core.resources.DrawableProvider -import im.vector.app.features.displayname.getBestName import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker import im.vector.app.features.home.room.detail.timeline.helper.AvatarSizeProvider import im.vector.app.features.home.room.detail.timeline.helper.VoiceBroadcastEventsGroup @@ -37,7 +36,6 @@ import im.vector.app.features.voicebroadcast.model.asVoiceBroadcastEvent import im.vector.app.features.voicebroadcast.recording.VoiceBroadcastRecorder import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.getRoom -import org.matrix.android.sdk.api.session.getUserOrDefault import org.matrix.android.sdk.api.util.toMatrixItem import javax.inject.Inject @@ -77,7 +75,7 @@ class VoiceBroadcastItemFactory @Inject constructor( voiceBroadcast = voiceBroadcast, voiceBroadcastState = voiceBroadcastContent.voiceBroadcastState, duration = voiceBroadcastEventsGroup.getDuration(), - recorderName = params.event.root.stateKey?.let { session.getUserOrDefault(it) }?.toMatrixItem()?.getBestName().orEmpty(), + recorderName = params.event.senderInfo.disambiguatedDisplayName, recorder = voiceBroadcastRecorder, player = voiceBroadcastPlayer, playbackTracker = playbackTracker, From 8a2f28bc376fd2f34c97270cdff7336332efe50e Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Thu, 12 Jan 2023 18:29:56 +0100 Subject: [PATCH 5/6] Add comment to explain the error handling --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index ab4b6c2269..1cb418163f 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -282,6 +282,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor( } } catch (failure: VoiceBroadcastFailure.ListeningError.DownloadError) { isPreparingNextPlayer = false + // Do not change the playingState if the current player is still valid, + // the error will be thrown again when switching to the next player if (playingState == State.Buffering || tryOrNull { currentMediaPlayer?.isPlaying } != true) { playingState = State.Error(failure) } @@ -453,6 +455,8 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override fun onError(mp: MediaPlayer, what: Int, extra: Int): Boolean { Timber.d("## Voice Broadcast | onError: what=$what, extra=$extra") + // Do not change the playingState if the current player is still valid, + // the error will be thrown again when switching to the next player if (playingState == State.Buffering || tryOrNull { currentMediaPlayer?.isPlaying } != true) { playingState = State.Error(VoiceBroadcastFailure.ListeningError.UnableToPlay(what, extra)) } From 169c9b221ca69039406a50744749c5336c4af85a Mon Sep 17 00:00:00 2001 From: Florian Renaud Date: Fri, 13 Jan 2023 15:46:48 +0100 Subject: [PATCH 6/6] Throw an error if the media player which has completed is not the expected one --- .../voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt index 1cb418163f..2e1600e4e2 100644 --- a/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt +++ b/vector/src/main/java/im/vector/app/features/voicebroadcast/listening/VoiceBroadcastPlayerImpl.kt @@ -436,7 +436,11 @@ class VoiceBroadcastPlayerImpl @Inject constructor( override fun onCompletion(mp: MediaPlayer) { // Release media player as soon as it completed mp.release() - currentMediaPlayer = null + if (currentMediaPlayer == mp) { + currentMediaPlayer = null + } else { + error("The media player which has completed mismatches the current media player instance.") + } // Next media player is already attached to this player and will start playing automatically if (nextMediaPlayer != null) return