From b1857474fadbe437be72d9d4bd80354f1ed662d1 Mon Sep 17 00:00:00 2001 From: Brian Keane Date: Fri, 20 Mar 2026 16:26:34 -0500 Subject: [PATCH 1/7] Add AVPlayer-based streaming player for faster audio startup Introduces StreamingStationPlayer as an alternative to PlayolaStationPlayer that streams audio via AVPlayer instead of downloading entire files first. This significantly reduces startup latency for long audio files. New files: - FadeScheduleBuilder: pure fade schedule generation (shared utility) - AVPlayerProviding: protocol wrapping AVPlayer for testability - StreamingSpinPlayer: AVPlayer-based single spin playback - StreamingStationPlayer: orchestrator with schedule polling - ScheduleService: extracted shared schedule-fetching logic Modified: - ListeningSessionReporter: added publisher-based init for both players 37 new tests across 3 test suites, all passing. Co-Authored-By: Claude Opus 4.6 --- .../Player/ListeningSessionReporter.swift | 29 +- .../Player/ScheduleService.swift | 106 +++++ .../Player/Streaming/AVPlayerProviding.swift | 104 +++++ .../Streaming/FadeScheduleBuilder.swift | 91 +++++ .../Streaming/StreamingSpinPlayer.swift | 335 ++++++++++++++++ .../Streaming/StreamingStationPlayer.swift | 362 ++++++++++++++++++ TODOS.md | 15 + .../FadeScheduleBuilderTests.swift | 180 +++++++++ .../StreamingSpinPlayerTests.swift | 307 +++++++++++++++ .../StreamingStationPlayerTests.swift | 166 ++++++++ 10 files changed, 1691 insertions(+), 4 deletions(-) create mode 100644 Sources/PlayolaPlayer/Player/ScheduleService.swift create mode 100644 Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift create mode 100644 Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift create mode 100644 Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift create mode 100644 Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift create mode 100644 TODOS.md create mode 100644 Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift create mode 100644 Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift create mode 100644 Tests/PlayolaPlayerTests/StreamingStationPlayerTests.swift diff --git a/Sources/PlayolaPlayer/Player/ListeningSessionReporter.swift b/Sources/PlayolaPlayer/Player/ListeningSessionReporter.swift index 1362e0d..a3cabab 100644 --- a/Sources/PlayolaPlayer/Player/ListeningSessionReporter.swift +++ b/Sources/PlayolaPlayer/Player/ListeningSessionReporter.swift @@ -54,6 +54,7 @@ public class ListeningSessionReporter { var currentSessionStationId: String? var disposeBag = Set() weak var stationPlayer: PlayolaStationPlayer? + private let stationIdGetter: () -> String? var currentListeningSessionID: String? private let errorReporter = PlayolaErrorReporter.shared private let authProvider: PlayolaAuthenticationProvider? @@ -73,8 +74,29 @@ public class ListeningSessionReporter { self.authProvider = authProvider self.urlSession = urlSession self.baseURL = baseURL + self.stationIdGetter = { [weak stationPlayer] in stationPlayer?.stationId } - stationPlayer.$stationId.sink { [weak self] stationId in + subscribeToStationId(stationPlayer.$stationId.eraseToAnyPublisher()) + } + + init( + stationIdPublisher: AnyPublisher, + stationIdGetter: @escaping () -> String?, + authProvider: PlayolaAuthenticationProvider? = nil, + urlSession: URLSessionProtocol = URLSession.shared, + baseURL: URL = URL(string: "https://admin-api.playola.fm")! + ) { + self.stationPlayer = nil + self.authProvider = authProvider + self.urlSession = urlSession + self.baseURL = baseURL + self.stationIdGetter = stationIdGetter + + subscribeToStationId(stationIdPublisher) + } + + private func subscribeToStationId(_ publisher: AnyPublisher) { + publisher.sink { [weak self] stationId in guard let self else { return } if let stationId { Task { @@ -97,7 +119,6 @@ public class ListeningSessionReporter { try await self.endListeningSession() self.stopPeriodicNotifications() } catch { - // Just log the error but don't fail critically since this is cleanup Task { await self.errorReporter.reportError( error, @@ -195,7 +216,7 @@ public class ListeningSessionReporter { withTimeInterval: 10.0, repeats: true, block: { [weak self] _ in guard let self else { return } - guard let stationId = self.stationPlayer?.stationId else { + guard let stationId = self.stationIdGetter() else { let error = ListeningSessionError.invalidResponse( "Missing stationId in periodic notification") Task { @@ -303,7 +324,6 @@ public class ListeningSessionReporter { return request } - // TODO: Find a better way of doing this. Protocols + ObservableObject has issues. #if DEBUG internal init( authProvider: PlayolaAuthenticationProvider? = nil, @@ -311,6 +331,7 @@ public class ListeningSessionReporter { baseURL: URL = URL(string: "https://admin-api.playola.fm")! ) { self.stationPlayer = nil + self.stationIdGetter = { nil } self.authProvider = authProvider self.urlSession = urlSession self.baseURL = baseURL diff --git a/Sources/PlayolaPlayer/Player/ScheduleService.swift b/Sources/PlayolaPlayer/Player/ScheduleService.swift new file mode 100644 index 0000000..a0a941b --- /dev/null +++ b/Sources/PlayolaPlayer/Player/ScheduleService.swift @@ -0,0 +1,106 @@ +import Foundation +import PlayolaCore +import os.log + +enum ScheduleService { + private static let logger = OSLog( + subsystem: "PlayolaPlayer", + category: "ScheduleService") + + static func getSchedule( + stationId: String, + baseUrl: URL, + errorReporter: PlayolaErrorReporter = .shared + ) async throws -> Schedule { + let url = baseUrl.appending(path: "/v1/stations/\(stationId)/schedule") + .appending(queryItems: [ + URLQueryItem(name: "includeRelatedTexts", value: "true"), + URLQueryItem(name: "lockedIn", value: "true"), + ]) + + do { + let (data, response) = try await URLSession.shared.data(from: url) + + guard let httpResponse = response as? HTTPURLResponse else { + let error = StationPlayerError.networkError("Invalid response type") + Task { + await errorReporter.reportError( + error, + context: "Non-HTTP response received from schedule endpoint: \(url.absoluteString)", + level: .error) + } + throw error + } + + guard (200...299).contains(httpResponse.statusCode) else { + let responseText = String(data: data, encoding: .utf8) ?? "Unable to decode response" + let error = StationPlayerError.networkError("HTTP error: \(httpResponse.statusCode)") + + Task { + if httpResponse.statusCode == 404 { + await errorReporter.reportError( + error, + context: "Station not found: \(stationId) | Response: \(responseText.prefix(100))", + level: .error) + } else { + await errorReporter.reportError( + error, + context: + "HTTP \(httpResponse.statusCode) error getting schedule for station: \(stationId) | " + + "Response: \(responseText.prefix(100))", + level: .error) + } + } + throw error + } + + return try decodeSchedule(from: data, stationId: stationId, errorReporter: errorReporter) + } catch let error as StationPlayerError { + throw error + } catch { + Task { + await errorReporter.reportError( + error, context: "Failed to fetch schedule for station: \(stationId)", level: .error) + } + throw error + } + } + + private static func decodeSchedule( + from data: Data, stationId: String, errorReporter: PlayolaErrorReporter + ) throws -> Schedule { + let decoder = JSONDecoderWithIsoFull() + + do { + let spins = try decoder.decode([Spin].self, from: data) + guard !spins.isEmpty else { + let error = StationPlayerError.scheduleError( + "No spins returned in schedule for station ID: \(stationId)") + Task { + await errorReporter.reportError(error, level: .error) + } + throw error + } + return Schedule(stationId: spins[0].stationId, spins: spins) + } catch let decodingError as DecodingError { + let context: String + switch decodingError { + case .dataCorrupted(let reportedContext): + context = "Corrupted data: \(reportedContext.debugDescription)" + case .keyNotFound(let key, _): + context = "Missing key: \(key)" + case .typeMismatch(let type, _): + context = "Type mismatch for: \(type)" + default: + context = "Unknown decoding error" + } + + Task { + await errorReporter.reportError( + decodingError, context: "Failed to decode schedule: \(context)", level: .error) + } + + throw StationPlayerError.scheduleError("Invalid schedule data: \(context)") + } + } +} diff --git a/Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift b/Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift new file mode 100644 index 0000000..7565668 --- /dev/null +++ b/Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift @@ -0,0 +1,104 @@ +import AVFoundation +import Foundation + +/// Protocol wrapping AVPlayer for testability. +/// +/// Follows the same pattern as URLSessionProtocol in the codebase. +/// In production, AVPlayer conforms via a wrapper. In tests, MockAVPlayer conforms. +@MainActor +public protocol AVPlayerProviding: AnyObject { + var volume: Float { get set } + var currentTimeSeconds: Double { get } + func play() + func pause() + func seek(to time: CMTime) async -> Bool + + /// Loads a URL and waits until the player is ready to play. + /// Throws if the item fails to load. + func loadURL(_ url: URL) async throws + + /// Tears down the current item. + func clearItem() +} + +/// Production AVPlayer wrapper that handles AVPlayerItem KVO internally. +@MainActor +public class AVPlayerWrapper: AVPlayerProviding { + private var player: AVPlayer? + private var statusObservation: NSKeyValueObservation? + + public var volume: Float { + get { player?.volume ?? 0 } + set { player?.volume = newValue } + } + + public var currentTimeSeconds: Double { + player?.currentTime().seconds ?? 0 + } + + public func play() { + player?.play() + } + + public func pause() { + player?.pause() + } + + public func seek(to time: CMTime) async -> Bool { + guard let player else { return false } + return await withCheckedContinuation { continuation in + player.seek(to: time) { finished in + continuation.resume(returning: finished) + } + } + } + + public func loadURL(_ url: URL) async throws { + let item = AVPlayerItem(url: url) + let newPlayer = AVPlayer(playerItem: item) + newPlayer.automaticallyWaitsToMinimizeStalling = true + self.player = newPlayer + + try await waitForReady(item: item) + } + + private func waitForReady(item: AVPlayerItem) async throws { + try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in + self.statusObservation = item.observe(\.status, options: [.new]) { + [weak self] observedItem, _ in + guard let self else { return } + + Task { @MainActor in + self.statusObservation?.invalidate() + self.statusObservation = nil + + switch observedItem.status { + case .readyToPlay: + continuation.resume() + case .failed: + let error = + observedItem.error + ?? StationPlayerError.playbackError("AVPlayerItem failed to load") + continuation.resume(throwing: error) + case .unknown: + break + @unknown default: + break + } + } + } + } + } + + public func clearItem() { + statusObservation?.invalidate() + statusObservation = nil + player?.pause() + player?.replaceCurrentItem(with: nil) + player = nil + } + + deinit { + statusObservation?.invalidate() + } +} diff --git a/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift b/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift new file mode 100644 index 0000000..13d3bba --- /dev/null +++ b/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift @@ -0,0 +1,91 @@ +import Foundation +import PlayolaCore + +public struct FadeStep: Sendable, Equatable { + public let timeMS: Int + public let volume: Float + + public init(timeMS: Int, volume: Float) { + self.timeMS = timeMS + self.volume = volume + } +} + +public enum FadeScheduleBuilder { + public static let fadeDurationMS: Double = 1500 + public static let fadeSteps: Int = 48 + + /// Expands a spin's fades into discrete interpolation steps. + /// + /// Each fade in the spin is expanded into `fadeSteps` evenly-spaced steps + /// over `fadeDurationMS` milliseconds, linearly interpolating from the previous + /// volume level to the fade's target volume. + /// + /// Example: A fade at 30000ms from 1.0 to 0.0 with 48 steps over 1500ms produces + /// 49 entries (steps 0...48) starting at timeMS=30000 and ending at timeMS=31500. + public static func buildFadeSchedule( + for spin: Spin, + fadeDurationMS: Double = FadeScheduleBuilder.fadeDurationMS, + fadeSteps: Int = FadeScheduleBuilder.fadeSteps + ) -> [FadeStep] { + let fades = spin.fades + guard !fades.isEmpty else { return [] } + + var schedule: [FadeStep] = [] + let sortedFades = fades.sorted { $0.atMS < $1.atMS } + var fromVolume = spin.startingVolume + let stepDurationMS = fadeDurationMS / Double(fadeSteps) + + for fade in sortedFades { + let toVolume = fade.toVolume + let fadeStartMS = fade.atMS + + for step in 0...fadeSteps { + let progress = Float(step) / Float(fadeSteps) + let volume = fromVolume + (toVolume - fromVolume) * progress + let timeMS = fadeStartMS + Int(Double(step) * stepDurationMS) + schedule.append(FadeStep(timeMS: timeMS, volume: volume)) + } + + fromVolume = toVolume + } + + return schedule + } + + /// Returns the volume at a given millisecond position based on a fade schedule. + /// + /// Walks backward through the schedule to find the last step at or before `ms`. + /// If `ms` is before all steps, returns `startingVolume`. + public static func volumeAtMS( + _ ms: Int, + in schedule: [FadeStep], + startingVolume: Float + ) -> Float { + var result = startingVolume + + for step in schedule { + if step.timeMS <= ms { + result = step.volume + } else { + break + } + } + + return result + } + + /// Returns the index of the first fade step that hasn't been applied yet + /// (i.e., the first step whose timeMS > currentMS). + public static func firstUnprocessedIndex( + in schedule: [FadeStep], + afterMS currentMS: Int + ) -> Int { + for (index, step) in schedule.enumerated() { + if step.timeMS > currentMS { + return index + } + } + return schedule.count + } +} diff --git a/Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift b/Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift new file mode 100644 index 0000000..85167e8 --- /dev/null +++ b/Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift @@ -0,0 +1,335 @@ +import AVFoundation +import Combine +import Foundation +import PlayolaCore +import os.log + +/// Delegate for StreamingSpinPlayer events. +@MainActor +protocol StreamingSpinPlayerDelegate: AnyObject { + func streamingPlayer(_ player: StreamingSpinPlayer, startedPlaying spin: Spin) + func streamingPlayer( + _ player: StreamingSpinPlayer, didChangeState state: StreamingSpinPlayer.State) + func streamingPlayer(_ player: StreamingSpinPlayer, didEncounterError error: Error) +} + +/// Handles playback of a single spin using AVPlayer for streaming. +/// +/// ``` +/// State Machine: +/// ┌───────────┐ load() ┌──────────┐ readyToPlay ┌────────┐ +/// │ available ├───────────►│ loading ├──────────────►│ loaded │ +/// └─────▲─────┘ └────┬─────┘ └───┬────┘ +/// │ │ .failed │ play/schedulePlay +/// │ ▼ ▼ +/// │ ┌─────────┐ ┌──────────┐ +/// │ │ error │ │ playing │ +/// │ └─────────┘ └────┬─────┘ +/// │ │ endtime+1s +/// │ clear() ▼ +/// └────────────────────────────────────────── finished +/// ``` +@MainActor +public class StreamingSpinPlayer { + public enum State: Equatable { + case available + case loading + case loaded + case playing + case finished + case error + } + + private static let logger = OSLog( + subsystem: "PlayolaPlayer", + category: "StreamingSpinPlayer") + + private static let fadeTimerInterval: TimeInterval = 1.0 / 30.0 // ~33ms + private static let cleanupBufferTime: TimeInterval = 1.0 + + // MARK: - Public Properties + let id = UUID() + var spin: Spin? + weak var delegate: StreamingSpinPlayerDelegate? + + public var state: State = .available { + didSet { + if oldValue != state { + delegate?.streamingPlayer(self, didChangeState: state) + } + } + } + + // MARK: - Audio + private var avPlayer: AVPlayerProviding? + + // MARK: - Fade Automation + private(set) var fadeSchedule: [FadeStep] = [] + private(set) var nextFadeIndex: Int = 0 + private var fadeTimer: Timer? + + // MARK: - Scheduling Timers + private var playTimer: Timer? + private var clearTimer: Timer? + + // MARK: - Dependencies + private let errorReporter = PlayolaErrorReporter.shared + private let playerFactory: () -> AVPlayerProviding + + // MARK: - Playback State + private var playbackStartOffsetMS: Int = 0 + + // MARK: - Lifecycle + + init( + delegate: StreamingSpinPlayerDelegate? = nil, + playerFactory: (() -> AVPlayerProviding)? = nil + ) { + self.delegate = delegate + self.playerFactory = playerFactory ?? { AVPlayerWrapper() } + } + + // MARK: - Loading + + /// Loads a spin for streaming playback. + /// + /// Creates an AVPlayer with the spin's download URL and waits for the player + /// to report readyToPlay status. + func load(_ spin: Spin) async -> Result { + os_log( + "Loading spin: %@", log: StreamingSpinPlayer.logger, type: .info, spin.id) + + guard let audioFileUrl = spin.audioBlock.downloadUrl else { + let error = StationPlayerError.playbackError("Invalid audio file URL in spin") + Task { + await errorReporter.reportError(error, context: "Missing download URL", level: .error) + } + self.state = .available + return .failure(error) + } + + self.state = .loading + self.spin = spin + + // Build fade schedule while player buffers + self.fadeSchedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + + let player = playerFactory() + self.avPlayer = player + + do { + try await player.loadURL(audioFileUrl) + os_log( + "Player ready for spin: %@", + log: StreamingSpinPlayer.logger, type: .info, spin.id) + self.state = .loaded + return .success(()) + } catch { + os_log( + "Player failed for spin: %@ - %@", + log: StreamingSpinPlayer.logger, type: .error, + spin.id, error.localizedDescription) + Task { + await self.errorReporter.reportError( + error, context: "AVPlayerItem failed to load", level: .error) + } + self.state = .error + return .failure(error) + } + } + + // MARK: - Playback + + /// Plays the loaded spin immediately from the specified position (in seconds). + func playNow(from offsetSeconds: Double) { + guard let avPlayer, state == .loaded || state == .playing else { + os_log( + "Cannot playNow - not loaded (state: %@)", + log: StreamingSpinPlayer.logger, type: .info, + String(describing: state)) + return + } + + guard let spin else { return } + + let offsetMS = Int(offsetSeconds * 1000) + + // If past endOfMessage, skip + if offsetMS >= spin.audioBlock.endOfMessageMS { + os_log( + "Offset %d past endOfMessage %d - skipping", + log: StreamingSpinPlayer.logger, type: .info, + offsetMS, spin.audioBlock.endOfMessageMS) + clear() + return + } + + self.playbackStartOffsetMS = offsetMS + + // Set volume from fade schedule at current position + let volume = FadeScheduleBuilder.volumeAtMS( + offsetMS, in: fadeSchedule, startingVolume: spin.startingVolume) + avPlayer.volume = volume + + // Seek to position and play + let seekTime = CMTime(seconds: offsetSeconds, preferredTimescale: 600) + Task { + _ = await avPlayer.seek(to: seekTime) + avPlayer.play() + + self.state = .playing + delegate?.streamingPlayer(self, startedPlaying: spin) + + // Start fade automation from the right index + nextFadeIndex = FadeScheduleBuilder.firstUnprocessedIndex( + in: fadeSchedule, afterMS: offsetMS) + startFadeTimer() + setupClearTimer() + } + } + + /// Schedules playback to start at a specific date (for future spins). + func schedulePlay(at scheduledDate: Date) { + guard state == .loaded else { + os_log( + "Cannot schedulePlay - not loaded (state: %@)", + log: StreamingSpinPlayer.logger, type: .info, + String(describing: state)) + return + } + + guard let spin else { return } + + os_log( + "Scheduling play at %@ for spin: %@", + log: StreamingSpinPlayer.logger, type: .info, + ISO8601DateFormatter().string(from: scheduledDate), spin.id) + + // Set initial volume + avPlayer?.volume = spin.startingVolume + + self.playbackStartOffsetMS = 0 + + self.playTimer = Timer( + fire: scheduledDate, + interval: 0, + repeats: false + ) { [weak self] timer in + timer.invalidate() + guard let self else { return } + + Task { @MainActor in + guard self.playTimer === timer, let avPlayer = self.avPlayer, let spin = self.spin else { + return + } + + avPlayer.play() + self.state = .playing + self.delegate?.streamingPlayer(self, startedPlaying: spin) + self.nextFadeIndex = 0 + self.startFadeTimer() + self.setupClearTimer() + } + } + + RunLoop.main.add(self.playTimer!, forMode: .default) + } + + // MARK: - Fade Timer + + private func startFadeTimer() { + guard !fadeSchedule.isEmpty, nextFadeIndex < fadeSchedule.count else { return } + + fadeTimer = Timer.scheduledTimer( + withTimeInterval: StreamingSpinPlayer.fadeTimerInterval, + repeats: true + ) { [weak self] _ in + guard let self else { return } + Task { @MainActor in + self.processFades() + } + } + } + + private func processFades() { + guard state == .playing, let avPlayer else { + stopFadeTimer() + return + } + + let currentTimeMS = Int(avPlayer.currentTimeSeconds * 1000) + + while nextFadeIndex < fadeSchedule.count { + let step = fadeSchedule[nextFadeIndex] + if step.timeMS <= currentTimeMS { + avPlayer.volume = step.volume + nextFadeIndex += 1 + } else { + break + } + } + + if nextFadeIndex >= fadeSchedule.count { + stopFadeTimer() + } + } + + private func stopFadeTimer() { + fadeTimer?.invalidate() + fadeTimer = nil + } + + // MARK: - Clear Timer + + private func setupClearTimer() { + guard let spin else { return } + + clearTimer = Timer( + fire: spin.endtime.addingTimeInterval(StreamingSpinPlayer.cleanupBufferTime), + interval: 0, + repeats: false + ) { [weak self] timer in + timer.invalidate() + guard let self else { return } + Task { @MainActor in + guard self.clearTimer === timer else { return } + self.clearTimer = nil + self.state = .finished + self.clear() + } + } + + RunLoop.main.add(clearTimer!, forMode: .default) + } + + // MARK: - Cleanup + + func stop() { + os_log( + "StreamingSpinPlayer.stop() - ID: %@, spin: %@", + log: StreamingSpinPlayer.logger, type: .info, + id.uuidString, spin?.id ?? "nil") + clear() + } + + func clear() { + avPlayer?.pause() + avPlayer?.clearItem() + avPlayer = nil + + stopFadeTimer() + + playTimer?.invalidate() + playTimer = nil + + clearTimer?.invalidate() + clearTimer = nil + + fadeSchedule = [] + nextFadeIndex = 0 + playbackStartOffsetMS = 0 + + spin = nil + state = .available + } +} diff --git a/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift new file mode 100644 index 0000000..7db0b3c --- /dev/null +++ b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift @@ -0,0 +1,362 @@ +import AVFoundation +import Combine +import Foundation +import PlayolaCore +import os.log + +/// A streaming audio player for Playola stations using AVPlayer. +/// +/// Unlike `PlayolaStationPlayer` which downloads entire files before playback, +/// `StreamingStationPlayer` streams audio via AVPlayer for faster startup. +/// +/// ``` +/// Lifecycle: +/// play(stationId:) → fetch schedule → load current spin → start playback +/// → poll every 20s for upcoming spins +/// stop() → cancel tasks → stop all players → nil stationId +/// +/// Spin Management: +/// spinPlayers: [String: StreamingSpinPlayer] — keyed by spin ID +/// - Current spin: playNow(from: offset) +/// - Future spins: schedulePlay(at: airtime) +/// - Finished spins: cleared automatically via clearTimer +/// ``` +@MainActor +final public class StreamingStationPlayer: ObservableObject { + public enum State: Sendable { + case loading + case playing(Spin) + case idle + } + + private static let logger = OSLog( + subsystem: "PlayolaPlayer", + category: "StreamingStationPlayer") + + private static let schedulePollingInterval: TimeInterval = 20.0 + private static let scheduleWindow: TimeInterval = 600 // 10 minutes + + // MARK: - Public Properties + + @Published public var stationId: String? + @Published public var state: StreamingStationPlayer.State = .idle + + public var isPlaying: Bool { + switch state { + case .playing: return true + default: return false + } + } + + // MARK: - Dependencies + + var baseUrl = URL(string: "https://admin-api.playola.fm")! + var listeningSessionReporter: ListeningSessionReporter? + private let errorReporter = PlayolaErrorReporter.shared + private var authProvider: PlayolaAuthenticationProvider? + private let playerFactory: () -> AVPlayerProviding + private let audioSessionManager = AudioSessionManager() + + // MARK: - Internal State + + var spinPlayers: [String: StreamingSpinPlayer] = [:] + private var currentSchedule: Schedule? + private var scheduleOffset: TimeInterval? + private var schedulingTask: Task? + + // Audio interruption state + private var isSuspended = false + private var wasPlayingBeforeInterruption = false + private var interruptedStationId: String? + + // MARK: - Lifecycle + + public init(playerFactory: (() -> AVPlayerProviding)? = nil) { + self.playerFactory = playerFactory ?? { AVPlayerWrapper() } + + #if os(iOS) || os(tvOS) + NotificationCenter.default.addObserver( + self, + selector: #selector(handleAudioSessionInterruption(_:)), + name: AVAudioSession.interruptionNotification, + object: nil + ) + + NotificationCenter.default.addObserver( + self, + selector: #selector(handleAudioRouteChange(_:)), + name: AVAudioSession.routeChangeNotification, + object: nil + ) + #endif + } + + /// Configure with authentication provider. + public func configure( + authProvider: PlayolaAuthenticationProvider, + baseURL: URL = URL(string: "https://admin-api.playola.fm")! + ) { + self.authProvider = authProvider + self.baseUrl = baseURL + self.listeningSessionReporter = ListeningSessionReporter( + stationIdPublisher: $stationId.eraseToAnyPublisher(), + stationIdGetter: { [weak self] in self?.stationId }, + authProvider: authProvider, + baseURL: baseURL + ) + } + + // MARK: - Playback + + /// Begins streaming playback of the specified station. + public func play(stationId: String, atDate: Date? = nil) async throws { + isSuspended = false + wasPlayingBeforeInterruption = false + interruptedStationId = nil + + schedulingTask?.cancel() + self.scheduleOffset = atDate?.timeIntervalSinceNow + self.stationId = stationId + self.state = .loading + + try await audioSessionManager.activate() + + let schedule = try await ScheduleService.getSchedule( + stationId: stationId, baseUrl: baseUrl) + self.currentSchedule = schedule + + let currentSpins = schedule.current(offsetTimeInterval: scheduleOffset) + guard let firstSpin = currentSpins.first else { + let error = StationPlayerError.scheduleError("No available spins to play") + Task { + await errorReporter.reportError( + error, + context: "Schedule for station \(stationId) contains no current spins | " + + "Total spins: \(schedule.spins.count)", + level: .error) + } + throw error + } + + try await loadAndPlaySpin(firstSpin) + + schedulingTask = Task { + await scheduleUpcomingSpins() + } + } + + /// Stops playback and releases resources. + public func stop() { + os_log("Stop called", log: StreamingStationPlayer.logger, type: .info) + + schedulingTask?.cancel() + schedulingTask = nil + + for (_, player) in spinPlayers { + player.stop() + } + spinPlayers.removeAll() + + self.stationId = nil + self.currentSchedule = nil + self.state = .idle + } + + // MARK: - Spin Loading + + private func loadAndPlaySpin(_ spin: Spin) async throws { + let player = getOrCreateSpinPlayer(for: spin) + + let result = await player.load(spin) + switch result { + case .success: + let timing = spin.playbackTiming + switch timing { + case .playing: + let elapsedSeconds = Date().timeIntervalSince(spin.airtime) + player.playNow(from: elapsedSeconds) + self.state = .playing(spin) + case .future: + player.schedulePlay(at: spin.airtime) + case .tooLateToStart, .past: + player.clear() + spinPlayers.removeValue(forKey: spin.id) + } + case .failure(let error): + spinPlayers.removeValue(forKey: spin.id) + throw error + } + } + + private func getOrCreateSpinPlayer(for spin: Spin) -> StreamingSpinPlayer { + if let existing = spinPlayers[spin.id] { + return existing + } + let player = StreamingSpinPlayer( + delegate: self, + playerFactory: playerFactory + ) + spinPlayers[spin.id] = player + return player + } + + // MARK: - Schedule Polling + + private func scheduleUpcomingSpins() async { + guard let stationId else { return } + + while !Task.isCancelled { + do { + try await performScheduleUpdate(stationId: stationId) + } catch is CancellationError { + return + } catch { + os_log( + "Schedule update failed: %@", + log: StreamingStationPlayer.logger, type: .error, + error.localizedDescription) + } + + do { + try await Task.sleep(for: .seconds(StreamingStationPlayer.schedulePollingInterval)) + } catch { + return + } + } + } + + private func performScheduleUpdate(stationId: String) async throws { + let updatedSchedule = try await ScheduleService.getSchedule( + stationId: stationId, baseUrl: baseUrl) + + let spinsToLoad = updatedSchedule.current(offsetTimeInterval: scheduleOffset).filter { + $0.airtime < .now + StreamingStationPlayer.scheduleWindow + } + + for spin in spinsToLoad { + try Task.checkCancellation() + if spinPlayers[spin.id] == nil { + do { + try await loadAndPlaySpin(spin) + } catch { + os_log( + "Failed to load spin %@: %@", + log: StreamingStationPlayer.logger, type: .error, + spin.id, error.localizedDescription) + } + } + } + } + + // MARK: - Audio Interruptions + + #if os(iOS) || os(tvOS) + @objc public func handleAudioRouteChange(_ notification: Notification) { + guard let userInfo = notification.userInfo, + let reasonValue = userInfo[AVAudioSessionRouteChangeReasonKey] as? UInt, + let reason = AVAudioSession.RouteChangeReason(rawValue: reasonValue) + else { return } + + switch reason { + case .oldDeviceUnavailable: + guard + let previousRoute = userInfo[AVAudioSessionRouteChangePreviousRouteKey] + as? AVAudioSessionRouteDescription + else { return } + + let wasUsingHeadphones = previousRoute.outputs.contains { + [.headphones, .bluetoothA2DP, .bluetoothHFP, .bluetoothLE].contains($0.portType) + } + + if wasUsingHeadphones && isPlaying { + interruptedStationId = stationId + wasPlayingBeforeInterruption = true + stop() + } + default: + break + } + } + + @objc public func handleAudioSessionInterruption(_ notification: Notification) { + guard let userInfo = notification.userInfo, + let typeValue = userInfo[AVAudioSessionInterruptionTypeKey] as? UInt, + let type = AVAudioSession.InterruptionType(rawValue: typeValue) + else { return } + + switch type { + case .began: + isSuspended = true + wasPlayingBeforeInterruption = isPlaying + interruptedStationId = stationId + schedulingTask?.cancel() + schedulingTask = nil + + case .ended: + isSuspended = false + guard let optionsValue = userInfo[AVAudioSessionInterruptionOptionKey] as? UInt else { + return + } + let options = AVAudioSession.InterruptionOptions(rawValue: optionsValue) + if options.contains(.shouldResume) && wasPlayingBeforeInterruption { + resumeAfterInterruption() + } + + @unknown default: + break + } + } + + private func resumeAfterInterruption() { + guard let stationToResume = interruptedStationId else { return } + + Task { @MainActor in + do { + try await self.audioSessionManager.activate() + try await self.play(stationId: stationToResume) + } catch { + os_log( + "Failed to resume after interruption: %@", + log: StreamingStationPlayer.logger, type: .error, + error.localizedDescription) + await errorReporter.reportError( + error, context: "Failed to resume playback after interruption", level: .error) + } + + self.interruptedStationId = nil + self.wasPlayingBeforeInterruption = false + } + } + #endif +} + +// MARK: - StreamingSpinPlayerDelegate + +extension StreamingStationPlayer: StreamingSpinPlayerDelegate { + func streamingPlayer(_ player: StreamingSpinPlayer, startedPlaying spin: Spin) { + os_log( + "Started playing: %@", + log: StreamingStationPlayer.logger, type: .info, spin.id) + self.state = .playing(spin) + } + + func streamingPlayer( + _ player: StreamingSpinPlayer, didChangeState state: StreamingSpinPlayer.State + ) { + if state == .finished { + if let spinId = player.spin?.id ?? spinPlayers.first(where: { $0.value === player })?.key { + spinPlayers.removeValue(forKey: spinId) + } + } + } + + func streamingPlayer(_ player: StreamingSpinPlayer, didEncounterError error: Error) { + os_log( + "Spin player error: %@", + log: StreamingStationPlayer.logger, type: .error, + error.localizedDescription) + Task { + await errorReporter.reportError(error, context: "StreamingSpinPlayer error", level: .error) + } + } +} diff --git a/TODOS.md b/TODOS.md new file mode 100644 index 0000000..5be5ea1 --- /dev/null +++ b/TODOS.md @@ -0,0 +1,15 @@ +# TODOs + +## P3: Server-side audio normalization +Pre-normalize audio files to a target LUFS during upload/ingest on the backend. +Currently, the AVAudioEngine-based player applies client-side normalization via +`AVAudioUnitEQ.globalGain` (±24dB). The new streaming player and web player skip +normalization entirely. Server-side pre-normalization would solve loudness consistency +for all clients permanently. + +**Effort**: L (human) → M (CC) +**Depends on**: Backend audio pipeline access +**Context**: Added during streaming player implementation (2026-03-20). The streaming +player (AVPlayer-based) cannot do client-side normalization because AVPlayer doesn't +connect to AVAudioEngine. The web player also doesn't normalize. This is the clean +long-term solution. diff --git a/Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift b/Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift new file mode 100644 index 0000000..d780646 --- /dev/null +++ b/Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift @@ -0,0 +1,180 @@ +import Foundation +import Testing + +@testable import PlayolaCore +@testable import PlayolaPlayer + +struct FadeScheduleBuilderTests { + + // MARK: - buildFadeSchedule + + @Test("Empty fades array produces empty schedule") + func testBuildFadeScheduleNoFades() { + let spin = Spin.mockWith(fades: []) + let schedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + #expect(schedule.isEmpty) + } + + @Test("Single fade produces fadeSteps+1 entries") + func testBuildFadeScheduleSingleFade() { + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [Fade(atMS: 30000, toVolume: 0.0)] + ) + let schedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + #expect(schedule.count == FadeScheduleBuilder.fadeSteps + 1) + } + + @Test("Single fade starts at fade atMS and ends at atMS + fadeDuration") + func testBuildFadeScheduleTiming() { + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [Fade(atMS: 30000, toVolume: 0.0)] + ) + let schedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + + #expect(schedule.first?.timeMS == 30000) + #expect(schedule.last?.timeMS == 31500) + } + + @Test("Single fade interpolates from startingVolume to target") + func testBuildFadeScheduleInterpolation() { + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [Fade(atMS: 10000, toVolume: 0.0)] + ) + let schedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + + // First step should be startingVolume + #expect(schedule.first!.volume == 1.0) + // Last step should be target volume + #expect(schedule.last!.volume == 0.0) + // Midpoint should be approximately 0.5 + let midIndex = FadeScheduleBuilder.fadeSteps / 2 + #expect(abs(schedule[midIndex].volume - 0.5) < 0.02) + } + + @Test("Multiple fades chain correctly") + func testBuildFadeScheduleMultipleFades() { + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [ + Fade(atMS: 10000, toVolume: 0.0), + Fade(atMS: 20000, toVolume: 1.0), + ] + ) + let schedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + let stepsPerFade = FadeScheduleBuilder.fadeSteps + 1 + + #expect(schedule.count == stepsPerFade * 2) + + // First fade: 1.0 → 0.0 + #expect(schedule[0].volume == 1.0) + #expect(schedule[stepsPerFade - 1].volume == 0.0) + + // Second fade: 0.0 → 1.0 + #expect(schedule[stepsPerFade].volume == 0.0) + #expect(schedule[stepsPerFade * 2 - 1].volume == 1.0) + } + + @Test("Fades are sorted by atMS regardless of input order") + func testBuildFadeScheduleSortsInput() { + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [ + Fade(atMS: 20000, toVolume: 1.0), + Fade(atMS: 10000, toVolume: 0.0), + ] + ) + let schedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + + // First fade should start at 10000 + #expect(schedule.first?.timeMS == 10000) + } + + @Test("Custom fade duration and steps are respected") + func testBuildFadeScheduleCustomParams() { + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [Fade(atMS: 5000, toVolume: 0.5)] + ) + let schedule = FadeScheduleBuilder.buildFadeSchedule( + for: spin, fadeDurationMS: 1000, fadeSteps: 10) + + #expect(schedule.count == 11) + #expect(schedule.first?.timeMS == 5000) + #expect(schedule.last?.timeMS == 6000) + } + + // MARK: - volumeAtMS + + @Test("volumeAtMS before any fade returns startingVolume") + func testVolumeAtMSBeforeFades() { + let schedule = [ + FadeStep(timeMS: 10000, volume: 1.0), + FadeStep(timeMS: 11500, volume: 0.0), + ] + let volume = FadeScheduleBuilder.volumeAtMS(5000, in: schedule, startingVolume: 0.8) + #expect(volume == 0.8) + } + + @Test("volumeAtMS at exact fade time returns that fade's volume") + func testVolumeAtMSExactMatch() { + let schedule = [ + FadeStep(timeMS: 10000, volume: 0.5), + FadeStep(timeMS: 11000, volume: 0.0), + ] + let volume = FadeScheduleBuilder.volumeAtMS(10000, in: schedule, startingVolume: 1.0) + #expect(volume == 0.5) + } + + @Test("volumeAtMS after all fades returns last fade's volume") + func testVolumeAtMSAfterAllFades() { + let schedule = [ + FadeStep(timeMS: 10000, volume: 0.5), + FadeStep(timeMS: 11500, volume: 0.0), + ] + let volume = FadeScheduleBuilder.volumeAtMS(20000, in: schedule, startingVolume: 1.0) + #expect(volume == 0.0) + } + + @Test("volumeAtMS with empty schedule returns startingVolume") + func testVolumeAtMSEmptySchedule() { + let volume = FadeScheduleBuilder.volumeAtMS(5000, in: [], startingVolume: 0.7) + #expect(volume == 0.7) + } + + // MARK: - firstUnprocessedIndex + + @Test("firstUnprocessedIndex returns 0 when all fades are in the future") + func testFirstUnprocessedIndexAllFuture() { + let schedule = [ + FadeStep(timeMS: 10000, volume: 0.5), + FadeStep(timeMS: 11000, volume: 0.0), + ] + let index = FadeScheduleBuilder.firstUnprocessedIndex(in: schedule, afterMS: 5000) + #expect(index == 0) + } + + @Test("firstUnprocessedIndex returns count when all fades are past") + func testFirstUnprocessedIndexAllPast() { + let schedule = [ + FadeStep(timeMS: 10000, volume: 0.5), + FadeStep(timeMS: 11000, volume: 0.0), + ] + let index = FadeScheduleBuilder.firstUnprocessedIndex(in: schedule, afterMS: 20000) + #expect(index == schedule.count) + } + + @Test("firstUnprocessedIndex returns correct index mid-schedule") + func testFirstUnprocessedIndexMidSchedule() { + let schedule = [ + FadeStep(timeMS: 10000, volume: 0.8), + FadeStep(timeMS: 10500, volume: 0.5), + FadeStep(timeMS: 11000, volume: 0.2), + FadeStep(timeMS: 11500, volume: 0.0), + ] + let index = FadeScheduleBuilder.firstUnprocessedIndex(in: schedule, afterMS: 10500) + #expect(index == 2) + } +} diff --git a/Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift b/Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift new file mode 100644 index 0000000..2894921 --- /dev/null +++ b/Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift @@ -0,0 +1,307 @@ +import AVFoundation +import Foundation +import Testing + +@testable import PlayolaCore +@testable import PlayolaPlayer + +// MARK: - Mock AVPlayer + +@MainActor +final class MockAVPlayer: AVPlayerProviding { + var volume: Float = 1.0 + var currentTimeSeconds: Double = 0.0 + + var playCallCount = 0 + var pauseCallCount = 0 + var seekTarget: CMTime? + var loadedURL: URL? + var clearItemCallCount = 0 + + var shouldFailLoad = false + var loadError: Error = StationPlayerError.playbackError("Mock load failure") + + func play() { + playCallCount += 1 + } + + func pause() { + pauseCallCount += 1 + } + + func seek(to time: CMTime) async -> Bool { + seekTarget = time + return true + } + + func loadURL(_ url: URL) async throws { + loadedURL = url + if shouldFailLoad { + throw loadError + } + } + + func clearItem() { + clearItemCallCount += 1 + } +} + +// MARK: - Mock Delegate + +@MainActor +final class MockStreamingSpinPlayerDelegate: StreamingSpinPlayerDelegate { + var startedPlayingSpins: [Spin] = [] + var stateChanges: [StreamingSpinPlayer.State] = [] + var errors: [Error] = [] + + func streamingPlayer(_ player: StreamingSpinPlayer, startedPlaying spin: Spin) { + startedPlayingSpins.append(spin) + } + + func streamingPlayer( + _ player: StreamingSpinPlayer, didChangeState state: StreamingSpinPlayer.State + ) { + stateChanges.append(state) + } + + func streamingPlayer(_ player: StreamingSpinPlayer, didEncounterError error: Error) { + errors.append(error) + } +} + +// MARK: - Tests + +@MainActor +struct StreamingSpinPlayerTests { + + private func createPlayer(mockPlayer: MockAVPlayer? = nil) -> ( + StreamingSpinPlayer, MockAVPlayer, MockStreamingSpinPlayerDelegate + ) { + let mock = mockPlayer ?? MockAVPlayer() + let delegate = MockStreamingSpinPlayerDelegate() + let player = StreamingSpinPlayer( + delegate: delegate, + playerFactory: { mock } + ) + return (player, mock, delegate) + } + + // MARK: - Initial State + + @Test("Initial state is available") + func testInitialState() { + let (player, _, _) = createPlayer() + #expect(player.state == .available) + #expect(player.spin == nil) + } + + // MARK: - Load + + @Test("Load with nil downloadUrl fails immediately") + func testLoadNilDownloadUrl() async { + let (player, _, _) = createPlayer() + let audioBlock = AudioBlock( + id: "test", title: "Test", artist: "Test", durationMS: 30000, + endOfMessageMS: 28000, beginningOfOutroMS: 25000, endOfIntroMS: 5000, + lengthOfOutroMS: 5000, downloadUrl: nil, s3Key: "key", s3BucketName: "bucket", + type: "song", createdAt: Date(), updatedAt: Date(), album: nil, popularity: nil, + youTubeId: nil, isrc: nil, spotifyId: nil, imageUrl: nil as URL? + ) + let spin = Spin.mockWith(audioBlock: audioBlock) + + let result = await player.load(spin) + + switch result { + case .success: + Issue.record("Expected failure for nil downloadUrl") + case .failure: + #expect(player.state == .available) + } + } + + @Test("Successful load transitions to loaded state") + func testLoadSuccess() async { + let mock = MockAVPlayer() + let (player, _, delegate) = createPlayer(mockPlayer: mock) + let spin = Spin.mockWith() + + let result = await player.load(spin) + + switch result { + case .success: + #expect(player.state == .loaded) + #expect(player.spin?.id == spin.id) + #expect(mock.loadedURL != nil) + #expect(delegate.stateChanges.contains(.loading)) + #expect(delegate.stateChanges.contains(.loaded)) + case .failure(let error): + Issue.record("Expected success but got: \(error)") + } + } + + @Test("Failed load transitions to error state") + func testLoadFailure() async { + let mock = MockAVPlayer() + mock.shouldFailLoad = true + let (player, _, delegate) = createPlayer(mockPlayer: mock) + let spin = Spin.mockWith() + + let result = await player.load(spin) + + switch result { + case .success: + Issue.record("Expected failure") + case .failure: + #expect(player.state == .error) + #expect(delegate.stateChanges.contains(.loading)) + #expect(delegate.stateChanges.contains(.error)) + } + } + + @Test("Load builds fade schedule") + func testFadeScheduleBuiltDuringLoad() async { + let mock = MockAVPlayer() + let (player, _, _) = createPlayer(mockPlayer: mock) + + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [Fade(atMS: 10000, toVolume: 0.0)] + ) + + _ = await player.load(spin) + + #expect(!player.fadeSchedule.isEmpty) + #expect(player.fadeSchedule.count == FadeScheduleBuilder.fadeSteps + 1) + } + + // MARK: - Clear + + @Test("Clear resets all state") + func testClear() async { + let mock = MockAVPlayer() + let (player, _, _) = createPlayer(mockPlayer: mock) + + let spin = Spin.mockWith() + _ = await player.load(spin) + + player.clear() + + #expect(player.state == .available) + #expect(player.spin == nil) + #expect(mock.pauseCallCount == 1) + #expect(mock.clearItemCallCount == 1) + #expect(player.fadeSchedule.isEmpty) + } + + // MARK: - Stop + + @Test("Stop calls clear") + func testStop() async { + let mock = MockAVPlayer() + let (player, _, _) = createPlayer(mockPlayer: mock) + + let spin = Spin.mockWith() + _ = await player.load(spin) + + player.stop() + + #expect(player.state == .available) + #expect(player.spin == nil) + #expect(mock.pauseCallCount == 1) + } + + // MARK: - PlayNow + + @Test("PlayNow with offset past endOfMessage clears player") + func testPlayNowPastEndOfMessage() async { + let mock = MockAVPlayer() + let (player, _, _) = createPlayer(mockPlayer: mock) + + let spin = Spin.mockWith( + audioBlock: AudioBlock.mockWith(endOfMessageMS: 30000) + ) + _ = await player.load(spin) + + player.playNow(from: 31.0) // 31 seconds > 30s endOfMessage + + #expect(player.state == .available) + #expect(player.spin == nil) + } + + @Test("PlayNow when not loaded is a no-op") + func testPlayNowNotLoaded() { + let (player, mock, _) = createPlayer() + player.spin = Spin.mockWith() + + player.playNow(from: 5.0) + + #expect(mock.playCallCount == 0) + #expect(player.state == .available) + } + + @Test("PlayNow sets volume from fade schedule") + func testPlayNowSetsVolume() async { + let mock = MockAVPlayer() + let (player, _, _) = createPlayer(mockPlayer: mock) + + let spin = Spin.mockWith( + startingVolume: 0.8, + fades: [] + ) + _ = await player.load(spin) + + player.playNow(from: 0.0) + + // Give the async Task inside playNow a moment + try? await Task.sleep(for: .milliseconds(50)) + + #expect(mock.volume == 0.8) + } + + // MARK: - SchedulePlay + + @Test("SchedulePlay when not loaded is a no-op") + func testSchedulePlayNotLoaded() { + let (player, mock, _) = createPlayer() + + player.schedulePlay(at: Date().addingTimeInterval(10)) + + #expect(mock.playCallCount == 0) + } + + @Test("SchedulePlay sets initial volume") + func testSchedulePlaySetsVolume() async { + let mock = MockAVPlayer() + let (player, _, _) = createPlayer(mockPlayer: mock) + + let spin = Spin.mockWith(startingVolume: 0.6) + _ = await player.load(spin) + + player.schedulePlay(at: Date().addingTimeInterval(10)) + + #expect(mock.volume == 0.6) + } + + // MARK: - State Transitions + + @Test("Delegate receives state changes") + func testDelegateReceivesStateChanges() { + let (player, _, delegate) = createPlayer() + + player.state = .loading + player.state = .loaded + player.state = .playing + + #expect(delegate.stateChanges == [.loading, .loaded, .playing]) + } + + @Test("Duplicate state changes are not reported") + func testNoDuplicateStateChanges() { + let (player, _, delegate) = createPlayer() + + player.state = .loading + player.state = .loading // duplicate + + #expect(delegate.stateChanges == [.loading]) + } +} diff --git a/Tests/PlayolaPlayerTests/StreamingStationPlayerTests.swift b/Tests/PlayolaPlayerTests/StreamingStationPlayerTests.swift new file mode 100644 index 0000000..6a19f0c --- /dev/null +++ b/Tests/PlayolaPlayerTests/StreamingStationPlayerTests.swift @@ -0,0 +1,166 @@ +import AVFoundation +import Combine +import Foundation +import Testing + +@testable import PlayolaCore +@testable import PlayolaPlayer + +// MARK: - Mock Schedule Service + +/// Captures the mock AVPlayers created during tests so we can inspect them. +@MainActor +final class MockAVPlayerTracker { + var createdPlayers: [MockAVPlayer] = [] + + func createPlayer() -> MockAVPlayer { + let player = MockAVPlayer() + createdPlayers.append(player) + return player + } +} + +// MARK: - Tests + +@MainActor +struct StreamingStationPlayerTests { + + private func createStationPlayer(tracker: MockAVPlayerTracker? = nil) -> ( + StreamingStationPlayer, MockAVPlayerTracker + ) { + let playerTracker = tracker ?? MockAVPlayerTracker() + let stationPlayer = StreamingStationPlayer( + playerFactory: { playerTracker.createPlayer() } + ) + return (stationPlayer, playerTracker) + } + + // MARK: - Initial State + + @Test("Initial state is idle") + func testInitialState() { + let (player, _) = createStationPlayer() + #expect(player.stationId == nil) + switch player.state { + case .idle: + break + default: + Issue.record("Expected idle state") + } + #expect(!player.isPlaying) + } + + // MARK: - Stop + + @Test("Stop resets all state") + func testStop() { + let (player, _) = createStationPlayer() + player.stationId = "test-station" + + player.stop() + + #expect(player.stationId == nil) + switch player.state { + case .idle: + break + default: + Issue.record("Expected idle state after stop") + } + #expect(player.spinPlayers.isEmpty) + } + + @Test("Stop clears all spin players") + func testStopClearsSpinPlayers() async { + let tracker = MockAVPlayerTracker() + let (player, _) = createStationPlayer(tracker: tracker) + + // Manually add a spin player to simulate state + let spin = Spin.mockWith() + let spinPlayer = StreamingSpinPlayer( + delegate: player, + playerFactory: { tracker.createPlayer() } + ) + _ = await spinPlayer.load(spin) + player.spinPlayers[spin.id] = spinPlayer + + player.stop() + + #expect(player.spinPlayers.isEmpty) + } + + // MARK: - Configure + + @Test("Configure sets baseUrl") + func testConfigureSetsBaseUrl() { + let (player, _) = createStationPlayer() + let customURL = URL(string: "https://custom-api.example.com")! + + player.configure( + authProvider: StreamingMockAuthProvider(), + baseURL: customURL + ) + + #expect(player.baseUrl == customURL) + } + + @Test("Configure creates listening session reporter") + func testConfigureCreatesReporter() { + let (player, _) = createStationPlayer() + + player.configure(authProvider: StreamingMockAuthProvider()) + + #expect(player.listeningSessionReporter != nil) + } + + // MARK: - Spin Player Delegate + + @Test("Finished spin player is removed from spinPlayers") + func testFinishedSpinPlayerRemoved() async { + let tracker = MockAVPlayerTracker() + let (stationPlayer, _) = createStationPlayer(tracker: tracker) + + let spin = Spin.mockWith() + let spinPlayer = StreamingSpinPlayer( + delegate: stationPlayer, + playerFactory: { tracker.createPlayer() } + ) + _ = await spinPlayer.load(spin) + stationPlayer.spinPlayers[spin.id] = spinPlayer + + // Simulate the spin finishing + stationPlayer.streamingPlayer(spinPlayer, didChangeState: .finished) + + #expect(stationPlayer.spinPlayers[spin.id] == nil) + } + + // MARK: - isPlaying + + @Test("isPlaying returns true when state is playing") + func testIsPlayingTrue() { + let (player, _) = createStationPlayer() + player.state = .playing(Spin.mockWith()) + #expect(player.isPlaying) + } + + @Test("isPlaying returns false when state is idle") + func testIsPlayingFalseIdle() { + let (player, _) = createStationPlayer() + player.state = .idle + #expect(!player.isPlaying) + } + + @Test("isPlaying returns false when state is loading") + func testIsPlayingFalseLoading() { + let (player, _) = createStationPlayer() + player.state = .loading + #expect(!player.isPlaying) + } +} + +// MARK: - Mock Auth Provider + +@MainActor +private final class StreamingMockAuthProvider: PlayolaAuthenticationProvider { + nonisolated func getCurrentToken() async -> String? { "mock-token" } + nonisolated func refreshToken() async -> String? { "mock-refreshed-token" } +} From b2e77e086f1a4ba4338758ef7fb7b067c2b48c39 Mon Sep 17 00:00:00 2001 From: Brian Keane Date: Fri, 20 Mar 2026 19:25:47 -0500 Subject: [PATCH 2/7] Add streaming/download toggle to example app and fix station switching - Add player mode picker (Streaming vs Download) to ContentView - Stop current player before switching stations - Hard-code default station ID for testing Co-Authored-By: Claude Opus 4.6 --- .../PlayolaPlayerExample/ContentView.swift | 279 ++++++++++-------- .../Player/PlayolaStationPlayer.swift | 2 +- 2 files changed, 162 insertions(+), 119 deletions(-) diff --git a/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift b/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift index 3385984..561f90d 100644 --- a/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift +++ b/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift @@ -8,6 +8,11 @@ import PlayolaPlayer import SwiftUI +enum PlayerMode: String, CaseIterable { + case streaming = "Streaming" + case download = "Download" +} + // Main thread responsiveness monitor class MainThreadMonitor: ObservableObject { @Published var isResponsive = true @@ -35,7 +40,7 @@ class MainThreadMonitor: ObservableObject { let elapsed = displayLink.timestamp - lastUpdate if elapsed >= 1.0 { fps = Double(frameCount) / elapsed - isResponsive = fps > 30 // Consider unresponsive if below 30 FPS + isResponsive = fps > 30 frameCount = 0 lastUpdate = displayLink.timestamp } @@ -43,15 +48,62 @@ class MainThreadMonitor: ObservableObject { } struct ContentView: View { - @ObservedObject var player = PlayolaStationPlayer.shared + @ObservedObject var downloadPlayer = PlayolaStationPlayer.shared + @ObservedObject var streamingPlayer = StreamingStationPlayer() @StateObject private var threadMonitor = MainThreadMonitor() @State private var showingStationPicker = false @State private var showingScheduleViewer = false @State private var selectedStationId: String = "9d79fd38-1940-4312-8fe8-3b9b50d49c6c" + @State private var playerMode: PlayerMode = .streaming + + private var isPlaying: Bool { + switch playerMode { + case .streaming: return streamingPlayer.isPlaying + case .download: return downloadPlayer.isPlaying + } + } + + private var nowPlayingSpin: Spin? { + switch playerMode { + case .streaming: + if case .playing(let spin) = streamingPlayer.state { return spin } + case .download: + if case .playing(let spin) = downloadPlayer.state { return spin } + } + return nil + } + + private var isLoading: Bool { + switch playerMode { + case .streaming: + if case .loading = streamingPlayer.state { return true } + case .download: + if case .loading = downloadPlayer.state { return true } + } + return false + } + + private var isIdle: Bool { + switch playerMode { + case .streaming: + if case .idle = streamingPlayer.state { return true } + case .download: + if case .idle = downloadPlayer.state { return true } + } + return false + } + + private var loadingProgress: Float? { + if case .download = playerMode, + case .loading(let progress) = downloadPlayer.state + { + return progress + } + return nil + } var body: some View { ZStack { - // Background gradient LinearGradient( gradient: Gradient(colors: [Color.black, Color.gray.opacity(0.3)]), startPoint: .topLeading, @@ -60,7 +112,7 @@ struct ContentView: View { .ignoresSafeArea() VStack(spacing: 30) { - // Header with thread monitor + // Header with thread monitor and player toggle HStack { VStack(alignment: .leading, spacing: 5) { Text("Main Thread Monitor") @@ -75,25 +127,46 @@ struct ContentView: View { } .padding() + // Player mode toggle + VStack(spacing: 8) { + Picker("Player Mode", selection: $playerMode) { + ForEach(PlayerMode.allCases, id: \.self) { mode in + Text(mode.rawValue).tag(mode) + } + } + .pickerStyle(.segmented) + .padding(.horizontal) + .onChange(of: playerMode) { _, _ in + stopCurrentPlayer() + } + + Text( + playerMode == .streaming + ? "AVPlayer streaming (fast startup)" + : "AVAudioEngine download (full file)" + ) + .font(.caption2) + .foregroundColor(.white.opacity(0.5)) + } + Spacer() - // Main content VStack(spacing: 25) { - // Album art placeholder with animation + // Album art placeholder ZStack { RoundedRectangle(cornerRadius: 20) .fill(Color.white.opacity(0.1)) .frame(width: 250, height: 250) - if player.isPlaying { + if isPlaying { Image(systemName: "music.note") .font(.system(size: 80)) .foregroundColor(.white.opacity(0.7)) - .rotationEffect(.degrees(player.isPlaying ? 360 : 0)) + .rotationEffect(.degrees(isPlaying ? 360 : 0)) .animation( - player.isPlaying + isPlaying ? Animation.linear(duration: 3).repeatForever(autoreverses: false) : .default, - value: player.isPlaying + value: isPlaying ) } else { Image(systemName: "radio") @@ -104,7 +177,7 @@ struct ContentView: View { // Now playing info VStack(spacing: 10) { - if case .playing(let spin) = player.state { + if let spin = nowPlayingSpin { Text(spin.audioBlock.title) .font(.title2) .fontWeight(.semibold) @@ -115,19 +188,24 @@ struct ContentView: View { .font(.headline) .foregroundColor(.white.opacity(0.7)) .lineLimit(1) - } else if case .loading(let progress) = player.state { + } else if isLoading { VStack(spacing: 15) { Text("Loading Station...") .font(.headline) .foregroundColor(.white.opacity(0.8)) - ProgressView(value: progress) - .progressViewStyle(LinearProgressViewStyle(tint: .white)) - .frame(width: 200) - - Text("\(Int(progress * 100))%") - .font(.caption) - .foregroundColor(.white.opacity(0.6)) + if let progress = loadingProgress { + ProgressView(value: progress) + .progressViewStyle(LinearProgressViewStyle(tint: .white)) + .frame(width: 200) + + Text("\(Int(progress * 100))%") + .font(.caption) + .foregroundColor(.white.opacity(0.6)) + } else { + ProgressView() + .progressViewStyle(CircularProgressViewStyle(tint: .white)) + } } } else { Text("Ready to Play") @@ -139,49 +217,31 @@ struct ContentView: View { // Offset playback controls VStack(spacing: 20) { - // Time offset buttons Text("Play from different times:") .font(.caption) .foregroundColor(.white.opacity(0.6)) HStack(spacing: 15) { - Button("5min ago") { - playWithOffset(-300) // 5 minutes ago - } - .buttonStyle(OffsetButtonStyle()) - - Button("1min ago") { - playWithOffset(-60) // 1 minute ago - } - .buttonStyle(OffsetButtonStyle()) - - Button("10sec ago") { - playWithOffset(-10) // 10 seconds ago - } - .buttonStyle(OffsetButtonStyle()) + Button("5min ago") { playWithOffset(-300) } + .buttonStyle(OffsetButtonStyle()) + Button("1min ago") { playWithOffset(-60) } + .buttonStyle(OffsetButtonStyle()) + Button("10sec ago") { playWithOffset(-10) } + .buttonStyle(OffsetButtonStyle()) } HStack(spacing: 15) { - Button("10sec future") { - playWithOffset(10) // 10 seconds from now - } - .buttonStyle(OffsetButtonStyle()) - - Button("1min future") { - playWithOffset(60) // 1 minute from now - } - .buttonStyle(OffsetButtonStyle()) - - Button("5min future") { - playWithOffset(300) // 5 minutes from now - } - .buttonStyle(OffsetButtonStyle()) + Button("10sec future") { playWithOffset(10) } + .buttonStyle(OffsetButtonStyle()) + Button("1min future") { playWithOffset(60) } + .buttonStyle(OffsetButtonStyle()) + Button("5min future") { playWithOffset(300) } + .buttonStyle(OffsetButtonStyle()) } } // Main playback controls HStack(spacing: 40) { - // Station picker Button( action: { showingStationPicker.toggle() }, label: { @@ -190,23 +250,21 @@ struct ContentView: View { .foregroundColor(.white.opacity(0.8)) }) - // Play/Stop button (current time) Button( action: { playOrPause() }, label: { ZStack { Circle() - .fill(buttonColor(for: player.state)) + .fill(currentButtonColor) .frame(width: 80, height: 80) - Image(systemName: buttonIcon(for: player.state)) + Image(systemName: currentButtonIcon) .font(.title) .foregroundColor(.white) - .offset(x: shouldOffsetIcon(for: player.state) ? 3 : 0) // Center play icon + .offset(x: isIdle ? 3 : 0) } }) - // Schedule viewer Button( action: { showingScheduleViewer.toggle() }, label: { @@ -221,28 +279,48 @@ struct ContentView: View { } } .sheet(isPresented: $showingStationPicker) { - StationPickerView(selectedStationId: $selectedStationId) + StationPickerView( + selectedStationId: $selectedStationId, + playerMode: playerMode, + streamingPlayer: streamingPlayer + ) } .sheet(isPresented: $showingScheduleViewer) { ScheduleViewer(selectedStationId: selectedStationId) } } + private var currentButtonColor: Color { + if isPlaying { return .red } + if isLoading { return .orange } + return .green + } + + private var currentButtonIcon: String { + if isIdle { return "play.fill" } + return "stop.fill" + } + + func stopCurrentPlayer() { + Task { + await downloadPlayer.stop() + streamingPlayer.stop() + } + } + func playOrPause() { Task { - switch await player.state { - case .loading: - // Cancel loading - await player.stop() - case .playing: - // Stop playing - await player.stop() - case .idle: - // Start playing + if isPlaying || isLoading { + stopCurrentPlayer() + } else { do { - try await player.play(stationId: selectedStationId) + switch playerMode { + case .streaming: + try await streamingPlayer.play(stationId: selectedStationId) + case .download: + try await downloadPlayer.play(stationId: selectedStationId) + } } catch { - // Handle errors gracefully (including cancellation during loading) print("Failed to start playback: \(error)") } } @@ -251,18 +329,15 @@ struct ContentView: View { func playWithOffset(_ offsetSeconds: TimeInterval) { Task { - // Always stop current playback first - await player.stop() - - // Calculate the target date + stopCurrentPlayer() let atDate = Date().addingTimeInterval(offsetSeconds) - do { - try await player.play( - stationId: selectedStationId, - atDate: atDate - ) - print("Started playback with offset: \(offsetSeconds) seconds (at: \(atDate))") + switch playerMode { + case .streaming: + try await streamingPlayer.play(stationId: selectedStationId, atDate: atDate) + case .download: + try await downloadPlayer.play(stationId: selectedStationId, atDate: atDate) + } } catch { print("Failed to start offset playback: \(error)") } @@ -277,7 +352,6 @@ struct ThreadResponsivenessIndicator: View { var body: some View { VStack(spacing: 8) { - // Visual spinner that shows thread responsiveness ZStack { Circle() .stroke(Color.white.opacity(0.2), lineWidth: 3) @@ -361,6 +435,8 @@ struct StationInfo: Codable, Identifiable { struct StationPickerView: View { @Environment(\.dismiss) var dismiss @Binding var selectedStationId: String + var playerMode: PlayerMode + var streamingPlayer: StreamingStationPlayer @State private var stations: [StationInfo] = [] @State private var isLoading = true @State private var errorMessage: String? @@ -389,10 +465,16 @@ struct StationPickerView: View { action: { Task { do { - // Use playolaID for playola stations let stationId = station.playolaID ?? station.id selectedStationId = stationId - try await PlayolaStationPlayer.shared.play(stationId: stationId) + switch playerMode { + case .streaming: + streamingPlayer.stop() + try await streamingPlayer.play(stationId: stationId) + case .download: + await PlayolaStationPlayer.shared.stop() + try await PlayolaStationPlayer.shared.play(stationId: stationId) + } } catch { print("Failed to start playback: \(error)") } @@ -401,7 +483,6 @@ struct StationPickerView: View { }, label: { HStack { - // Show image if available if let imageURL = station.imageURL, let url = URL(string: imageURL) { AsyncImage(url: url) { image in image @@ -454,12 +535,10 @@ struct StationPickerView: View { let response = try JSONDecoder().decode(StationListsResponse.self, from: data) - // Get stations from in_development_list and artist_list var allStations: [StationInfo] = [] for list in response.stationLists { if list.id == "in_development_list" || list.id == "artist_list" { - // Filter to only include playola type stations let playolaStations = list.stations.filter { $0.type == "playola" } allStations.append(contentsOf: playolaStations) } @@ -478,42 +557,6 @@ struct StationPickerView: View { } } -func isLoading(_ state: PlayolaStationPlayer.State) -> Bool { - if case .loading = state { - return true - } - return false -} - -func buttonColor(for state: PlayolaStationPlayer.State) -> Color { - switch state { - case .loading: - return Color.orange - case .playing: - return Color.red - case .idle: - return Color.green - } -} - -func buttonIcon(for state: PlayolaStationPlayer.State) -> String { - switch state { - case .loading: - return "stop.fill" - case .playing: - return "stop.fill" - case .idle: - return "play.fill" - } -} - -func shouldOffsetIcon(for state: PlayolaStationPlayer.State) -> Bool { - if case .idle = state { - return true - } - return false -} - // Custom button style for offset buttons struct OffsetButtonStyle: ButtonStyle { func makeBody(configuration: Configuration) -> some View { diff --git a/Sources/PlayolaPlayer/Player/PlayolaStationPlayer.swift b/Sources/PlayolaPlayer/Player/PlayolaStationPlayer.swift index 437ec14..5461481 100644 --- a/Sources/PlayolaPlayer/Player/PlayolaStationPlayer.swift +++ b/Sources/PlayolaPlayer/Player/PlayolaStationPlayer.swift @@ -59,7 +59,7 @@ public enum StationPlayerError: Error, LocalizedError { /// ``` @MainActor final public class PlayolaStationPlayer: ObservableObject { - var baseUrl = URL(string: "https://admin-api.playola.fm/v1")! + var baseUrl = URL(string: "https://admin-api.playola.fm")! @Published public var stationId: String? private var interruptedStationId: String? var currentSchedule: Schedule? From c54dc5ed903f9a3ee2eca6afda0e0fb06ddb96d6 Mon Sep 17 00:00:00 2001 From: Brian Keane Date: Fri, 20 Mar 2026 19:32:18 -0500 Subject: [PATCH 3/7] Fix SwiftLint violations: large tuple and for-where - Replace 3-member tuple return with PlayerTestContext struct - Use `where` clause instead of `if` inside `for` loop Co-Authored-By: Claude Opus 4.6 --- .../Streaming/FadeScheduleBuilder.swift | 6 +- .../StreamingSpinPlayerTests.swift | 132 +++++++++--------- 2 files changed, 70 insertions(+), 68 deletions(-) diff --git a/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift b/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift index 13d3bba..721f3e3 100644 --- a/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift +++ b/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift @@ -81,10 +81,8 @@ public enum FadeScheduleBuilder { in schedule: [FadeStep], afterMS currentMS: Int ) -> Int { - for (index, step) in schedule.enumerated() { - if step.timeMS > currentMS { - return index - } + for (index, step) in schedule.enumerated() where step.timeMS > currentMS { + return index } return schedule.count } diff --git a/Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift b/Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift index 2894921..1600fdb 100644 --- a/Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift +++ b/Tests/PlayolaPlayerTests/StreamingSpinPlayerTests.swift @@ -74,32 +74,36 @@ final class MockStreamingSpinPlayerDelegate: StreamingSpinPlayerDelegate { @MainActor struct StreamingSpinPlayerTests { - private func createPlayer(mockPlayer: MockAVPlayer? = nil) -> ( - StreamingSpinPlayer, MockAVPlayer, MockStreamingSpinPlayerDelegate - ) { + private struct PlayerTestContext { + let player: StreamingSpinPlayer + let mock: MockAVPlayer + let delegate: MockStreamingSpinPlayerDelegate + } + + private func createPlayer(mockPlayer: MockAVPlayer? = nil) -> PlayerTestContext { let mock = mockPlayer ?? MockAVPlayer() let delegate = MockStreamingSpinPlayerDelegate() let player = StreamingSpinPlayer( delegate: delegate, playerFactory: { mock } ) - return (player, mock, delegate) + return PlayerTestContext(player: player, mock: mock, delegate: delegate) } // MARK: - Initial State @Test("Initial state is available") func testInitialState() { - let (player, _, _) = createPlayer() - #expect(player.state == .available) - #expect(player.spin == nil) + let ctx = createPlayer() + #expect(ctx.player.state == .available) + #expect(ctx.player.spin == nil) } // MARK: - Load @Test("Load with nil downloadUrl fails immediately") func testLoadNilDownloadUrl() async { - let (player, _, _) = createPlayer() + let ctx = createPlayer() let audioBlock = AudioBlock( id: "test", title: "Test", artist: "Test", durationMS: 30000, endOfMessageMS: 28000, beginningOfOutroMS: 25000, endOfIntroMS: 5000, @@ -109,31 +113,31 @@ struct StreamingSpinPlayerTests { ) let spin = Spin.mockWith(audioBlock: audioBlock) - let result = await player.load(spin) + let result = await ctx.player.load(spin) switch result { case .success: Issue.record("Expected failure for nil downloadUrl") case .failure: - #expect(player.state == .available) + #expect(ctx.player.state == .available) } } @Test("Successful load transitions to loaded state") func testLoadSuccess() async { let mock = MockAVPlayer() - let (player, _, delegate) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith() - let result = await player.load(spin) + let result = await ctx.player.load(spin) switch result { case .success: - #expect(player.state == .loaded) - #expect(player.spin?.id == spin.id) + #expect(ctx.player.state == .loaded) + #expect(ctx.player.spin?.id == spin.id) #expect(mock.loadedURL != nil) - #expect(delegate.stateChanges.contains(.loading)) - #expect(delegate.stateChanges.contains(.loaded)) + #expect(ctx.delegate.stateChanges.contains(.loading)) + #expect(ctx.delegate.stateChanges.contains(.loaded)) case .failure(let error): Issue.record("Expected success but got: \(error)") } @@ -143,35 +147,35 @@ struct StreamingSpinPlayerTests { func testLoadFailure() async { let mock = MockAVPlayer() mock.shouldFailLoad = true - let (player, _, delegate) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith() - let result = await player.load(spin) + let result = await ctx.player.load(spin) switch result { case .success: Issue.record("Expected failure") case .failure: - #expect(player.state == .error) - #expect(delegate.stateChanges.contains(.loading)) - #expect(delegate.stateChanges.contains(.error)) + #expect(ctx.player.state == .error) + #expect(ctx.delegate.stateChanges.contains(.loading)) + #expect(ctx.delegate.stateChanges.contains(.error)) } } @Test("Load builds fade schedule") func testFadeScheduleBuiltDuringLoad() async { let mock = MockAVPlayer() - let (player, _, _) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith( startingVolume: 1.0, fades: [Fade(atMS: 10000, toVolume: 0.0)] ) - _ = await player.load(spin) + _ = await ctx.player.load(spin) - #expect(!player.fadeSchedule.isEmpty) - #expect(player.fadeSchedule.count == FadeScheduleBuilder.fadeSteps + 1) + #expect(!ctx.player.fadeSchedule.isEmpty) + #expect(ctx.player.fadeSchedule.count == FadeScheduleBuilder.fadeSteps + 1) } // MARK: - Clear @@ -179,18 +183,18 @@ struct StreamingSpinPlayerTests { @Test("Clear resets all state") func testClear() async { let mock = MockAVPlayer() - let (player, _, _) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith() - _ = await player.load(spin) + _ = await ctx.player.load(spin) - player.clear() + ctx.player.clear() - #expect(player.state == .available) - #expect(player.spin == nil) + #expect(ctx.player.state == .available) + #expect(ctx.player.spin == nil) #expect(mock.pauseCallCount == 1) #expect(mock.clearItemCallCount == 1) - #expect(player.fadeSchedule.isEmpty) + #expect(ctx.player.fadeSchedule.isEmpty) } // MARK: - Stop @@ -198,15 +202,15 @@ struct StreamingSpinPlayerTests { @Test("Stop calls clear") func testStop() async { let mock = MockAVPlayer() - let (player, _, _) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith() - _ = await player.load(spin) + _ = await ctx.player.load(spin) - player.stop() + ctx.player.stop() - #expect(player.state == .available) - #expect(player.spin == nil) + #expect(ctx.player.state == .available) + #expect(ctx.player.spin == nil) #expect(mock.pauseCallCount == 1) } @@ -215,42 +219,42 @@ struct StreamingSpinPlayerTests { @Test("PlayNow with offset past endOfMessage clears player") func testPlayNowPastEndOfMessage() async { let mock = MockAVPlayer() - let (player, _, _) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith( audioBlock: AudioBlock.mockWith(endOfMessageMS: 30000) ) - _ = await player.load(spin) + _ = await ctx.player.load(spin) - player.playNow(from: 31.0) // 31 seconds > 30s endOfMessage + ctx.player.playNow(from: 31.0) // 31 seconds > 30s endOfMessage - #expect(player.state == .available) - #expect(player.spin == nil) + #expect(ctx.player.state == .available) + #expect(ctx.player.spin == nil) } @Test("PlayNow when not loaded is a no-op") func testPlayNowNotLoaded() { - let (player, mock, _) = createPlayer() - player.spin = Spin.mockWith() + let ctx = createPlayer() + ctx.player.spin = Spin.mockWith() - player.playNow(from: 5.0) + ctx.player.playNow(from: 5.0) - #expect(mock.playCallCount == 0) - #expect(player.state == .available) + #expect(ctx.mock.playCallCount == 0) + #expect(ctx.player.state == .available) } @Test("PlayNow sets volume from fade schedule") func testPlayNowSetsVolume() async { let mock = MockAVPlayer() - let (player, _, _) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith( startingVolume: 0.8, fades: [] ) - _ = await player.load(spin) + _ = await ctx.player.load(spin) - player.playNow(from: 0.0) + ctx.player.playNow(from: 0.0) // Give the async Task inside playNow a moment try? await Task.sleep(for: .milliseconds(50)) @@ -262,22 +266,22 @@ struct StreamingSpinPlayerTests { @Test("SchedulePlay when not loaded is a no-op") func testSchedulePlayNotLoaded() { - let (player, mock, _) = createPlayer() + let ctx = createPlayer() - player.schedulePlay(at: Date().addingTimeInterval(10)) + ctx.player.schedulePlay(at: Date().addingTimeInterval(10)) - #expect(mock.playCallCount == 0) + #expect(ctx.mock.playCallCount == 0) } @Test("SchedulePlay sets initial volume") func testSchedulePlaySetsVolume() async { let mock = MockAVPlayer() - let (player, _, _) = createPlayer(mockPlayer: mock) + let ctx = createPlayer(mockPlayer: mock) let spin = Spin.mockWith(startingVolume: 0.6) - _ = await player.load(spin) + _ = await ctx.player.load(spin) - player.schedulePlay(at: Date().addingTimeInterval(10)) + ctx.player.schedulePlay(at: Date().addingTimeInterval(10)) #expect(mock.volume == 0.6) } @@ -286,22 +290,22 @@ struct StreamingSpinPlayerTests { @Test("Delegate receives state changes") func testDelegateReceivesStateChanges() { - let (player, _, delegate) = createPlayer() + let ctx = createPlayer() - player.state = .loading - player.state = .loaded - player.state = .playing + ctx.player.state = .loading + ctx.player.state = .loaded + ctx.player.state = .playing - #expect(delegate.stateChanges == [.loading, .loaded, .playing]) + #expect(ctx.delegate.stateChanges == [.loading, .loaded, .playing]) } @Test("Duplicate state changes are not reported") func testNoDuplicateStateChanges() { - let (player, _, delegate) = createPlayer() + let ctx = createPlayer() - player.state = .loading - player.state = .loading // duplicate + ctx.player.state = .loading + ctx.player.state = .loading // duplicate - #expect(delegate.stateChanges == [.loading]) + #expect(ctx.delegate.stateChanges == [.loading]) } } From 4658b78739c8f4a0d3e4213d98701a9abc032e44 Mon Sep 17 00:00:00 2001 From: Brian Keane Date: Fri, 20 Mar 2026 19:49:35 -0500 Subject: [PATCH 4/7] Fix concurrency bugs, memory leak, and race conditions from review - P1: AVPlayerWrapper.waitForReady now keeps observing on .unknown status instead of leaking the continuation; throws on @unknown default - P1: StreamingSpinPlayer.playNow guards state after await seek to prevent clear() race from leaving player in .playing with dangling timers - P1: StreamingStationPlayer adds deinit to remove NotificationCenter observers, preventing retain cycle / memory leak - P2: stopCurrentPlayer() is now async; callers await it before starting new playback to prevent stop/play races Co-Authored-By: Claude Opus 4.6 --- .../PlayolaPlayerExample/ContentView.swift | 14 ++++++-------- .../Player/Streaming/AVPlayerProviding.swift | 16 +++++++++++----- .../Player/Streaming/StreamingSpinPlayer.swift | 8 ++++++-- .../Streaming/StreamingStationPlayer.swift | 6 ++++++ 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift b/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift index 561f90d..d1f0e6c 100644 --- a/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift +++ b/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift @@ -137,7 +137,7 @@ struct ContentView: View { .pickerStyle(.segmented) .padding(.horizontal) .onChange(of: playerMode) { _, _ in - stopCurrentPlayer() + Task { await stopCurrentPlayer() } } Text( @@ -301,17 +301,15 @@ struct ContentView: View { return "stop.fill" } - func stopCurrentPlayer() { - Task { - await downloadPlayer.stop() - streamingPlayer.stop() - } + func stopCurrentPlayer() async { + await downloadPlayer.stop() + streamingPlayer.stop() } func playOrPause() { Task { if isPlaying || isLoading { - stopCurrentPlayer() + await stopCurrentPlayer() } else { do { switch playerMode { @@ -329,7 +327,7 @@ struct ContentView: View { func playWithOffset(_ offsetSeconds: TimeInterval) { Task { - stopCurrentPlayer() + await stopCurrentPlayer() let atDate = Date().addingTimeInterval(offsetSeconds) do { switch playerMode { diff --git a/Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift b/Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift index 7565668..e78e464 100644 --- a/Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift +++ b/Sources/PlayolaPlayer/Player/Streaming/AVPlayerProviding.swift @@ -69,21 +69,27 @@ public class AVPlayerWrapper: AVPlayerProviding { guard let self else { return } Task { @MainActor in - self.statusObservation?.invalidate() - self.statusObservation = nil - switch observedItem.status { case .readyToPlay: + self.statusObservation?.invalidate() + self.statusObservation = nil continuation.resume() case .failed: + self.statusObservation?.invalidate() + self.statusObservation = nil let error = observedItem.error ?? StationPlayerError.playbackError("AVPlayerItem failed to load") continuation.resume(throwing: error) case .unknown: - break + // Not yet determined — keep observing. + return @unknown default: - break + self.statusObservation?.invalidate() + self.statusObservation = nil + continuation.resume( + throwing: StationPlayerError.playbackError( + "AVPlayerItem entered unexpected status")) } } } diff --git a/Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift b/Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift index 85167e8..ff6acad 100644 --- a/Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift +++ b/Sources/PlayolaPlayer/Player/Streaming/StreamingSpinPlayer.swift @@ -173,10 +173,14 @@ public class StreamingSpinPlayer { // Seek to position and play let seekTime = CMTime(seconds: offsetSeconds, preferredTimescale: 600) - Task { + Task { [weak self] in + guard let self else { return } _ = await avPlayer.seek(to: seekTime) - avPlayer.play() + // Guard against clear() called during the await + guard self.state == .loaded || self.state == .playing else { return } + + avPlayer.play() self.state = .playing delegate?.streamingPlayer(self, startedPlaying: spin) diff --git a/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift index 7db0b3c..0ecad52 100644 --- a/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift +++ b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift @@ -91,6 +91,12 @@ final public class StreamingStationPlayer: ObservableObject { #endif } + deinit { + #if os(iOS) || os(tvOS) + NotificationCenter.default.removeObserver(self) + #endif + } + /// Configure with authentication provider. public func configure( authProvider: PlayolaAuthenticationProvider, From 52847967ccd8d23a5256e1c871236c2d6d26493c Mon Sep 17 00:00:00 2001 From: Brian Keane Date: Fri, 20 Mar 2026 20:02:14 -0500 Subject: [PATCH 5/7] Fix review round 2: idle state, @StateObject, overlapping fades - P1: StreamingStationPlayer transitions to .idle when last spin finishes and spinPlayers is empty - P1: Change @ObservedObject to @StateObject for streamingPlayer in ContentView so SwiftUI owns its lifecycle - P2: FadeScheduleBuilder skips overlapping steps to ensure monotonic time ordering when fades are closer than fadeDurationMS - Add test for overlapping fade edge case Co-Authored-By: Claude Opus 4.6 --- .../PlayolaPlayerExample/ContentView.swift | 2 +- .../Streaming/FadeScheduleBuilder.swift | 8 +++++++ .../Streaming/StreamingStationPlayer.swift | 3 +++ .../FadeScheduleBuilderTests.swift | 23 +++++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift b/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift index d1f0e6c..a2ce7bf 100644 --- a/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift +++ b/PlayolaPlayerExample/PlayolaPlayerExample/ContentView.swift @@ -49,7 +49,7 @@ class MainThreadMonitor: ObservableObject { struct ContentView: View { @ObservedObject var downloadPlayer = PlayolaStationPlayer.shared - @ObservedObject var streamingPlayer = StreamingStationPlayer() + @StateObject var streamingPlayer = StreamingStationPlayer() @StateObject private var threadMonitor = MainThreadMonitor() @State private var showingStationPicker = false @State private var showingScheduleViewer = false diff --git a/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift b/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift index 721f3e3..013b822 100644 --- a/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift +++ b/Sources/PlayolaPlayer/Player/Streaming/FadeScheduleBuilder.swift @@ -36,6 +36,8 @@ public enum FadeScheduleBuilder { var fromVolume = spin.startingVolume let stepDurationMS = fadeDurationMS / Double(fadeSteps) + var lastTimeMS = Int.min + for fade in sortedFades { let toVolume = fade.toVolume let fadeStartMS = fade.atMS @@ -44,7 +46,13 @@ public enum FadeScheduleBuilder { let progress = Float(step) / Float(fadeSteps) let volume = fromVolume + (toVolume - fromVolume) * progress let timeMS = fadeStartMS + Int(Double(step) * stepDurationMS) + + // Ensure monotonic time ordering — skip steps that overlap with previous fade + if timeMS <= lastTimeMS { + continue + } schedule.append(FadeStep(timeMS: timeMS, volume: volume)) + lastTimeMS = timeMS } fromVolume = toVolume diff --git a/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift index 0ecad52..eef0b44 100644 --- a/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift +++ b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift @@ -353,6 +353,9 @@ extension StreamingStationPlayer: StreamingSpinPlayerDelegate { if let spinId = player.spin?.id ?? spinPlayers.first(where: { $0.value === player })?.key { spinPlayers.removeValue(forKey: spinId) } + if spinPlayers.isEmpty { + self.state = .idle + } } } diff --git a/Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift b/Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift index d780646..5b5d709 100644 --- a/Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift +++ b/Tests/PlayolaPlayerTests/FadeScheduleBuilderTests.swift @@ -177,4 +177,27 @@ struct FadeScheduleBuilderTests { let index = FadeScheduleBuilder.firstUnprocessedIndex(in: schedule, afterMS: 10500) #expect(index == 2) } + + // MARK: - Overlapping Fades + + @Test("Overlapping fades produce monotonic time ordering") + func testOverlappingFadesMonotonic() { + let spin = Spin.mockWith( + startingVolume: 1.0, + fades: [ + Fade(atMS: 10000, toVolume: 0.5), + Fade(atMS: 10500, toVolume: 0.0), // Starts before first fade ends (10000 + 1500) + ] + ) + + let schedule = FadeScheduleBuilder.buildFadeSchedule(for: spin) + + // Verify all times are strictly increasing + for i in 1.. schedule[i - 1].timeMS, + "Times must increase: [\(i)] \(schedule[i].timeMS) <= [\(i-1)] \(schedule[i-1].timeMS)" + ) + } + } } From 98c58cf41c78f6738bbe158683da0eef05f061fd Mon Sep 17 00:00:00 2001 From: Brian Keane Date: Fri, 20 Mar 2026 20:11:13 -0500 Subject: [PATCH 6/7] Guard against stop() during loadAndPlaySpin await After awaiting player.load(), check that stationId is still set before proceeding to playback. If stop() ran during the suspension, clean up the player instead of overwriting the .idle state. Co-Authored-By: Claude Opus 4.6 --- .../Player/Streaming/StreamingStationPlayer.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift index eef0b44..1fef633 100644 --- a/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift +++ b/Sources/PlayolaPlayer/Player/Streaming/StreamingStationPlayer.swift @@ -174,6 +174,14 @@ final public class StreamingStationPlayer: ObservableObject { let player = getOrCreateSpinPlayer(for: spin) let result = await player.load(spin) + + // Guard against stop() called during the await + guard stationId != nil else { + player.clear() + spinPlayers.removeValue(forKey: spin.id) + return + } + switch result { case .success: let timing = spin.playbackTiming From 03e2ce3428b691f2b3105947756429cba0be30a5 Mon Sep 17 00:00:00 2001 From: Brian Keane Date: Fri, 20 Mar 2026 20:33:51 -0500 Subject: [PATCH 7/7] Add PR review comment reaction guideline to CLAUDE.md Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 887cc97..938fbb8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,4 +28,8 @@ ## Type Usage - Model types should conform to Codable, Sendable, Equatable & Hashable - Use strong typing and avoid optionals where possible -- Use proper access control (public, internal, private) \ No newline at end of file +- Use proper access control (public, internal, private) + +## PR Review Comments +- After addressing a PR review comment, add a 👍 reaction to it via `gh api /repos/{owner}/{repo}/pulls/comments/{id}/reactions -f content='+1'` +- For PR-level comments, use `gh api /repos/{owner}/{repo}/issues/comments/{id}/reactions -f content='+1'` \ No newline at end of file