From ed35e1f911cefe64a0f68ee46c62b81d01bbe7f6 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Fri, 27 Jan 2023 13:01:51 +0100 Subject: [PATCH 1/2] Improve error handling during a voice broadcast playback --- .../VoiceBroadcastPlaybackViewModel.swift | 89 +++++++++++++++---- .../View/VoiceBroadcastPlaybackView.swift | 2 +- .../VoiceBroadcastPlaybackModels.swift | 1 + .../VoiceBroadcastPlaybackScreenState.swift | 2 +- changelog.d/7311.change | 1 + 5 files changed, 75 insertions(+), 20 deletions(-) create mode 100644 changelog.d/7311.change diff --git a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift index fd1ca9157c..af229c35ec 100644 --- a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift +++ b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift @@ -44,8 +44,17 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic private var reloadVoiceBroadcastChunkQueue: Bool = false private var seekToChunkTime: TimeInterval? + /// The last chunk we tried to load + private var lastChunkProcessed: UInt = 0 + /// The last chunk correctly loaded and added to the player's queue private var lastChunkAddedToPlayer: UInt = 0 + private var hasAttachmentErrors: Bool = false { + didSet { + updateErrorState() + } + } + private var isPlayingLastChunk: Bool { // We can't play the last chunk if the brodcast is not stopped guard state.broadcastState == .stopped else { @@ -60,18 +69,19 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic return state.bindings.progress + 1000 >= state.playingState.duration - Float(chunkDuration) } - private var playingChunk: VoiceBroadcastChunk? { + /// Current chunk loaded in the audio player + private var currentChunk: VoiceBroadcastChunk? { guard let currentAudioPlayerUrl = audioPlayer?.currentUrl, - let playingEventId = voiceBroadcastAttachmentCacheManagerLoadResults.first(where: { result in + let currentEventId = voiceBroadcastAttachmentCacheManagerLoadResults.first(where: { result in result.url == currentAudioPlayerUrl })?.eventIdentifier else { return nil } - let playingChunk = voiceBroadcastAggregator.voiceBroadcast.chunks.first(where: { chunk in - chunk.attachment.eventId == playingEventId + let currentChunk = voiceBroadcastAggregator.voiceBroadcast.chunks.first(where: { chunk in + chunk.attachment.eventId == currentEventId }) - return playingChunk + return currentChunk } private var isLivePlayback: Bool { @@ -112,7 +122,8 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic playbackState: .stopped, playingState: VoiceBroadcastPlayingState(duration: Float(voiceBroadcastAggregator.voiceBroadcast.duration), isLive: false, canMoveForward: false, canMoveBackward: false), bindings: VoiceBroadcastPlaybackViewStateBindings(progress: 0), - decryptionState: VoiceBroadcastPlaybackDecryptionState(errorCount: 0)) + decryptionState: VoiceBroadcastPlaybackDecryptionState(errorCount: 0), + showPlaybackError: false) super.init(initialViewState: viewState) displayLink = CADisplayLink(target: WeakTarget(self, selector: #selector(handleDisplayLinkTick)), selector: WeakTarget.triggerSelector) @@ -198,8 +209,8 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic // If we known the last chunk sequence, use it to check if we need to stop // Note: it's possible to be in .stopped state and to still have a last chunk sequence at 0 (old versions or a crash during recording). In this case, we use isPlayingLastChunk as a fallback solution if voiceBroadcastAggregator.voiceBroadcastLastChunkSequence > 0 { - // we should stop only if we have already added the last chunk to the player - shouldStop = (lastChunkAddedToPlayer == voiceBroadcastAggregator.voiceBroadcastLastChunkSequence) + // we should stop only if we have already processed the last chunk + shouldStop = (lastChunkProcessed == voiceBroadcastAggregator.voiceBroadcastLastChunkSequence) } else { shouldStop = isPlayingLastChunk } @@ -236,10 +247,12 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic private func seek(to seekTime: Float) { // Flush the chunks queue and the current audio player playlist + lastChunkProcessed = 0 lastChunkAddedToPlayer = 0 voiceBroadcastChunkQueue = [] reloadVoiceBroadcastChunkQueue = isProcessingVoiceBroadcastChunk audioPlayer?.removeAllPlayerItems() + hasAttachmentErrors = false let chunks = reorderVoiceBroadcastChunks(chunks: Array(voiceBroadcastAggregator.voiceBroadcast.chunks)) @@ -326,6 +339,8 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic return } + self.lastChunkProcessed = chunk.sequence + switch result { case .success(let result): guard result.eventIdentifier == chunk.attachment.eventId else { @@ -369,19 +384,46 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic audioPlayer.seekToTime(time) self.seekToChunkTime = nil } - + + self.hasAttachmentErrors = false + self.processNextVoiceBroadcastChunk() + case .failure (let error): - MXLog.error("[VoiceBroadcastPlaybackViewModel] processVoiceBroadcastChunkQueue: loadAttachment error", context: error) - if self.voiceBroadcastChunkQueue.count == 0 { - // No more chunk to try. Go to error - self.state.playbackState = .error + MXLog.error("[VoiceBroadcastPlaybackViewModel] processVoiceBroadcastChunkQueue: loadAttachment error", context: ["error": error, "chunk": chunk.sequence]) + self.hasAttachmentErrors = true + // If nothing has been added to the player's queue, exit the buffer state + if self.lastChunkAddedToPlayer == 0 { + self.pause() } } - - self.processNextVoiceBroadcastChunk() } } + private func resetErrorState() { + state.showPlaybackError = false + } + + private func updateErrorState() { + // Show an error if the playback state is .error + var showPlaybackError = state.playbackState == .error + + // Or if there is an attachment error + if hasAttachmentErrors { + // only if the audio player is not playing and has nothing left to play + let audioPlayerIsPlaying = audioPlayer?.isPlaying ?? false + let currentPlayerTime = audioPlayer?.currentTime ?? 0 + let currentPlayerDuration = audioPlayer?.duration ?? 0 + let currentChunkSequence = currentChunk?.sequence ?? 0 + let hasNoMoreChunkToPlay = (currentChunk == nil && lastChunkAddedToPlayer == 0) || (currentChunkSequence == lastChunkAddedToPlayer) + if !audioPlayerIsPlaying && hasNoMoreChunkToPlay && (currentPlayerDuration - currentPlayerTime < 0.2) { + showPlaybackError = true + } + } + + state.showPlaybackError = showPlaybackError + + } + private func updateDuration() { let duration = voiceBroadcastAggregator.voiceBroadcast.duration state.playingState.duration = Float(duration) @@ -404,10 +446,11 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic } else { seek(to: state.bindings.progress) } + resetErrorState() } @objc private func handleDisplayLinkTick() { - guard let playingSequence = self.playingChunk?.sequence else { + guard let playingSequence = self.currentChunk?.sequence else { return } @@ -438,7 +481,7 @@ class VoiceBroadcastPlaybackViewModel: VoiceBroadcastPlaybackViewModelType, Voic state.playingState.remainingTimeLabel = label state.playingState.canMoveBackward = state.bindings.progress > 0 - state.playingState.canMoveForward = state.bindings.progress < state.playingState.duration + state.playingState.canMoveForward = (state.playingState.duration - state.bindings.progress) > 500 } private func handleVoiceBroadcastChunksProcessing() { @@ -504,6 +547,7 @@ extension VoiceBroadcastPlaybackViewModel: VoiceBroadcastAggregatorDelegate { // MARK: - VoiceMessageAudioPlayerDelegate extension VoiceBroadcastPlaybackViewModel: VoiceMessageAudioPlayerDelegate { func audioPlayerDidFinishLoading(_ audioPlayer: VoiceMessageAudioPlayer) { + updateErrorState() } func audioPlayerDidStartPlaying(_ audioPlayer: VoiceMessageAudioPlayer) { @@ -511,6 +555,7 @@ extension VoiceBroadcastPlaybackViewModel: VoiceMessageAudioPlayerDelegate { state.playingState.isLive = isLivePlayback isPlaybackInitialized = true displayLink.isPaused = false + resetErrorState() } func audioPlayerDidPausePlaying(_ audioPlayer: VoiceMessageAudioPlayer) { @@ -522,6 +567,9 @@ extension VoiceBroadcastPlaybackViewModel: VoiceMessageAudioPlayerDelegate { func audioPlayerDidStopPlaying(_ audioPlayer: VoiceMessageAudioPlayer) { MXLog.debug("[VoiceBroadcastPlaybackViewModel] audioPlayerDidStopPlaying") state.playbackState = .stopped + + updateErrorState() + state.playingState.isLive = false audioPlayer.deregisterDelegate(self) self.mediaServiceProvider.deregisterNowPlayingInfoDelegate(forPlayer: audioPlayer) @@ -531,11 +579,16 @@ extension VoiceBroadcastPlaybackViewModel: VoiceMessageAudioPlayerDelegate { func audioPlayer(_ audioPlayer: VoiceMessageAudioPlayer, didFailWithError error: Error) { state.playbackState = .error + self.updateErrorState() } func audioPlayerDidFinishPlaying(_ audioPlayer: VoiceMessageAudioPlayer) { MXLog.debug("[VoiceBroadcastPlaybackViewModel] audioPlayerDidFinishPlaying: \(audioPlayer.playerItems.count)") - stopIfVoiceBroadcastOver() + if hasAttachmentErrors { + stop() + } else { + stopIfVoiceBroadcastOver() + } } } diff --git a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/View/VoiceBroadcastPlaybackView.swift b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/View/VoiceBroadcastPlaybackView.swift index 6ac146ce2f..b4bcaa7aa6 100644 --- a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/View/VoiceBroadcastPlaybackView.swift +++ b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/View/VoiceBroadcastPlaybackView.swift @@ -114,7 +114,7 @@ struct VoiceBroadcastPlaybackView: View { .fixedSize(horizontal: false, vertical: true) .accessibilityIdentifier("decryptionErrorView") } - else if viewModel.viewState.playbackState == .error { + else if viewModel.viewState.showPlaybackError { VoiceBroadcastPlaybackErrorView() } else { HStack (spacing: 34.0) { diff --git a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackModels.swift b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackModels.swift index aeb1f4f61e..7a810a1673 100644 --- a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackModels.swift +++ b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackModels.swift @@ -59,6 +59,7 @@ struct VoiceBroadcastPlaybackViewState: BindableState { var playingState: VoiceBroadcastPlayingState var bindings: VoiceBroadcastPlaybackViewStateBindings var decryptionState: VoiceBroadcastPlaybackDecryptionState + var showPlaybackError: Bool } struct VoiceBroadcastPlaybackViewStateBindings { diff --git a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackScreenState.swift b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackScreenState.swift index 84f210d817..59b434ca96 100644 --- a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackScreenState.swift +++ b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/VoiceBroadcastPlaybackScreenState.swift @@ -43,7 +43,7 @@ enum MockVoiceBroadcastPlaybackScreenState: MockScreenState, CaseIterable { var screenView: ([Any], AnyView) { let details = VoiceBroadcastPlaybackDetails(senderDisplayName: "Alice", avatarData: AvatarInput(mxContentUri: "", matrixItemId: "!fakeroomid:matrix.org", displayName: "The name of the room")) - let viewModel = MockVoiceBroadcastPlaybackViewModel(initialViewState: VoiceBroadcastPlaybackViewState(details: details, broadcastState: .started, playbackState: .stopped, playingState: VoiceBroadcastPlayingState(duration: 10.0, isLive: true, canMoveForward: false, canMoveBackward: false), bindings: VoiceBroadcastPlaybackViewStateBindings(progress: 0), decryptionState: VoiceBroadcastPlaybackDecryptionState(errorCount: 0))) + let viewModel = MockVoiceBroadcastPlaybackViewModel(initialViewState: VoiceBroadcastPlaybackViewState(details: details, broadcastState: .started, playbackState: .stopped, playingState: VoiceBroadcastPlayingState(duration: 10.0, isLive: true, canMoveForward: false, canMoveBackward: false), bindings: VoiceBroadcastPlaybackViewStateBindings(progress: 0), decryptionState: VoiceBroadcastPlaybackDecryptionState(errorCount: 0), showPlaybackError: false)) return ( [false, viewModel], diff --git a/changelog.d/7311.change b/changelog.d/7311.change new file mode 100644 index 0000000000..d9b992237e --- /dev/null +++ b/changelog.d/7311.change @@ -0,0 +1 @@ +Improve error handling during a voice broadcast playback. From 28f0f34b43251b7d84d6c003f22bd749a49eaab5 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Wed, 1 Feb 2023 14:43:38 +0100 Subject: [PATCH 2/2] Cleanup --- .../MatrixSDK/VoiceBroadcastPlaybackViewModel.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift index af229c35ec..35933ffced 100644 --- a/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift +++ b/RiotSwiftUI/Modules/Room/VoiceBroadcastPlayback/MatrixSDK/VoiceBroadcastPlaybackViewModel.swift @@ -579,7 +579,7 @@ extension VoiceBroadcastPlaybackViewModel: VoiceMessageAudioPlayerDelegate { func audioPlayer(_ audioPlayer: VoiceMessageAudioPlayer, didFailWithError error: Error) { state.playbackState = .error - self.updateErrorState() + updateErrorState() } func audioPlayerDidFinishPlaying(_ audioPlayer: VoiceMessageAudioPlayer) {