From 8a1f0b96df8b34f4a0982eb0e9f5f4092ab9a95f Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Thu, 9 Feb 2023 11:50:11 +0100 Subject: [PATCH] Fix some voice message issues (#7325, #7217) Fix #7325: prevent setting the audio session to inactive during recording Fix #7217: ensure that an audio player has its content loaded when it reaches the end to allow seek and replay. --- .../VoiceMessageAudioPlayer.swift | 11 ++++++- .../VoiceMessageAudioRecorder.swift | 17 ++++++---- .../VoiceMessageController.swift | 32 ++++++++++++++++++- .../VoiceMessageMediaServiceProvider.swift | 4 ++- .../VoiceMessagePlaybackController.swift | 2 ++ changelog.d/7217.bugfix | 1 + changelog.d/7325.bugfix | 1 + 7 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 changelog.d/7217.bugfix create mode 100644 changelog.d/7325.bugfix diff --git a/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioPlayer.swift b/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioPlayer.swift index 4a242a91ce..920e0aeb4e 100644 --- a/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioPlayer.swift +++ b/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioPlayer.swift @@ -53,7 +53,7 @@ class VoiceMessageAudioPlayer: NSObject { return false } - return (audioPlayer.rate > 0) + return audioPlayer.currentItem != nil && (audioPlayer.rate > 0) } var duration: TimeInterval { @@ -118,6 +118,13 @@ class VoiceMessageAudioPlayer: NSObject { } } + func reloadContentIfNeeded() { + if let url, let audioPlayer, audioPlayer.currentItem == nil { + self.url = nil + loadContentFromURL(url) + } + } + func removeAllPlayerItems() { audioPlayer?.removeAllItems() } @@ -130,6 +137,8 @@ class VoiceMessageAudioPlayer: NSObject { func play() { isStopped = false + reloadContentIfNeeded() + do { try AVAudioSession.sharedInstance().setCategory(AVAudioSession.Category.playback) try AVAudioSession.sharedInstance().setActive(true) diff --git a/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioRecorder.swift b/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioRecorder.swift index d82200109f..9ab618e97f 100644 --- a/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioRecorder.swift +++ b/Riot/Modules/Room/VoiceMessages/VoiceMessageAudioRecorder.swift @@ -73,15 +73,18 @@ class VoiceMessageAudioRecorder: NSObject, AVAudioRecorderDelegate { } } - func stopRecording() { + func stopRecording(releaseAudioSession: Bool = true) { audioRecorder?.stop() - do { - try AVAudioSession.sharedInstance().setActive(false) - } catch { - delegateContainer.notifyDelegatesWithBlock { delegate in - (delegate as? VoiceMessageAudioRecorderDelegate)?.audioRecorder(self, didFailWithError: VoiceMessageAudioRecorderError.genericError) } + + if releaseAudioSession { + MXLog.debug("[VoiceMessageAudioRecorder] stopRecording() - releasing audio session") + do { + try AVAudioSession.sharedInstance().setActive(false) + } catch { + delegateContainer.notifyDelegatesWithBlock { delegate in + (delegate as? VoiceMessageAudioRecorderDelegate)?.audioRecorder(self, didFailWithError: VoiceMessageAudioRecorderError.genericError) } + } } - } func peakPowerForChannelNumber(_ channelNumber: Int) -> Float { diff --git a/Riot/Modules/Room/VoiceMessages/VoiceMessageController.swift b/Riot/Modules/Room/VoiceMessages/VoiceMessageController.swift index 94d93dedb3..bd2ef8bf67 100644 --- a/Riot/Modules/Room/VoiceMessages/VoiceMessageController.swift +++ b/Riot/Modules/Room/VoiceMessages/VoiceMessageController.swift @@ -199,15 +199,25 @@ public class VoiceMessageController: NSObject, VoiceMessageToolbarViewDelegate, // MARK: - AudioRecorderDelegate func audioRecorderDidStartRecording(_ audioRecorder: VoiceMessageAudioRecorder) { + guard self.audioRecorder === audioRecorder else { + return + } notifiedRemainingTime = false updateUI() } func audioRecorderDidFinishRecording(_ audioRecorder: VoiceMessageAudioRecorder) { + guard self.audioRecorder === audioRecorder else { + return + } updateUI() } func audioRecorder(_ audioRecorder: VoiceMessageAudioRecorder, didFailWithError: Error) { + guard self.audioRecorder === audioRecorder else { + MXLog.error("[VoiceMessageController] audioRecorder failed but it's not the current one.") + return + } isInLockedMode = false updateUI() @@ -217,20 +227,34 @@ public class VoiceMessageController: NSObject, VoiceMessageToolbarViewDelegate, // MARK: - VoiceMessageAudioPlayerDelegate func audioPlayerDidStartPlaying(_ audioPlayer: VoiceMessageAudioPlayer) { + guard self.audioPlayer === audioPlayer else { + return + } updateUI() } func audioPlayerDidPausePlaying(_ audioPlayer: VoiceMessageAudioPlayer) { + guard self.audioPlayer === audioPlayer else { + return + } updateUI() } func audioPlayerDidStopPlaying(_ audioPlayer: VoiceMessageAudioPlayer) { + guard self.audioPlayer === audioPlayer else { + return + } updateUI() } func audioPlayerDidFinishPlaying(_ audioPlayer: VoiceMessageAudioPlayer) { + guard self.audioPlayer === audioPlayer else { + return + } audioPlayer.seekToTime(0.0) { [weak self] _ in self?.updateUI() + // Reload its content if necessary, otherwise the seek won't work + self?.audioPlayer?.reloadContentIfNeeded() } } @@ -280,7 +304,13 @@ public class VoiceMessageController: NSObject, VoiceMessageToolbarViewDelegate, isInLockedMode = false audioPlayer?.stop() - audioRecorder?.stopRecording() + + // Check if we are recording before stopping the recording, because it will try to pause the audio session and it can be problematic if another player or recorder is running + if let audioRecorder, audioRecorder.isRecording { + audioRecorder.stopRecording() + } + // Also, we can release it now, which will prevent the service provider from trying to manage an old audio recorder. + audioRecorder = nil deleteRecordingAtURL(temporaryFileURL) diff --git a/Riot/Modules/Room/VoiceMessages/VoiceMessageMediaServiceProvider.swift b/Riot/Modules/Room/VoiceMessages/VoiceMessageMediaServiceProvider.swift index c6dc8b8b4f..0a9283a3c4 100644 --- a/Riot/Modules/Room/VoiceMessages/VoiceMessageMediaServiceProvider.swift +++ b/Riot/Modules/Room/VoiceMessages/VoiceMessageMediaServiceProvider.swift @@ -191,7 +191,9 @@ import MediaPlayer continue } - audioRecorder.stopRecording() + // We should release the audio session only if we want to pause all services + let shouldReleaseAudioSession = (service == nil) + audioRecorder.stopRecording(releaseAudioSession: shouldReleaseAudioSession) } guard let audioPlayersEnumerator = audioPlayers.objectEnumerator() else { diff --git a/Riot/Modules/Room/VoiceMessages/VoiceMessagePlaybackController.swift b/Riot/Modules/Room/VoiceMessages/VoiceMessagePlaybackController.swift index 9bc0b705f2..59912aa48b 100644 --- a/Riot/Modules/Room/VoiceMessages/VoiceMessagePlaybackController.swift +++ b/Riot/Modules/Room/VoiceMessages/VoiceMessagePlaybackController.swift @@ -144,6 +144,8 @@ class VoiceMessagePlaybackController: VoiceMessageAudioPlayerDelegate, VoiceMess audioPlayer.seekToTime(0.0) { [weak self] _ in guard let self = self else { return } self.state = .stopped + // Reload its content if necessary, otherwise the seek won't work + self.audioPlayer?.reloadContentIfNeeded() } } diff --git a/changelog.d/7217.bugfix b/changelog.d/7217.bugfix new file mode 100644 index 0000000000..aaa993008b --- /dev/null +++ b/changelog.d/7217.bugfix @@ -0,0 +1 @@ +A voice message is now replayable. diff --git a/changelog.d/7325.bugfix b/changelog.d/7325.bugfix new file mode 100644 index 0000000000..5edaaba0d7 --- /dev/null +++ b/changelog.d/7325.bugfix @@ -0,0 +1 @@ +Fix an issue where a voice message recording was failing.