Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling during a voice broadcast playback #7321

Merged
merged 2 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -504,13 +547,15 @@ extension VoiceBroadcastPlaybackViewModel: VoiceBroadcastAggregatorDelegate {
// MARK: - VoiceMessageAudioPlayerDelegate
extension VoiceBroadcastPlaybackViewModel: VoiceMessageAudioPlayerDelegate {
func audioPlayerDidFinishLoading(_ audioPlayer: VoiceMessageAudioPlayer) {
updateErrorState()
}

func audioPlayerDidStartPlaying(_ audioPlayer: VoiceMessageAudioPlayer) {
state.playbackState = .playing
state.playingState.isLive = isLivePlayback
isPlaybackInitialized = true
displayLink.isPaused = false
resetErrorState()
}

func audioPlayerDidPausePlaying(_ audioPlayer: VoiceMessageAudioPlayer) {
Expand All @@ -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)
Expand All @@ -531,11 +579,16 @@ extension VoiceBroadcastPlaybackViewModel: VoiceMessageAudioPlayerDelegate {

func audioPlayer(_ audioPlayer: VoiceMessageAudioPlayer, didFailWithError error: Error) {
state.playbackState = .error
updateErrorState()
}

func audioPlayerDidFinishPlaying(_ audioPlayer: VoiceMessageAudioPlayer) {
MXLog.debug("[VoiceBroadcastPlaybackViewModel] audioPlayerDidFinishPlaying: \(audioPlayer.playerItems.count)")
stopIfVoiceBroadcastOver()
if hasAttachmentErrors {
stop()
} else {
stopIfVoiceBroadcastOver()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ struct VoiceBroadcastPlaybackViewState: BindableState {
var playingState: VoiceBroadcastPlayingState
var bindings: VoiceBroadcastPlaybackViewStateBindings
var decryptionState: VoiceBroadcastPlaybackDecryptionState
var showPlaybackError: Bool
}

struct VoiceBroadcastPlaybackViewStateBindings {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
1 change: 1 addition & 0 deletions changelog.d/7311.change
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve error handling during a voice broadcast playback.