diff --git a/ios/Playback/Sources/S2Playback/Engine/MusicPlaybackController.swift b/ios/Playback/Sources/S2Playback/Engine/MusicPlaybackController.swift index c62031f467..96100496ff 100644 --- a/ios/Playback/Sources/S2Playback/Engine/MusicPlaybackController.swift +++ b/ios/Playback/Sources/S2Playback/Engine/MusicPlaybackController.swift @@ -556,6 +556,12 @@ public final class MusicPlaybackController { guard newState != state else { return } state = newState if newState == .paused || newState == .idle || newState == .ended { pauseEngine() } + reportState(newState) + } + + /// The current track's state, on the callback queue. Repeated by ``stayPaused()`` when a play fails and the + /// engine was already paused: `setState` would otherwise drop it, and the owner would never hear the refusal. + private func reportState(_ newState: State) { let uid = current?.track.uid let callback = callbackLock.withLock { callbacks.state } if let callback { callbackQueue.async { callback(newState, uid) } } @@ -633,11 +639,17 @@ public final class MusicPlaybackController { } /// A play the engine couldn't start: paused, as if it had been asked to pause, so the owner and - /// Now Playing show it paused. Nothing plays until the next play. + /// Now Playing show it paused. Nothing plays until the next play. Reported again when already + /// paused — the state didn't change, but the owner still has to hear that this play was refused. + /// Not a decode failure: the track stays put. private func stayPaused() { playWhenReady = false stopTicker() - setState(.paused) + if state == .paused { + reportState(.paused) + } else { + setState(.paused) + } } /// Opens `slot`'s source, or waits for the open already under way; false (after reporting it) diff --git a/ios/Playback/Tests/S2PlaybackTests/MusicPlaybackControllerTests.swift b/ios/Playback/Tests/S2PlaybackTests/MusicPlaybackControllerTests.swift index d5cc901f06..8a27b8274e 100644 --- a/ios/Playback/Tests/S2PlaybackTests/MusicPlaybackControllerTests.swift +++ b/ios/Playback/Tests/S2PlaybackTests/MusicPlaybackControllerTests.swift @@ -335,12 +335,24 @@ final class MusicPlaybackControllerTests: XCTestCase { controller.play() controller.syncForTesting() - XCTAssertEqual(log.states, [.loading, .paused]) + // Already paused, so the state didn't change — the refusal is reported again anyway, for this track. + XCTAssertEqual(log.states, [.loading, .paused, .paused]) + XCTAssertEqual(log.failures, []) controller.startEngine = { try $0.start() } controller.play() controller.syncForTesting() - XCTAssertEqual(log.states, [.loading, .paused, .playing]) + XCTAssertEqual(log.states, [.loading, .paused, .paused, .playing]) + } + + func testPausingWhileAlreadyPausedDoesNotRepeatTheReport() throws { + let (controller, log) = try makeController() + controller.load(current: track("A", TestSignal.noise(frames: 12_000, seed: 1)), next: nil, playWhenReady: false) + controller.syncForTesting() + + controller.pause() + controller.syncForTesting() + XCTAssertEqual(log.states, [.loading, .paused]) } func testALoadThatPlaysWhenTheEngineCantStartLoadsPausedAndStaysPaused() throws { @@ -351,6 +363,7 @@ final class MusicPlaybackControllerTests: XCTestCase { controller.load(current: track("A", TestSignal.noise(frames: 12_000, seed: 1)), next: nil, playWhenReady: true) controller.syncForTesting() XCTAssertEqual(log.states, [.loading, .paused]) + XCTAssertEqual(log.failures, [], "a start failure is not a song that failed to decode") // The failed play is forgotten: a seek, which restarts the stream, doesn't try again. controller.startEngine = { try $0.start() } diff --git a/ios/S2/Platform/Audio/EngineAudioPlayer.swift b/ios/S2/Platform/Audio/EngineAudioPlayer.swift index dc15c416c5..1ec76b5de0 100644 --- a/ios/S2/Platform/Audio/EngineAudioPlayer.swift +++ b/ios/S2/Platform/Audio/EngineAudioPlayer.swift @@ -13,7 +13,10 @@ import Shared /// **Audio session.** `onWillPlay` runs before anything plays (`play()`, or a load that plays) and `onPaused` on /// every pause; `PlaybackSystemCoordinator` points them at `AudioSessionController.activate()` and /// `playbackPaused()`. A session that won't activate (a call, another app holding the hardware) cancels the play: -/// `play()` does nothing and a load loads paused, so the engine reports paused, as when the engine can't start. +/// `play()` does nothing and a load loads paused. Both report paused for that track, on the calling thread, so Kotlin +/// can drop the intent. A refused load's report is not the track becoming ready — the engine's own loading and paused +/// reports, afterwards and in order, are. A refused play of a track that's already ready has no engine transition, so +/// the paused report is the only one. final class EngineAudioPlayer: NSObject, IosAudioPlayer { /// Called on the main thread before playback starts; false cancels it. var onWillPlay: () -> Bool = { true } @@ -80,6 +83,7 @@ final class EngineAudioPlayer: NSObject, IosAudioPlayer { } let plays = playWhenReady && onWillPlay() engine.load(current: track, next: next.flatMap(engineTrack), startMs: startMs, playWhenReady: plays) + if playWhenReady && !plays { reportPaused(current.id) } } func setNext(next: IosAudioTrack?) { @@ -87,10 +91,19 @@ final class EngineAudioPlayer: NSObject, IosAudioPlayer { } func play() { - guard onWillPlay() else { return } + guard onWillPlay() else { + if let currentId { reportPaused(currentId) } + return + } engine.play() } + /// The session refused playback of `trackId`. Synchronous, on the call's thread (the main thread): Kotlin is still + /// inside the load or play and can tell this from the engine's later ready-paused. + private func reportPaused(_ trackId: String) { + listener?.onStateChanged(trackId: trackId, state: .paused) + } + func pause() { engine.pause() onPaused() diff --git a/ios/S2/Platform/Audio/PlaybackSystemCoordinator.swift b/ios/S2/Platform/Audio/PlaybackSystemCoordinator.swift index 42b82cdc35..621941181b 100644 --- a/ios/S2/Platform/Audio/PlaybackSystemCoordinator.swift +++ b/ios/S2/Platform/Audio/PlaybackSystemCoordinator.swift @@ -74,9 +74,14 @@ final class PlaybackSystemCoordinator: NowPlayingCommandHandler { player.onPaused = {} } - /// Read from the flow's value, not `isPlaying`, which lags it by a hop through the observer task. + /// Read from the flows' values, not `isPlaying`, which lags them by a hop through the observer task. + /// Playing, or loading with the intent to play. An idle, ended or failed track is paused and must not + /// advertise playback — including to an interruption, which resumes only when this was true. private var playerIsPlaying: Bool { - playback.playbackStateFlow.value is PlaybackState.Playing + let state = playback.playbackStateFlow.value + if state is PlaybackState.Playing { return true } + if state is PlaybackState.Loading { return playback.playWhenReadyFlow.value.boolValue } + return false } private func observe() { @@ -87,6 +92,11 @@ final class PlaybackSystemCoordinator: NowPlayingCommandHandler { self?.publish() } }, + Task { [weak self] in + for await _ in playback.playWhenReadyFlow { + self?.publish() + } + }, Task { [weak self] in for await _ in playback.progressFlow { self?.publish() @@ -108,7 +118,8 @@ final class PlaybackSystemCoordinator: NowPlayingCommandHandler { // MARK: - Following the player /// Publishes the player as it is now: the current song, its position and whether it plays. - private func publish() { + /// `force` writes the item again, artwork included, instead of `updatePlayback`'s throttled position update. + private func publish(force: Bool = false) { guard let current = playback.queueOperations.queueStateFlow.value.currentItem else { guard nowPlaying.item != nil else { return } // The queue emptied: nothing is playing, so give the session back to other apps. @@ -125,7 +136,7 @@ final class PlaybackSystemCoordinator: NowPlayingCommandHandler { current.song.type == .audio ? .tracks : .interval(forward: Self.skipForwardSeconds, backward: Self.skipBackwardSeconds) ) - if item == nowPlaying.item { + if !force, item == nowPlaying.item { nowPlaying.updatePlayback(position: position, isPlaying: isPlaying, speed: speed) } else { nowPlaying.setItem(item, position: position, isPlaying: isPlaying, speed: speed) @@ -148,14 +159,19 @@ final class PlaybackSystemCoordinator: NowPlayingCommandHandler { } /// Every audio object died with the media server: a new engine, and the current item loaded into it - /// where it was, playing on if it was. + /// where it was. Playing — or still loading with the intent to play — resumes; paused stays paused. + /// Now Playing is written in full even when the item and state didn't change: the system copy was wiped + /// with the server, and `updatePlayback` would throttle an unchanged paused or loading item. private func rebuildEngine() { guard let engine = makeEngine() else { return } - let wasPlaying = playerIsPlaying + let resume = playerIsPlaying let position = playback.getProgress() player.replaceEngine(engine) - playback.load(seekPosition: position, skipUnloadable: false) { _ in } - if wasPlaying { playback.play() } + playback.load(seekPosition: position, skipUnloadable: false) { [weak self] _ in + MainActor.assumeIsolated { self?.publish(force: true) } + } + if resume { playback.play() } + publish(force: true) } // MARK: - NowPlayingCommandHandler diff --git a/ios/S2Tests/EngineAudioPlayerTests.swift b/ios/S2Tests/EngineAudioPlayerTests.swift index baa1469ac0..03b04d86f0 100644 --- a/ios/S2Tests/EngineAudioPlayerTests.swift +++ b/ios/S2Tests/EngineAudioPlayerTests.swift @@ -122,9 +122,11 @@ struct EngineAudioPlayerTests { player.load(current: track("a"), next: nil, startMs: 0, playWhenReady: true) player.play() - // The load still happens, paused; the play never reaches the engine. + // The load still happens, paused; the play never reaches the engine. Each refusal is a paused + // report for that track, delivered on the call so it stays ahead of the engine's own events. #expect(engine.loads.map(\.playWhenReady) == [false]) #expect(engine.commands.isEmpty) + #expect(listener.calls == ["state a paused", "state a paused"]) } @Test func positionAndDurationOnlyCountForTheTrackKotlinThinksIsCurrent() { @@ -261,4 +263,103 @@ struct EngineAudioPlayerTests { #expect(engine.loads.last?.current.url.absoluteString == "demo://2") #expect(graph.playerController.queueOperations.queueStateFlow.value.currentItem?.song.name == "Hyperballad") } + + private func intendsToPlay(_ controller: IosPlayerController) -> Bool { + controller.playWhenReadyFlow.value.boolValue + } + + @Test func aSessionRefusalClearsIntentWithoutFinishingTheLoadEarly() async throws { + player.onWillPlay = { false } + let graph = makeTestGraph(audioPlayer: player) + let controller = graph.playerController + _ = try await controller.queueOperations.setQueue(songs: TestSongs.demo, shuffleSongs: nil, position: 0, context: PlayContextNone.shared) + var completed = false + controller.skipToNext(ignoreRepeat: true) { _ in completed = true } + + #expect(await waitUntil { engine.loads.count == 1 }) + #expect(engine.loads.last?.playWhenReady == false) + #expect(!engine.commands.contains("play")) + #expect(!intendsToPlay(controller)) + #expect(!completed) + #expect(controller.playbackStateFlow.value is PlaybackState.Loading) + + let id = try #require(engine.loads.last?.current.id) + engine.emit(.state(.loading, trackId: id)) + #expect(controller.playbackStateFlow.value is PlaybackState.Loading) + engine.emit(.state(.paused, trackId: id)) + #expect(await waitUntil { controller.playbackStateFlow.value is PlaybackState.Paused }) + #expect(completed) + #expect(!intendsToPlay(controller)) + #expect(controller.queueOperations.queueStateFlow.value.currentItem?.song.name == "Hyperballad") + + // The cancelled intent is what a later queue change preserves: the new song loads paused. + let next = try #require(controller.queueOperations.getNext(ignoreRepeat: true)) + controller.queueOperations.setCurrentItem(currentItem: next) + #expect(await waitUntil { engine.loads.count == 2 }) + #expect(engine.loads.last?.playWhenReady == false) + #expect(!intendsToPlay(controller)) + + player.onWillPlay = { true } + controller.play() + #expect(await waitUntil { engine.commands.contains("play") }) + #expect(intendsToPlay(controller)) + } + + @Test func aRefusedPlayOfAReadySongClearsIntentWithNoEngineTransition() async throws { + let graph = makeTestGraph(audioPlayer: player) + let controller = graph.playerController + try await queueDemoSongs(on: graph, skipUnloadable: false) + #expect(await waitUntil { !engine.loads.isEmpty }) + let id = try #require(engine.loads.first?.current.id) + engine.emit(.state(.paused, trackId: id)) + #expect(await waitUntil { controller.playbackStateFlow.value is PlaybackState.Paused }) + + player.onWillPlay = { false } + controller.play() + + #expect(!engine.commands.contains("play")) + #expect(!intendsToPlay(controller)) + #expect(controller.playbackStateFlow.value is PlaybackState.Paused) + } + + @Test func aPausedReportForTheSongASkipReplacedIsIgnored() async throws { + let graph = makeTestGraph(audioPlayer: player) + let controller = graph.playerController + try await queueDemoSongs(on: graph, skipUnloadable: false) + #expect(await waitUntil { !engine.loads.isEmpty }) + let first = try #require(engine.loads.first?.current.id) + controller.play() + engine.emit(.state(.playing, trackId: first)) + #expect(await waitUntil { controller.playbackStateFlow.value is PlaybackState.Playing }) + #expect(intendsToPlay(controller)) + #expect(await waitUntil { engine.nexts.last??.id != nil }) + + controller.skipToNext(ignoreRepeat: true, completion: nil) + #expect(await waitUntil { engine.loads.count == 2 }) + engine.emit(.state(.paused, trackId: first)) + + #expect(intendsToPlay(controller)) + #expect(controller.queueOperations.queueStateFlow.value.currentItem?.song.name == "Hyperballad") + #expect(controller.playbackStateFlow.value is PlaybackState.Loading) + } + + @Test func aLoadCompletionThatPlaysKeepsTheNewIntent() async throws { + player.onWillPlay = { false } + let graph = makeTestGraph(audioPlayer: player) + let controller = graph.playerController + _ = try await controller.queueOperations.setQueue(songs: TestSongs.demo, shuffleSongs: nil, position: 0, context: PlayContextNone.shared) + controller.skipToNext(ignoreRepeat: true) { _ in + player.onWillPlay = { true } + controller.play() + } + + #expect(await waitUntil { engine.loads.count == 1 }) + #expect(!intendsToPlay(controller)) + let id = try #require(engine.loads.last?.current.id) + engine.emit(.state(.loading, trackId: id)) + engine.emit(.state(.paused, trackId: id)) + + #expect(await waitUntil { intendsToPlay(controller) && engine.commands.contains("play") }) + #expect(controller.queueOperations.queueStateFlow.value.currentItem?.song.name == "Hyperballad") + } } diff --git a/ios/S2Tests/PlaybackSystemCoordinatorTests.swift b/ios/S2Tests/PlaybackSystemCoordinatorTests.swift index 0c0544a5a1..aba0fa7d3e 100644 --- a/ios/S2Tests/PlaybackSystemCoordinatorTests.swift +++ b/ios/S2Tests/PlaybackSystemCoordinatorTests.swift @@ -37,13 +37,22 @@ struct PlaybackSystemCoordinatorTests { var nowPlayingInfo: [String: Any]? { didSet { history.append(nowPlayingInfo) } } - var playbackState: MPNowPlayingPlaybackState = .unknown + var playbackState: MPNowPlayingPlaybackState = .unknown { + didSet { if playbackState != oldValue { playbackStates.append(playbackState) } } + } /// Every write, in order. private(set) var history: [[String: Any]?] = [] + /// Every playback-state change, in order. + private(set) var playbackStates: [MPNowPlayingPlaybackState] = [] func value(_ key: String) -> T? { nowPlayingInfo?[key] as? T } } + private final class EngineSource { + var make: () -> AudioEngine? + init(_ engine: AudioEngine?) { make = { engine } } + } + private final class FakeCommandCenter: RemoteCommandCenter { var handlers: [RemoteCommand: (RemoteCommandEvent) -> MPRemoteCommandHandlerStatus] = [:] @@ -65,6 +74,7 @@ struct PlaybackSystemCoordinatorTests { private let commands = FakeCommandCenter() private let graph: IosAppGraph private let coordinator: PlaybackSystemCoordinator + private let engineSource: EngineSource private var rebuiltEngines: [FakeAudioEngine] = [] init() { @@ -72,12 +82,14 @@ struct PlaybackSystemCoordinatorTests { graph = makeTestGraph(audioPlayer: player) let rebuilt = FakeAudioEngine() rebuiltEngines = [rebuilt] + let source = EngineSource(rebuilt) + engineSource = source coordinator = PlaybackSystemCoordinator( playback: graph.playerController, player: player, session: AudioSessionController(session: session, notificationCenter: center), nowPlaying: NowPlayingController(infoCenter: info, commandCenter: commands), - makeEngine: { rebuilt } + makeEngine: { source.make() } ) coordinator.start() } @@ -94,6 +106,14 @@ struct PlaybackSystemCoordinatorTests { info.value(MPNowPlayingInfoPropertyPlaybackRate) } + private var defaultRate: Double? { + info.value(MPNowPlayingInfoPropertyDefaultPlaybackRate) + } + + private var intendsToPlay: Bool { + graph.playerController.playWhenReadyFlow.value.boolValue + } + /// Queues `songs` and loads the first, as far as the engine reporting it ready. private func loadQueue(_ songs: [Song] = TestSongs.demo) async throws -> String { let controller = graph.playerController @@ -242,4 +262,181 @@ struct PlaybackSystemCoordinatorTests { #expect(commands.handlers.values.isEmpty) #expect(title == nil) } + + /// Queues `songs` and asks for the first, leaving the engine unreported so playback stays loading. + private func beginLoad(_ songs: [Song] = TestSongs.demo) async throws { + let controller = graph.playerController + _ = try await controller.queueOperations.setQueue(songs: songs, shuffleSongs: nil, position: 0, context: PlayContextNone.shared) + controller.load(seekPosition: nil, skipUnloadable: false) { _ in } + #expect(await waitUntil { !engine.loads.isEmpty && title == "Paranoid Android" }) + } + + @Test func aSkipHoldsThePauseButtonThroughLoading() async throws { + _ = try await playQueue() + graph.playerController.setPlaybackSpeed(multiplier: 1.5) + #expect(await waitUntil { rate == 1.5 && info.playbackState == .playing }) + let statesBefore = info.playbackStates.count + + commands.fire(.nextTrack) + + #expect(await waitUntil { title == "Hyperballad" }) + let hyperballad = info.history.compactMap { $0 }.filter { $0[MPMediaItemPropertyTitle] as? String == "Hyperballad" } + #expect(!hyperballad.isEmpty) + #expect(hyperballad.allSatisfy { ($0[MPNowPlayingInfoPropertyElapsedPlaybackTime] as? TimeInterval) == 0 }) + #expect(hyperballad.allSatisfy { ($0[MPNowPlayingInfoPropertyPlaybackRate] as? Double) == 1.5 }) + #expect(hyperballad.allSatisfy { ($0[MPNowPlayingInfoPropertyDefaultPlaybackRate] as? Double) == 1.5 }) + #expect(!info.playbackStates.dropFirst(statesBefore).contains(.paused)) + #expect(info.playbackState == .playing) + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + #expect(engine.loads.last?.playWhenReady == true) + + let nextId = try #require(engine.loads.last?.current.id) + engine.emit(.state(.playing, trackId: nextId)) + #expect(await waitUntil { graph.playerController.playbackStateFlow.value is PlaybackState.Playing }) + #expect(rate == 1.5) + #expect(info.playbackState == .playing) + } + + @Test func loadingShowsPlayUntilAskedAndTheButtonFollowsIntentImmediately() async throws { + try await beginLoad() + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + #expect(!intendsToPlay) + #expect(info.playbackState == .paused) + #expect(rate == 0) + + graph.playerController.setPlaybackSpeed(multiplier: 1.5) + #expect(await waitUntil { defaultRate == 1.5 }) + #expect(rate == 0) + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + + commands.fire(.play) + #expect(await waitUntil { info.playbackState == .playing && rate == 1.5 }) + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + #expect(intendsToPlay) + + commands.fire(.pause) + #expect(await waitUntil { info.playbackState == .paused && rate == 0 }) + #expect(defaultRate == 1.5) + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + #expect(!intendsToPlay) + + commands.fire(.togglePlayPause) + #expect(await waitUntil { info.playbackState == .playing && rate == 1.5 }) + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + #expect(intendsToPlay) + } + + @Test func anInterruptionWhileLoadingWithIntentPausesAndResumes() async throws { + try await beginLoad() + commands.fire(.play) + #expect(await waitUntil { info.playbackState == .playing }) + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + + center.post(name: AVAudioSession.interruptionNotification, object: session, userInfo: [ + AVAudioSessionInterruptionTypeKey: AVAudioSession.InterruptionType.began.rawValue, + ]) + #expect(await waitUntil { !intendsToPlay }) + + center.post(name: AVAudioSession.interruptionNotification, object: session, userInfo: [ + AVAudioSessionInterruptionTypeKey: AVAudioSession.InterruptionType.ended.rawValue, + AVAudioSessionInterruptionOptionKey: AVAudioSession.InterruptionOptions.shouldResume.rawValue, + ]) + #expect(await waitUntil { intendsToPlay && engine.commands.contains("play") }) + } + + @Test func aResetRepublishesUnchangedPausedPlaybackInsideTheThrottle() async throws { + let id = try await loadQueue() + graph.playerController.setPlaybackSpeed(multiplier: 1.25) + #expect(await waitUntil { title == "Paranoid Android" && defaultRate == 1.25 }) + engine.emit(.position(trackId: id, ms: 12_000)) + #expect(await waitUntil { elapsed == 12 }) + + info.nowPlayingInfo = nil + info.playbackState = .unknown + center.post(name: AVAudioSession.mediaServicesWereResetNotification, object: session) + + #expect(await waitUntil { title == "Paranoid Android" && elapsed == 12 && defaultRate == 1.25 }) + #expect(rate == 0) + #expect(info.playbackState == .paused) + #expect(!intendsToPlay) + let rebuilt = try #require(rebuiltEngines.first) + #expect(rebuilt.loads.first?.playWhenReady == false) + #expect(!rebuilt.commands.contains("play")) + } + + @Test func aResetWhileLoadingRepublishesAndRestoresPlayIntent() async throws { + try await beginLoad() + commands.fire(.nextTrack) + #expect(await waitUntil { title == "Hyperballad" && info.playbackState == .playing }) + #expect(graph.playerController.playbackStateFlow.value is PlaybackState.Loading) + graph.playerController.setPlaybackSpeed(multiplier: 1.5) + #expect(await waitUntil { rate == 1.5 }) + + info.nowPlayingInfo = nil + info.playbackState = .unknown + center.post(name: AVAudioSession.mediaServicesWereResetNotification, object: session) + + let rebuilt = try #require(rebuiltEngines.first) + #expect(await waitUntil { + title == "Hyperballad" && (rebuilt.commands.contains("play") || rebuilt.loads.contains { $0.playWhenReady }) + }) + #expect(intendsToPlay) + #expect(info.playbackState == .playing) + #expect(rate == 1.5) + #expect(defaultRate == 1.5) + } + + @Test func aResetWhilePlayingRestoresIntentInTheReplacementEngine() async throws { + _ = try await playQueue() + #expect(await waitUntil { rate == 1 && info.playbackState == .playing }) + info.nowPlayingInfo = nil + info.playbackState = .unknown + + center.post(name: AVAudioSession.mediaServicesWereResetNotification, object: session) + + let rebuilt = try #require(rebuiltEngines.first) + #expect(await waitUntil { + title == "Paranoid Android" && (rebuilt.commands.contains("play") || rebuilt.loads.contains { $0.playWhenReady }) + }) + #expect(intendsToPlay) + #expect(info.playbackState == .playing) + #expect(rate == 1) + } + + @Test func aResetOfAnEmptyQueueDoesNotRestartOrPublishStaleMetadata() async throws { + _ = try await loadQueue() + #expect(await waitUntil { title != nil }) + graph.playerController.clearQueue() + #expect(await waitUntil { title == nil }) + + info.nowPlayingInfo = ["planted": "stale"] + let writes = info.history.count + center.post(name: AVAudioSession.mediaServicesWereResetNotification, object: session) + await drainMainQueue() + + #expect(info.history.count == writes) + #expect(info.nowPlayingInfo?["planted"] as? String == "stale") + let rebuilt = try #require(rebuiltEngines.first) + #expect(rebuilt.loads.isEmpty) + #expect(!rebuilt.commands.contains("play")) + #expect(!intendsToPlay) + } + + @Test func aResetWhoseEngineCannotBeBuiltDoesNotRestartOrPublish() async throws { + _ = try await playQueue() + #expect(await waitUntil { info.playbackState == .playing }) + let plays = engine.commands.filter { $0 == "play" }.count + engineSource.make = { nil } + info.nowPlayingInfo = nil + info.playbackState = .unknown + let writes = info.history.count + + center.post(name: AVAudioSession.mediaServicesWereResetNotification, object: session) + await drainMainQueue() + + #expect(info.history.count == writes) + #expect(title == nil) + #expect(engine.commands.filter { $0 == "play" }.count == plays) + #expect(intendsToPlay) + } } diff --git a/shared/src/commonMain/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerController.kt b/shared/src/commonMain/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerController.kt index 39c24ca1df..92782008ad 100644 --- a/shared/src/commonMain/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerController.kt +++ b/shared/src/commonMain/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerController.kt @@ -122,7 +122,36 @@ class IosPlayerController( /** The engine's last reported state for [current]. */ private var engineState = IosAudioPlayerState.Idle - private var playWhenReady = false + /** + * Whether the user wants playback to run. This is the value [load]s, [play], [pause], failures and queue changes + * actually use — not a copy beside it. Distinct from [playbackStateFlow]: a track can be loading either way, and an + * idle, ended or failed track does not keep the intent. + */ + private val _playWhenReady = MutableStateFlow(false) + + /** [playWhenReady], for system surfaces. Collectors read this flow; they don't keep their own copy. */ + val playWhenReadyFlow: StateFlow = _playWhenReady.asStateFlow() + + private var playWhenReady: Boolean + get() = _playWhenReady.value + set(value) { + _playWhenReady.value = value + } + + /** Non-zero while a [player] call is on the stack, so a paused report it makes is its refusal of that call. */ + private var engineCallDepth = 0 + + /** The current load was handed to the engine asking to play. */ + private var loadAskedToPlay = false + + /** [player.play] was called and the engine has not yet reported what came of it. */ + private var playCallPending = false + + /** + * The load's own ready-paused arrived while [playCallPending]: the play is still in flight, and a further paused + * report is that play being refused (the engine was already paused, so the report repeats). + */ + private var playAwaitingRejection = false /** The completion of the last [load] (or skip), called once its item is ready or nothing could load. */ private var pendingLoad: PendingLoad? = null @@ -287,7 +316,11 @@ class IosPlayerController( feed.sent = true feed.opensAtPosition = stream.opensAtPosition val handedBack = engineNext?.takeIf { !it.failed }?.handedOver - player.load(feed.track(stream), handedBack, (startMs - feed.offsetMs).toLong(), playWhenReady) + loadAskedToPlay = playWhenReady + playCallPending = false + playAwaitingRejection = false + // A session that refuses this load reports paused inside the call, before the engine is ready. + callEngine { player.load(feed.track(stream), handedBack, (startMs - feed.offsetMs).toLong(), playWhenReady) } feedNext() } @@ -362,6 +395,7 @@ class IosPlayerController( engineNext = null engineState = IosAudioPlayerState.Idle playWhenReady = false + clearPlayRequest() completePending(Result.failure(IllegalStateException("Queue empty"))) } @@ -372,14 +406,38 @@ class IosPlayerController( state: IosAudioPlayerState ) { val currentFeed = current ?: return - // A failed track has been dealt with: skipped, or stopped at. + // A failed track has been dealt with: skipped, or stopped at. So has a report for a track already replaced. if (trackId != currentFeed.id || currentFeed.failed) return + // A refusal delivered inside load or play, before the engine has become ready. Cancelling the intent here must + // not complete the load: the engine's own loading and paused reports, still to come, are what make it ready. + if (state == IosAudioPlayerState.Paused && engineCallDepth > 0 && !currentFeed.ready) { + if (playWhenReady) playWhenReady = false + clearPlayRequest() + publishState() + return + } engineState = state when (state) { - IosAudioPlayerState.Playing, IosAudioPlayerState.Paused -> if (!currentFeed.failed && !currentFeed.ready) { - currentFeed.ready = true - loadFailures = 0 - completePending(Result.success(pendingLoad?.attempt == 1)) + IosAudioPlayerState.Paused -> { + // Reconcile before the load completion: it may ask to play, and that newer intent has to survive. + when { + playCallPending && !currentFeed.ready -> { + // The load's ready-paused, queued before a play that hasn't been answered yet. + playCallPending = false + playAwaitingRejection = true + } + + playWhenReady && (loadAskedToPlay || playCallPending || playAwaitingRejection) -> { + playWhenReady = false + clearPlayRequest() + } + } + markReady(currentFeed) + } + + IosAudioPlayerState.Playing -> { + clearPlayRequest() + markReady(currentFeed) } IosAudioPlayerState.Ended -> { @@ -392,6 +450,29 @@ class IosPlayerController( publishState() } + /** The engine can play [feed]: the first playing or paused report after it was handed over, not a refusal before that. */ + private fun markReady(feed: Feed) { + if (feed.failed || feed.ready) return + feed.ready = true + loadFailures = 0 + completePending(Result.success(pendingLoad?.attempt == 1)) + } + + private fun clearPlayRequest() { + loadAskedToPlay = false + playCallPending = false + playAwaitingRejection = false + } + + private inline fun callEngine(block: () -> Unit) { + engineCallDepth++ + try { + block() + } finally { + engineCallDepth-- + } + } + /** The engine moved on to its next track: the queue's current item follows. */ private fun onTransition(trackId: String) { val arrived = engineNext?.takeIf { it.id == trackId } ?: return @@ -485,6 +566,7 @@ class IosPlayerController( next = null engineNext = null playWhenReady = false + clearPlayRequest() player.stop() engineState = IosAudioPlayerState.Idle } @@ -511,6 +593,7 @@ class IosPlayerController( } completePending(Result.failure(IllegalStateException("Nothing to load"))) playWhenReady = false + clearPlayRequest() engineState = IosAudioPlayerState.Ended publishState() } @@ -594,7 +677,9 @@ class IosPlayerController( else -> { if (isNearEnd(getProgress() ?: 0, item.song)) seekNow(0) - player.play() + playCallPending = true + // A session that refuses a track that's already ready reports paused even though the engine never moves. + callEngine { player.play() } } } publishState() @@ -615,6 +700,8 @@ class IosPlayerController( private fun pauseNow() { playWhenReady = false + clearPlayRequest() + // Still told to the engine while the state stays [PlaybackState.Loading], so a load that was going to play stops. if (current != null) player.pause() } @@ -875,6 +962,7 @@ class IosPlayerController( val playedOut = queue.remove(uids) if (playedOut && current != null) { playWhenReady = false + clearPlayRequest() } sync() if (playedOut && current != null) { diff --git a/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/FakeIosAudioPlayer.kt b/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/FakeIosAudioPlayer.kt index 20137cb5ad..06153f5ccb 100644 --- a/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/FakeIosAudioPlayer.kt +++ b/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/FakeIosAudioPlayer.kt @@ -35,6 +35,18 @@ class FakeIosAudioPlayer : IosAudioPlayer { private var playWhenReady = false + /** + * When false, [play] and a load that asked to play are refused: the track is still prepared, ending paused, and a + * paused state is reported for its id. [deferRefusal] posts that play refusal for [settle], as an engine that + * can't start does; otherwise it is reported inside the call, as a session that won't activate. A refused load + * always ends with the engine's own paused report (what makes the load ready); the in-call report, when not + * deferred, only cancels the intent early. + */ + var acceptsPlay = true + + /** See [acceptsPlay]. A refused load is deferred by the engine's own paused report either way. */ + var deferRefusal = false + override fun setListener(listener: IosAudioPlayerListener?) { this.listener = listener } @@ -48,8 +60,10 @@ class FakeIosAudioPlayer : IosAudioPlayer { calls += "load ${current.url}@$startMs${if (playWhenReady) " playing" else ""}${next?.let { " next ${it.url}" } ?: ""}" this.current = current this.next = next - this.playWhenReady = playWhenReady + val starts = playWhenReady && acceptsPlay + this.playWhenReady = starts position = startMs + if (playWhenReady && !starts && !deferRefusal) listener?.onStateChanged(current.id, IosAudioPlayerState.Paused) setState(IosAudioPlayerState.Loading) if (startMs > 0 && current.isUnseekable()) { // The engine starts it at its beginning instead, and says so before its state. @@ -62,7 +76,7 @@ class FakeIosAudioPlayer : IosAudioPlayer { // A failed current track counts as ended, and the engine carries on into its next. if (next != null) transition() else setState(IosAudioPlayerState.Ended) } else { - setState(if (playWhenReady) IosAudioPlayerState.Playing else IosAudioPlayerState.Paused) + setState(if (starts) IosAudioPlayerState.Playing else IosAudioPlayerState.Paused) } } @@ -73,6 +87,10 @@ class FakeIosAudioPlayer : IosAudioPlayer { override fun play() { calls += "play" + if (!acceptsPlay) { + current?.id?.let(::refuse) + return + } playWhenReady = true if (current != null && state == IosAudioPlayerState.Paused) setState(IosAudioPlayerState.Playing) } @@ -164,6 +182,19 @@ class FakeIosAudioPlayer : IosAudioPlayer { /** Clears [calls], so a test sees only what's asked of the engine after this. */ fun clearCalls() = calls.clear() + /** Queues a state report for [trackId], delivered by [settle]. An id the controller has replaced is ignored by it. */ + fun emitState( + state: IosAudioPlayerState, + trackId: String + ) { + post { listener?.onStateChanged(trackId, state) } + } + + private fun refuse(trackId: String) { + val report: () -> Unit = { listener?.onStateChanged(trackId, IosAudioPlayerState.Paused) } + if (deferRefusal) post(report) else report() + } + private fun transition() { val arrived = checkNotNull(next) current = arrived diff --git a/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerControllerTest.kt b/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerControllerTest.kt index 2ec736b594..f0fd08b9cd 100644 --- a/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerControllerTest.kt +++ b/shared/src/commonTest/kotlin/com/simplecityapps/shuttle/shared/playback/IosPlayerControllerTest.kt @@ -695,4 +695,246 @@ class IosPlayerControllerTest { controller.currentSong shouldBe b controller.playbackState() shouldBe PlaybackState.Playing } + + // Play intent, kept apart from the engine's state + + @Test + fun `play intent follows play, pause and toggle - and a load does not assume it`() = test { controller -> + controller.queueOperations.setQueue(listOf(a, b), null, 0) + controller.playWhenReadyFlow.value shouldBe false + + controller.load { } + controller.playWhenReadyFlow.value shouldBe false + engine.settle() + + controller.play() + engine.settle() + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + + controller.pause() + engine.settle() + controller.playWhenReadyFlow.value shouldBe false + + controller.togglePlayback() + engine.settle() + controller.playWhenReadyFlow.value shouldBe true + + controller.togglePlayback() + engine.settle() + controller.playWhenReadyFlow.value shouldBe false + controller.playbackState() shouldBe PlaybackState.Paused + } + + @Test + fun `play, pause and toggle while a song is still loading update intent without leaving loading`() = test { controller -> + controller.queueOperations.setQueue(listOf(a), null, 0) + + controller.load { } + controller.playbackState() shouldBe PlaybackState.Loading + controller.playWhenReadyFlow.value shouldBe false + + controller.play() + controller.pause() + controller.togglePlayback() + + // The engine hasn't reported ready, so the state is still loading; the intent moved with each call. + controller.playbackState() shouldBe PlaybackState.Loading + controller.playWhenReadyFlow.value shouldBe true + engine.calls shouldContain "play" + engine.calls shouldContain "pause" + engine.settle() + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + } + + @Test + fun `a play asked before the engine settles survives its queued paused report - and a stale track does not`() = test { controller -> + controller.queueOperations.setQueue(listOf(a, b), null, 0) + controller.load { } + controller.play() + controller.pause() + controller.play() + engine.emitState(IosAudioPlayerState.Paused, "stale") + + engine.settle() + + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + controller.currentSong shouldBe a + } + + @Test + fun `a paused load's completion that plays keeps that newer intent`() = test { controller -> + controller.queueOperations.setQueue(listOf(a), null, 0) + + controller.load { result -> + result.onSuccess { controller.play() } + } + engine.settle() + + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + } + + @Test + fun `a session that refuses a playing load cancels the intent without finishing early or skipping`() = test { controller -> + engine.acceptsPlay = false + controller.queueOperations.setQueue(listOf(a, b), null, 0) + var result: Result? = null + + controller.skipToNext { result = it } + + // Reported inside the load, before the engine's paused report has made the song ready. + result shouldBe null + controller.playbackState() shouldBe PlaybackState.Loading + controller.playWhenReadyFlow.value shouldBe false + controller.currentSong shouldBe b + + engine.settle() + result?.isSuccess shouldBe true + controller.playbackState() shouldBe PlaybackState.Paused + controller.playWhenReadyFlow.value shouldBe false + controller.currentSong shouldBe b + } + + @Test + fun `an engine that pauses a playing load clears intent when it becomes ready and does not skip`() = test { controller -> + engine.acceptsPlay = false + engine.deferRefusal = true + controller.queueOperations.setQueue(listOf(a, b, c), null, 0) + var result: Result? = null + + controller.skipToNext { result = it } + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Loading + result shouldBe null + + engine.settle() + + result?.isSuccess shouldBe true + controller.currentSong shouldBe b + controller.playWhenReadyFlow.value shouldBe false + controller.playbackState() shouldBe PlaybackState.Paused + engine.calls.none { it.startsWith("load song:3") } shouldBe true + } + + @Test + fun `a refusal's load completion that plays keeps the new intent`() = test { controller -> + engine.acceptsPlay = false + engine.deferRefusal = true + controller.queueOperations.setQueue(listOf(a, b), null, 0) + + controller.skipToNext { + engine.acceptsPlay = true + controller.play() + } + engine.settle() + + controller.currentSong shouldBe b + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + } + + @Test + fun `a refused play of a ready song clears intent - a queue change stays paused - and an explicit next plays`() = test { controller -> + controller.start(listOf(a, b, c)) + engine.acceptsPlay = false + engine.clearCalls() + + controller.play() + controller.playWhenReadyFlow.value shouldBe false + controller.currentSong shouldBe a + + controller.queueOperations.setCurrentItem(controller.queueOperations.getQueue()[1]) + engine.settle() + engine.calls.first { it.startsWith("load") } shouldBe "load song:2@0" + controller.playWhenReadyFlow.value shouldBe false + controller.playbackState() shouldBe PlaybackState.Paused + + engine.acceptsPlay = true + engine.clearCalls() + controller.skipToNext() + engine.settle() + engine.calls.first() shouldBe "load song:3@0 playing" + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + } + + @Test + fun `an engine that is already paused and can't start reports it again and the queue stays put`() = test { controller -> + controller.start(listOf(a, b), play = false) + engine.acceptsPlay = false + engine.deferRefusal = true + engine.clearCalls() + + controller.play() + controller.playWhenReadyFlow.value shouldBe true + engine.settle() + + controller.playWhenReadyFlow.value shouldBe false + controller.currentSong shouldBe a + controller.playbackState() shouldBe PlaybackState.Paused + engine.calls shouldBe listOf("play") + } + + @Test + fun `a paused report for the song a skip replaced is ignored`() = test { controller -> + controller.start(listOf(a, b, c)) + controller.skipToNext() + engine.settle() + + engine.emitState(IosAudioPlayerState.Paused, "stale") + engine.settle() + + controller.currentSong shouldBe b + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + } + + @Test + fun `reaching the end, exhausting failures, or emptying the queue clears intent - a gapless move keeps it`() = test { controller -> + controller.start(listOf(a, b)) + engine.finishTrack() + controller.currentSong shouldBe b + controller.playWhenReadyFlow.value shouldBe true + + controller.start(listOf(a)) + engine.finishTrack() + controller.playWhenReadyFlow.value shouldBe false + controller.playbackState() shouldBe PlaybackState.Paused + + engine.failing += listOf(url(b), url(c)) + controller.start(listOf(a, b, c)) + controller.skipToNext() + engine.settle() + controller.currentSong shouldBe c + controller.playWhenReadyFlow.value shouldBe false + + controller.start(listOf(a, b), play = false) + controller.clearQueue() + controller.playWhenReadyFlow.value shouldBe false + controller.queueOperations.getSize() shouldBe 0 + + controller.start(listOf(a, b)) + controller.clearQueue() + controller.playWhenReadyFlow.value shouldBe true + controller.playbackState() shouldBe PlaybackState.Playing + } + + @Test + fun `removing the current song keeps a playing intent and removing the rest of the queue clears it`() = test { controller -> + controller.start(listOf(a, b, c)) + + controller.removeQueueItem(controller.queueOperations.getQueue()[0]) + engine.settle() + controller.currentSong shouldBe b + controller.playWhenReadyFlow.value shouldBe true + + val rest = controller.queueOperations.getQueue() + controller.queueOperations.remove(rest) + engine.settle() + controller.playWhenReadyFlow.value shouldBe false + controller.playbackState() shouldBe PlaybackState.Paused + } }