diff --git a/Sources/Kaset/Services/Audio/DefaultOutputDeviceMonitor.swift b/Sources/Kaset/Services/Audio/DefaultOutputDeviceMonitor.swift new file mode 100644 index 000000000..a4b3f8f48 --- /dev/null +++ b/Sources/Kaset/Services/Audio/DefaultOutputDeviceMonitor.swift @@ -0,0 +1,315 @@ +import CoreAudio +import Foundation + +// MARK: - RouteChangeEvent + +/// One observed change of the system output device. +struct RouteChangeEvent: Equatable { + let at: ContinuousClock.Instant + /// True when the device that had been the default went away, as opposed to the user + /// switching outputs or a new device arriving. + let isDisappearance: Bool + /// Set once this disappearance has explained a pause, so it cannot explain another. + var isConsumed = false +} + +// MARK: - RouteChangeRecord + +/// Thread-safe log of recent system output-device changes. +/// +/// Core Audio delivers notifications on its own thread while these are read on the MainActor, +/// and consumers classify events by how closely they followed a route change — tens of +/// milliseconds. Storing this in actor-isolated state would date a change to whenever a hop +/// happened to be scheduled instead of to when the route actually changed. +/// +/// A log rather than a latest-pair: classification runs after an unbounded hop, so further +/// route events can land before a queued command is judged, and keeping only the newest pair +/// would discard the very event that explains it. +private final class RouteChangeRecord: @unchecked Sendable { + private let lock = NSLock() + private var events: [RouteChangeEvent] = [] + private var defaultDeviceID: AudioDeviceID? + /// Far longer than any classification window, and still a hard bound on growth. + private static let retention = Duration.seconds(30) + private static let capacity = 32 + + func prime(defaultDeviceID: AudioDeviceID?) { + self.lock.withLock { self.defaultDeviceID = defaultDeviceID } + } + + /// Appends a change, marking it a disappearance when the device that had been the default is + /// gone — the difference between headphones being unplugged and the user picking a different + /// output. + /// + /// `instant` must be captured at listener entry, before any contention. This lock is also + /// held by `claimRouteLoss`, so stamping the event after acquiring it would date a + /// disappearance *after* a pause that got there first — exactly the ordering the late + /// re-attribution exists to serve, silently defeated. + /// + /// The Core Audio queries run under the lock. A `nil` dispatch queue means the listener runs + /// directly on the notifying thread with no serialization, so overlapping callbacks would + /// otherwise interleave their reads and pair one callback's device ID with another's + /// usability result. Each transition is therefore consistent with the baseline it was + /// compared against, and the log is ordered the same way that baseline advanced: an arrival + /// stamp that predates the previous commit is clamped forward rather than sorted behind it, + /// which would describe a transition against a baseline that no longer applied. + /// Uncontended — the ordinary case — the stamp is exactly arrival time. + func recordChange( + at instant: ContinuousClock.Instant, + currentDefaultDeviceID: () -> AudioDeviceID?, + isDeviceUsable: (AudioDeviceID) -> Bool + ) { + self.lock.withLock { + let isDisappearance = self.defaultDeviceID.map { !isDeviceUsable($0) } ?? false + let at = self.events.last.map { max(instant, $0.at) } ?? instant + self.events.append(RouteChangeEvent(at: at, isDisappearance: isDisappearance)) + self.events.removeAll { at - $0.at > Self.retention } + if self.events.count > Self.capacity { + self.events.removeFirst(self.events.count - Self.capacity) + } + self.defaultDeviceID = currentDefaultDeviceID() + } + } + + func routeRestored(since instant: ContinuousClock.Instant) -> Bool { + self.lock.withLock { + DefaultOutputDeviceMonitor.routeRestored(in: self.events, since: instant) + } + } + + /// Claims the disappearance that explains a pause admitted at `instant`, if there is one. + /// + /// Claiming is what keeps a single disconnect from excusing every pause that follows it: + /// a user pressing Pause moments after the system already did must keep its own meaning. + func claimRouteLoss(admittedAt instant: ContinuousClock.Instant, within window: Duration) -> Bool { + self.lock.withLock { + guard let index = DefaultOutputDeviceMonitor.routeLossIndex( + in: self.events, + admittedAt: instant, + within: window + ) else { return false } + self.events[index].isConsumed = true + return true + } + } +} + +// MARK: - DefaultOutputDeviceMonitor + +/// Reports system default-output-device changes: Bluetooth connect/disconnect, +/// headphone plug/unplug, and manual output switches. +/// +/// `EqualizerService` installs its own listener, but that one is gated behind the +/// equalizer being enabled. Playback-state correctness must not depend on a +/// user-facing audio feature being switched on, so this monitor stays independent. +/// +/// Known limitation: this watches only which device is the default output. A route change +/// *within* one device — some Macs expose speakers and the headphone jack as data sources on +/// the same built-in device — leaves that selection untouched and goes unseen. Covering it +/// means also listening on the current device's output data source and retargeting those +/// listeners on every default-device change. Bluetooth, the case this was built for, always +/// swaps the device itself; consumers degrade to their pre-existing behavior when a change is +/// missed rather than misbehaving. +@MainActor +final class DefaultOutputDeviceMonitor { + static let shared = DefaultOutputDeviceMonitor() + + private var handler: (() -> Void)? + private var isListening = false + private let logger = DiagnosticsLogger.player + // swiftformat:disable:next modifierOrder + nonisolated private static let record = RouteChangeRecord() + + private init() {} + + /// Claims the disappearance explaining a pause admitted at `instant`, if there is one. + /// + /// macOS stops playback when a route vanishes by sending the app a `pause` remote command, + /// which is indistinguishable from the user pressing Pause. Pairing it with a device that + /// just went away is what tells the two apart — and why a device merely being added, or the + /// user switching outputs by hand, deliberately does not qualify. + /// + /// Each disappearance can explain at most one pause. Otherwise a user pressing Pause shortly + /// after the system already paused for the same disconnect would also read as system-driven, + /// losing their explicit intent and letting a later reconnect resume against it. + /// + /// `instant` must be when the command was admitted, not when it is being handled: commands + /// are drained onto the MainActor asynchronously, so measuring against the handling time + /// would let a delayed main actor age a genuine route pause out of the window, or backdate + /// a user pause into one. + nonisolated func claimRouteLossPause(admittedAt instant: ContinuousClock.Instant, within window: Duration) -> Bool { + Self.record.claimRouteLoss(admittedAt: instant, within: window) + } + + /// Pure decision behind ``claimRouteLossPause(admittedAt:within:)``: which logged + /// disappearance, if any, explains a pause admitted at `instant`. + /// + /// Reconstructs the route timeline *as of* `instant` rather than as of now. Classification + /// runs after an unbounded hop, so route events can land in between; judging by the latest + /// state would let a reconnect arriving after the pause retroactively turn a genuine + /// route-loss pause into a user pause and disable recovery. + nonisolated static func routeLossIndex( + in events: [RouteChangeEvent], + admittedAt instant: ContinuousClock.Instant, + within window: Duration + ) -> Int? { + guard let index = events.lastIndex(where: { event in + event.isDisappearance + && !event.isConsumed + && event.at <= instant + && instant - event.at <= window + }) else { return nil } + + // Output coming back between that disappearance and the command means there was a route + // to play to when the command was admitted, so the pause is the user's. A restoration + // after the command cannot explain a pause that had already been admitted. + let disappearance = events[index] + let restoredBeforeCommand = events.contains { event in + !event.isDisappearance && event.at > disappearance.at && event.at <= instant + } + return restoredBeforeCommand ? nil : index + } + + /// Whether output came back after `instant` and is still available. + /// + /// Not merely "something changed": a disconnect fires this monitor too, and under flapping + /// the newest event after a route-loss pause can be another disappearance. Resuming then + /// would start playback with no usable output — and because playback is already paused, that + /// second loss may produce no pause command to re-arm the marker. + nonisolated func routeRestored(since instant: ContinuousClock.Instant) -> Bool { + Self.record.routeRestored(since: instant) + } + + /// Pure decision behind ``routeRestored(since:)``. + nonisolated static func routeRestored( + in events: [RouteChangeEvent], + since instant: ContinuousClock.Instant + ) -> Bool { + guard let latest = events.last(where: { $0.at > instant }) else { return false } + return !latest.isDisappearance + } + + /// Installs `handler` and starts listening on first use. Later calls replace the handler. + func start(onChange handler: @escaping () -> Void) { + self.handler = handler + guard !self.isListening else { return } + + Self.record.prime(defaultDeviceID: Self.currentDefaultOutputDeviceID()) + + var address = Self.defaultOutputDeviceAddress + let status = AudioObjectAddPropertyListenerBlock( + AudioObjectID(kAudioObjectSystemObject), + &address, + nil, + Self.listener + ) + guard status == noErr else { + self.logger.warning("failed to listen for default-output changes: \(status)") + return + } + self.isListening = true + } + + private func notifyChange() { + self.handler?() + } + + // Core Audio invokes this on its own callback queue. It is a `nonisolated static` + // constant so it does not inherit MainActor isolation from the enclosing class — + // otherwise Swift 6's runtime isolation check trips with `dispatch_assert_queue_fail` + // the first time the block fires off-main. The hop to MainActor happens in the Task. + // swiftformat:disable:next modifierOrder + nonisolated private static let listener: + @Sendable (UInt32, UnsafePointer) -> Void = { _, _ in + // Before any lock contention, so the stamp is arrival time rather than acquisition. + let changedAt = ContinuousClock.now + DefaultOutputDeviceMonitor.record.recordChange( + at: changedAt, + currentDefaultDeviceID: DefaultOutputDeviceMonitor.currentDefaultOutputDeviceID, + isDeviceUsable: DefaultOutputDeviceMonitor.isDeviceUsable + ) + Task { @MainActor in + DefaultOutputDeviceMonitor.shared.notifyChange() + } + } + + // swiftformat:disable:next modifierOrder + nonisolated private static var defaultOutputDeviceAddress: AudioObjectPropertyAddress { + AudioObjectPropertyAddress( + mSelector: kAudioHardwarePropertyDefaultOutputDevice, + mScope: kAudioObjectPropertyScopeGlobal, + mElement: kAudioObjectPropertyElementMain + ) + } + + // swiftformat:disable:next modifierOrder + nonisolated private static func currentDefaultOutputDeviceID() -> AudioDeviceID? { + var address = Self.defaultOutputDeviceAddress + var deviceID = AudioDeviceID(0) + var size = UInt32(MemoryLayout.size) + let status = AudioObjectGetPropertyData( + AudioObjectID(kAudioObjectSystemObject), + &address, + 0, + nil, + &size, + &deviceID + ) + guard status == noErr, deviceID != AudioDeviceID(kAudioObjectUnknown) else { return nil } + return deviceID + } + + // Whether `deviceID` is still a device the system could play through. + // + // Core Audio can mark a device dead before dropping its ID from the device list, so being + // listed is not enough: an unplugged Bluetooth output often lingers as an ID whose + // `DeviceIsAlive` has already gone to zero, and treating that as present would classify + // the pause it triggers as the user's. + // swiftformat:disable:next modifierOrder + nonisolated private static func isDeviceUsable(_ deviceID: AudioDeviceID) -> Bool { + guard self.availableDeviceIDs().contains(deviceID) else { return false } + + var address = AudioObjectPropertyAddress( + mSelector: kAudioDevicePropertyDeviceIsAlive, + mScope: kAudioObjectPropertyScopeGlobal, + mElement: kAudioObjectPropertyElementMain + ) + var isAlive = UInt32(0) + var size = UInt32(MemoryLayout.size) + let status = AudioObjectGetPropertyData(deviceID, &address, 0, nil, &size, &isAlive) + // A device that cannot even answer the query is not one we can play through. + guard status == noErr else { return false } + return isAlive != 0 + } + + // swiftformat:disable:next modifierOrder + nonisolated private static func availableDeviceIDs() -> Set { + var address = AudioObjectPropertyAddress( + mSelector: kAudioHardwarePropertyDevices, + mScope: kAudioObjectPropertyScopeGlobal, + mElement: kAudioObjectPropertyElementMain + ) + var size = UInt32(0) + guard AudioObjectGetPropertyDataSize( + AudioObjectID(kAudioObjectSystemObject), + &address, + 0, + nil, + &size + ) == noErr, size > 0 else { return [] } + + var deviceIDs = [AudioDeviceID]( + repeating: 0, + count: Int(size) / MemoryLayout.size + ) + guard AudioObjectGetPropertyData( + AudioObjectID(kAudioObjectSystemObject), + &address, + 0, + nil, + &size, + &deviceIDs + ) == noErr else { return [] } + return Set(deviceIDs) + } +} diff --git a/Sources/Kaset/Services/Player/MusicPlaybackIntent.swift b/Sources/Kaset/Services/Player/MusicPlaybackIntent.swift index aced272c4..1bfcd6b3d 100644 --- a/Sources/Kaset/Services/Player/MusicPlaybackIntent.swift +++ b/Sources/Kaset/Services/Player/MusicPlaybackIntent.swift @@ -11,11 +11,31 @@ struct MusicPlaybackIntent: Equatable { let generation: UInt64 } +// MARK: - MusicPauseOrigin + +/// Why playback is being paused. The two are indistinguishable at the remote-command layer — +/// macOS asks an app to pause a vanished route exactly as it asks for a user's Pause — so the +/// distinction has to be carried explicitly. +enum MusicPauseOrigin: Equatable { + case user + /// A remote pause that no route loss explained when it arrived. Treated as the user's, but + /// its admission instant is kept so a route event published afterwards can still claim it. + case unattributedRemote(admittedAt: ContinuousClock.Instant) + /// The system took the audio route away, as of when the command was admitted. + case routeLoss(at: ContinuousClock.Instant) +} + // MARK: - MusicRemoteTransportCommand enum MusicRemoteTransportCommand: Equatable { case play - case pause + /// A remote `pause` no route loss explained at admission. Carries the admission instant so + /// a route event that lands afterwards can still re-attribute it. + case pause(admittedAt: ContinuousClock.Instant) + /// A `pause` the system imposed by taking the audio route away, not a user request. + /// Carries the ingress admission instant so the recovery marker is dated to when the + /// command arrived rather than to whenever the MainActor got around to draining it. + case pauseForRouteChange(admittedAt: ContinuousClock.Instant) case togglePlayPause case next case previous @@ -216,9 +236,23 @@ extension PlayerService { case .play: self.clearRemoteMusicSkipCoalescingTarget() await self.resume(intent: intent) - case .pause: + case let .pause(admittedAt): + self.clearRemoteMusicSkipCoalescingTarget() + await self.pause(intent: intent, origin: .unattributedRemote(admittedAt: admittedAt)) + // Classification ran before this command was enqueued, and this queue drains + // later. A disappearance recorded in between — whose callback found no marker to + // attribute — is still unclaimed, and nothing else is guaranteed to come back for + // it. Retry now that the marker exists, completing the symmetry: whichever of the + // route event and the pause lands second performs the attribution. + NowPlayingManager.shared.attributeRouteLossPauseIfPending() + case let .pauseForRouteChange(admittedAt): self.clearRemoteMusicSkipCoalescingTarget() - await self.pause(intent: intent) + await self.pause(intent: intent, origin: .routeLoss(at: admittedAt)) + // A reconnect can be recorded and handled before this pause drains, spending + // both recovery attempts while there was still no marker to act on. Schedule + // from here too, so installing the marker is itself a trigger. Harmless when + // no route has returned: the resume re-checks that for itself. + NowPlayingManager.shared.resumeAfterRouteRestoredSoon() case .togglePlayPause: self.clearRemoteMusicSkipCoalescingTarget() await self.playPause(intent: intent) diff --git a/Sources/Kaset/Services/Player/NowPlayingManager.swift b/Sources/Kaset/Services/Player/NowPlayingManager.swift index 540ea312a..904396a03 100644 --- a/Sources/Kaset/Services/Player/NowPlayingManager.swift +++ b/Sources/Kaset/Services/Player/NowPlayingManager.swift @@ -111,6 +111,20 @@ final class RemoteMusicCommandIngress: @unchecked Sendable { } } +// MARK: - NowPlayingInfoCenter + +/// The slice of `MPNowPlayingInfoCenter` the claim logic writes, so withdrawal can be tested +/// without a live system center. +@MainActor +protocol NowPlayingInfoCenter: AnyObject { + var nowPlayingInfo: [String: Any]? { get set } + var playbackState: MPNowPlayingPlaybackState { get set } +} + +// MARK: - MPNowPlayingInfoCenter + NowPlayingInfoCenter + +extension MPNowPlayingInfoCenter: NowPlayingInfoCenter {} + // MARK: - NowPlayingManager /// Manages remote-command routing and the app's Now Playing ownership. @@ -141,6 +155,13 @@ final class NowPlayingManager { private static let defaultSkipInterval: TimeInterval = 15 nonisolated static let nativeClaimServiceIdentifier = "com.sertacozercan.Kaset.native-now-playing-claim" + @ObservationIgnored private var routeRestoreTask: Task? + @ObservationIgnored private var routeRestoreGeneration: UInt64 = 0 + private static let routeRestoreDelays: [Duration] = [ + .milliseconds(500), + .milliseconds(2000), + ] + private init() {} // MARK: - Now Playing Claim @@ -151,8 +172,9 @@ final class NowPlayingManager { } /// What Kaset should tell the system Now Playing center for the current player state. - /// `handsOff` lets WebKit replace an existing fallback during active playback, while - /// `release` clears a native claim only when no resumable media remains. + /// Both non-claim cases withdraw Kaset's tagged entry — they differ only in why: + /// `handsOff` because WebKit's own Now Playing client owns the card during playback, + /// `release` because no resumable media remains. enum NowPlayingClaim: Equatable { case handsOff case release @@ -187,8 +209,19 @@ final class NowPlayingManager { } switch state { - case .playing, .buffering, .loading: + case .playing, .buffering: + // `buffering` is a stall inside playback that already started, so WebKit owns + // its card by now. Claiming here would recreate the competing second entry. + // Nothing in the music player assigns `buffering` today; if that changes, decide + // this case by whether playback was ever confirmed rather than by the state name, + // the way `activeVideo` already does with `isPlaybackConfirmed`. return .handsOff + case .loading: + // Playback is starting and WebKit has not published its card yet. Withdrawing + // here would leave the app with no Now Playing entry — and no media keys — for + // the whole load, so hold a playing-state claim until `.playing` hands over. + guard let track else { return .release } + return .claim(title: track.title, artist: track.artist, playbackState: .playing) case .idle, .paused, .ended, .error: guard let track else { return .release } return .claim(title: track.title, artist: track.artist, playbackState: .paused) @@ -209,25 +242,39 @@ final class NowPlayingManager { self.applyNowPlayingClaim(claim) } - /// Maps a claim onto `MPNowPlayingInfoCenter`. Hands-off only clears info we still own. + /// Maps a claim onto the Now Playing center, withdrawing our entry unless we want one. private func applyNowPlayingClaim(_ claim: NowPlayingClaim) { - let center = MPNowPlayingInfoCenter.default() + self.isAssertingNativeClaim = Self.applyClaim( + claim, + isAssertingNativeClaim: self.isAssertingNativeClaim, + to: MPNowPlayingInfoCenter.default() + ) + } + + /// Applies `claim` to `center`, returning whether Kaset asserts a native claim afterwards. + /// + /// Withdrawing is the interesting half. WebKit registers its own Now Playing client with the + /// system rather than writing `MPNowPlayingInfoCenter`, so a Kaset claim left in place during + /// playback does not get replaced — it survives as a second, competing entry for the same + /// app. Whichever entry was updated most recently owns the media keys, so a stale claim can + /// silently capture them and answer Play/Pause from a playback state that no longer matches + /// the page. Clearing here cannot disturb WebKit's card; they are separate clients. + /// + /// Only metadata carrying our tag is ever cleared, so a card published by anything else is + /// left alone. + @discardableResult + static func applyClaim( + _ claim: NowPlayingClaim, + isAssertingNativeClaim: Bool, + to center: any NowPlayingInfoCenter + ) -> Bool { switch claim { - case .handsOff: - guard self.isAssertingNativeClaim else { return } - guard Self.isNativeClaim(center.nowPlayingInfo) else { - self.isAssertingNativeClaim = false - return - } - // Preserve the fallback until WebKit atomically replaces the app-wide metadata. - // A non-destructive state update cannot clear a concurrently published WebKit card. - center.playbackState = .playing - case .release: - guard self.isAssertingNativeClaim else { return } - self.isAssertingNativeClaim = false - guard Self.isNativeClaim(center.nowPlayingInfo) else { return } + case .handsOff, .release: + guard isAssertingNativeClaim else { return false } + guard self.isNativeClaim(center.nowPlayingInfo) else { return false } center.playbackState = .stopped center.nowPlayingInfo = nil + return false case let .claim(title, artist, playbackState): var info: [String: Any] = [ MPMediaItemPropertyTitle: title, @@ -241,7 +288,7 @@ final class NowPlayingManager { case .playing: .playing case .paused: .paused } - self.isAssertingNativeClaim = true + return true } } @@ -303,11 +350,72 @@ final class NowPlayingManager { self.logger.info("NowPlayingManager configured") self.observeSettingsChanges() + self.observeOutputDeviceChanges() self.updateNowPlayingClaim() self.restartNowPlayingObservation() } + // MARK: - Transport Reconciliation + + private func observeOutputDeviceChanges() { + DefaultOutputDeviceMonitor.shared.start { [weak self] in + self?.handleOutputDeviceChange() + } + } + + /// Runs once the route event has been published, which is the earliest moment a pause that + /// beat it can be attributed correctly. + private func handleOutputDeviceChange() { + self.attributeRouteLossPauseIfPending() + self.resumeAfterRouteRestoredSoon() + } + + /// Attributes a pending unattributed pause to a route loss, if one now explains it. + /// + /// Safe to call from either side of the race and at any time: it is a no-op without a pending + /// pause, and the claim is one-shot, so a disconnect still explains at most one pause. + func attributeRouteLossPauseIfPending() { + guard let playerService = self.playerService else { return } + playerService.reattributeRemotePauseToRouteLoss { admittedAt in + DefaultOutputDeviceMonitor.shared.claimRouteLossPause( + admittedAt: admittedAt, + within: Self.routeLossPauseWindow + ) + } + guard playerService.routeLossPauseAt != nil else { return } + self.resumeAfterRouteRestoredSoon() + } + + /// Resumes playback that a vanished audio route had stopped, once a route is back. + /// + /// The device is not immediately usable when the notification lands, and WebKit settles a + /// route change in stages, so try twice rather than racing it once. Bursts of notifications + /// collapse into the latest request. `resumeAfterRouteRestored` itself decides whether a + /// resume is warranted, so both a disconnect and a reconnect can drive this safely. + func resumeAfterRouteRestoredSoon() { + self.routeRestoreGeneration &+= 1 + let generation = self.routeRestoreGeneration + self.routeRestoreTask?.cancel() + self.routeRestoreTask = Task { @MainActor [weak self] in + defer { + if self?.routeRestoreGeneration == generation { + self?.routeRestoreTask = nil + } + } + var elapsed = Duration.zero + for delay in Self.routeRestoreDelays { + try? await Task.sleep(for: delay - elapsed) + elapsed = delay + guard !Task.isCancelled, + let self, + self.routeRestoreGeneration == generation + else { return } + await self.playerService?.resumeAfterRouteRestored() + } + } + } + /// Registers the YouTube video player for media-key routing. /// Additive: without this call (or when video is inactive), all commands /// route to the music player exactly as before. @@ -486,7 +594,14 @@ final class NowPlayingManager { issuedAtMilliseconds: capturedCommand.issuedAtMilliseconds ) } else { - self.enqueueMusicRemoteCommand(.pause, capturedCommand: capturedCommand, player: player) + let command: MusicRemoteTransportCommand = Self.isRouteChangePause(capturedCommand) + ? .pauseForRouteChange(admittedAt: capturedCommand.admittedAt) + : .pause(admittedAt: capturedCommand.admittedAt) + self.enqueueMusicRemoteCommand( + command, + capturedCommand: capturedCommand, + player: player + ) } case .togglePlayPause: if self.routesToYouTubeVideo, let youtube = self.youtubePlayerService { @@ -538,6 +653,30 @@ final class NowPlayingManager { } } + /// Whether the `pause` we just received is the system reacting to a vanished audio route + /// (unplugged headphones, a Bluetooth device disconnecting) rather than a user request. + /// + /// macOS delivers both as the same remote command, so the only thing separating them is + /// that a route-driven one lands immediately after the output device it was playing to + /// disappears — measured at 40–60ms. The window absorbs scheduling jitter while staying far + /// below the time it takes a person to reach for the key, and requiring a *disappearance* + /// rather than any device change keeps a deliberate pause next to a manual output switch + /// from being mistaken for one. + /// + /// The comparison anchors on when the ingress admitted the command, not on when this drain + /// runs: the hop to the MainActor is unbounded, and dating the decision to handling time + /// would let a busy main actor push a real route pause out of the window. + private static func isRouteChangePause(_ capturedCommand: CapturedRemoteMusicCommand) -> Bool { + DefaultOutputDeviceMonitor.shared.claimRouteLossPause( + admittedAt: capturedCommand.admittedAt, + within: self.routeLossPauseWindow + ) + } + + /// How closely a `pause` must follow a device disappearing to be attributed to it. Measured + /// at 40-60ms; generous enough for scheduling jitter, far below human reaction time. + private static let routeLossPauseWindow = Duration.milliseconds(1500) + private func handleNextPreviousMediaKey( direction: RemoteMusicCommandDirection, capturedCommand: CapturedRemoteMusicCommand, diff --git a/Sources/Kaset/Services/Player/PlayerService+PlaybackControls.swift b/Sources/Kaset/Services/Player/PlayerService+PlaybackControls.swift index 0798e853e..e5d273922 100644 --- a/Sources/Kaset/Services/Player/PlayerService+PlaybackControls.swift +++ b/Sources/Kaset/Services/Player/PlayerService+PlaybackControls.swift @@ -368,6 +368,8 @@ extension PlayerService { guard shouldHideMiniPlayer || didStartPlayback || shouldRecordInteraction else { return } self.showMiniPlayer = false + self.routeLossPauseAt = nil + self.unattributedRemotePauseAt = nil self.state = .playing if shouldRecordInteraction { @@ -483,12 +485,39 @@ extension PlayerService { await self.pause(intent: intent) } - func pause(intent: MusicPlaybackIntent) async { + /// Pauses playback. + /// + /// `origin` separates a pause the system imposed from one the user asked for — losing an + /// audio route arrives as an ordinary `pause` remote command. A system pause must not record + /// a standing intent to stay paused: `isExplicitPauseIntentActive` makes the observer + /// re-pause the page the instant anything resumes it, so a media key handled by WebKit would + /// start playback and be killed a moment later, and clearing `shouldResumeAfterInterruption` + /// additionally blocks transport recovery from resuming. + func pause(intent: MusicPlaybackIntent, origin: MusicPauseOrigin = .user) async { guard self.acceptsMusicPlaybackIntent(intent) else { return } - self.logger.debug("Pausing playback") - self.shouldResumeAfterInterruption = false + self.logger.debug("Pausing playback (origin: \(String(describing: origin)))") self.isAwaitingPlaybackConfirmation = false - self.isExplicitPauseIntentActive = true + switch origin { + case .user: + self.shouldResumeAfterInterruption = false + self.isExplicitPauseIntentActive = true + self.routeLossPauseAt = nil + self.unattributedRemotePauseAt = nil + case let .unattributedRemote(admittedAt): + // Behaves as the user's until proven otherwise. Core Audio publishes a route change + // only after several synchronous device queries while this command drains + // independently, so a genuine route-loss pause can arrive first and look deliberate. + self.shouldResumeAfterInterruption = false + self.isExplicitPauseIntentActive = true + self.routeLossPauseAt = nil + self.unattributedRemotePauseAt = admittedAt + case let .routeLoss(at): + self.unattributedRemotePauseAt = nil + // Dated to when the command was admitted, not to now: this runs after an unbounded + // hop to the MainActor, and a marker stamped late would sort *after* a reconnect + // that already happened, leaving `hasAudioRouteChanged` permanently false. + self.routeLossPauseAt = at + } if self.isPendingRestoredLoadDeferred { self.state = .paused @@ -525,9 +554,59 @@ extension PlayerService { await self.resume(intent: intent) } + /// Re-attributes a remote pause to the route loss that caused it, once the route event is + /// published and `claim` confirms it explains that pause. + /// + /// Classification at admission is a race the pause can win: the Core Audio listener runs + /// several synchronous device queries before publishing, so the command may be judged + /// against a route log that does not yet contain the disappearance. Retrying from the other + /// side closes it — the route event is the thing that arrives late, not the pause. + func reattributeRemotePauseToRouteLoss(claim: (ContinuousClock.Instant) -> Bool) { + // The marker is checked before `claim` on purpose: this can run before the pause has + // drained, and consuming the disappearance here would leave nothing for the admission + // classification to find. Whichever of the two arrives second does the attribution. + guard let admittedAt = self.unattributedRemotePauseAt, + !self.isPlaying, + claim(admittedAt) + else { return } + self.unattributedRemotePauseAt = nil + self.shouldResumeAfterInterruption = true + self.isExplicitPauseIntentActive = false + self.routeLossPauseAt = admittedAt + self.logger.info("Re-attributed a remote pause to the audio route loss that caused it") + } + + /// Resumes playback that the system stopped when its audio route disappeared, once a route + /// is available again. + /// + /// The offer stands for as long as the pause does — there is no expiry. Intent, not elapsed + /// time, is what retires it: a user pause or playback actually starting clears + /// `routeLossPauseAt`, so a resume can only ever continue exactly what the route loss + /// interrupted. Reconnecting headphones an hour later is still the same interrupted song. + /// + /// The marker deliberately survives issuing the resume; only confirmed playback clears it. + /// A route needs a moment to become usable, so the first attempt can be rejected while it + /// settles, and retiring the marker here would leave nothing for the retry to act on. + /// + /// Beyond restoring what the user was listening to, this is what rebinds WebKit's media + /// session: while it stays stale its Now Playing entry keeps swallowing media keys without + /// acting on them, which is why F8 does nothing until playback runs once. + func resumeAfterRouteRestored() async { + guard self.routeLossPauseAt.map({ self.hasAudioRouteReturned($0) }) == true, + // The disconnect drives this same path, and its route change predates the pause, + // so only output that came back afterwards — and is still there — qualifies. + !self.isPlaying, + !self.isExplicitPauseIntentActive, + self.currentTrack != nil || self.pendingPlayVideoId != nil + else { return } + self.logger.info("Audio route restored — resuming playback paused by the route loss") + await self.resume() + } + func resume(intent: MusicPlaybackIntent) async { guard self.acceptsMusicPlaybackIntent(intent) else { return } self.logger.debug("Resuming playback") + self.unattributedRemotePauseAt = nil self.isStoppingPlayback = false self.shouldResumeAfterInterruption = true self.isAwaitingPlaybackConfirmation = true diff --git a/Sources/Kaset/Services/Player/PlayerService.swift b/Sources/Kaset/Services/Player/PlayerService.swift index 8a0a499bc..6a1bbc7ab 100644 --- a/Sources/Kaset/Services/Player/PlayerService.swift +++ b/Sources/Kaset/Services/Player/PlayerService.swift @@ -116,6 +116,28 @@ final class PlayerService: NSObject, PlayerServiceProtocol { var isAwaitingPlaybackConfirmation = false var isExplicitPauseIntentActive = false + /// When the system paused playback because its audio route disappeared, if that is why + /// playback is currently stopped. + /// + /// The route coming back is the cue to resume. That matters beyond convenience: WebKit's + /// Now Playing registration survives the route loss as an entry that still captures the + /// media keys but no longer acts on them, and playing again is the only thing that rebinds + /// it. Until then F8 reaches a session that does nothing. + @ObservationIgnored var routeLossPauseAt: ContinuousClock.Instant? + + /// A remote pause that no route loss explained when it arrived. + /// + /// The Core Audio listener timestamps a route change immediately but publishes it only after + /// several synchronous device queries, while the `pause` command it provokes drains + /// independently. When the command wins that race the pause looks deliberate, so its instant + /// is kept and the attribution retried once the route event lands. + @ObservationIgnored var unattributedRemotePauseAt: ContinuousClock.Instant? + + /// Whether output came back after the given instant and is still available. Injectable for tests. + @ObservationIgnored var hasAudioRouteReturned: @MainActor (ContinuousClock.Instant) -> Bool = { + DefaultOutputDeviceMonitor.shared.routeRestored(since: $0) + } + /// Currently playing track. var currentTrack: Song? { didSet { diff --git a/Tests/KasetTests/NowPlayingClaimTests.swift b/Tests/KasetTests/NowPlayingClaimTests.swift index 1876183ee..5760bb95a 100644 --- a/Tests/KasetTests/NowPlayingClaimTests.swift +++ b/Tests/KasetTests/NowPlayingClaimTests.swift @@ -3,14 +3,32 @@ import MediaPlayer import Testing @testable import Kaset +// MARK: - NowPlayingClaimTests + @Suite("Now playing claim", .serialized, .tags(.service)) +@MainActor struct NowPlayingClaimTests { - @Test("Actively playing or starting yields hands-off (WebKit owns the card)") + @Test("Started playback yields hands-off (WebKit owns the card)") func activePlaybackIsHandsOff() { + // `buffering` is a stall inside playback that already started, so it belongs with + // `playing`: re-claiming mid-track would recreate the competing second entry. let track = (title: "Song", artist: "Artist") #expect(NowPlayingManager.desiredClaim(state: .playing, track: track, activeVideo: nil) == .handsOff) #expect(NowPlayingManager.desiredClaim(state: .buffering, track: track, activeVideo: nil) == .handsOff) - #expect(NowPlayingManager.desiredClaim(state: .loading, track: track, activeVideo: nil) == .handsOff) + } + + @Test("A starting track keeps a playing claim until WebKit publishes its card") + func startingPlaybackKeepsClaimUntilHandover() { + // Withdrawing during the load would leave the app with no Now Playing entry, and + // therefore no media keys, for as long as the track takes to start. + let track = (title: "Song", artist: "Artist") + let expected = NowPlayingManager.NowPlayingClaim.claim( + title: "Song", + artist: "Artist", + playbackState: .playing + ) + #expect(NowPlayingManager.desiredClaim(state: .loading, track: track, activeVideo: nil) == expected) + #expect(NowPlayingManager.desiredClaim(state: .loading, track: nil, activeVideo: nil) == .release) } @Test("Not playing with a track yields a minimal claim") @@ -93,6 +111,79 @@ struct NowPlayingClaimTests { ) == expected) } + @Test("Hands-off and release both withdraw Kaset's entry so only one card can exist") + func nonClaimDecisionsWithdrawTheEntry() { + // WebKit registers a separate Now Playing client rather than overwriting + // `MPNowPlayingInfoCenter.default()`, so a claim kept during playback survives as a + // second entry for the same app and can capture the media keys with a stale state. + // Both non-claim decisions must therefore map onto the same withdrawal. + let track = (title: "Song", artist: "Artist") + let playingClaim = NowPlayingManager.desiredClaim(state: .playing, track: track, activeVideo: nil) + let emptyClaim = NowPlayingManager.desiredClaim(state: .idle, track: nil, activeVideo: nil) + + #expect(playingClaim == .handsOff) + #expect(emptyClaim == .release) + for claim in [playingClaim, emptyClaim] { + switch claim { + case .handsOff, .release: + break + case .claim: + Issue.record("Playback and empty states must not publish a native claim") + } + } + } + + @Test("Handing the card to WebKit withdraws our entry") + func handsOffClearsOwnedMetadata() { + // The central fix: the hands-off branch used to wait for WebKit to replace the app-wide + // metadata, which never happens, leaving a second permanently stale Control Center entry. + let center = MockNowPlayingInfoCenter() + NowPlayingManager.applyClaim( + .claim(title: "Song", artist: "Artist", playbackState: .paused), + isAssertingNativeClaim: false, + to: center + ) + #expect(NowPlayingManager.isNativeClaim(center.nowPlayingInfo)) + + let stillAsserting = NowPlayingManager.applyClaim( + .handsOff, + isAssertingNativeClaim: true, + to: center + ) + + #expect(stillAsserting == false) + #expect(center.nowPlayingInfo == nil) + #expect(center.playbackState == .stopped) + } + + @Test("Releasing with nothing to play withdraws our entry too") + func releaseClearsOwnedMetadata() { + let center = MockNowPlayingInfoCenter() + NowPlayingManager.applyClaim( + .claim(title: "Song", artist: "Artist", playbackState: .paused), + isAssertingNativeClaim: false, + to: center + ) + + NowPlayingManager.applyClaim(.release, isAssertingNativeClaim: true, to: center) + + #expect(center.nowPlayingInfo == nil) + } + + @Test("A card we do not own is never cleared") + func withdrawalLeavesForeignMetadataAlone() { + // WebKit publishes through its own client, but anything landing in the shared center + // that lacks our tag must survive a withdrawal untouched. + let center = MockNowPlayingInfoCenter() + center.nowPlayingInfo = [MPMediaItemPropertyTitle: "Someone else's card"] + center.playbackState = .playing + + NowPlayingManager.applyClaim(.handsOff, isAssertingNativeClaim: true, to: center) + + #expect(center.nowPlayingInfo?.isEmpty == false) + #expect(center.playbackState == .playing) + } + @Test("Only tagged native metadata is treated as Kaset's claim") func nativeClaimOwnershipTag() { let nativeInfo: [String: Any] = [ @@ -105,3 +196,11 @@ struct NowPlayingClaimTests { #expect(NowPlayingManager.isNativeClaim(nil) == false) } } + +// MARK: - MockNowPlayingInfoCenter + +@MainActor +final class MockNowPlayingInfoCenter: NowPlayingInfoCenter { + var nowPlayingInfo: [String: Any]? + var playbackState: MPNowPlayingPlaybackState = .unknown +} diff --git a/Tests/KasetTests/PlayerServiceRouteChangePauseTests.swift b/Tests/KasetTests/PlayerServiceRouteChangePauseTests.swift new file mode 100644 index 000000000..2dc2b9170 --- /dev/null +++ b/Tests/KasetTests/PlayerServiceRouteChangePauseTests.swift @@ -0,0 +1,175 @@ +import Foundation +import Testing +@testable import Kaset + +/// Losing an audio route (Bluetooth disconnecting, headphones unplugged) reaches the app as an +/// ordinary `pause` remote command. Recording it as a deliberate user pause is what broke media +/// keys after reconnecting: `isExplicitPauseIntentActive` makes the observer re-pause the page +/// the moment a WebKit-handled media key resumes it, so playback starts and dies immediately. +@Suite("Player service route-change pause", .serialized, .tags(.service)) +@MainActor +struct PlayerServiceRouteChangePauseTests { + @Test("A user pause records a standing intent to stay paused") + func userPauseHoldsPauseIntent() async { + let playerService = self.makePlayingService() + + await playerService.pause(intent: playerService.currentMusicPlaybackIntent) + + #expect(playerService.state == .paused) + #expect(playerService.isExplicitPauseIntentActive) + #expect(playerService.shouldResumeAfterInterruption == false) + } + + @Test("A route-change pause stops playback without claiming the user wants it paused") + func routeChangePauseKeepsResumeIntent() async { + let playerService = self.makePlayingService() + + await playerService.pause( + intent: playerService.currentMusicPlaybackIntent, + origin: .routeLoss(at: .now) + ) + + #expect(playerService.state == .paused) + // The two flags that would otherwise fight the next resume. + #expect(playerService.isExplicitPauseIntentActive == false) + #expect(playerService.shouldResumeAfterInterruption) + } + + @Test("After a route-change pause the page resuming is adopted, not undone") + func resumeAfterRouteChangePauseSurvives() async { + let playerService = self.makePlayingService() + await playerService.pause( + intent: playerService.currentMusicPlaybackIntent, + origin: .routeLoss(at: .now) + ) + + // A media key handled by WebKit resumes the page; the observer reports it. + playerService.updatePlaybackState( + isPlaying: true, + progress: 137, + duration: 300, + observedVideoId: "route-change" + ) + + #expect(playerService.state == .playing) + } + + @Test("After a user pause the page resuming is still refused") + func resumeAfterUserPauseIsStillRefused() async { + let playerService = self.makePlayingService() + await playerService.pause(intent: playerService.currentMusicPlaybackIntent) + + playerService.updatePlaybackState( + isPlaying: true, + progress: 137, + duration: 300, + observedVideoId: "route-change" + ) + + #expect(playerService.state == .paused) + } + + @Test("A returning route resumes what the route loss stopped") + func routeRestoredResumes() async { + let playerService = self.makeRouteLossPausedService(routeReturned: true) + + await playerService.resumeAfterRouteRestored() + + #expect(playerService.isAwaitingPlaybackConfirmation) + } + + @Test("The recovery marker survives an unconfirmed resume so the retry can act") + func markerSurvivesUntilPlaybackConfirms() async { + // A route needs a moment to become usable, so the first attempt can be rejected while + // it settles. Retiring the marker on issue would leave the retry nothing to act on. + let playerService = self.makeRouteLossPausedService(routeReturned: true) + + await playerService.resumeAfterRouteRestored() + #expect(playerService.routeLossPauseAt != nil) + + playerService.confirmPlaybackStarted() + #expect(playerService.routeLossPauseAt == nil) + } + + @Test("The disconnect's own reconciliation does not resume") + func disconnectDoesNotResumeItself() async { + // Losing the route schedules the same reconciliation the reconnect uses, and its route + // change predates the pause. Mistaking it for a reconnect would restart the music the + // system just stopped. + let playerService = self.makeRouteLossPausedService(routeReturned: false) + + await playerService.resumeAfterRouteRestored() + + #expect(playerService.isAwaitingPlaybackConfirmation == false) + #expect(playerService.state == .paused) + } + + @Test("A user pause is never resumed by a returning route") + func userPauseIsNotResumed() async { + let playerService = self.makeRouteLossPausedService(routeReturned: true) + await playerService.pause(intent: playerService.currentMusicPlaybackIntent) + + await playerService.resumeAfterRouteRestored() + + #expect(playerService.isAwaitingPlaybackConfirmation == false) + #expect(playerService.state == .paused) + } + + @Test("A pause that beat the route event is re-attributed once it lands") + func lateRouteEventReattributesThePause() async { + // The Core Audio listener publishes only after several synchronous device queries, so a + // genuine route-loss pause can drain first and look deliberate. Classifying at admission + // alone would leave the explicit pause intent set and recovery permanently disabled. + let playerService = self.makePlayingService() + let admittedAt = ContinuousClock.now + await playerService.pause( + intent: playerService.currentMusicPlaybackIntent, + origin: .unattributedRemote(admittedAt: admittedAt) + ) + #expect(playerService.isExplicitPauseIntentActive) + + playerService.reattributeRemotePauseToRouteLoss { $0 == admittedAt } + + #expect(playerService.isExplicitPauseIntentActive == false) + #expect(playerService.shouldResumeAfterInterruption) + #expect(playerService.routeLossPauseAt == admittedAt) + } + + @Test("A pause no route loss explains keeps its user semantics") + func unclaimedPauseStaysDeliberate() async { + let playerService = self.makePlayingService() + await playerService.pause( + intent: playerService.currentMusicPlaybackIntent, + origin: .unattributedRemote(admittedAt: .now) + ) + + // No disappearance can account for it, so the claim is refused. + playerService.reattributeRemotePauseToRouteLoss { _ in false } + + #expect(playerService.isExplicitPauseIntentActive) + #expect(playerService.routeLossPauseAt == nil) + } + + private func makeRouteLossPausedService(routeReturned: Bool) -> PlayerService { + let playerService = self.makePlayingService() + playerService.hasAudioRouteReturned = { _ in routeReturned } + playerService.state = .paused + playerService.routeLossPauseAt = .now + return playerService + } + + private func makePlayingService() -> PlayerService { + let playerService = PlayerService() + playerService.currentTrack = Song( + id: "route-change", + title: "route-change", + artists: [], + duration: 300, + videoId: "route-change" + ) + playerService.state = .playing + playerService.shouldResumeAfterInterruption = true + playerService.isExplicitPauseIntentActive = false + return playerService + } +} diff --git a/Tests/KasetTests/RouteDisappearanceClassificationTests.swift b/Tests/KasetTests/RouteDisappearanceClassificationTests.swift new file mode 100644 index 000000000..1cdd5f171 --- /dev/null +++ b/Tests/KasetTests/RouteDisappearanceClassificationTests.swift @@ -0,0 +1,143 @@ +import Foundation +import Testing +@testable import Kaset + +/// A vanished audio route and a user pressing Pause arrive as the same remote command, so the +/// only thing separating them is a device disappearing just before the command was admitted. +/// +/// Classification runs after an unbounded hop to the MainActor, so every case here is expressed +/// in terms of the instant the command was *admitted* — never the instant it is judged, and +/// never assuming the route stood still in between. +@Suite("Route disappearance classification", .tags(.service)) +struct RouteDisappearanceClassificationTests { + private let window = Duration.milliseconds(1500) + private let base = ContinuousClock.now + + @Test("A pause admitted just after the output device vanished is route-driven") + func pauseFollowingDisappearanceQualifies() { + let events = [self.disappearance(at: 0)] + + #expect(self.routeLossIndex(in: events, admittedAt: 60) == 0) + } + + @Test("A pause admitted before the device vanished is the user's") + func pausePrecedingDisappearanceDoesNotQualify() { + // The command was already in flight when the route dropped, so the route cannot be + // what prompted it. + let events = [self.disappearance(at: 100)] + + #expect(self.routeLossIndex(in: events, admittedAt: 90) == nil) + } + + @Test("A pause admitted long after the device vanished is the user's") + func pauseOutsideWindowDoesNotQualify() { + let events = [self.disappearance(at: 0)] + + #expect(self.routeLossIndex(in: events, admittedAt: 1600) == nil) + } + + @Test("Output restored before the pause makes it the user's") + func restorationBeforeTheCommandDoesNotQualify() { + // Disconnect then reconnect quickly: there was somewhere to play by the time the + // command arrived, so pausing then was deliberate. + let events = [self.disappearance(at: 0), self.restoration(at: 200)] + + #expect(self.routeLossIndex(in: events, admittedAt: 250) == nil) + } + + @Test("Output restored after the pause still leaves it route-driven") + func restorationAfterTheCommandStillQualifies() { + // The reconnect landed while the command was still queued for the MainActor. Judging by + // the latest route state would misread this as a user pause and disable recovery. + let events = [self.disappearance(at: 0), self.restoration(at: 160)] + + #expect(self.routeLossIndex(in: events, admittedAt: 60) == 0) + } + + @Test("A later disconnect does not hide the one that explains a queued pause") + func newerDisappearanceDoesNotShadowTheRelevantOne() { + // Flapping Bluetooth can complete a whole disconnect/reconnect/disconnect cycle before + // the first pause is drained. Keeping only the newest events would leave nothing that + // predates the command, and a genuine route pause would read as the user's. + let events = [ + self.disappearance(at: 0), + self.restoration(at: 400), + self.disappearance(at: 800), + ] + + #expect(self.routeLossIndex(in: events, admittedAt: 60) == 0) + } + + @Test("A disappearance already claimed cannot explain a second pause") + func consumedDisappearanceDoesNotQualify() { + // The system's own route-loss pause claims the disconnect. A user pause moments later + // must keep its own meaning, or a future reconnect would resume against it. + var events = [self.disappearance(at: 0)] + events[0].isConsumed = true + + #expect(self.routeLossIndex(in: events, admittedAt: 60) == nil) + } + + @Test("Route changes that never removed a device do not qualify") + func changeWithoutDisappearanceDoesNotQualify() { + // Plugging in a new output or switching by hand changes the default device without + // taking one away. + let events = [self.restoration(at: 0)] + + #expect(self.routeLossIndex(in: events, admittedAt: 60) == nil) + } + + // MARK: - Route restoration + + @Test("Output coming back after the pause counts as restored") + func restorationAfterMarkerQualifies() { + let events = [self.disappearance(at: 0), self.restoration(at: 400)] + + #expect(DefaultOutputDeviceMonitor.routeRestored(in: events, since: self.base + .milliseconds(60))) + } + + @Test("Nothing after the pause is not a restoration") + func noEventAfterMarkerDoesNotQualify() { + let events = [self.disappearance(at: 0)] + + #expect(DefaultOutputDeviceMonitor.routeRestored( + in: events, + since: self.base + .milliseconds(60) + ) == false) + } + + @Test("A route lost again after coming back is not restored") + func laterDisappearanceRevokesRestoration() { + // Flapping Bluetooth: reconnect then drop again before the delayed retry runs. Treating + // any later event as a restoration would resume onto an output that is gone, and with + // playback already paused there may be no second pause command to re-arm recovery. + let events = [ + self.disappearance(at: 0), + self.restoration(at: 400), + self.disappearance(at: 800), + ] + + #expect(DefaultOutputDeviceMonitor.routeRestored( + in: events, + since: self.base + .milliseconds(60) + ) == false) + } + + // MARK: - Helpers + + private func disappearance(at milliseconds: Int) -> RouteChangeEvent { + RouteChangeEvent(at: self.base + .milliseconds(milliseconds), isDisappearance: true) + } + + private func restoration(at milliseconds: Int) -> RouteChangeEvent { + RouteChangeEvent(at: self.base + .milliseconds(milliseconds), isDisappearance: false) + } + + private func routeLossIndex(in events: [RouteChangeEvent], admittedAt milliseconds: Int) -> Int? { + DefaultOutputDeviceMonitor.routeLossIndex( + in: events, + admittedAt: self.base + .milliseconds(milliseconds), + within: self.window + ) + } +} diff --git a/docs/adr/0033-audio-route-loss-recovery.md b/docs/adr/0033-audio-route-loss-recovery.md new file mode 100644 index 000000000..eba64c8e0 --- /dev/null +++ b/docs/adr/0033-audio-route-loss-recovery.md @@ -0,0 +1,119 @@ +# ADR-0033: Audio Route Loss Recovery and Now Playing Ownership + +## Status + +Implemented + +## Context + +Media keys (F8) stopped controlling playback after Bluetooth headphones disconnected and +reconnected. Clicking Play in Kaset's window restored them. The behavior was reproduced with a +timestamped file trace (see `docs/common-bug-patterns.md`) rather than inferred; three separate +defects turned out to stack. + +### 1. WebKit's Now Playing session survives a route loss as a zombie + +WebKit registers its own Now Playing client with the system, independent of the app's +`MPNowPlayingInfoCenter`. When the audio route disappears, that registration outlives the media +session it described: Control Center still shows its card, and the card still captures the media +keys — but its transport actions do nothing. Pressing the card's own play button is a no-op, +while Kaset's card next to it works. + +The registration cannot be withdrawn from the app side. Clearing `navigator.mediaSession` +metadata and `playbackState` was tried and only blanks the card's contents; WebKit keeps the +registration as long as a media element that has played still exists. The only thing that +rebinds the session to a live route is playback actually running again — which is exactly what +the manual "click Play" workaround did. + +### 2. A route-loss pause was recorded as a deliberate user pause + +macOS stops playback on a vanished route by sending the app an ordinary `pause` remote command, +indistinguishable from the user pressing Pause. Kaset's handler set +`isExplicitPauseIntentActive`, whose purpose is to stop YouTube's autoplay from overriding a +user's pause. With it set, `applyObservedPlaybackState` re-pauses the page the instant anything +resumes it — so even a media key that did reach a healthy session started playback and had it +killed a moment later. + +### 3. The native Now Playing claim could never be released + +`NowPlayingManager` published a tagged minimal claim while paused so media keys still reached +Kaset (ADR-less, from #387). Its hands-off branch waited for "WebKit to atomically replace the +app-wide metadata" before standing down. WebKit never does that — it does not write +`MPNowPlayingInfoCenter` at all — so the claim was never withdrawn and Kaset kept a second, +permanently stale Control Center entry once it had paused even once. + +## Decision + +**Resume playback when the audio route returns**, scoped precisely to playback the route loss +itself stopped. This restores what the user was listening to and, as a direct consequence, +rebinds WebKit's media session so the media keys work again for everything afterward. + +Supporting decisions: + +- **Classify the pause at its source.** `MusicPauseOrigin` distinguishes `.user` from + `.routeLoss(at:)`. Only a user pause records the standing intent to stay paused. +- **Identify a route loss by a device disappearing**, not by any output change. A manual output + switch or a device being added must not arm recovery. `DefaultOutputDeviceMonitor` remembers + the previous default device and checks both device-list membership and + `kAudioDevicePropertyDeviceIsAlive` — Core Audio can mark a device dead before dropping its + ID. +- **Attribute from both sides.** Core Audio's listener timestamps a change immediately but only + publishes it after several synchronous device queries, while the `pause` it provokes drains + independently. Either can win. A pause that arrives first is treated as the user's but keeps + its admission instant, and the route event re-attributes it when it lands — the route event is + the thing that arrives late, not the pause. Classifying only at admission would leave the + explicit-pause intent set with no way to undo it, which is precisely the original failure. +- **Anchor timing to command admission.** Remote commands drain onto the MainActor + asynchronously, so both the classification window and the recovery marker use the ingress + admission instant. Dating either to handling time lets a busy main actor misclassify a real + route pause, or sort the marker after a reconnect that already happened. +- **Bound recovery by intent, not by a timer.** The marker is retired by a user pause or by + playback actually starting — never by elapsed time. A resume can therefore only ever continue + exactly what the route loss interrupted, so reconnecting an hour later is still the same + interrupted song. It deliberately survives *issuing* a resume so the second attempt can retry + while the route is still settling. +- **Release the native claim whenever WebKit owns the card**, since the event it was waiting for + does not exist. + +## Consequences + +### Positive + +- Media keys work after a Bluetooth disconnect/reconnect without touching the app. +- Reconnecting headphones continues the interrupted track, matching AirPods behavior elsewhere. +- Exactly one Control Center entry during playback instead of a stale duplicate. + +### Negative + +- Playback restarts on its own when a route returns. This is a deliberate behavior change; it is + narrowly scoped to a pause the system imposed, and any user transport action retires it. +- Classification rests on a 1.5s correlation window between a device disappearing and the `pause` + command (measured at 40–60ms). It is a heuristic, not a signal the OS provides. + +### Out of scope: the YouTube video source + +Only the music player recovers. When `PlaybackArbiter` routes media keys to +`YouTubePlayerService`, the same system pause still reaches `handleRemotePause` unclassified, no +recovery marker is recorded, and reconnecting cannot rebind that video's WebKit media session — +so the media-key failure described above remains for video playback. + +This is a gap, not a regression: nothing about the video path changed. It is left out because it +needs its own recovery marker and resume path in a separate service, and none of it could be +verified against the reproduction that drove this work, which was music-only. Shipping an +unverified parallel implementation alongside a verified one was judged worse than recording the +gap. It deserves its own change, reproduced and traced the same way. + +### Known limitation + +`DefaultOutputDeviceMonitor` watches only which device is the *default output*. A route change +within one device — some Macs expose speakers and the headphone jack as data sources on the same +built-in device — leaves that selection untouched and goes unseen, so recovery does not arm. +Covering it requires listening on the current device's output data source and retargeting those +listeners on every default-device change. Bluetooth always swaps the device itself. A missed +change degrades to the previous behavior rather than misbehaving. + +## References + +- ADR-0001: WebView-Based Playback +- ADR-0026: Generation-Scoped Web Playback Bridge +- `docs/common-bug-patterns.md` — the trace workflow that localized this diff --git a/docs/adr/README.md b/docs/adr/README.md index 4fc7ee573..2ab160c32 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -67,3 +67,4 @@ What becomes easier or more difficult because of this change? | [0030](0030-account-scoped-favorites.md) | Account-Scoped Favorites Persistence | Accepted | | [0031](0031-saved-album-library-reconciliation.md) | Saved-Album Library Identity and Reconciliation | Accepted | | [0032](0032-youtube-ask-gemini.md) | Watch-Scoped YouTube Ask Gemini | Accepted; fixed WEB profile enabled in production | +| [0033](0033-audio-route-loss-recovery.md) | Audio Route Loss Recovery and Now Playing Ownership | Accepted |