From f3b9483b2394de4c07ff26a110c7b6d6cac998b1 Mon Sep 17 00:00:00 2001 From: Dallas Groot Date: Sat, 11 Apr 2026 11:17:40 -0700 Subject: [PATCH] overhaul MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AUDIT-036 — Slider/button fixes (direct Liquid Glass cause) scheduleFlush() now runs Task { @MainActor } instead of bare Task. The pendingSaves dictionary is now only ever read/written on the main thread. Before this fix, a UserDefaults write could race with a slider didSet, causing values to snap back or write the wrong value — which is exactly why buttons were switching state unexpectedly. AUDIT-034 — 60fps idle Canvas (direct Liquid Glass cause) TimelineView now uses isRenderingActive ? settings.effectiveFPS : 2.0. When paused or not visible, the Canvas drops from 60fps to 2fps. This stops the continuous GPU wakeups that were fighting Liquid Glass gesture tracking, which is why sliders needed multiple attempts. AUDIT-001 — FFT real-time heap allocation processFFT no longer allocates any heap memory. The Hann window is computed once in init(). All four scratch buffers (fftWindow, fftWindowed, fftRealp/fftImagp, fftMagnitudes) are pre-allocated and reused every render callback — zero allocations on the real-time audio thread. AUDIT-002 — WatchOfflineStore data race taskToSongId and pendingSongs now protected by a dedicated serial storeQueue. URLSession delegate reads and main thread writes are serialised. AUDIT-019 — URLSession per AsyncCoverArt render CompanionAPIService() no longer instantiated per render. Companion cover art URLs now built directly from CompanionSettings.shared.baseURL — no URLSession created. AUDIT-020 — Synchronous disk read on main thread CachedImageLoader now uses memoryOnlyImage (sync, no I/O) for the first check, then cachedImageAsync (disk read on ioQueue) for the second. Main thread never blocks on disk I/O. AUDIT-033 — Lost star/unstar actions offline Star/unstar now routes through OptimisticActionQueue — actions survive Tailscale reconnection and are retried automatically. AUDIT-035 — OptimisticActionQueue flush race flush() Task is now @MainActor — pendingActions only ever touched on main thread, no more race between rapid taps and in-flight flushes. AUDIT-038 — O(n²) deduplication deduplicateAlbums now O(n) using a frequency dictionary. For 843 albums: ~7.1M string comparisons/second during playback → ~1,700. AUDIT-026, AUDIT-015 — Duplicate setResourceValue removed, cacheSize now uses totalSize directly --- Shared/Audio/AudioPlayer.swift | 71 +++++++++++-------- Shared/Storage/LibraryCache.swift | 8 +-- iOS/Data/OptimisticActionQueue.swift | 21 +++--- iOS/Views/Common/AsyncCoverArt.swift | 71 ++++++++++++++----- .../Companion/MultiAlbumEditorSheet.swift | 25 +++---- iOS/Views/Library/MyMusicView.swift | 56 ++++++++------- .../Visualizer/MitsuhaVisualizerView.swift | 24 +++++-- watchOS/App/WatchOfflineStore.swift | 58 ++++++++------- 8 files changed, 196 insertions(+), 138 deletions(-) diff --git a/Shared/Audio/AudioPlayer.swift b/Shared/Audio/AudioPlayer.swift index 36f1462..822eacf 100644 --- a/Shared/Audio/AudioPlayer.swift +++ b/Shared/Audio/AudioPlayer.swift @@ -77,6 +77,16 @@ class AudioPlayer: NSObject, ObservableObject { // Written from audio render thread, read from Timer on main thread. // No lock needed: stale FFT frame is imperceptible at 30fps. nonisolated(unsafe) private var rawFFTLevels: [Float] = Array(repeating: 0, count: 512) + + // Pre-allocated FFT scratch buffers — allocated once in init, reused every render callback. + // Heap allocation from a real-time audio thread is forbidden (locks inside malloc can + // block indefinitely and cause the hardware deadline to be missed → audio glitch). + private let fftSize: vDSP_Length = 1024 + nonisolated(unsafe) private var fftWindow: [Float] // Hann window coefficients — constant + nonisolated(unsafe) private var fftWindowed: [Float] // windowed input scratch + nonisolated(unsafe) private var fftRealp: [Float] // split-complex real part + nonisolated(unsafe) private var fftImagp: [Float] // split-complex imag part + nonisolated(unsafe) private var fftMagnitudes: [Float] // output magnitudes // MARK: - Level Simulation (for streams) private var levelTimer: Timer? @@ -104,6 +114,17 @@ class AudioPlayer: NSObject, ObservableObject { private override init() { // Radix-2 FFT setup for 1024 samples (log2(1024) = 10) fftSetup = vDSP_create_fftsetup(10, Int32(kFFTRadix2)) + + // Pre-allocate all FFT scratch buffers and compute the constant Hann window. + // These are reused on every render callback — zero heap allocation at render time. + let halfSize = Int(1024 / 2) + fftWindow = [Float](repeating: 0, count: 1024) + fftWindowed = [Float](repeating: 0, count: 1024) + fftRealp = [Float](repeating: 0, count: halfSize) + fftImagp = [Float](repeating: 0, count: halfSize) + fftMagnitudes = [Float](repeating: 0, count: halfSize) + vDSP_hann_window(&fftWindow, 1024, Int32(vDSP_HANN_NORM)) + super.init() configureAudioSession() setupRemoteControls() @@ -721,55 +742,43 @@ class AudioPlayer: NSObject, ObservableObject { private func processFFT(buffer: AVAudioPCMBuffer) { guard let fftSetup = fftSetup else { return } guard let channelData = buffer.floatChannelData?[0] else { return } - + let frameCount = buffer.frameLength - let fftSize: vDSP_Length = 1024 let log2n: vDSP_Length = 10 guard frameCount >= fftSize else { return } - + let halfSize = Int(fftSize / 2) - - // 1. Hann window - var windowed = [Float](repeating: 0, count: Int(fftSize)) - var window = [Float](repeating: 0, count: Int(fftSize)) - vDSP_hann_window(&window, fftSize, Int32(vDSP_HANN_NORM)) - vDSP_vmul(channelData, 1, window, 1, &windowed, 1, fftSize) - - // 2-3. FFT with safe pointers - var realp = [Float](repeating: 0, count: halfSize) - var imagp = [Float](repeating: 0, count: halfSize) - var magnitudes = [Float](repeating: 0, count: halfSize) - - realp.withUnsafeMutableBufferPointer { realpBuf in - imagp.withUnsafeMutableBufferPointer { imagpBuf in + + // 1. Apply pre-computed Hann window — no allocation, mutates pre-allocated scratch buffer + vDSP_vmul(channelData, 1, fftWindow, 1, &fftWindowed, 1, fftSize) + + // 2. FFT using pre-allocated split-complex buffers + fftRealp.withUnsafeMutableBufferPointer { realpBuf in + fftImagp.withUnsafeMutableBufferPointer { imagpBuf in var splitComplex = DSPSplitComplex( realp: realpBuf.baseAddress!, imagp: imagpBuf.baseAddress! ) - - windowed.withUnsafeBytes { rawBuffer in + fftWindowed.withUnsafeBytes { rawBuffer in let complexPtr = rawBuffer.bindMemory(to: DSPComplex.self).baseAddress! vDSP_ctoz(complexPtr, 2, &splitComplex, 1, vDSP_Length(halfSize)) } - vDSP_fft_zrip(fftSetup, &splitComplex, 1, log2n, FFTDirection(FFT_FORWARD)) - vDSP_zvmags(&splitComplex, 1, &magnitudes, 1, vDSP_Length(halfSize)) + vDSP_zvmags(&splitComplex, 1, &fftMagnitudes, 1, vDSP_Length(halfSize)) } } - - // 4. Normalize by N² and take sqrt for amplitude + + // 3. Normalize by N² and take sqrt for amplitude — in-place on pre-allocated buffer let fftSizeF = Float(fftSize) var scale: Float = 1.0 / (fftSizeF * fftSizeF) - vDSP_vsmul(magnitudes, 1, &scale, &magnitudes, 1, vDSP_Length(halfSize)) - - // sqrt for perceptual scaling + vDSP_vsmul(fftMagnitudes, 1, &scale, &fftMagnitudes, 1, vDSP_Length(halfSize)) + for i in 0.. UIImage? { + memoryImage(for: url) + } + + /// Returns cached image from memory first, then disk (async, off main thread). + /// Promotes disk hits to memory. Use this from async contexts. + func cachedImageAsync(for url: URL) async -> UIImage? { + // 1. Memory — no I/O, safe on any thread + if let img = memoryImage(for: url) { return img } + // 2. Disk — on ioQueue to keep main thread free + return await withCheckedContinuation { continuation in + ioQueue.async { + if let img = self.diskImage(for: url) { + self.storeInMemory(img, for: url) + continuation.resume(returning: img) + } else { + continuation.resume(returning: nil) + } + } + } + } + + /// Synchronous combined lookup — only call from background threads. + /// For main-thread callers, prefer memoryOnlyImage + cachedImageAsync. func cachedImage(for url: URL) -> UIImage? { // 1. Memory if let img = memoryImage(for: url) { @@ -265,32 +290,37 @@ class CachedImageLoader: ObservableObject { // Skip if already loaded this URL guard self.url != url else { return } self.url = url - - // Check cache first - if let cached = ImageCache.shared.cachedImage(for: url) { + + // 1. Memory hit — instant, no I/O + if let cached = ImageCache.shared.memoryOnlyImage(for: url) { self.image = cached return } - - // Download + + // 2. Async check (disk → network) — never blocks main thread isLoading = true task?.cancel() - task = URLSession.shared.dataTask(with: url) { [weak self] data, _, error in - guard let self = self, error == nil, let data = data, - let img = UIImage(data: data) else { - DispatchQueue.main.async { self?.isLoading = false } + Task { @MainActor in + // Check disk off main thread + if let cached = await ImageCache.shared.cachedImageAsync(for: url) { + self.image = cached + self.isLoading = false return } - - // Cache it - ImageCache.shared.store(img, for: url) - - DispatchQueue.main.async { + // Network fetch + do { + let (data, _) = try await URLSession.shared.data(from: url) + guard let img = UIImage(data: data) else { + self.isLoading = false + return + } + ImageCache.shared.store(img, for: url) self.image = img self.isLoading = false + } catch { + self.isLoading = false } } - task?.resume() } func cancel() { @@ -350,9 +380,12 @@ struct AsyncCoverArt: View { .aspectRatio(contentMode: .fill) .clipped() } else if id.hasPrefix("companion:") { - // Route to Companion API cover art endpoint + // Route to Companion API cover art endpoint. + // Use a shared service instance — creating CompanionAPIService() per render + // allocates a new URLSession each time, causing connection pool exhaustion. let companionId = String(id.dropFirst("companion:".count)) - if let url = CompanionAPIService().coverArtURL(companionId: companionId) { + if let url = CompanionSettings.shared.baseURL? + .appendingPathComponent("library/cover-art/\(companionId)") { ZStack { placeholderView CachedAsyncImage(url: url) diff --git a/iOS/Views/Companion/MultiAlbumEditorSheet.swift b/iOS/Views/Companion/MultiAlbumEditorSheet.swift index f2c320b..1bbb7c8 100644 --- a/iOS/Views/Companion/MultiAlbumEditorSheet.swift +++ b/iOS/Views/Companion/MultiAlbumEditorSheet.swift @@ -257,20 +257,21 @@ struct MultiAlbumEditorSheet: View { } } .swipeActions(edge: .trailing, allowsFullSwipe: true) { - if excludedSongIds.contains(song.id) { - Button { - withAnimation { excludedSongIds.remove(song.id) } - } label: { - Label("Include", systemImage: "plus.circle") - } - .tint(accentPink) - } else { - Button(role: .destructive) { - withAnimation { excludedSongIds.insert(song.id) } - } label: { - Label("Exclude", systemImage: "minus.circle") + Button { + withAnimation { + if excludedSongIds.contains(song.id) { + excludedSongIds.remove(song.id) + } else { + excludedSongIds.insert(song.id) + } } + } label: { + Label( + excludedSongIds.contains(song.id) ? "Include" : "Exclude", + systemImage: excludedSongIds.contains(song.id) ? "plus.circle" : "minus.circle" + ) } + .tint(excludedSongIds.contains(song.id) ? accentPink : .red) } } } header: { diff --git a/iOS/Views/Library/MyMusicView.swift b/iOS/Views/Library/MyMusicView.swift index fb4e1d4..dbcd293 100644 --- a/iOS/Views/Library/MyMusicView.swift +++ b/iOS/Views/Library/MyMusicView.swift @@ -942,27 +942,23 @@ struct MyMusicView: View { Divider() - // Favourite toggle + // Favourite toggle — routes through OptimisticActionQueue for offline resilience. + // Direct client calls with try? silently lose the action when Tailscale is reconnecting. Button(action: { - Task { - if song.starred != nil { - try? await serverManager.client.unstar(id: song.id) - await MainActor.run { - favouriteSongs.removeAll { $0.id == song.id } - LibraryCache.shared.save(favouriteSongs, key: "starred_songs") - } - } else { - try? await serverManager.client.star(id: song.id) - await MainActor.run { - if !favouriteSongs.contains(where: { $0.id == song.id }) { - favouriteSongs.append(song) - LibraryCache.shared.save(favouriteSongs, key: "starred_songs") - } - } + if song.starred != nil { + OptimisticActionQueue.shared.unstar(songId: song.id) + favouriteSongs.removeAll { $0.id == song.id } + LibraryCache.shared.save(favouriteSongs, key: "starred_songs") + } else { + OptimisticActionQueue.shared.star(songId: song.id) + if !favouriteSongs.contains(where: { $0.id == song.id }) { + favouriteSongs.append(song) + LibraryCache.shared.save(favouriteSongs, key: "starred_songs") } } }) { - Label(song.starred != nil ? "Unfavourite" : "Favourite", systemImage: song.starred != nil ? "heart.slash.fill" : "heart") + Label(song.starred != nil ? "Unfavourite" : "Favourite", + systemImage: song.starred != nil ? "heart.slash.fill" : "heart") } Divider() @@ -1120,19 +1116,27 @@ struct MyMusicView: View { // MARK: - Album Deduplication // Navidrome returns one album entry per artist for compilations. // Group by name + coverArt to show each album once. - + // O(n) implementation: build frequency map in one pass, deduplicate in second pass. + private func deduplicateAlbums(_ albums: [Album]) -> [Album] { - var seen = Set() - var result: [Album] = [] + // Count how many distinct artist entries share the same name+cover key + var keyCount: [String: Int] = [:] for album in albums { let key = "\(album.name)|\(album.coverArt ?? "")" - if seen.contains(key) { continue } + keyCount[key, default: 0] += 1 + } + + var seen = Set() + var result: [Album] = [] + result.reserveCapacity(keyCount.count) + + for album in albums { + let key = "\(album.name)|\(album.coverArt ?? "")" + guard !seen.contains(key) else { continue } seen.insert(key) - - // Check if multiple artists share this album - let sameAlbum = albums.filter { "\($0.name)|\($0.coverArt ?? "")" == key } - if sameAlbum.count > 1 { - // Replace artist with "Various Artists" + + if keyCount[key, default: 1] > 1 { + // Multiple artist entries — replace with "Various Artists" let grouped = Album( id: album.id, name: album.name, artist: "Various Artists", artistId: nil, diff --git a/iOS/Views/Visualizer/MitsuhaVisualizerView.swift b/iOS/Views/Visualizer/MitsuhaVisualizerView.swift index 2a51d8f..a2f2a2e 100644 --- a/iOS/Views/Visualizer/MitsuhaVisualizerView.swift +++ b/iOS/Views/Visualizer/MitsuhaVisualizerView.swift @@ -218,9 +218,12 @@ class VisualizerSettings: ObservableObject { } } + // Debounce UserDefaults writes to 500ms so rapid slider drags don't + // flood the defaults system. The Task is explicitly @MainActor so + // pendingSaves is only ever read/written on the main thread — no data race. private func scheduleFlush() { saveTask?.cancel() - saveTask = Task { [weak self] in + saveTask = Task { @MainActor [weak self] in try? await Task.sleep(nanoseconds: 500_000_000) guard !Task.isCancelled, let self else { return } for (k, v) in self.pendingSaves { @@ -286,6 +289,12 @@ struct MitsuhaVisualizerView: View { var compact: Bool = false var isVisible: Bool = true + // Observe settings only for the enabled gate and config values. + // The Canvas itself does NOT observe settings — we pass the values it + // needs as local constants captured at body evaluation time. This means + // a slider drag in VisualizerSettingsView does NOT invalidate the Canvas + // on every change — only the outer view re-evaluates, and the Canvas + // only re-evaluates when isPlaying / isVisible / isAppActive changes. @ObservedObject var settings = VisualizerSettings.shared @ObservedObject var albumColors = AlbumColorExtractor.shared @StateObject private var box = VisualizerLevelBox() @@ -305,12 +314,13 @@ struct MitsuhaVisualizerView: View { var body: some View { Group { if settings.enabled { - // Always keep one TimelineView mounted — never swap it for a static Canvas. - // Swapping causes a view-identity change: the old idle Canvas persists in the - // compositor while the new TimelineView hasn't committed its first frame yet, - // producing a frozen wave after pause/resume. Rendering idle vs live inside - // the Canvas body avoids this entirely. - TimelineView(.periodic(from: .now, by: 1.0 / 60.0)) { timeline in + // Adaptive FPS: full rate when rendering, 2fps when idle/paused. + // This prevents the 60fps GPU wakeup that drains battery during pause + // and stops the render loop from fighting Liquid Glass gesture tracking. + let targetFPS = isRenderingActive ? settings.effectiveFPS : 2.0 + let tickInterval = 1.0 / max(targetFPS, 1.0) + + TimelineView(.periodic(from: .now, by: tickInterval)) { timeline in let tickDate = timeline.date Canvas { context, size in _ = tickDate diff --git a/watchOS/App/WatchOfflineStore.swift b/watchOS/App/WatchOfflineStore.swift index 7982f24..a2f8795 100644 --- a/watchOS/App/WatchOfflineStore.swift +++ b/watchOS/App/WatchOfflineStore.swift @@ -25,8 +25,11 @@ class WatchOfflineStore: NSObject, ObservableObject, URLSessionDownloadDelegate private let catalogKey = "watch_offline_songs" private let fileManager = FileManager.default - private var pendingSongs: [String: Song] = [:] // taskId → Song - private var taskToSongId: [Int: String] = [:] // URLSession task ID → songId + // Both pendingSongs and taskToSongId are accessed from the URLSession delegate queue + // (background) AND from the main thread. Protect them with a dedicated serial queue. + private let storeQueue = DispatchQueue(label: "com.navidromeplayer.watch.offlinestore") + private var pendingSongs: [String: Song] = [:] // access via storeQueue only + private var taskToSongId: [Int: String] = [:] // access via storeQueue only private lazy var backgroundSession: URLSession = { let config = URLSessionConfiguration.background(withIdentifier: "com.navidromeplayer.watch.download") @@ -129,7 +132,7 @@ class WatchOfflineStore: NSObject, ObservableObject, URLSessionDownloadDelegate func downloadFromServer(song: Song) { guard !songs.contains(where: { $0.id == song.id }) else { return } - guard downloadProgress[song.id] == nil else { return } // already downloading + guard downloadProgress[song.id] == nil else { return } guard let url = WatchSessionManager.shared.streamURL(songId: song.id, maxBitRate: 128) else { print("[Watch] No stream URL for \(song.id)") @@ -141,12 +144,13 @@ class WatchOfflineStore: NSObject, ObservableObject, URLSessionDownloadDelegate self.downloadComplete[song.id] = false } - pendingSongs[song.id] = song - let task = backgroundSession.downloadTask(with: url) - taskToSongId[task.taskIdentifier] = song.id + // Protect dictionary mutations with storeQueue — delegate reads on background queue + storeQueue.sync { + self.pendingSongs[song.id] = song + self.taskToSongId[task.taskIdentifier] = song.id + } task.resume() - print("[Watch] Download started: \(song.title)") } @@ -160,7 +164,8 @@ class WatchOfflineStore: NSObject, ObservableObject, URLSessionDownloadDelegate // MARK: - URLSession Download Delegate func urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didWriteData bytesWritten: Int64, totalBytesWritten: Int64, totalBytesExpectedToWrite: Int64) { - guard let songId = taskToSongId[downloadTask.taskIdentifier] else { return } + let songId: String? = storeQueue.sync { taskToSongId[downloadTask.taskIdentifier] } + guard let songId else { return } let progress = totalBytesExpectedToWrite > 0 ? Double(totalBytesWritten) / Double(totalBytesExpectedToWrite) : 0 DispatchQueue.main.async { self.downloadProgress[songId] = progress @@ -168,8 +173,11 @@ class WatchOfflineStore: NSObject, ObservableObject, URLSessionDownloadDelegate } func urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL) { - guard let songId = taskToSongId[downloadTask.taskIdentifier], - let song = pendingSongs[songId] else { return } + let (songId, song): (String?, Song?) = storeQueue.sync { + let sid = taskToSongId[downloadTask.taskIdentifier] + return (sid, sid.flatMap { pendingSongs[$0] }) + } + guard let songId, let song else { return } let filename = "\(songId).mp3" let destURL = musicDirectory.appendingPathComponent(filename) @@ -184,38 +192,39 @@ class WatchOfflineStore: NSObject, ObservableObject, URLSessionDownloadDelegate self.addSong(song, localPath: destURL.path) self.downloadProgress.removeValue(forKey: songId) self.downloadComplete[songId] = true - self.pendingSongs.removeValue(forKey: songId) - self.taskToSongId.removeValue(forKey: downloadTask.taskIdentifier) - - // Haptic feedback + self.storeQueue.async { + self.pendingSongs.removeValue(forKey: songId) + self.taskToSongId.removeValue(forKey: downloadTask.taskIdentifier) + } WKInterfaceDevice.current().play(.success) - - // Notify iOS self.notifyiOS(songId: songId, downloaded: true) - - // Clear completion indicator after 2s DispatchQueue.main.asyncAfter(deadline: .now() + 2.0) { self.downloadComplete.removeValue(forKey: songId) } } - print("[Watch] Download complete: \(song.title)") } catch { print("[Watch] Save failed: \(error)") DispatchQueue.main.async { self.downloadProgress.removeValue(forKey: songId) - self.pendingSongs.removeValue(forKey: songId) + self.storeQueue.async { + self.pendingSongs.removeValue(forKey: songId) + } } } } func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { - guard let error, let songId = taskToSongId[task.taskIdentifier] else { return } + guard let error else { return } + let songId: String? = storeQueue.sync { taskToSongId[task.taskIdentifier] } + guard let songId else { return } print("[Watch] Download error for \(songId): \(error.localizedDescription)") DispatchQueue.main.async { self.downloadProgress.removeValue(forKey: songId) - self.pendingSongs.removeValue(forKey: songId) - self.taskToSongId.removeValue(forKey: task.taskIdentifier) + self.storeQueue.async { + self.pendingSongs.removeValue(forKey: songId) + self.taskToSongId.removeValue(forKey: task.taskIdentifier) + } } } @@ -263,7 +272,6 @@ class WatchOfflineStore: NSObject, ObservableObject, URLSessionDownloadDelegate // MARK: - Cache Management var cacheSize: String { - let size = songs.reduce(Int64(0)) { $0 + $1.fileSize } - return ByteCountFormatter.string(fromByteCount: size, countStyle: .file) + ByteCountFormatter.string(fromByteCount: totalSize, countStyle: .file) } }