Fix: VoiceMessageMediaManager should only load ogg files

element-hq · Oct 4, 2023 · dcad4f9 · dcad4f9
1 parent c31e250
commit dcad4f9
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 40 deletions.
diff --git a/ElementX/Sources/Services/VoiceMessage/VoiceMessageCache.swift b/ElementX/Sources/Services/VoiceMessage/VoiceMessageCache.swift
@@ -29,18 +29,20 @@ class VoiceMessageCache {
         return newURL
     }
 
-    func fileExists(for mediaSource: MediaSourceProxy, withExtension fileExtension: String? = nil) -> Bool {
+    func fileURL(for mediaSource: MediaSourceProxy, withExtension fileExtension: String? = nil) -> URL? {
         var url = temporaryFilesFolderURL.appendingPathComponent(mediaSource.url.lastPathComponent)
         if let fileExtension {
             url = url.deletingPathExtension().appendingPathExtension(fileExtension)
         }
-        return FileManager.default.fileExists(atPath: url.path())
+        return FileManager.default.fileExists(atPath: url.path()) ? url : nil
     }
 
-    func cache(mediaSource: MediaSourceProxy, using fileURL: URL) throws {
+    func cache(mediaSource: MediaSourceProxy, using fileURL: URL) throws -> URL {
         setupTemporaryFilesFolder()
         let url = cacheURL(for: mediaSource)
+        try? FileManager.default.removeItem(at: url)
         try FileManager.default.copyItem(at: fileURL, to: url)
+        return url
     }
 
     func clearCache() {
@@ -56,12 +58,8 @@ class VoiceMessageCache {
     // MARK: - Private
 
     private func setupTemporaryFilesFolder() {
-        let url = temporaryFilesFolderURL
-        guard !FileManager.default.directoryExists(at: url) else {
-            return
-        }
         do {
-            try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true, attributes: nil)
+            try FileManager.default.createDirectoryIfNeeded(at: temporaryFilesFolderURL, withIntermediateDirectories: true)
         } catch {
             MXLog.error("Failed to setup audio cache manager.")
         }

diff --git a/ElementX/Sources/Services/VoiceMessage/VoiceMessageMediaManager.swift b/ElementX/Sources/Services/VoiceMessage/VoiceMessageMediaManager.swift
@@ -24,10 +24,9 @@ class VoiceMessageMediaManager: VoiceMessageMediaManagerProtocol {
     private let mediaProvider: MediaProviderProtocol
     private let cache: VoiceMessageCache
 
-    private let supportedMimeTypePrefix = "audio/"
-    /// File extensions supported for playing voice messages without conversion
-    private let supportedAudioExtensions = ["mp3", "mp4", "m4a", "wav", "aac"]
-    /// Preferred audio file extension
+    private let supportedVoiceMessageMimeType = "audio/ogg"
+
+    /// Preferred audio file extension after conversion
     private let preferredAudioExtension = "m4a"
 
     init(mediaProvider: MediaProviderProtocol) {
@@ -40,40 +39,29 @@ class VoiceMessageMediaManager: VoiceMessageMediaManagerProtocol {
     }
 
     func loadVoiceMessageFromSource(_ source: MediaSourceProxy, body: String?) async throws -> URL {
-        guard let mimeType = source.mimeType, mimeType.starts(with: supportedMimeTypePrefix) else {
+        guard let mimeType = source.mimeType, mimeType == supportedVoiceMessageMimeType else {
             throw VoiceMessageMediaManagerError.unsupportedMimeTye
         }
 
-        if !cache.fileExists(for: source) {
-            guard case .success(let fileHandle) = await mediaProvider.loadFileFromSource(source, body: body) else {
-                throw MediaProviderError.failedRetrievingFile
-            }
-            try cache.cache(mediaSource: source, using: fileHandle.url)
+        // Do we already have a converted version?
+        if let fileURL = cache.fileURL(for: source, withExtension: preferredAudioExtension) {
+            return fileURL
         }
 
-        var url = cache.cacheURL(for: source)
-
-        // Convert from ogg if needed
-        if !hasSupportedAudioExtension(url) {
-            let audioConverter = AudioConverter()
-            let originalURL = url
-            url = cache.cacheURL(for: source, replacingExtension: preferredAudioExtension)
-            // Do we already have a converted version?
-            if !cache.fileExists(for: source, withExtension: preferredAudioExtension) {
-                try audioConverter.convertToMPEG4AAC(sourceURL: originalURL, destinationURL: url)
-            }
-
-            // we don't need the original file anymore
-            try? FileManager.default.removeItem(at: originalURL)
+        // Otherwise, load the file from source
+        guard case .success(let fileHandle) = await mediaProvider.loadFileFromSource(source, body: body) else {
+            throw MediaProviderError.failedRetrievingFile
         }
+        let fileURL = try cache.cache(mediaSource: source, using: fileHandle.url)
+
+        // Convert from ogg
+        let audioConverter = AudioConverter()
+        let convertedFileURL = cache.cacheURL(for: source, replacingExtension: preferredAudioExtension)
+        try audioConverter.convertToMPEG4AAC(sourceURL: fileURL, destinationURL: convertedFileURL)
 
-        return url
-    }
-
-    // MARK: - Private
-
-    /// Returns true if the URL has a supported audio extension
-    private func hasSupportedAudioExtension(_ url: URL) -> Bool {
-        supportedAudioExtensions.contains(url.pathExtension.lowercased())
+        // we don't need the original file anymore
+        try? FileManager.default.removeItem(at: fileURL)
+
+        return convertedFileURL
     }
 }