Skip to content

Commit fd00e3b

Browse files
authored
Merge pull request #101 from playola-radio/briankeane/engine-recovery-generation-gate
fix(sdk): generation-gate + .error state in engine-config recovery
2 parents dda547d + 82da3ba commit fd00e3b

1 file changed

Lines changed: 42 additions & 9 deletions

File tree

Sources/PlayolaPlayer/Player/PlayolaStationPlayer.swift

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ final public class PlayolaStationPlayer: ObservableObject {
189189
/// True once the engine-config-change observer has been registered (lazily,
190190
/// on first playback) so we register it exactly once.
191191
private var engineConfigObserverRegistered = false
192+
private var engineConfigObserver: (any NSObjectProtocol)?
192193

193194
/// Registers the AVAudioEngineConfigurationChange observer, scoped to the
194195
/// SDK's OWN engine. AVAudioEngine stops itself on a hardware/format/route
@@ -204,12 +205,19 @@ final public class PlayolaStationPlayer: ObservableObject {
204205
private func registerEngineConfigObserverIfNeeded() {
205206
guard !engineConfigObserverRegistered else { return }
206207
engineConfigObserverRegistered = true
207-
NotificationCenter.default.addObserver(
208-
self,
209-
selector: #selector(handleAudioEngineConfigurationChange(_:)),
210-
name: .AVAudioEngineConfigurationChange,
211-
object: mainMixer.engine
212-
)
208+
// Deliver on the main queue: AVAudioEngineConfigurationChange fires on an
209+
// unspecified thread, and the handler reads @MainActor state (isSuspended,
210+
// isPlaying, stationId, playGeneration) synchronously before hopping into a
211+
// Task. `queue: .main` guarantees those reads run on the main actor.
212+
engineConfigObserver = NotificationCenter.default.addObserver(
213+
forName: .AVAudioEngineConfigurationChange,
214+
object: mainMixer.engine,
215+
queue: .main
216+
) { [weak self] notification in
217+
MainActor.assumeIsolated {
218+
self?.handleAudioEngineConfigurationChange(notification)
219+
}
220+
}
213221
}
214222

215223
private static let logger = OSLog(
@@ -947,16 +955,41 @@ final public class PlayolaStationPlayer: ObservableObject {
947955
/// has paused for an interruption (the host drives resume then). Only acts
948956
/// while actively playing; re-syncs to the live wall clock via play().
949957
///
950-
/// Internal (not public): it's a notification selector target, not host API —
951-
/// the host never calls it. `@objc` is required for `#selector`.
952-
@objc func handleAudioEngineConfigurationChange(_ notification: Notification) {
958+
/// Internal: delivered on the main queue from a block observer (see
959+
/// registerEngineConfigObserverIfNeeded), so it runs on the main actor. Not
960+
/// host API — the host never calls it.
961+
func handleAudioEngineConfigurationChange(_ notification: Notification) {
953962
os_log("Audio engine configuration changed", log: PlayolaStationPlayer.logger, type: .info)
954963
guard !isSuspended, isPlaying, let stationToRecover = stationId else { return }
964+
// Snapshot the recovery attempt: stationToRecover + generation together. Any
965+
// host stop()/play() after this point bumps playGeneration and invalidates
966+
// the recovery — we must not override the host's explicit choice (same
967+
// supersession discipline resumeAfterInterruption() uses).
968+
let generation = playGeneration
955969
Task { @MainActor in
956970
do {
957971
try await mainMixer.restartEngine()
972+
} catch {
973+
// Surface via state (mirrors resumeAfterInterruption) so a host driving
974+
// UI from $state isn't left on a stale .playing with dead audio — and
975+
// report — but only if a host stop()/play() hasn't superseded us during
976+
// the restart (a superseded recovery is moot; don't add Sentry noise).
977+
if generation == playGeneration {
978+
state = .error(
979+
.playbackError(
980+
"Failed to recover audio engine after configuration change: "
981+
+ error.localizedDescription))
982+
await errorReporter.reportError(
983+
error, context: "Failed to recover engine after configuration change", level: .error)
984+
}
985+
return
986+
}
987+
// A host stop()/play() during restartEngine took over — don't override it.
988+
guard generation == playGeneration else { return }
989+
do {
958990
try await play(stationId: stationToRecover)
959991
} catch {
992+
// play() publishes .error itself on terminal failure when still current.
960993
await errorReporter.reportError(
961994
error, context: "Failed to recover engine after configuration change", level: .error)
962995
}

0 commit comments

Comments
 (0)