From 6311792126446c91d528a7064fc1252f296340c2 Mon Sep 17 00:00:00 2001 From: GeneralD Date: Mon, 15 Jun 2026 04:06:17 +0900 Subject: [PATCH] perf(#255): emit now-playing artwork only on track changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The media-remote-helper fired a repeating timer that base64-encoded the now-playing artwork (hundreds of KB–several MB) and streamed it over IPC on every tick, pegging the daemon at ~28% CPU even when the track never changed. Tag each helper payload with an `event` ("track-change" | "tick") and attach `artwork_base64` only on track-change. The daemon backfills the cached cover on ticks and clears it when a track-change carries no artwork (genuinely cover-less track). Composes with the #270 decode memoization: #270 avoids re-decoding an unchanged base64, #255 avoids re-transmitting it. --- .claude/CLAUDE.md | 2 +- .../MediaRemoteDataSourceImpl.swift | 41 +++++-- .../Resources/media-remote-helper.swift | 30 ++++-- Sources/VersionHandler/Resources/version.txt | 2 +- .../MediaRemoteDataSourceImplTests.swift | 102 ++++++++++++++++-- 5 files changed, 153 insertions(+), 24 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 4062baa..4ae2bf0 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -220,7 +220,7 @@ Presenters subscribe to Interactors via Combine. Interactors access UseCases via ### Key Design Decisions -**MediaRemoteDataSource via swift-interpret helper**: `MediaRemote.framework` is a private framework, and `MRMediaRemoteGetNowPlayingInfo` only returns data when the **host process** carries an Apple-internal entitlement (`com.apple.private.tcc.allow` family). Apple-signed binaries — including `/usr/bin/swift` — qualify; any third-party binary (Developer ID, ad-hoc, anything notarized outside Apple) does **not**, because AMFI strips Apple-private entitlements from non-Apple-signed Mach-Os at load time. The helper Swift source (`Resources/media-remote-helper.swift`) is therefore spawned via the **absolute path** `/usr/bin/swift ` so that the Apple-signed `xcode_select` tool-shim is unconditionally the host process. **Never go through `/usr/bin/env swift`** — `env` respects `$PATH` and a developer with a Homebrew / swift.org / asdf swift earlier in `PATH` would silently fall into a non-Apple-signed binary, reintroducing the same regression. **Never pre-compile the helper with `swiftc`** — the resulting binary becomes the host, loses the entitlement, and `MRMediaRemoteGetNowPlayingInfo` silently returns no info on macOS 26+ (regression tracked in #261). The helper runs as a persistent subprocess and streams JSON over a pipe, using `MRMediaRemoteRegisterForNowPlayingNotifications` for event-driven updates. The 1–2 s `swift-frontend -interpret` cost on first launch is the price of admission; there is no Apple-supported alternative for third parties. +**MediaRemoteDataSource via swift-interpret helper**: `MediaRemote.framework` is a private framework, and `MRMediaRemoteGetNowPlayingInfo` only returns data when the **host process** carries an Apple-internal entitlement (`com.apple.private.tcc.allow` family). Apple-signed binaries — including `/usr/bin/swift` — qualify; any third-party binary (Developer ID, ad-hoc, anything notarized outside Apple) does **not**, because AMFI strips Apple-private entitlements from non-Apple-signed Mach-Os at load time. The helper Swift source (`Resources/media-remote-helper.swift`) is therefore spawned via the **absolute path** `/usr/bin/swift ` so that the Apple-signed `xcode_select` tool-shim is unconditionally the host process. **Never go through `/usr/bin/env swift`** — `env` respects `$PATH` and a developer with a Homebrew / swift.org / asdf swift earlier in `PATH` would silently fall into a non-Apple-signed binary, reintroducing the same regression. **Never pre-compile the helper with `swiftc`** — the resulting binary becomes the host, loses the entitlement, and `MRMediaRemoteGetNowPlayingInfo` silently returns no info on macOS 26+ (regression tracked in #261). The helper runs as a persistent subprocess and streams JSON over a pipe, using `MRMediaRemoteRegisterForNowPlayingNotifications` for event-driven updates. The 1–2 s `swift-frontend -interpret` cost on first launch is the price of admission; there is no Apple-supported alternative for third parties. **Artwork emission is scoped to track changes (#255)**: each JSON line carries an `event` tag (`"track-change"` for notification-driven + initial fetches, `"tick"` for the 3 s periodic snapshot). Base64-encoding the cover (hundreds of KB–several MB) on every tick pegged daemon CPU for no benefit, so `artwork_base64` is sent only on `track-change`; `MediaRemoteDataSourceImpl` backfills the cached cover on ticks and clears it when a `track-change` arrives cover-less. This composes with the daemon-side decode memoization (`lastArtworkBase64`/`lastArtworkData`, #270): #270 avoids re-*decoding* an unchanged payload, #255 avoids re-*transmitting* it. **ProcessGateway OS boundary**: `ProcessGateway` centralizes OS-bound work in Domain (resource sampling, process management, lock files, launchctl, executable discovery, streaming subprocesses). `DarwinGateway` is the live macOS implementation, and `DependencyInjection` wires it into handlers and data sources so application logic no longer reaches directly into `Process`, `flock`, `getrusage`, or `which`. diff --git a/Sources/MediaRemoteDataSource/MediaRemoteDataSourceImpl.swift b/Sources/MediaRemoteDataSource/MediaRemoteDataSourceImpl.swift index 7c4310b..002fb73 100644 --- a/Sources/MediaRemoteDataSource/MediaRemoteDataSourceImpl.swift +++ b/Sources/MediaRemoteDataSource/MediaRemoteDataSourceImpl.swift @@ -46,7 +46,9 @@ extension MediaRemoteDataSourceImpl: MediaRemoteDataSource { NowPlaying( title: json["title"] as? String, artist: json["artist"] as? String, - artworkData: artworkData(from: json["artwork_base64"] as? String), + artworkData: artworkData( + base64: json["artwork_base64"] as? String, + isTrackChange: (json["event"] as? String) == HelperEvent.trackChange.rawValue), duration: json["duration"] as? Double, rawElapsed: json["elapsed"] as? Double, playbackRate: json["rate"] as? Double ?? 1.0, @@ -98,15 +100,23 @@ extension MediaRemoteDataSourceImpl { state.iterator = nextIterator } - /// Decodes the artwork base64 payload, reusing the previous result while the - /// payload is unchanged. The helper re-broadcasts the full now-playing payload - /// on every event, so without memoization the (potentially megabyte-scale) - /// artwork would be re-decoded on each elapsed-time tick (#270). - private func artworkData(from base64: String?) -> Data? { - guard let base64 else { return nil } + /// Resolves the artwork for one payload, reusing the cached cover whenever the + /// helper omits it. The helper sends `artwork_base64` only on `track-change` + /// events; periodic `tick`s drop it to avoid re-base64-encoding the + /// (potentially megabyte-scale) image every interval (#255). Decoding is also + /// memoized so an unchanged base64 is decoded only once (#270). + private func artworkData(base64: String?, isTrackChange: Bool) -> Data? { state.lock.lock() defer { state.lock.unlock() } - if state.lastArtworkBase64 == base64 { return state.lastArtworkData } + guard let base64 else { + // No artwork field. On a tick the cover is unchanged — reuse the + // cache. On a track-change it means the new track genuinely has no + // cover, so drop the stale one instead of showing the previous track's. + guard isTrackChange else { return state.lastArtworkData } + state.clearArtwork() + return nil + } + guard state.lastArtworkBase64 != base64 else { return state.lastArtworkData } let decoded = decodeBase64(base64) state.lastArtworkBase64 = base64 state.lastArtworkData = decoded @@ -114,10 +124,25 @@ extension MediaRemoteDataSourceImpl { } } +extension MediaRemoteDataSourceImpl { + /// Wire-protocol event tag emitted by `media-remote-helper.swift`. Only + /// `track-change` payloads carry artwork; `tick` payloads omit it (#255). + fileprivate enum HelperEvent: String { + case trackChange = "track-change" + case tick + } +} + private final class StreamStateBox: @unchecked Sendable { let lock = NSLock() var iterator: AsyncStream.AsyncIterator? var isPolling = false var lastArtworkBase64: String? var lastArtworkData: Data? + + /// Drops the cached cover. The caller must already hold `lock`. + func clearArtwork() { + lastArtworkBase64 = nil + lastArtworkData = nil + } } diff --git a/Sources/MediaRemoteDataSource/Resources/media-remote-helper.swift b/Sources/MediaRemoteDataSource/Resources/media-remote-helper.swift index dc586cd..c8d4fee 100644 --- a/Sources/MediaRemoteDataSource/Resources/media-remote-helper.swift +++ b/Sources/MediaRemoteDataSource/Resources/media-remote-helper.swift @@ -13,7 +13,16 @@ typealias RegisterFn = @convention(c) (DispatchQueue) -> Void let getInfo = unsafeBitCast(sym, to: GetInfoFn.self) let register = unsafeBitCast(regSym, to: RegisterFn.self) -@Sendable func fetchAndPrint() { +// Distinguishes a genuine now-playing change (track switch, play/pause, seek — +// delivered via MediaRemote notifications) from a periodic snapshot refresh +// (`tick`). The client uses this to decide whether the artwork field is +// authoritative for the current track. See `MediaRemoteDataSourceImpl`. +enum Event: String { + case trackChange = "track-change" + case tick +} + +@Sendable func fetchAndPrint(event: Event) { getInfo(DispatchQueue.main) { dict in guard let d = dict as? [String: Any], let title = d["kMRMediaRemoteNowPlayingInfoTitle"] as? String, @@ -23,7 +32,7 @@ let register = unsafeBitCast(regSym, to: RegisterFn.self) fflush(stdout) return } - var r: [String: Any] = ["has_info": true] + var r: [String: Any] = ["has_info": true, "event": event.rawValue] r["title"] = d["kMRMediaRemoteNowPlayingInfoTitle"] r["artist"] = d["kMRMediaRemoteNowPlayingInfoArtist"] r["duration"] = d["kMRMediaRemoteNowPlayingInfoDuration"] @@ -35,7 +44,13 @@ let register = unsafeBitCast(regSym, to: RegisterFn.self) // per polling interval for timestamp-less sources). let ts = (d["kMRMediaRemoteNowPlayingInfoTimestamp"] as? Date) ?? Date() r["timestamp"] = ts.timeIntervalSinceReferenceDate - if let art = d["kMRMediaRemoteNowPlayingInfoArtworkData"] as? Data { + // Artwork is large (hundreds of KB–several MB). Base64-encoding it and + // streaming it over IPC on every periodic `tick` pegged the daemon's CPU + // for no benefit while the track was unchanged (#255), so it is emitted + // only on `track-change`; the client reuses the last cover on ticks. A + // track-change that carries no artwork signals a genuinely cover-less + // track, so the client clears its cache. + if event == .trackChange, let art = d["kMRMediaRemoteNowPlayingInfoArtworkData"] as? Data { r["artwork_base64"] = art.base64EncodedString() } if let json = try? JSONSerialization.data(withJSONObject: r), @@ -57,15 +72,16 @@ for name in [ ] { NotificationCenter.default.addObserver( forName: NSNotification.Name(name), object: nil, queue: .main - ) { _ in fetchAndPrint() } + ) { _ in fetchAndPrint(event: .trackChange) } } // Periodic fallback for snapshot refresh (rawElapsed/timestamp/playbackRate). // The client (LyricsPresenter) interpolates elapsed on every DisplayLink tick // from this snapshot, so 3s polling is sufficient for lyric sync. pause/seek // is delivered immediately via `kMRMediaRemoteNowPlayingInfoDidChangeNotification`. -Timer.scheduledTimer(withTimeInterval: 3.0, repeats: true) { _ in fetchAndPrint() } +// Ticks omit artwork (see fetchAndPrint) — only the lightweight snapshot fields. +Timer.scheduledTimer(withTimeInterval: 3.0, repeats: true) { _ in fetchAndPrint(event: .tick) } -// Initial fetch -fetchAndPrint() +// Initial fetch — carries artwork so the first frame has a cover. +fetchAndPrint(event: .trackChange) RunLoop.main.run() diff --git a/Sources/VersionHandler/Resources/version.txt b/Sources/VersionHandler/Resources/version.txt index b70ae75..7243b12 100644 --- a/Sources/VersionHandler/Resources/version.txt +++ b/Sources/VersionHandler/Resources/version.txt @@ -1 +1 @@ -2.14.1 +2.14.2 diff --git a/Tests/MediaRemoteDataSourceTests/MediaRemoteDataSourceImplTests.swift b/Tests/MediaRemoteDataSourceTests/MediaRemoteDataSourceImplTests.swift index dc1f8d5..a169934 100644 --- a/Tests/MediaRemoteDataSourceTests/MediaRemoteDataSourceImplTests.swift +++ b/Tests/MediaRemoteDataSourceTests/MediaRemoteDataSourceImplTests.swift @@ -232,6 +232,96 @@ struct MediaRemoteDataSourceImplTests { } } + @Test("tick payload without artwork reuses the last track-change cover (#255)") + func tickReusesLastArtwork() async throws { + let artwork = Data("cover".utf8).base64EncodedString() + let gateway = StreamingGateway(streamPlans: [ + [ + Self.jsonLine( + title: "Song", artist: "Artist", hasInfo: true, artworkBase64: artwork, + event: "track-change"), + Self.jsonLine(title: "Song", artist: "Artist", hasInfo: true, event: "tick"), + ] + ]) + let decoder = CountingDecoder() + + await withDependencies { + $0.processGateway = gateway + } operation: { + let dataSource = MediaRemoteDataSourceImpl(decodeBase64: decoder.decode) + let artworks = [await dataSource.poll(), await dataSource.poll()].map { + result -> Data? in + guard case .info(let nowPlaying) = result else { + Issue.record("Expected .info, got \(result)") + return nil + } + return nowPlaying.artworkData + } + + // The tick omits artwork, so the cached cover is reused — and decoded + // only once. + #expect(artworks == [Data("cover".utf8), Data("cover".utf8)]) + #expect(decoder.count == 1) + } + } + + @Test("track-change payload without artwork clears the cached cover (#255)") + func trackChangeWithoutArtworkClearsCache() async throws { + let artwork = Data("cover".utf8).base64EncodedString() + let gateway = StreamingGateway(streamPlans: [ + [ + Self.jsonLine( + title: "First", artist: "Artist", hasInfo: true, artworkBase64: artwork, + event: "track-change"), + Self.jsonLine(title: "Second", artist: "Artist", hasInfo: true, event: "track-change"), + Self.jsonLine(title: "Second", artist: "Artist", hasInfo: true, event: "tick"), + ] + ]) + let decoder = CountingDecoder() + + await withDependencies { + $0.processGateway = gateway + } operation: { + let dataSource = MediaRemoteDataSourceImpl(decodeBase64: decoder.decode) + let artworks = [ + await dataSource.poll(), await dataSource.poll(), await dataSource.poll(), + ].map { result -> Data? in + guard case .info(let nowPlaying) = result else { + Issue.record("Expected .info, got \(result)") + return nil + } + return nowPlaying.artworkData + } + + // The cover-less track-change drops the cache, and the following tick + // has nothing to reuse. + #expect(artworks == [Data("cover".utf8), nil, nil]) + #expect(decoder.count == 1) + } + } + + @Test("tick before any track-change yields no artwork (#255)") + func tickBeforeAnyArtworkYieldsNil() async throws { + let gateway = StreamingGateway(streamPlans: [ + [Self.jsonLine(title: "Song", artist: "Artist", hasInfo: true, event: "tick")] + ]) + let decoder = CountingDecoder() + + await withDependencies { + $0.processGateway = gateway + } operation: { + let dataSource = MediaRemoteDataSourceImpl(decodeBase64: decoder.decode) + let result = await dataSource.poll() + + guard case .info(let nowPlaying) = result else { + Issue.record("Expected .info, got \(result)") + return + } + #expect(nowPlaying.artworkData == nil) + #expect(decoder.count == 0) + } + } + @Test("poll spawns the helper via the Apple-signed swift interpreter") func pollInvokesInterpretMode() async throws { let gateway = StreamingGateway(streamPlans: [ @@ -261,7 +351,8 @@ struct MediaRemoteDataSourceImplTests { extension MediaRemoteDataSourceImplTests { fileprivate static func jsonLine( - title: String, artist: String, hasInfo: Bool, artworkBase64: String? = nil + title: String, artist: String, hasInfo: Bool, artworkBase64: String? = nil, + event: String? = nil ) -> String { let payload: [String: Any] = [ "has_info": hasInfo, @@ -272,12 +363,9 @@ extension MediaRemoteDataSourceImplTests { "rate": 1.0, "timestamp": 10.0, ] - let payloadWithArtwork = artworkBase64.map { - payload.merging(["artwork_base64": $0]) { _, new in new } - } - guard - let data = try? JSONSerialization.data(withJSONObject: payloadWithArtwork ?? payload) - else { return "" } + .merging(artworkBase64.map { ["artwork_base64": $0] } ?? [:]) { _, new in new } + .merging(event.map { ["event": $0] } ?? [:]) { _, new in new } + guard let data = try? JSONSerialization.data(withJSONObject: payload) else { return "" } return String(decoding: data, as: UTF8.self) } }