From f9ad44f96a7276bb0629b2b22003084a4cf1b01d Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Tue, 24 Oct 2023 18:49:42 +0200 Subject: [PATCH] Fix playback and recording of voice messages (#1948) --- .../Mocks/Generated/GeneratedMocks.swift | 6 +- .../View/ComposerToolbar.swift | 4 +- .../Audio/Recorder/AudioRecorder.swift | 106 ++++++++++++++---- .../Recorder/AudioRecorderProtocol.swift | 3 +- .../MediaPlayer/MediaPlayerProvider.swift | 6 +- .../MediaPlayerProviderProtocol.swift | 1 + .../RoomTimelineController.swift | 1 + .../VoiceMessage/VoiceMessageRecorder.swift | 6 +- .../Sources/MediaPlayerProviderTests.swift | 33 +++--- ...est_encryptedHistoryRoomTimelineView.1.png | 4 +- 10 files changed, 115 insertions(+), 55 deletions(-) diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index 1cedf6dd64..318767ce18 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -447,11 +447,11 @@ class AudioRecorderMock: AudioRecorderProtocol { var deleteRecordingCalled: Bool { return deleteRecordingCallsCount > 0 } - var deleteRecordingClosure: (() -> Void)? + var deleteRecordingClosure: (() async -> Void)? - func deleteRecording() { + func deleteRecording() async { deleteRecordingCallsCount += 1 - deleteRecordingClosure?() + await deleteRecordingClosure?() } //MARK: - averagePowerForChannelNumber diff --git a/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift b/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift index 66a51e6018..da5f7f5134 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift @@ -243,7 +243,7 @@ struct ComposerToolbar: View { } stopRecording: { if let voiceMessageRecordingStartTime, Date.now.timeIntervalSince(voiceMessageRecordingStartTime) < voiceMessageMinimumRecordingDuration { context.send(viewAction: .cancelVoiceMessageRecording) - withAnimation { + withElementAnimation { showVoiceMessageRecordingTooltip = true } } else { @@ -273,7 +273,7 @@ struct ComposerToolbar: View { .allowsHitTesting(false) .onAppear { DispatchQueue.main.asyncAfter(deadline: .now() + voiceMessageTooltipDuration) { - withAnimation { + withElementAnimation { showVoiceMessageRecordingTooltip = false } } diff --git a/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift b/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift index 6f34fafe74..90a851e679 100644 --- a/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift +++ b/ElementX/Sources/Services/Audio/Recorder/AudioRecorder.swift @@ -42,42 +42,38 @@ class AudioRecorder: NSObject, AudioRecorderProtocol, AVAudioRecorderDelegate { audioRecorder?.isRecording ?? false } + private let dispatchQueue = DispatchQueue(label: "io.element.elementx.audio_recorder", qos: .userInitiated) + private var stopped = false + func record(with recordID: AudioRecordingIdentifier) async -> Result { + stopped = false guard await requestRecordPermission() else { return .failure(.recordPermissionNotGranted) } - - let settings = [AVFormatIDKey: Int(kAudioFormatMPEG4AAC), - AVSampleRateKey: 48000, - AVEncoderBitRateKey: 128_000, - AVNumberOfChannelsKey: 1, - AVEncoderAudioQualityKey: AVAudioQuality.high.rawValue] - - do { - try AVAudioSession.sharedInstance().setCategory(.playAndRecord, mode: .default) - try AVAudioSession.sharedInstance().setActive(true) - let url = URL.temporaryDirectory.appendingPathComponent("voice-message-\(recordID.identifier).m4a") - audioRecorder = try AVAudioRecorder(url: url, settings: settings) - audioRecorder?.delegate = self - audioRecorder?.isMeteringEnabled = true - audioRecorder?.record() + let result = await startRecording(with: recordID) + switch result { + case .success: actionsSubject.send(.didStartRecording) - } catch { - MXLog.error("audio recording failed: \(error)") + case .failure(let error): actionsSubject.send(.didFailWithError(error: error)) } - return .success(()) + return result } func stopRecording() async { - guard let audioRecorder, audioRecorder.isRecording else { - return + await withCheckedContinuation { continuation in + stopRecording { + continuation.resume() + } } - audioRecorder.stop() } - func deleteRecording() { - audioRecorder?.deleteRecording() + func deleteRecording() async { + await withCheckedContinuation { continuation in + deleteRecording { + continuation.resume() + } + } } func peakPowerForChannelNumber(_ channelNumber: Int) -> Float { @@ -121,7 +117,69 @@ class AudioRecorder: NSObject, AudioRecorderProtocol, AVAudioRecorderDelegate { } } } - + + // MARK: - Private + + private func startRecording(with recordID: AudioRecordingIdentifier) async -> Result { + await withCheckedContinuation { continuation in + startRecording(with: recordID) { result in + continuation.resume(returning: result) + } + } + } + + private func startRecording(with recordID: AudioRecordingIdentifier, completion: @escaping (Result) -> Void) { + dispatchQueue.async { [weak self] in + guard let self, !self.stopped else { + completion(.failure(.recordingCancelled)) + return + } + let settings = [AVFormatIDKey: Int(kAudioFormatMPEG4AAC), + AVSampleRateKey: 48000, + AVEncoderBitRateKey: 128_000, + AVNumberOfChannelsKey: 1, + AVEncoderAudioQualityKey: AVAudioQuality.high.rawValue] + + do { + try AVAudioSession.sharedInstance().setCategory(.playAndRecord, mode: .default) + try AVAudioSession.sharedInstance().setActive(true) + let url = URL.temporaryDirectory.appendingPathComponent("voice-message-\(recordID.identifier).m4a") + audioRecorder = try AVAudioRecorder(url: url, settings: settings) + audioRecorder?.delegate = self + audioRecorder?.isMeteringEnabled = true + audioRecorder?.record() + completion(.success(())) + } catch { + MXLog.error("audio recording failed: \(error)") + completion(.failure(.genericError)) + } + } + } + + private func stopRecording(completion: @escaping () -> Void) { + dispatchQueue.async { [weak self] in + defer { + completion() + } + guard let self else { return } + stopped = true + guard let audioRecorder, audioRecorder.isRecording else { + return + } + audioRecorder.stop() + } + } + + private func deleteRecording(completion: @escaping () -> Void) { + dispatchQueue.async { [weak self] in + defer { + completion() + } + guard let self else { return } + audioRecorder?.deleteRecording() + } + } + // MARK: - AVAudioRecorderDelegate func audioRecorderDidFinishRecording(_ recorder: AVAudioRecorder, successfully success: Bool) { diff --git a/ElementX/Sources/Services/Audio/Recorder/AudioRecorderProtocol.swift b/ElementX/Sources/Services/Audio/Recorder/AudioRecorderProtocol.swift index 291d8371cc..13c1714d6c 100644 --- a/ElementX/Sources/Services/Audio/Recorder/AudioRecorderProtocol.swift +++ b/ElementX/Sources/Services/Audio/Recorder/AudioRecorderProtocol.swift @@ -33,6 +33,7 @@ extension AudioRecordingIdentifier { enum AudioRecorderError: Error { case genericError case recordPermissionNotGranted + case recordingCancelled } enum AudioRecorderAction { @@ -49,7 +50,7 @@ protocol AudioRecorderProtocol: AnyObject { func record(with recordID: AudioRecordingIdentifier) async -> Result func stopRecording() async - func deleteRecording() + func deleteRecording() async func averagePowerForChannelNumber(_ channelNumber: Int) -> Float } diff --git a/ElementX/Sources/Services/MediaPlayer/MediaPlayerProvider.swift b/ElementX/Sources/Services/MediaPlayer/MediaPlayerProvider.swift index 45258fd4f9..ca890bd1b5 100644 --- a/ElementX/Sources/Services/MediaPlayer/MediaPlayerProvider.swift +++ b/ElementX/Sources/Services/MediaPlayer/MediaPlayerProvider.swift @@ -48,7 +48,6 @@ class MediaPlayerProvider: MediaPlayerProviderProtocol { return audioPlayerStates[audioPlayerStateID] } - @MainActor func register(audioPlayerState: AudioPlayerState) { guard let audioPlayerStateID = audioPlayerStateID(for: audioPlayerState.id) else { MXLog.error("Failed to build a key to register this audioPlayerState: \(audioPlayerState)") @@ -57,7 +56,6 @@ class MediaPlayerProvider: MediaPlayerProviderProtocol { audioPlayerStates[audioPlayerStateID] = audioPlayerState } - @MainActor func unregister(audioPlayerState: AudioPlayerState) { guard let audioPlayerStateID = audioPlayerStateID(for: audioPlayerState.id) else { MXLog.error("Failed to build a key to register this audioPlayerState: \(audioPlayerState)") @@ -66,12 +64,12 @@ class MediaPlayerProvider: MediaPlayerProviderProtocol { audioPlayerStates[audioPlayerStateID] = nil } - func detachAllStates(except exception: AudioPlayerState?) async { + func detachAllStates(except exception: AudioPlayerState?) { for key in audioPlayerStates.keys { if let exception, key == audioPlayerStateID(for: exception.id) { continue } - await audioPlayerStates[key]?.detachAudioPlayer() + audioPlayerStates[key]?.detachAudioPlayer() } } diff --git a/ElementX/Sources/Services/MediaPlayer/MediaPlayerProviderProtocol.swift b/ElementX/Sources/Services/MediaPlayer/MediaPlayerProviderProtocol.swift index 6966dc5a02..e32e44a2b0 100644 --- a/ElementX/Sources/Services/MediaPlayer/MediaPlayerProviderProtocol.swift +++ b/ElementX/Sources/Services/MediaPlayer/MediaPlayerProviderProtocol.swift @@ -20,6 +20,7 @@ enum MediaPlayerProviderError: Error { case unsupportedMediaType } +@MainActor protocol MediaPlayerProviderProtocol { func player(for mediaSource: MediaSourceProxy) -> Result diff --git a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift index 1b3ef56890..7d03573f57 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift @@ -309,6 +309,7 @@ class RoomTimelineController: RoomTimelineControllerProtocol { if audioPlayer.state == .playing { audioPlayer.pause() } else { + audioPlayerState.attachAudioPlayer(audioPlayer) audioPlayer.play() } } diff --git a/ElementX/Sources/Services/VoiceMessage/VoiceMessageRecorder.swift b/ElementX/Sources/Services/VoiceMessage/VoiceMessageRecorder.swift index a20b9fc330..7731fe6b15 100644 --- a/ElementX/Sources/Services/VoiceMessage/VoiceMessageRecorder.swift +++ b/ElementX/Sources/Services/VoiceMessage/VoiceMessageRecorder.swift @@ -68,14 +68,14 @@ class VoiceMessageRecorder: VoiceMessageRecorderProtocol { func cancelRecording() async { await audioRecorder.stopRecording() - audioRecorder.deleteRecording() + await audioRecorder.deleteRecording() recordingURL = nil previewAudioPlayerState = nil } func deleteRecording() async { await stopPlayback() - audioRecorder.deleteRecording() + await audioRecorder.deleteRecording() previewAudioPlayerState = nil recordingURL = nil } @@ -189,7 +189,7 @@ class VoiceMessageRecorder: VoiceMessageRecorderProtocol { // Build the preview audio player let mediaSource = MediaSourceProxy(url: url, mimeType: mp4accMimeType) - guard case .success(let mediaPlayer) = mediaPlayerProvider.player(for: mediaSource), let audioPlayer = mediaPlayer as? AudioPlayerProtocol else { + guard case .success(let mediaPlayer) = await mediaPlayerProvider.player(for: mediaSource), let audioPlayer = mediaPlayer as? AudioPlayerProtocol else { return .failure(.previewNotAvailable) } previewAudioPlayer = audioPlayer diff --git a/UnitTests/Sources/MediaPlayerProviderTests.swift b/UnitTests/Sources/MediaPlayerProviderTests.swift index eb0eaaebd6..a93d550255 100644 --- a/UnitTests/Sources/MediaPlayerProviderTests.swift +++ b/UnitTests/Sources/MediaPlayerProviderTests.swift @@ -19,6 +19,7 @@ import Combine import Foundation import XCTest +@MainActor class MediaPlayerProviderTests: XCTestCase { private var mediaPlayerProvider: MediaPlayerProvider! @@ -41,7 +42,7 @@ class MediaPlayerProviderTests: XCTestCase { } let mediaSourceVideo = MediaSourceProxy(url: someURL, mimeType: "video/mp4") - switch mediaPlayerProvider.player(for: mediaSourceWithoutMimeType) { + switch mediaPlayerProvider.player(for: mediaSourceVideo) { case .failure(.unsupportedMediaType): // Ok break @@ -72,11 +73,11 @@ class MediaPlayerProviderTests: XCTestCase { // By default, there should be no player state XCTAssertNil(mediaPlayerProvider.playerState(for: audioPlayerStateId)) - let audioPlayerState = await AudioPlayerState(id: audioPlayerStateId, duration: 10.0) - await mediaPlayerProvider.register(audioPlayerState: audioPlayerState) + let audioPlayerState = AudioPlayerState(id: audioPlayerStateId, duration: 10.0) + mediaPlayerProvider.register(audioPlayerState: audioPlayerState) XCTAssertEqual(audioPlayerState, mediaPlayerProvider.playerState(for: audioPlayerStateId)) - await mediaPlayerProvider.unregister(audioPlayerState: audioPlayerState) + mediaPlayerProvider.unregister(audioPlayerState: audioPlayerState) XCTAssertNil(mediaPlayerProvider.playerState(for: audioPlayerStateId)) } @@ -84,17 +85,17 @@ class MediaPlayerProviderTests: XCTestCase { let audioPlayer = AudioPlayerMock() audioPlayer.actions = PassthroughSubject().eraseToAnyPublisher() - let audioPlayerStates = await Array(repeating: AudioPlayerState(id: .timelineItemIdentifier(.random), duration: 0), count: 10) + let audioPlayerStates = Array(repeating: AudioPlayerState(id: .timelineItemIdentifier(.random), duration: 0), count: 10) for audioPlayerState in audioPlayerStates { - await mediaPlayerProvider.register(audioPlayerState: audioPlayerState) - await audioPlayerState.attachAudioPlayer(audioPlayer) - let isAttached = await audioPlayerState.isAttached + mediaPlayerProvider.register(audioPlayerState: audioPlayerState) + audioPlayerState.attachAudioPlayer(audioPlayer) + let isAttached = audioPlayerState.isAttached XCTAssertTrue(isAttached) } - await mediaPlayerProvider.detachAllStates(except: nil) + mediaPlayerProvider.detachAllStates(except: nil) for audioPlayerState in audioPlayerStates { - let isAttached = await audioPlayerState.isAttached + let isAttached = audioPlayerState.isAttached XCTAssertFalse(isAttached) } } @@ -103,18 +104,18 @@ class MediaPlayerProviderTests: XCTestCase { let audioPlayer = AudioPlayerMock() audioPlayer.actions = PassthroughSubject().eraseToAnyPublisher() - let audioPlayerStates = await Array(repeating: AudioPlayerState(id: .timelineItemIdentifier(.random), duration: 0), count: 10) + let audioPlayerStates = Array(repeating: AudioPlayerState(id: .timelineItemIdentifier(.random), duration: 0), count: 10) for audioPlayerState in audioPlayerStates { - await mediaPlayerProvider.register(audioPlayerState: audioPlayerState) - await audioPlayerState.attachAudioPlayer(audioPlayer) - let isAttached = await audioPlayerState.isAttached + mediaPlayerProvider.register(audioPlayerState: audioPlayerState) + audioPlayerState.attachAudioPlayer(audioPlayer) + let isAttached = audioPlayerState.isAttached XCTAssertTrue(isAttached) } let exception = audioPlayerStates[1] - await mediaPlayerProvider.detachAllStates(except: exception) + mediaPlayerProvider.detachAllStates(except: exception) for audioPlayerState in audioPlayerStates { - let isAttached = await audioPlayerState.isAttached + let isAttached = audioPlayerState.isAttached if audioPlayerState == exception { XCTAssertTrue(isAttached) } else { diff --git a/UnitTests/__Snapshots__/PreviewTests/test_encryptedHistoryRoomTimelineView.1.png b/UnitTests/__Snapshots__/PreviewTests/test_encryptedHistoryRoomTimelineView.1.png index bd37c3995c..989432a732 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_encryptedHistoryRoomTimelineView.1.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_encryptedHistoryRoomTimelineView.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bcbd11ba0914a24d5a68406599a0ce1d061825c25af02a0fe04584d86f117cb5 -size 68842 +oid sha256:0503798dc7a22145db714caf0d301a34771da4ffa2de43d82d0b23499a743196 +size 96896