From e4383bbe2c5ba542335ab6fd7d4c7911a51f6468 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:34:22 +0300 Subject: [PATCH 1/3] feat: play a text's narration, and bring the old library across MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M7. Per-text audio, with the transport the prototype had and the behaviours it got wrong. The scrubber commits once. The prototype bound its slider straight to `player.seek` with zero tolerance, so every drag delta was a frame-accurate seek; they cancelled each other and fought the 10 Hz observer writing the old value back under the finger. Here a drag moves `scrubTime` and nothing else, engine reports are ignored while it is non-nil, and the finger coming up issues exactly one seek. That requirement is about calls rather than about sound, which is why `AudioEngine` is a protocol: against `AVPlayer` it can only be checked by ear, against a spy it is an assertion. The playhead is published at most once a second. The display shows `m:ss`, and the prototype woke the actor laying out CJK text ten times a second to render a string that had not changed. The engine observes at 4 Hz and the model forwards only when the second changes; the exact playhead is kept unpublished for persistence, so stopping mid-sentence resumes mid-sentence. Now-playing info is pushed on transitions only. The system extrapolates the position from the last elapsed time and rate it was given, so telling it less often is also telling it better. The audio session is activated on first play, never at launch — activating one you are not using interrupts whatever the reader was listening to, for an app that might never make a sound. Interruptions pause and resume; audio the reader paused themselves is never resumed for them; unplugging headphones pauses and does not resume, because plugging them back in must not start their audio in the open. Narration is copied in and recorded by a path **relative** to the audio directory. The prototype stored an absolute path containing the container UUID, which changes across an iOS reinstall or restore, so every attachment broke at once long after the mistake. The playhead is written while the reader is using the app rather than only on teardown: macOS delivers neither a scene-phase change nor `onDisappear` reliably on quit, so saving only at the end saved nothing. It is written when playback stops, once a minute while playing, when another text takes the transport, and on scene-phase change. Also here, because it is the other half of the same milestone: the prototype's `library.db` is migrated on first launch. Texts, revealed words split out of the newline-joined string they were packed into, and `last_opened_at`. `scroll_offset` is discarded deliberately — a pixel offset means nothing after a font change and the prototype never restored it either. An `audio_path` that still resolves is copied inside; one that does not is reported rather than failing the migration. The file is renamed rather than deleted, so a migration that went wrong has not cost anything. And a defect found on the way: `ReaderMetrics.minimumHitTarget` had gone missing and left its documentation attached to `tokenCornerRadius`, which is why the transport's first buttons were 9x11 targets. Verified on the running app: attach, play, scrub, pause, quit, relaunch — the transport comes back parked at 0:04 where it was left. The migration was run against the real prototype database: 3 texts and 152 revealed words recovered from the blob, counts matching the source exactly. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 15 + README.md | 2 +- .../LibraryRepository.swift | 105 +++-- .../PrototypeLibrary.swift | 264 +++++++++++ Sources/HanReaderPlayback/AudioEngine.swift | 176 +++++++ .../HanReaderPlayback/HanReaderPlayback.swift | 1 - .../NowPlayingController.swift | 139 ++++++ Sources/HanReaderPlayback/PlaybackModel.swift | 344 ++++++++++++++ .../HanReaderPlayback/PlaybackSession.swift | 125 +++++ Sources/HanReaderUI/Design/DesignTokens.swift | 5 + Sources/HanReaderUI/HanReaderRootView.swift | 9 + Sources/HanReaderUI/Screens/AudioBar.swift | 171 +++++++ .../HanReaderUI/Screens/ReaderScreen.swift | 207 ++++++++- .../HanReaderUI/Services/AppServices.swift | 35 ++ .../Services/AudioAttachment.swift | 95 ++++ .../PrototypeLibraryTests.swift | 282 +++++++++++ .../PlaybackModelTests.swift | 437 ++++++++++++++++++ .../AudioAttachmentTests.swift | 156 +++++++ Tests/HanReaderUITests/AudioBarTests.swift | 57 +++ 19 files changed, 2581 insertions(+), 44 deletions(-) create mode 100644 Sources/HanReaderPersistence/PrototypeLibrary.swift create mode 100644 Sources/HanReaderPlayback/AudioEngine.swift create mode 100644 Sources/HanReaderPlayback/NowPlayingController.swift create mode 100644 Sources/HanReaderPlayback/PlaybackModel.swift create mode 100644 Sources/HanReaderPlayback/PlaybackSession.swift create mode 100644 Sources/HanReaderUI/Screens/AudioBar.swift create mode 100644 Sources/HanReaderUI/Services/AudioAttachment.swift create mode 100644 Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift create mode 100644 Tests/HanReaderPlaybackTests/PlaybackModelTests.swift create mode 100644 Tests/HanReaderUITests/AudioAttachmentTests.swift create mode 100644 Tests/HanReaderUITests/AudioBarTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 400504e..fa7fd75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,21 @@ it needs to read like release notes rather than a commit log. ## [Unreleased] +### Added + +- Per-text audio. Attach a narration to any text and it plays in a transport + below the reader, with a scrubber, ten-second skips, five speeds and lock + screen and Control Centre controls. The playhead is remembered, so the next + launch resumes mid-sentence rather than at the top. Tapping a word pauses + the narration while the word is spoken and resumes it afterwards — the + attached audio is usually the same narration, so hearing both is confusing + — and audio paused deliberately stays paused. Unplugging headphones pauses + rather than playing to the room. +- Libraries from the prototype are migrated on first launch: texts, revealed + words split out of the newline-joined string they were packed into, and + which text was open last. The old file is kept under a new name rather than + deleted. + ### Fixed - Readings for heteronyms now take the surrounding words into account. 重 in diff --git a/README.md b/README.md index 1d238d8..7c55ab9 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ framework imports. Details in [`Docs/ARCHITECTURE.md`](Docs/ARCHITECTURE.md). - [x] **M4** — word segmentation and the line-breaking core - [x] **M5** — **the macOS reader: first usable version** - [x] **M6** — **BKRS DSL import** -- [ ] **M7** — audio playback +- [x] **M7** — **audio playback** - [ ] **M8** — iOS and iPadOS - [ ] **M9** — accessibility and Russian localization - [ ] **M10** — 1.0 diff --git a/Sources/HanReaderPersistence/LibraryRepository.swift b/Sources/HanReaderPersistence/LibraryRepository.swift index f04a622..c966c7a 100644 --- a/Sources/HanReaderPersistence/LibraryRepository.swift +++ b/Sources/HanReaderPersistence/LibraryRepository.swift @@ -153,6 +153,31 @@ public struct LibraryRepository: Sendable { } } + /// Stores a playhead without disturbing the reading position. + /// + /// The mirror of `save(_:)`, which writes a position and leaves the + /// playhead alone: the transport moves while the page does not, and the + /// page scrolls while the audio does not, so each writes only its own + /// column. A row is created if the reader has only ever played audio and + /// never scrolled — otherwise the first playhead of a session would be + /// dropped silently by an UPDATE that matched nothing. + public func saveAudioTime( + _ time: Double, + for id: TextID, + at date: Date = .now, + ) async throws { + try await dbQueue.write { db in + try db.execute(sql: """ + INSERT INTO readingPosition + (textId, blockIndex, tokenIndex, characterOffset, audioTime, updatedAt) + VALUES (?, 0, 0, 0, ?, ?) + ON CONFLICT(textId) DO UPDATE SET + audioTime = excluded.audioTime, + updatedAt = excluded.updatedAt + """, arguments: [id.rawValue, time, date]) + } + } + /// Clears a stored playhead. Separate from `save(_:)`, which deliberately /// preserves one it was not given. public func clearAudioTime(of id: TextID) async throws { @@ -257,6 +282,49 @@ public struct LibraryRepository: Sendable { } } + // MARK: - Helpers + + /// The stored preview. Taken by character so a multi-byte script is not + /// cut mid-character, and collapsed so newlines do not render as gaps. + /// Scans only as far as it needs to. + /// + /// The obvious implementation -- trim, split, join, then take a prefix -- + /// allocates intermediates proportional to the *whole document* in order + /// to produce 120 characters. For a novel that is tens of megabytes of + /// garbage per import. + static func preview(of content: String, limit: Int = 120) -> String { + var out = "" + out.reserveCapacity(limit) + var pendingSpace = false + + for character in content { + if character.isWhitespace { + // Leading whitespace is dropped; interior runs collapse to one + // space, emitted only once something follows. + pendingSpace = !out.isEmpty + continue + } + if pendingSpace { + out.append(" ") + pendingSpace = false + if out.count == limit { + return out + } + } + out.append(character) + if out.count == limit { + return out + } + } + return out + } +} + +/// Attached narration. +/// +/// Split out so the repository's own body stays readable; these are one +/// coherent concern and they arrived together in M7. +extension LibraryRepository { // MARK: - Audio public func attachAudio(_ track: AudioTrack) async throws { @@ -295,43 +363,6 @@ public struct LibraryRepository: Sendable { ) } } - - // MARK: - Helpers - - /// The stored preview. Taken by character so a multi-byte script is not - /// cut mid-character, and collapsed so newlines do not render as gaps. - /// Scans only as far as it needs to. - /// - /// The obvious implementation -- trim, split, join, then take a prefix -- - /// allocates intermediates proportional to the *whole document* in order - /// to produce 120 characters. For a novel that is tens of megabytes of - /// garbage per import. - static func preview(of content: String, limit: Int = 120) -> String { - var out = "" - out.reserveCapacity(limit) - var pendingSpace = false - - for character in content { - if character.isWhitespace { - // Leading whitespace is dropped; interior runs collapse to one - // space, emitted only once something follows. - pendingSpace = !out.isEmpty - continue - } - if pendingSpace { - out.append(" ") - pendingSpace = false - if out.count == limit { - return out - } - } - out.append(character) - if out.count == limit { - return out - } - } - return out - } } /// Things the library can refuse to do. diff --git a/Sources/HanReaderPersistence/PrototypeLibrary.swift b/Sources/HanReaderPersistence/PrototypeLibrary.swift new file mode 100644 index 0000000..a797121 --- /dev/null +++ b/Sources/HanReaderPersistence/PrototypeLibrary.swift @@ -0,0 +1,264 @@ +// HanReader — MIT licensed. See LICENSE. + +public import Foundation +internal import GRDB +import HanReaderCore + +/// Brings a reader's texts across from the prototype. +/// +/// The prototype wrote `library.db` beside where this app now writes +/// `library.sqlite`, so an existing user has one sitting next to the other. +/// It is small — a handful of texts — and migrating it is a one-off. +/// +/// ### What comes across, and what does not +/// +/// - **Texts** come across whole. Deduplicated on content by `importText`, +/// so running this twice cannot produce two copies of anything. +/// - **Revealed words** come across, split out of the newline-joined blob the +/// prototype packed them into. That blob is why the new schema has a +/// `revealedWord` table: a set stored as a string cannot be queried, cannot +/// be indexed, and silently corrupts the moment a word contains a newline. +/// - **`last_opened_at`** comes across, so the reader's most recent text is +/// still the one that opens. +/// - **`scroll_offset` is discarded, deliberately.** It is a pixel offset, +/// which is meaningless after a font-size change, a window resize, or a +/// move between a Mac and a phone — and the prototype never restored it +/// either. The new `readingPosition` is anchored to a character offset +/// instead, and there is no way to derive one from a pixel count. +/// - **`audio_path`** is an absolute path containing the app container's +/// UUID. It is followed if the file is still there and copied inside, which +/// is the fix; if it is not, the text comes across without its audio and +/// the outcome says so. +/// +/// ### Running once +/// +/// Marked by renaming the file rather than by a flag, so the state lives with +/// the thing it describes. The prototype's `UserDefaults.bool(forKey: +/// "dictionaryParsed_bkrs_v1")` is the counter-example: delete the database +/// and the flag still says it was parsed. +public enum PrototypeLibrary { + /// What a migration did, for a report the reader can act on. + public struct Outcome: Sendable, Equatable { + public var texts = 0 + public var alreadyPresent = 0 + public var revealedWords = 0 + public var audioTracks = 0 + /// Things worth telling the reader — an audio file that was no longer + /// where the prototype left it, a row that could not be read. + public var notes: [String] = [] + + public var isEmpty: Bool { + texts == 0 && alreadyPresent == 0 + } + } + + /// The prototype's database, beside the current one. + public static func file(near library: URL) -> URL { + library.deletingLastPathComponent().appendingPathComponent("library.db") + } + + /// Where a migrated file is moved to, so it is kept rather than deleted — + /// a reader whose migration went wrong still has their texts. + public static func migratedFile(near library: URL) -> URL { + library.deletingLastPathComponent().appendingPathComponent("library.db.migrated") + } + + /// Migrates the prototype library if there is one, and returns nil if + /// there is not. + @discardableResult + public static func migrateIfPresent( + into repository: LibraryRepository, + locations: AppDatabase.Locations, + ) async throws + -> Outcome? + { + let source = file(near: locations.library) + guard FileManager.default.fileExists(atPath: source.path) else { return nil } + + let outcome = try await migrate(from: source, into: repository, locations: locations) + // Moved only on success. A migration that threw must be able to run + // again, or a transient failure costs the reader their library. + try? FileManager.default.removeItem(at: migratedFile(near: locations.library)) + try FileManager.default.moveItem(at: source, to: migratedFile(near: locations.library)) + return outcome + } + + static func migrate( + from source: URL, + into repository: LibraryRepository, + locations: AppDatabase.Locations, + ) async throws + -> Outcome + { + let old = try openReadOnly(source) + let texts = try await readTexts(from: old) + let progress = try await readProgress(from: old) + + let run = Migration(repository: repository, locations: locations) + var outcome = Outcome() + let identifiers = try await run.importTexts(texts, into: &outcome) + try await run.applyProgress(progress, identifiers: identifiers, into: &outcome) + return outcome + } + + /// Read-only: the prototype's file is the reader's only copy until the + /// migration succeeds, and nothing here needs to write to it. + private static func openReadOnly(_ source: URL) throws -> DatabaseQueue { + var configuration = Configuration() + configuration.readonly = true + return try DatabaseQueue(path: source.path, configuration: configuration) + } + + /// Projected to plain values inside the read. A GRDB `Row` is tied to its + /// statement and is not `Sendable`, so carrying one across an `await` is + /// both a compile error and, were it allowed, a dangling read. + private static func readTexts(from old: DatabaseQueue) async throws -> [OldText] { + try await old.read { db in + try Row.fetchAll(db, sql: """ + SELECT id, title, content, audio_path FROM texts ORDER BY id + """).map { + OldText( + id: $0["id"], + title: $0["title"], + content: $0["content"], + audioPath: $0["audio_path"], + ) + } + } + } + + private static func readProgress(from old: DatabaseQueue) async throws -> [OldProgress] { + try await old.read { db in + try Row.fetchAll(db, sql: """ + SELECT text_id, revealed_words, last_opened_at FROM reading_progress + """).map { + OldProgress( + textID: $0["text_id"], + revealedWords: $0["revealed_words"] ?? "", + lastOpenedAt: $0["last_opened_at"], + ) + } + } + } +} + +/// One migration run, holding what every step needs. +/// +/// A type rather than six parameters threaded through four functions — +/// which is what it was, until the destination and the repository had to +/// reach the audio step as well. +private struct Migration { + let repository: LibraryRepository + let locations: AppDatabase.Locations + + /// - Returns: old row id to new `TextID`, so the progress rows can find + /// the text they belong to. + func importTexts( + _ texts: [OldText], + into outcome: inout PrototypeLibrary.Outcome, + ) async throws + -> [Int64: TextID] + { + var identifiers: [Int64: TextID] = [:] + for text in texts { + guard let title = text.title, let content = text.content, let oldID = text.id else { + outcome.notes.append("A text could not be read and was skipped.") + continue + } + let result = try await repository.importText( + title: title, + content: content, + sourceName: "HanReader (prototype)", + ) + switch result { + case let .imported(id): + identifiers[oldID] = id + outcome.texts += 1 + case let .alreadyPresent(id): + identifiers[oldID] = id + outcome.alreadyPresent += 1 + } + if let path = text.audioPath, let id = identifiers[oldID] { + try await carryAudio(from: path, for: id, title: title, into: &outcome) + } + } + return identifiers + } + + func applyProgress( + _ progress: [OldProgress], + identifiers: [Int64: TextID], + into outcome: inout PrototypeLibrary.Outcome, + ) async throws { + for row in progress { + guard let oldID = row.textID, let id = identifiers[oldID] else { continue } + // Split on newlines, which is the whole of undoing the blob — + // and the empty entries are filtered because an empty set was + // stored as an empty string, which splits into one empty word. + for word in row.revealedWords.split(separator: "\n") where !word.isEmpty { + try await repository.reveal(String(word), in: id) + outcome.revealedWords += 1 + } + if let opened = row.lastOpenedAt { + try await repository.markOpened(id, at: Date(timeIntervalSince1970: opened)) + } + } + } + + /// Follows an absolute audio path and copies the file inside, if it is + /// still where the prototype left it. + private func carryAudio( + from path: String, + for id: TextID, + title: String, + into outcome: inout PrototypeLibrary.Outcome, + ) async throws { + let original = URL(fileURLWithPath: path) + guard FileManager.default.fileExists(atPath: original.path) else { + // The failure the relative-path rule exists to prevent, met here + // as history rather than as a bug: the container moved and the + // stored path no longer points at anything. + outcome.notes + .append( + "The audio for \u{201C}\(title)\u{201D} was not where the old version left it.", + ) + return + } + try FileManager.default.createDirectory( + at: locations.audio, + withIntermediateDirectories: true, + ) + let name = "text-\(id.rawValue).\(original.pathExtension.lowercased())" + let destination = locations.audio.appendingPathComponent(name) + if FileManager.default.fileExists(atPath: destination.path) { + try FileManager.default.removeItem(at: destination) + } + try FileManager.default.copyItem(at: original, to: destination) + try await repository.attachAudio(AudioTrack( + textID: id, + relativePath: name, + duration: nil, + importedAt: Date(), + )) + outcome.audioTracks += 1 + } +} + +/// One row of the prototype's `texts` table, as plain values. +/// +/// Optional throughout rather than trusted: this is a file written by a +/// previous version of the app, possibly years ago, and a migration that +/// traps on a null is a migration that loses the whole library over one bad +/// row. +private struct OldText: Sendable { + let id: Int64? + let title: String? + let content: String? + let audioPath: String? +} + +private struct OldProgress: Sendable { + let textID: Int64? + let revealedWords: String + let lastOpenedAt: Double? +} diff --git a/Sources/HanReaderPlayback/AudioEngine.swift b/Sources/HanReaderPlayback/AudioEngine.swift new file mode 100644 index 0000000..20201c2 --- /dev/null +++ b/Sources/HanReaderPlayback/AudioEngine.swift @@ -0,0 +1,176 @@ +// HanReader — MIT licensed. See LICENSE. + +import AVFoundation +public import Foundation +import HanReaderCore + +/// The player underneath `PlaybackModel`. +/// +/// A seam, and it earns its place: the transport's hardest requirement is +/// about *how many* seeks a drag issues, which is a statement about calls +/// made rather than about sound produced. Against `AVPlayer` that can only be +/// checked by ear; against a spy it is an assertion. The prototype's scrubber +/// issued a frame-accurate seek on every drag delta, and nothing short of +/// counting them catches that. +/// +/// Deliberately not an `AsyncSequence` of positions. The engine reports four +/// times a second forever, and a sequence would make cancellation the +/// caller's problem on every path out. +public protocol AudioEngine: AnyObject { + /// Seconds, or zero until the asset has loaded far enough to say. + var duration: Double { get } + var currentTime: Double { get } + + func load(_ url: URL) + func play() + func pause() + func seek(to time: Double) + /// Applied immediately when playing, and remembered for the next `play()` + /// when not. + func setRate(_ rate: Double) + + /// Called about four times a second while playing. What reaches the UI is + /// throttled further — see `PlaybackModel.report(_:)`. + var onTime: ((Double) -> Void)? { get set } + /// Called once the asset knows how long it is. + var onDuration: ((Double) -> Void)? { get set } + var onFinish: (() -> Void)? { get set } +} + +/// `AVPlayer`, behind the seam. +final class AVPlayerAudioEngine: AudioEngine { + /// Four times a second. + /// + /// The prototype observed at 10 Hz and published every tick, so the main + /// actor — the one laying out CJK text — was woken ten times a second to + /// re-render the same `m:ss` string. Four is already more than the + /// display needs; `PlaybackModel` throttles what it publishes down to + /// once a second on top of this. + static let observationInterval = 0.25 + + private let player = AVPlayer() + private var timeObserver: Any? + private var finishObserver: (any NSObjectProtocol)? + private var durationTask: Task? + private var rate = 1.0 + + /// Unsubscription, captured as a closure so `deinit` can run it. + /// + /// `deinit` is nonisolated and this engine is not: the player and its + /// observer token are neither `Sendable` nor reachable from there. A + /// closure built on the main actor, holding what it needs, is — and by + /// the time `deinit` runs nothing else holds this engine, so there is no + /// concurrent access for the `unsafe` to paper over. It is rebuilt + /// whenever the observers change. + private nonisolated(unsafe) var teardown: (() -> Void)? + + private(set) var duration = 0.0 + + var currentTime: Double { + let time = player.currentTime().seconds + return time.isFinite ? time : 0 + } + + var onTime: ((Double) -> Void)? + var onDuration: ((Double) -> Void)? + var onFinish: (() -> Void)? + + init() { + let interval = CMTime(seconds: Self.observationInterval, preferredTimescale: 600) + timeObserver = player.addPeriodicTimeObserver( + forInterval: interval, + queue: .main, + ) { [weak self] time in + let seconds = time.seconds + guard seconds.isFinite else { return } + Task { @MainActor [weak self] in + self?.onTime?(seconds) + } + } + rebuildTeardown() + } + + deinit { + teardown?() + } + + /// Rebuilt rather than appended to, so a reload cannot leave the previous + /// track's observers to be removed twice. + private func rebuildTeardown() { + let player = player + let time = timeObserver + let finish = finishObserver + teardown = { + if let time { + player.removeTimeObserver(time) + } + if let finish { + NotificationCenter.default.removeObserver(finish) + } + } + } + + func load(_ url: URL) { + durationTask?.cancel() + duration = 0 + if let finishObserver { + NotificationCenter.default.removeObserver(finishObserver) + } + + let asset = AVURLAsset(url: url) + let item = AVPlayerItem(asset: asset) + player.replaceCurrentItem(with: item) + + finishObserver = NotificationCenter.default.addObserver( + forName: AVPlayerItem.didPlayToEndTimeNotification, + object: item, + queue: .main, + ) { [weak self] _ in + Task { @MainActor [weak self] in + self?.onFinish?() + } + } + rebuildTeardown() + + // Loaded asynchronously rather than read from `item.duration`, which + // is `indefinite` until the asset has been inspected — and reading it + // synchronously blocks whichever thread asks. + durationTask = Task { [weak self] in + guard let loaded = try? await asset.load(.duration) else { return } + let seconds = loaded.seconds + guard seconds.isFinite, seconds > 0 else { return } + await MainActor.run { [weak self] in + self?.duration = seconds + self?.onDuration?(seconds) + } + } + } + + func play() { + // `rate` doubles as the play command: setting it non-zero starts + // playback at that speed, which is how a speed setting survives a + // pause and resume. + player.rate = Float(rate) + } + + func pause() { + player.pause() + } + + func seek(to time: Double) { + let target = CMTime(seconds: max(0, time), preferredTimescale: 600) + // A tolerance, deliberately. The prototype seeked with + // `toleranceBefore: .zero, toleranceAfter: .zero`, which forces a + // frame-accurate seek — expensive, and pointless for spoken audio + // where a tenth of a second is inaudible. + let tolerance = CMTime(seconds: 0.1, preferredTimescale: 600) + player.seek(to: target, toleranceBefore: tolerance, toleranceAfter: tolerance) + } + + func setRate(_ rate: Double) { + self.rate = rate + if player.rate != 0 { + player.rate = Float(rate) + } + } +} diff --git a/Sources/HanReaderPlayback/HanReaderPlayback.swift b/Sources/HanReaderPlayback/HanReaderPlayback.swift index 54bb582..c3f8476 100644 --- a/Sources/HanReaderPlayback/HanReaderPlayback.swift +++ b/Sources/HanReaderPlayback/HanReaderPlayback.swift @@ -10,7 +10,6 @@ import HanReaderCore /// `AVAudioSession` category, interruption handling and route-change handling, /// while macOS requires none of it. /// -/// - Note: This is scaffolding. Playback arrives in milestone M7. public enum HanReaderPlayback { /// Proves at compile time that this module links `HanReaderCore`. public static let coreVersion = HanReaderCore.version diff --git a/Sources/HanReaderPlayback/NowPlayingController.swift b/Sources/HanReaderPlayback/NowPlayingController.swift new file mode 100644 index 0000000..3979bed --- /dev/null +++ b/Sources/HanReaderPlayback/NowPlayingController.swift @@ -0,0 +1,139 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import MediaPlayer + +/// Puts the current track on the lock screen, in Control Centre, and on the +/// Mac's Now Playing control, and accepts the commands they send back. +/// +/// `MediaPlayer` exists on macOS as well as iOS, so this is shared with no +/// platform branch — which is worth saying because the instinct is to put it +/// behind one. +/// +/// ### Updated on transitions only +/// +/// Now-playing info is pushed when something a remote control would care +/// about changes — a new track, play, pause, a seek, a speed change — and +/// never on the playback tick. The system extrapolates the playhead from +/// `elapsedPlaybackTime` plus `playbackRate` and the time it received them, +/// so a correct implementation tells it *less* often, not more. Pushing on +/// every tick is both wasted main-actor work and a worse lock screen, since +/// each update restarts the extrapolation. +@MainActor +public final class NowPlayingController { + private let info: MPNowPlayingInfoCenter + private let commands: MPRemoteCommandCenter + /// Weak: the controller is owned alongside the model it drives, and a + /// strong reference here would make them keep each other alive. + private weak var model: PlaybackModel? + private var title: String? + private var handlers: [MPRemoteCommand: Any] = [:] + + /// Unsubscription, captured for `deinit` — which is nonisolated and so + /// cannot reach the main-actor dictionary above. Rebuilt when the + /// handlers are. Same bargain as `AVPlayerAudioEngine.teardown`: nothing + /// else holds this object by the time it runs. + private nonisolated(unsafe) var teardown: (() -> Void)? + + /// No injection point, deliberately. `MPNowPlayingInfoCenter` and + /// `MPRemoteCommandCenter` are process-wide singletons that cannot be + /// substituted meaningfully, and exposing them would put MediaPlayer + /// types in this module's public API for no testing benefit. What is + /// worth testing here — that state is pushed on transitions and not on + /// ticks — is a property of `PlaybackModel.onStateChange`, and is tested + /// there. + public init() { + info = .default() + commands = .shared() + } + + deinit { + teardown?() + } + + /// Starts reflecting a model, and accepting commands on its behalf. + public func attach(to model: PlaybackModel, title: String?) { + self.model = model + self.title = title + subscribe() + model.onStateChange = { [weak self] in self?.update() } + update() + } + + /// Clears the lock screen when the reader closes the text. + public func detach() { + model?.onStateChange = nil + model = nil + title = nil + info.nowPlayingInfo = nil + } + + // MARK: - Pushing state out + + private func update() { + guard let model, model.state == .ready else { + info.nowPlayingInfo = nil + return + } + var now: [String: Any] = [ + MPMediaItemPropertyTitle: title ?? "Audio", + MPNowPlayingInfoPropertyElapsedPlaybackTime: model.playhead, + // Zero while paused, and the *actual* speed while playing. A + // lock screen told `1.0` for a track playing at 1.5× drifts + // visibly, because it is extrapolating from what we said. + MPNowPlayingInfoPropertyPlaybackRate: model.isPlaying ? model.rate.rawValue : 0, + ] + if model.duration > 0 { + now[MPMediaItemPropertyPlaybackDuration] = model.duration + } + info.nowPlayingInfo = now + } + + // MARK: - Taking commands in + + private func subscribe() { + guard handlers.isEmpty else { return } + add(commands.playCommand) { $0.play() } + add(commands.pauseCommand) { $0.pause() } + add(commands.togglePlayPauseCommand) { $0.toggle() } + add(commands.skipForwardCommand) { $0.skip(by: Self.skipInterval) } + add(commands.skipBackwardCommand) { $0.skip(by: -Self.skipInterval) } + + commands.skipForwardCommand.preferredIntervals = [NSNumber(value: Self.skipInterval)] + commands.skipBackwardCommand.preferredIntervals = [NSNumber(value: Self.skipInterval)] + + // Dragging the lock screen scrubber. One seek, at the end of the + // drag, for the same reason the in-app scrubber commits once. + let changePosition = commands.changePlaybackPositionCommand + changePosition.isEnabled = true + handlers[changePosition] = changePosition.addTarget { [weak self] event in + guard let model = self?.model, + let event = event as? MPChangePlaybackPositionCommandEvent + else { return .commandFailed } + model.seek(to: event.positionTime) + return .success + } + + let registered = handlers + teardown = { + for (command, handler) in registered { + command.removeTarget(handler) + } + } + } + + private func add(_ command: MPRemoteCommand, _ action: @escaping (PlaybackModel) -> Void) { + command.isEnabled = true + handlers[command] = command.addTarget { [weak self] _ in + guard let model = self?.model else { return .noActionableNowPlayingItem } + action(model) + return .success + } + } + + /// Matches the in-app transport, because a reader who learns that the + /// button moves ten seconds should not find a different number on the + /// lock screen. + public static let skipInterval = 10.0 +} diff --git a/Sources/HanReaderPlayback/PlaybackModel.swift b/Sources/HanReaderPlayback/PlaybackModel.swift new file mode 100644 index 0000000..73c3a6e --- /dev/null +++ b/Sources/HanReaderPlayback/PlaybackModel.swift @@ -0,0 +1,344 @@ +// HanReader — MIT licensed. See LICENSE. + +public import Foundation +import HanReaderCore +public import Observation + +/// How fast the narration plays. +/// +/// A fixed set rather than a continuous slider: this is audio in a language +/// the reader is learning, where the useful range is "slower than a native +/// speaker" to "slightly brisk", and a slider invites fiddling with a value +/// nobody can name. +public enum PlaybackRate: Double, CaseIterable, Sendable, Hashable { + case half = 0.5 + case slow = 0.75 + case normal = 1.0 + case brisk = 1.25 + case fast = 1.5 + + /// For the menu. Not localized: these read as numbers in every language + /// this app ships in, and `0,75×` in a Russian locale would be a format + /// applied to something that is not a quantity of anything. + public var label: String { + rawValue == 1 ? "1×" : "\(rawValue.formatted(.number.precision(.fractionLength(0 ... 2))))×" + } +} + +/// The per-text audio transport. +/// +/// Scene-scoped: one per window, so two windows reading two texts play two +/// tracks, and closing a window stops its own. The prototype had one player +/// hanging off a per-window model, which is the same thing by accident rather +/// than by design. +@Observable +@MainActor +public final class PlaybackModel { + /// What the transport is showing. Separate from `isPlaying` because a + /// track that has not loaded yet is not a paused track — the controls + /// should be disabled rather than look ready. + public enum State: Equatable, Sendable { + case empty + case ready + case failed(String) + } + + public private(set) var state: State = .empty + public private(set) var isPlaying = false + public private(set) var duration = 0.0 + + /// The playhead, published **at most once a second**. + /// + /// The engine reports four times a second; the display shows `m:ss`. The + /// prototype published every tick straight to the main actor, so the + /// thread laying out CJK text was woken ten times a second to render a + /// string that had not changed. + public private(set) var observedTime = 0.0 + + /// Where the reader's finger is, while it is down. + /// + /// The whole of scrub-on-commit. While this is non-nil the engine is not + /// touched and its reports are ignored, so the thumb follows the finger + /// instead of fighting a stream of positions from a player that is still + /// where it was. + public private(set) var scrubTime: Double? + + /// What the transport draws: the finger if it is down, else the player. + public var displayTime: Double { + scrubTime ?? observedTime + } + + public var isScrubbing: Bool { + scrubTime != nil + } + + public var rate: PlaybackRate = .normal { + didSet { + guard rate != oldValue else { return } + engine.setRate(rate.rawValue) + onStateChange?() + } + } + + /// Called when something a lock screen or Control Centre would care about + /// changed — not on every tick. Updating now-playing info four times a + /// second is the other half of the same waste as publishing the playhead. + public var onStateChange: (() -> Void)? + + /// The exact playhead, for persistence. Not the published one, which is + /// rounded to the second for the display's benefit; a reader who stops + /// mid-sentence should resume mid-sentence. + @ObservationIgnored public private(set) var playhead = 0.0 + + /// True when playback was stopped by something other than the reader — + /// a phone call, headphones unplugged, a word being spoken — so it can be + /// resumed without resuming audio the reader had deliberately paused. + @ObservationIgnored private var wasInterrupted = false + + @ObservationIgnored private let engine: any AudioEngine + @ObservationIgnored private let session: any PlaybackSession + @ObservationIgnored private var hasActivatedSession = false + + public init( + engine: (any AudioEngine)? = nil, + session: (any PlaybackSession)? = nil, + ) { + self.engine = engine ?? AVPlayerAudioEngine() + self.session = session ?? SystemPlaybackSession() + configure() + } + + private func configure() { + engine.onTime = { [weak self] time in self?.report(time) } + engine.onDuration = { [weak self] seconds in + guard let self else { return } + duration = seconds + onStateChange?() + } + engine.onFinish = { [weak self] in self?.finish() } + session.onInterruption = { [weak self] interruption in + self?.handle(interruption) + } + } + + // MARK: - Loading + + /// Loads a track and parks the playhead where the reader left it. + /// + /// Does not start playing. Opening a text should not fill the room with + /// sound, and the stored playhead is a resume point rather than an + /// instruction. + public func load(_ url: URL, startingAt time: Double? = nil) { + engine.load(url) + state = .ready + duration = engine.duration + isPlaying = false + wasInterrupted = false + scrubTime = nil + let start = max(0, time ?? 0) + playhead = start + observedTime = start + if start > 0 { + engine.seek(to: start) + } + engine.setRate(rate.rawValue) + onStateChange?() + } + + /// Forgets the track, for a reader who closed the text or removed its + /// audio. + public func unload() { + engine.pause() + state = .empty + isPlaying = false + duration = 0 + observedTime = 0 + playhead = 0 + scrubTime = nil + wasInterrupted = false + deactivateSession() + onStateChange?() + } + + public func fail(_ message: String) { + state = .failed(message) + isPlaying = false + onStateChange?() + } + + // MARK: - Transport + + public func play() { + guard state == .ready else { return } + // The session is activated here and nowhere else. Activating it at + // launch would interrupt whatever the reader was listening to, for an + // app that might never play a sound. + activateSession() + // Pressing play on a finished track starts it again rather than doing + // nothing, which is what a playhead sitting at the end would mean. + if duration > 0, playhead >= duration - Self.endOfTrackTolerance { + seek(to: 0) + } + engine.play() + isPlaying = true + wasInterrupted = false + onStateChange?() + } + + public func pause() { + guard isPlaying else { return } + engine.pause() + isPlaying = false + wasInterrupted = false + onStateChange?() + } + + public func toggle() { + if isPlaying { + pause() + } else { + play() + } + } + + /// Jumps by a number of seconds, clamped to the track. + public func skip(by seconds: Double) { + guard state == .ready else { return } + seek(to: playhead + seconds) + } + + /// Moves the playhead and tells everyone, in one step. + public func seek(to time: Double) { + guard state == .ready else { return } + let target = clamp(time) + engine.seek(to: target) + playhead = target + // Published immediately rather than left to the next report, or the + // thumb springs back to where it was for a quarter of a second. + observedTime = target + onStateChange?() + } + + // MARK: - Scrubbing + + /// The reader's finger went down on the scrubber. + public func beginScrub(at time: Double) { + guard state == .ready else { return } + scrubTime = clamp(time) + } + + /// The finger moved. **No seek is issued**, by design. + public func scrub(to time: Double) { + guard scrubTime != nil else { return } + scrubTime = clamp(time) + } + + /// The finger came up, and exactly one seek is issued. + public func endScrub() { + guard let target = scrubTime else { return } + scrubTime = nil + seek(to: target) + } + + // MARK: - Speech and interruptions + + /// Ducks the narration while a word is spoken. + /// + /// For this app the attached audio is usually the *same* narration, so + /// hearing the word and the recording at once is confusing rather than + /// merely noisy. Resumes only if it was this that paused it — a reader + /// who paused the audio and then tapped a word should not find it + /// playing again. + public func pauseForSpeech() { + guard isPlaying else { return } + engine.pause() + isPlaying = false + wasInterrupted = true + onStateChange?() + } + + public func resumeAfterSpeech() { + guard wasInterrupted, state == .ready else { return } + engine.play() + isPlaying = true + wasInterrupted = false + onStateChange?() + } + + private func handle(_ interruption: PlaybackInterruption) { + switch interruption { + case .began: + guard isPlaying else { return } + engine.pause() + isPlaying = false + wasInterrupted = true + onStateChange?() + case .ended: + resumeAfterSpeech() + case .outputGone: + // Headphones pulled out. Pausing is not politeness, it is the + // difference between a quiet room and the reader's audio playing + // through the speaker to everyone around them — and it must not + // be resumable automatically, or plugging back in starts it in + // the open. + guard isPlaying else { return } + engine.pause() + isPlaying = false + wasInterrupted = false + onStateChange?() + } + } + + // MARK: - Engine reports + + /// Throttles the engine's four-a-second down to the one-a-second the + /// display can actually show, while keeping `playhead` exact. + private func report(_ time: Double) { + playhead = time + // Ignored while dragging: the engine is still reporting from where it + // was, and letting that through is what makes a scrubber stutter + // under the finger. + guard scrubTime == nil else { return } + guard Int(time) != Int(observedTime) else { return } + observedTime = time + } + + private func finish() { + engine.pause() + isPlaying = false + wasInterrupted = false + if duration > 0 { + playhead = duration + observedTime = duration + } + deactivateSession() + onStateChange?() + } + + // MARK: - Session + + private func activateSession() { + guard !hasActivatedSession else { return } + do { + try session.activate() + hasActivatedSession = true + } catch { + Log.error("playback", "could not activate the audio session: \(error)") + } + } + + private func deactivateSession() { + guard hasActivatedSession else { return } + session.deactivate() + hasActivatedSession = false + } + + private func clamp(_ time: Double) -> Double { + guard duration > 0 else { return max(0, time) } + return min(max(0, time), duration) + } + + /// How close to the end counts as finished, for deciding whether play + /// means "resume" or "start again". + static let endOfTrackTolerance = 0.25 +} diff --git a/Sources/HanReaderPlayback/PlaybackSession.swift b/Sources/HanReaderPlayback/PlaybackSession.swift new file mode 100644 index 0000000..42f1aab --- /dev/null +++ b/Sources/HanReaderPlayback/PlaybackSession.swift @@ -0,0 +1,125 @@ +// HanReader — MIT licensed. See LICENSE. + +import AVFoundation +import Foundation +import HanReaderCore + +/// Something that happened to the audio route or to the system's idea of who +/// owns the speaker. +public enum PlaybackInterruption: Sendable, Equatable { + /// A call, an alarm, another app taking over. + case began + /// The interruption ended and the system says it is our turn again. + case ended + /// The device that was playing went away — headphones unplugged, a + /// Bluetooth speaker walked out of range. + case outputGone +} + +/// The system's audio session, where there is one. +/// +/// This is the one genuine divergence in the media stack: iOS requires a +/// category, an activation, and two notification subscriptions before a +/// sound can be made politely, while macOS requires none of it. Behind a +/// protocol so the model above it has no platform branch at all, and so a +/// test can deliver an interruption without a phone call. +public protocol PlaybackSession: AnyObject { + /// Called on the first `play()`, never at launch. + func activate() throws + func deactivate() + var onInterruption: ((PlaybackInterruption) -> Void)? { get set } +} + +/// The real one. Everything inside the `#if` is iOS-only by necessity rather +/// than by preference — `AVAudioSession` does not exist on macOS, where the +/// system mixes applications without being asked. +public final class SystemPlaybackSession: PlaybackSession { + public var onInterruption: ((PlaybackInterruption) -> Void)? + + #if os(iOS) + private var observers: [any NSObjectProtocol] = [] + #endif + + public init() {} + + deinit { + #if os(iOS) + for observer in observers { + NotificationCenter.default.removeObserver(observer) + } + #endif + } + + public func activate() throws { + #if os(iOS) + let session = AVAudioSession.sharedInstance() + // `.spokenAudio` rather than plain `.playback`: it tells the system + // this is speech, so a navigation prompt ducks it rather than talking + // over it, and the "pause spoken audio" accessory controls apply. + try session.setCategory(.playback, mode: .spokenAudio) + try session.setActive(true) + subscribe() + #endif + } + + public func deactivate() { + #if os(iOS) + // Notifying others is what lets the music the reader interrupted + // start itself again. Failing here is not worth surfacing: the + // session is already idle and the reader has nothing to do about it. + try? AVAudioSession.sharedInstance().setActive( + false, + options: .notifyOthersOnDeactivation, + ) + #endif + } + + #if os(iOS) + private func subscribe() { + guard observers.isEmpty else { return } + let centre = NotificationCenter.default + observers.append(centre.addObserver( + forName: AVAudioSession.interruptionNotification, + object: nil, + queue: .main, + ) { [weak self] note in + guard let raw = note.userInfo?[AVAudioSessionInterruptionTypeKey] as? UInt, + let type = AVAudioSession.InterruptionType(rawValue: raw) + else { return } + switch type { + case .began: + self?.send(.began) + case .ended: + // Only when the system says so. Resuming after an + // interruption we were not invited back from is how an app + // ends up talking over a phone call. + let options = note.userInfo?[AVAudioSessionInterruptionOptionKey] as? UInt ?? 0 + if AVAudioSession.InterruptionOptions(rawValue: options) + .contains(.shouldResume) + { + self?.send(.ended) + } + @unknown default: + break + } + }) + observers.append(centre.addObserver( + forName: AVAudioSession.routeChangeNotification, + object: nil, + queue: .main, + ) { [weak self] note in + guard let raw = note.userInfo?[AVAudioSessionRouteChangeReasonKey] as? UInt, + let reason = AVAudioSession.RouteChangeReason(rawValue: raw), + reason == .oldDeviceUnavailable + else { return } + self?.send(.outputGone) + }) + } + + private func send(_ interruption: PlaybackInterruption) { + Task { @MainActor [weak self] in + self?.onInterruption?(interruption) + } + } + #endif +} diff --git a/Sources/HanReaderUI/Design/DesignTokens.swift b/Sources/HanReaderUI/Design/DesignTokens.swift index 5a8d4c7..4cf6bec 100644 --- a/Sources/HanReaderUI/Design/DesignTokens.swift +++ b/Sources/HanReaderUI/Design/DesignTokens.swift @@ -90,6 +90,11 @@ nonisolated enum ReaderMetrics { static let readingColumnPadding = 24.0 /// Minimum hit target. Apple's guideline is 44pt, and a tapped word is /// this app's primary interaction. + /// + /// The constant went missing at some point and left its documentation + /// attached to the next declaration, which is why the transport's first + /// buttons were drawn as 9×11 targets. + static let minimumHitTarget = 44.0 /// Corner radius on a token's highlight. static let tokenCornerRadius = 4.0 } diff --git a/Sources/HanReaderUI/HanReaderRootView.swift b/Sources/HanReaderUI/HanReaderRootView.swift index f720285..f5357a9 100644 --- a/Sources/HanReaderUI/HanReaderRootView.swift +++ b/Sources/HanReaderUI/HanReaderRootView.swift @@ -135,6 +135,12 @@ private struct LibraryAndReader: View { /// abandon a running import. @State private var dictionaryImport: DictionaryImportModel @State private var library: LibraryModel + /// Scene-scoped: one transport per window, so two windows reading two + /// texts play two narrations and closing a window stops its own. The + /// prototype had one player per window-sized model, which was the same + /// thing by accident rather than by design. + @State private var playback = PlaybackModel() + @State private var nowPlaying = NowPlayingController() @State private var importer: TextImporter @State private var isFileImporterPresented = false @State private var duplicateNotice = false @@ -192,9 +198,12 @@ private struct LibraryAndReader: View { if let selection { ReaderScreen( textID: selection, + title: library.items.first { $0.id == selection }?.title ?? "", services: services, settings: settings, speech: speech, + playback: playback, + nowPlaying: nowPlaying, // Part of the reader's identity, so an imported // dictionary reaches a text that is already open rather // than waiting for the next launch. diff --git a/Sources/HanReaderUI/Screens/AudioBar.swift b/Sources/HanReaderUI/Screens/AudioBar.swift new file mode 100644 index 0000000..10c878e --- /dev/null +++ b/Sources/HanReaderUI/Screens/AudioBar.swift @@ -0,0 +1,171 @@ +// HanReader — MIT licensed. See LICENSE. + +import HanReaderPlayback +import SwiftUI + +/// The transport for a text's narration. +/// +/// Persistent while a text with audio is open, which is fidelity invariant 6 +/// — the prototype's bottom bar, kept. It has one behaviour worth stating +/// outright, because getting it wrong is what made the prototype's scrubber +/// fight itself: **the slider commits once**. +/// +/// `Slider`'s `onEditingChanged` is the seam. While the finger is down the +/// binding writes to `PlaybackModel.scrub(to:)`, which moves nothing but the +/// thumb; when it comes up, `endScrub()` issues exactly one seek. The +/// prototype bound the slider straight to `player.seek`, so every drag delta +/// was a frame-accurate seek, and the position observer wrote the old value +/// back underneath the finger. +struct AudioBar: View { + let model: PlaybackModel + /// Shown beside the transport so the reader knows what is playing when + /// several windows are open. + let title: String + + @Environment(\.accessibilityReduceMotion) private var reduceMotion + + var body: some View { + VStack(spacing: 6) { + HStack(spacing: 12) { + playButton + skipButton(by: -NowPlayingController.skipInterval) + skipButton(by: NowPlayingController.skipInterval) + scrubber + speedMenu + } + if case let .failed(message) = model.state { + Text(verbatim: message) + .font(.caption) + .foregroundStyle(.red) + .frame(maxWidth: .infinity, alignment: .leading) + } + } + .padding(.horizontal, ReaderMetrics.readingColumnPadding) + .padding(.vertical, 10) + .background(.bar) + .accessibilityElement(children: .contain) + .accessibilityLabel(Text("Audio for \(title)")) + } + + private var playButton: some View { + Button { + model.toggle() + } label: { + Image(systemName: model.isPlaying ? "pause.fill" : "play.fill") + } + .buttonStyle(.borderless) + .modifier(TransportHitTarget()) + .disabled(model.state != .ready) + .keyboardShortcut(.space, modifiers: []) + .accessibilityLabel(model.isPlaying ? Text("Pause") : Text("Play")) + } + + private func skipButton(by seconds: Double) -> some View { + let isBack = seconds < 0 + return Button { + model.skip(by: seconds) + } label: { + Image(systemName: isBack ? "gobackward.10" : "goforward.10") + } + .buttonStyle(.borderless) + .modifier(TransportHitTarget()) + .disabled(model.state != .ready) + .accessibilityLabel( + isBack + ? Text("Back \(Int(-seconds)) seconds") + : Text("Forward \(Int(seconds)) seconds"), + ) + } + + private var scrubber: some View { + HStack(spacing: 8) { + timeLabel(model.displayTime) + Slider( + value: Binding( + get: { model.displayTime }, + // Moves the thumb only. No seek is issued here, which is + // the entire point. + set: { model.scrub(to: $0) }, + ), + in: 0 ... max(model.duration, 1), + ) { isEditing in + if isEditing { + model.beginScrub(at: model.displayTime) + } else { + model.endScrub() + } + } + .disabled(model.state != .ready || model.duration <= 0) + .accessibilityLabel(Text("Playback position")) + .accessibilityValue(Text(Self.spoken(model.displayTime))) + timeLabel(model.duration) + } + } + + private var speedMenu: some View { + Menu { + Picker("Speed", selection: Bindable(model).rate) { + ForEach(PlaybackRate.allCases, id: \.self) { rate in + Text(verbatim: rate.label).tag(rate) + } + } + .pickerStyle(.inline) + .labelsHidden() + } label: { + Text(verbatim: model.rate.label) + .font(.caption.monospacedDigit()) + } + .menuStyle(.borderlessButton) + .fixedSize() + .disabled(model.state != .ready) + .accessibilityLabel(Text("Playback speed")) + } + + private func timeLabel(_ seconds: Double) -> some View { + // Monospaced digits, or the whole row shifts sideways every time the + // seconds tick over from 1 to 2 — the layout jitter that makes a + // transport feel unsteady. + Text(verbatim: Self.clock(seconds)) + .font(.caption.monospacedDigit()) + .foregroundStyle(.secondary) + .accessibilityHidden(true) + } + + /// `m:ss`, or `h:mm:ss` for anything over an hour. + /// + /// Hand-formatted rather than `Duration.formatted`, which spells a + /// position as "1 min, 4 sec" — correct prose, and unreadable as a + /// changing clock. + static func clock(_ seconds: Double) -> String { + guard seconds.isFinite, seconds >= 0 else { return "0:00" } + let total = Int(seconds.rounded(.down)) + let (hours, minutes, secs) = (total / 3600, total / 60 % 60, total % 60) + return hours > 0 + ? String(format: "%d:%02d:%02d", hours, minutes, secs) + : String(format: "%d:%02d", minutes, secs) + } + + /// What VoiceOver says, which is prose rather than a clock: "4 minutes, + /// 12 seconds" reads correctly where "4:12" is announced as a time of + /// day. + static func spoken(_ seconds: Double) -> String { + guard seconds.isFinite, seconds >= 0 else { return "0 seconds" } + return Duration.seconds(Int(seconds.rounded(.down))) + .formatted(.units(allowed: [.hours, .minutes, .seconds], width: .wide)) + } +} + +/// Gives a transport button a target worth aiming at. +/// +/// An SF Symbol in a borderless button is as big as its glyph — the play +/// triangle measured 9×11 points, which is a target you hunt for rather than +/// hit. The width is the full guideline; the height is less, because a 44pt +/// row would make the bar as tall as the detail panel on a Mac. M8 raises it +/// for touch, where the guideline is not a guideline. +private struct TransportHitTarget: ViewModifier { + func body(content: Content) -> some View { + content + .frame(minWidth: ReaderMetrics.minimumHitTarget, minHeight: 24) + .contentShape(.rect) + } +} diff --git a/Sources/HanReaderUI/Screens/ReaderScreen.swift b/Sources/HanReaderUI/Screens/ReaderScreen.swift index e3038e0..55dfe27 100644 --- a/Sources/HanReaderUI/Screens/ReaderScreen.swift +++ b/Sources/HanReaderUI/Screens/ReaderScreen.swift @@ -12,9 +12,18 @@ import SwiftUI /// reset code here: there is nothing to reset. struct ReaderScreen: View { let textID: TextID + /// For the transport and the lock screen. Passed in rather than queried: + /// the sidebar already has it, and a second read to label a control the + /// reader is looking at is a read too many. + let title: String let services: AppServices let settings: Settings let speech: SpeechModel + /// Scene-scoped, so one window's narration is not another's. Passed in + /// rather than owned here: a model created per text would stop and + /// restart the transport every time the reader changed the font size. + let playback: PlaybackModel + let nowPlaying: NowPlayingController /// Changes when the dictionaries do, so the model — which caches /// readings and definitions — is rebuilt against the new ones. @@ -22,6 +31,13 @@ struct ReaderScreen: View { @State private var model: ReaderModel? @State private var isInspectorPresented = false + @State private var isAudioImporterPresented = false + /// Whether this text has a narration, which decides whether the + /// transport is on screen at all. + @State private var hasAudio = false + /// What was last written, so a periodic flush can tell whether anything + /// has moved since. + @State private var lastSavedPlayhead = 0.0 @Environment(\.accessibilityVoiceOverEnabled) private var voiceOverEnabled @Environment(\.scenePhase) private var scenePhase @@ -54,6 +70,12 @@ struct ReaderScreen: View { // sidebar would leave the old text's work running and its // position unwritten. if let previous = model { + // The playhead belongs to the text that is leaving, and + // `textID` is already the one arriving — so the id comes + // from the outgoing model. Without this, changing text in + // the sidebar lost the playhead silently, which is the + // prototype's behaviour this milestone exists to fix. + await savePlayhead(for: previous.textID) await previous.close() } speech.stop() @@ -64,21 +86,77 @@ struct ReaderScreen: View { ) model = next await next.load() + await loadAudio(for: next) } .onDisappear { // An utterance outlives the view that started it: the // synthesizer is app-scoped, so without this a word spoken // in one text carries on into the next. speech.stop() + // Read before unloading, which resets it. + let outgoing = textID let model = model - Task { await model?.close() } + // The transport is scene-scoped but the track is not: the + // next text has its own, and a lock screen still naming the + // last one is worse than an empty one. + let pending = pendingPlayhead() + playback.unload() + nowPlaying.detach() + Task { + await write(pending, for: outgoing) + await model?.close() + } } .onChange(of: scenePhase) { _, phase in // The position save is debounced, so going to the background // with one in flight loses it. On iOS the app may not be // running again afterwards to write it. guard phase != .active, let model else { return } - Task { await model.flushPosition() } + Task { + await model.flushPosition() + await savePlayhead(for: textID) + } + } + // The playhead is written while the reader is using the app, not + // only when they leave it. macOS delivers neither a scene-phase + // change nor `onDisappear` reliably on quit, so a transport that + // saved only on teardown saved nothing at all — which is exactly + // the prototype's bug wearing a different hat. + .onChange(of: playback.isPlaying) { _, playing in + guard !playing else { return } + Task { await savePlayhead(for: textID) } + } + .onChange(of: playback.observedTime) { _, time in + // And periodically while playing, so quitting mid-sentence + // costs seconds rather than the session. `observedTime` is + // already throttled to once a second, so this is a comparison + // once a second and a write once a minute. + guard playback.isPlaying, + abs(time - lastSavedPlayhead) >= Self.playheadSaveInterval + else { return } + lastSavedPlayhead = time + Task { await savePlayhead(for: textID) } + } + .onChange(of: speech.isSpeaking) { _, speaking in + // Resumed when the utterance ends rather than after a guessed + // delay. `resumeAfterSpeech` only acts if it was this that + // paused the audio, so a reader who had already paused it + // does not find it playing again. + if !speaking, settings.pausesAudioWhileSpeaking { + playback.resumeAfterSpeech() + } + } + .fileImporter( + isPresented: $isAudioImporterPresented, + allowedContentTypes: AudioAttachment.contentTypes, + ) { result in + switch result { + case let .success(url): + Task { await attachAudio(url) } + case let .failure(error): + hasAudio = true + playback.fail(error.localizedDescription) + } } .toolbar { toolbar } .alert( @@ -137,7 +215,7 @@ struct ReaderScreen: View { onTap: { id in model.tap(id) if settings.speaksOnTap, let token = model.document?[id] { - speech.speak(token.text, suppressed: voiceOverEnabled) + speak(token.text) } }, topBlock: $model.topBlock, @@ -147,9 +225,18 @@ struct ReaderScreen: View { state: model.detail, style: style, onExpand: { isInspectorPresented = true }, - onSpeak: { speech.speak($0, suppressed: voiceOverEnabled) }, + onSpeak: { speak($0) }, ) }) + // The transport sits below everything, including the detail panel, + // and only when the text actually has a narration — an empty + // transport is a control that does nothing, which is worse than no + // control. + .safeAreaInset(edge: .bottom, spacing: 0) { + if hasAudio { + AudioBar(model: playback, title: title) + } + } // One modifier for both platforms. On a regular width this is a side // panel; on a compact one SwiftUI presents it as a sheet by itself, // which is exactly the behaviour wanted and needs no `#if os(`. @@ -158,7 +245,7 @@ struct ReaderScreen: View { WordDetailContent( detail: detail, style: style, - onSpeak: { speech.speak($0, suppressed: voiceOverEnabled) }, + onSpeak: { speak($0) }, dictionaryNames: services.dictionaryNames, ) .inspectorColumnWidth(min: 260, ideal: 320, max: 480) @@ -171,8 +258,23 @@ struct ReaderScreen: View { } } + /// How far the playhead must move before it is written again while + /// playing. A minute of narration is a tolerable amount to lose to a + /// crash, and a write a minute is nothing. + static let playheadSaveInterval = 60.0 + @ToolbarContentBuilder private var toolbar: some ToolbarContent { + ToolbarItem { + Button { + isAudioImporterPresented = true + } label: { + Label( + hasAudio ? "Replace audio" : "Add audio", + systemImage: hasAudio ? "waveform.circle.fill" : "waveform.circle", + ) + } + } ToolbarItem { Picker(selection: Binding( get: { settings.wordSpacing }, @@ -205,6 +307,101 @@ struct ReaderScreen: View { } } +/// The narration: finding it, attaching it, and keeping the spoken word +/// from talking over it. +/// +/// An extension because it is one coherent concern that arrived whole in +/// M7, and because the screen's own body is the reading surface rather than +/// the transport. +extension ReaderScreen { + // MARK: - Audio + + /// Finds this text's narration, if it has one, and parks it where the + /// reader left off. + private func loadAudio(for model: ReaderModel) async { + do { + guard let track = try await services.library.audioTrack(for: textID) else { + hasAudio = false + playback.unload() + return + } + let file = AudioAttachment.file(for: track, in: services.locations) + guard FileManager.default.fileExists(atPath: file.path) else { + // The row outlived its file. Said plainly rather than shown + // as a transport that does nothing when pressed. + hasAudio = true + playback.fail("The audio file for this text is missing.") + return + } + hasAudio = true + lastSavedPlayhead = model.storedAudioTime ?? 0 + playback.load(file, startingAt: model.storedAudioTime) + nowPlaying.attach(to: playback, title: title) + } catch { + Log.error("reader", "could not read the audio track: \(error)") + hasAudio = false + } + } + + /// The playhead worth storing, or nil when there is nothing to store. + /// + /// Read synchronously, because every caller is about to unload the + /// transport or hand it to another text — and `unload()` resets it. + private func pendingPlayhead() -> Double? { + guard hasAudio, playback.state == .ready else { return nil } + return playback.playhead + } + + /// Writes the playhead, so the next launch resumes mid-sentence rather + /// than at the top. The prototype lost this on every relaunch. + /// + /// Called at every point the transport stops being this text's: a scene + /// going inactive, the reader choosing another text, the window closing. + /// Saving only on the first of those was its own version of the bug — + /// changing text in the sidebar is the commonest of the three. + private func savePlayhead(for id: TextID) async { + await write(pendingPlayhead(), for: id) + } + + private func write(_ time: Double?, for id: TextID) async { + guard let time else { return } + do { + lastSavedPlayhead = time + try await services.library.saveAudioTime(time, for: id) + } catch { + Log.error("reader", "could not save the playhead: \(error)") + } + } + + private func attachAudio(_ url: URL) async { + do { + let file = try await AudioAttachment.attach( + url, + to: textID, + in: services.locations, + library: services.library, + ) + hasAudio = true + playback.load(file) + nowPlaying.attach(to: playback, title: title) + } catch { + Log.error("reader", "could not attach audio: \(error)") + hasAudio = true + playback.fail(error.localizedDescription) + } + } + + /// Speaks a word, ducking the narration first if the reader asked for + /// that — which is the default, because the attached audio is usually + /// the same narration and hearing both is confusing. + private func speak(_ word: String) { + if settings.pausesAudioWhileSpeaking { + playback.pauseForSpeech() + } + speech.speak(word, suppressed: voiceOverEnabled) + } +} + /// Puts the word-detail panel where the platform wants it. /// /// Above the text on a regular width, which is the prototype's placement and diff --git a/Sources/HanReaderUI/Services/AppServices.swift b/Sources/HanReaderUI/Services/AppServices.swift index ce3ab52..c6bcf31 100644 --- a/Sources/HanReaderUI/Services/AppServices.swift +++ b/Sources/HanReaderUI/Services/AppServices.swift @@ -71,6 +71,7 @@ final class AppServices: Sendable { static func launch(bundle: Bundle = .main) async throws -> AppServices { let locations = try AppDatabase.Locations.standard() let library = try LibraryRepository(locations: locations) + await migratePrototypeLibrary(into: library, locations: locations) return try await opening( library: library, locations: locations, @@ -78,6 +79,40 @@ final class AppServices: Sendable { ) } + /// Brings a reader's prototype library across, once. + /// + /// Before the dictionaries are opened, because it only touches the + /// library and a reader with texts to migrate should have them whatever + /// the dictionaries do. Never fatal: a migration that fails leaves the + /// old file in place to try again next launch, and an app that refuses + /// to start because of it would be worse than one that starts without + /// the old texts. + private static func migratePrototypeLibrary( + into library: LibraryRepository, + locations: AppDatabase.Locations, + ) async { + do { + guard let outcome = try await PrototypeLibrary.migrateIfPresent( + into: library, + locations: locations, + ) else { return } + Log.info( + "services", + """ + migrated the previous library: \(outcome.texts) texts, \ + \(outcome.alreadyPresent) already present, \ + \(outcome.revealedWords) revealed words, \ + \(outcome.audioTracks) audio tracks + """, + ) + for note in outcome.notes { + Log.error("services", note) + } + } catch { + Log.error("services", "could not migrate the previous library: \(error)") + } + } + /// The same services with the dictionaries read again. /// /// Built rather than mutated. Importing a dictionary changes what every diff --git a/Sources/HanReaderUI/Services/AudioAttachment.swift b/Sources/HanReaderUI/Services/AudioAttachment.swift new file mode 100644 index 0000000..61ec607 --- /dev/null +++ b/Sources/HanReaderUI/Services/AudioAttachment.swift @@ -0,0 +1,95 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import HanReaderPersistence +import UniformTypeIdentifiers + +/// Copies a narration into the app's own storage and records it against a +/// text. +/// +/// ### Copy in, always +/// +/// Both platforms, no exceptions. The reader picked the file out of a +/// security-scoped location and that grant does not survive the launch, let +/// alone a restore from backup — so a reference to where they found it is a +/// reference that will stop working. Copying is also what makes the stored +/// path *ours*, and therefore relative. +/// +/// ### Relative paths, which is the whole point +/// +/// The prototype stored `dest.path` — an absolute path containing the app +/// container's UUID. That UUID changes when an iOS app is reinstalled or +/// restored from a backup, so every audio association in the library broke +/// at once, silently, long after the mistake. `LibraryRepository.attachAudio` +/// rejects an absolute path outright; this is the code that gives it a +/// relative one. +enum AudioAttachment { + /// Audio formats worth offering. AVFoundation reads more than this, but + /// a picker that lists everything invites a reader to choose a video + /// file and wonder why it has no picture. + static let contentTypes: [UTType] = [.audio, .mpeg4Audio, .mp3, .wav, .aiff] + + /// Copies `url` into the audio directory and records it against `text`. + /// + /// - Returns: the file inside the container, ready to play. + @discardableResult + static func attach( + _ url: URL, + to text: TextID, + in locations: AppDatabase.Locations, + library: LibraryRepository, + ) async throws + -> URL + { + // Scoped to the reader's choice. Without this the copy fails on a + // sandboxed build with a permission error that reads like a missing + // file. + let scoped = url.startAccessingSecurityScopedResource() + defer { + if scoped { + url.stopAccessingSecurityScopedResource() + } + } + + try FileManager.default.createDirectory( + at: locations.audio, + withIntermediateDirectories: true, + ) + + // Named after the text rather than after the source file, so two + // texts narrated by files both called `audio.m4a` do not collide, + // and so the name carries no part of the reader's own filesystem. + let name = "text-\(text.rawValue).\(url.pathExtension.lowercased())" + let destination = locations.audio.appendingPathComponent(name) + + if FileManager.default.fileExists(atPath: destination.path) { + try FileManager.default.removeItem(at: destination) + } + try FileManager.default.copyItem(at: url, to: destination) + + try await library.attachAudio(AudioTrack( + textID: text, + relativePath: name, + // Left for the player to discover. Reading it here would mean + // loading the asset twice, and a duration the transport has not + // confirmed is worse than none. + duration: nil, + importedAt: Date(), + )) + return destination + } + + /// Where a recorded track lives now. + /// + /// Resolved on every use rather than stored, because the container + /// directory is exactly the thing that moves. + static func file( + for track: AudioTrack, + in locations: AppDatabase.Locations, + ) + -> URL + { + locations.audio.appendingPathComponent(track.relativePath) + } +} diff --git a/Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift b/Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift new file mode 100644 index 0000000..421007e --- /dev/null +++ b/Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift @@ -0,0 +1,282 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import GRDB +import HanReaderCore +import Testing +@testable import HanReaderPersistence + +/// Builds a prototype database with the schema the old app actually wrote. +/// +/// Copied from the real file rather than invented: `texts` has no +/// `content_hash` and no `updated_at`, `reading_progress` is keyed on +/// `text_id` with no row of its own, and `revealed_words` is a single +/// newline-joined string. A fixture that drifted from that would test a +/// migration nobody needs. +private func makePrototype(at url: URL) throws -> DatabaseQueue { + let db = try DatabaseQueue(path: url.path) + try db.write { db in + try db.execute(sql: """ + CREATE TABLE texts ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + title TEXT NOT NULL, + content TEXT NOT NULL, + audio_path TEXT, + imported_at REAL NOT NULL + ); + CREATE TABLE reading_progress ( + text_id INTEGER PRIMARY KEY, + scroll_offset REAL NOT NULL DEFAULT 0, + revealed_words TEXT NOT NULL DEFAULT '', + last_opened_at REAL NOT NULL + ); + """) + } + return db +} + +private struct Workspace { + let root: URL + let locations: AppDatabase.Locations + let prototype: URL +} + +private func workspace() throws -> Workspace { + let root = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("prototype-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + let locations = AppDatabase.Locations( + library: root.appendingPathComponent("library.sqlite"), + dictionaries: root.appendingPathComponent("dictionaries", isDirectory: true), + audio: root.appendingPathComponent("audio", isDirectory: true), + ) + return Workspace( + root: root, + locations: locations, + prototype: PrototypeLibrary.file(near: locations.library), + ) +} + +@Suite("Prototype library migration") +struct PrototypeLibraryTests { + @Test("Texts, revealed words and last-opened all come across") + func migratesEverything() async throws { + let space = try workspace() + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute( + sql: "INSERT INTO texts (id, title, content, imported_at) VALUES (?, ?, ?, ?)", + arguments: [1, "孔乙己", "我从十二岁起，便在镇口的咸亨酒店里当伙计", 1_600_000_000.0], + ) + try db.execute(sql: """ + INSERT INTO reading_progress + (text_id, scroll_offset, revealed_words, last_opened_at) + VALUES (?, ?, ?, ?) + """, arguments: [1, 480.5, "伙计\n绰号\n镇", 1_700_000_000.0]) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let outcome = try #require(try await PrototypeLibrary.migrateIfPresent( + into: library, + locations: space.locations, + )) + + #expect(outcome.texts == 1) + #expect(outcome.revealedWords == 3) + + let items = try await library.items() + #expect(items.count == 1) + let id = try #require(items.first?.id) + #expect(items.first?.title == "孔乙己") + + // The blob, undone. + let revealed = try await library.revealedWords(in: id) + #expect(revealed == ["伙计", "绰号", "镇"]) + + let position = try await library.position(of: id) + // `scroll_offset` is a pixel count and there is nothing to convert it + // into, so it is dropped rather than guessed at. + #expect(position?.characterOffset ?? 0 == 0) + } + + /// The migration must be safe to run twice — a crash between migrating + /// and renaming the file leaves it there for the next launch. + @Test("Running twice does not duplicate anything") + func migrationIsIdempotent() async throws { + let space = try workspace() + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute( + sql: "INSERT INTO texts (id, title, content, imported_at) VALUES (?, ?, ?, ?)", + arguments: [1, "孔乙己", "我从十二岁起", 1.0], + ) + try db.execute(sql: """ + INSERT INTO reading_progress + (text_id, scroll_offset, revealed_words, last_opened_at) + VALUES (?, 0, ?, ?) + """, arguments: [1, "伙计\n伙计", 2.0]) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let first = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + let second = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + + #expect(first.texts == 1) + // Second time, deduplicated on content rather than inserted again. + #expect(second.texts == 0) + #expect(second.alreadyPresent == 1) + #expect(try await library.items().count == 1) + + let id = try #require(try await library.items().first?.id) + #expect(try await library.revealedWords(in: id) == ["伙计"]) + } + + @Test("An empty revealed-words string produces no words") + func emptyBlobIsNotOneEmptyWord() async throws { + let space = try workspace() + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute( + sql: "INSERT INTO texts (id, title, content, imported_at) VALUES (?, ?, ?, ?)", + arguments: [1, "空", "我爱你", 1.0], + ) + try db.execute(sql: """ + INSERT INTO reading_progress + (text_id, scroll_offset, revealed_words, last_opened_at) + VALUES (?, 0, '', ?) + """, arguments: [1, 2.0]) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let outcome = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + #expect(outcome.revealedWords == 0) + let id = try #require(try await library.items().first?.id) + #expect(try await library.revealedWords(in: id).isEmpty) + } + + /// The prototype stored an absolute path containing the container's + /// UUID. On a device that path is long gone, which is the whole reason + /// the new schema stores a relative one. + @Test("Audio whose absolute path no longer resolves is reported, not fatal") + func missingAudioIsReported() async throws { + let space = try workspace() + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute(sql: """ + INSERT INTO texts (id, title, content, audio_path, imported_at) + VALUES (?, ?, ?, ?, ?) + """, arguments: [ + 1, "孔乙己", "我从十二岁起", + "/var/mobile/Containers/Data/Application/DEAD-BEEF/audio/kong.m4a", 1.0, + ]) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let outcome = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + + // The text still arrives; only its audio is missing. + #expect(outcome.texts == 1) + #expect(outcome.audioTracks == 0) + #expect(outcome.notes.count == 1) + let id = try #require(try await library.items().first?.id) + #expect(try await library.audioTrack(for: id) == nil) + } + + /// And when the file *is* still there it is copied inside and recorded + /// relatively — the fix, applied to the reader's history. + @Test("Audio that still exists is copied in and stored relatively") + func audioIsCarriedAcross() async throws { + let space = try workspace() + let source = space.root.appendingPathComponent("narration.m4a") + try Data("not really audio".utf8).write(to: source) + + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute(sql: """ + INSERT INTO texts (id, title, content, audio_path, imported_at) + VALUES (?, ?, ?, ?, ?) + """, arguments: [1, "孔乙己", "我从十二岁起", source.path, 1.0]) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let outcome = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + + #expect(outcome.audioTracks == 1) + let id = try #require(try await library.items().first?.id) + let track = try #require(try await library.audioTrack(for: id)) + #expect(!track.relativePath.hasPrefix("/")) + #expect(!track.relativePath.contains("Containers")) + + let copied = AudioAttachmentPath.file(for: track, in: space.locations) + #expect(FileManager.default.fileExists(atPath: copied.path)) + } + + @Test("No prototype database means nothing to do") + func absentPrototypeIsNotAnError() async throws { + let space = try workspace() + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let outcome = try await PrototypeLibrary.migrateIfPresent( + into: library, + locations: space.locations, + ) + #expect(outcome == nil) + } + + /// Renamed rather than deleted: a reader whose migration went wrong + /// still has their texts. And renamed rather than flagged, so the state + /// lives with the file — delete the library and the migration is + /// genuinely not pending again, which a `UserDefaults` flag cannot say. + @Test("A migrated database is kept under a new name") + func migratedFileIsKept() async throws { + let space = try workspace() + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute( + sql: "INSERT INTO texts (id, title, content, imported_at) VALUES (?, ?, ?, ?)", + arguments: [1, "孔乙己", "我从十二岁起", 1.0], + ) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + _ = try await PrototypeLibrary.migrateIfPresent(into: library, locations: space.locations) + + #expect(!FileManager.default.fileExists(atPath: space.prototype.path)) + #expect(FileManager.default.fileExists( + atPath: PrototypeLibrary.migratedFile(near: space.locations.library).path, + )) + + // And so a second launch finds nothing to do. + #expect(try await PrototypeLibrary.migrateIfPresent( + into: library, + locations: space.locations, + ) == nil) + } +} + +/// The same resolution the UI layer does, repeated here because that code +/// lives above this module and a persistence test cannot import it. +private enum AudioAttachmentPath { + static func file(for track: AudioTrack, in locations: AppDatabase.Locations) -> URL { + locations.audio.appendingPathComponent(track.relativePath) + } +} diff --git a/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift b/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift new file mode 100644 index 0000000..fa798cc --- /dev/null +++ b/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift @@ -0,0 +1,437 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import Testing +@testable import HanReaderPlayback + +/// A player that makes no sound and counts what it was asked to do. +/// +/// The transport's hardest requirements are about calls rather than about +/// audio — how many seeks a drag issues, how often state is pushed to a lock +/// screen — and those are assertions only against a spy. +@MainActor +private final class SpyEngine: AudioEngine { + private(set) var seeks: [Double] = [] + private(set) var playCount = 0 + private(set) var pauseCount = 0 + private(set) var rates: [Double] = [] + private(set) var loaded: [URL] = [] + + var duration = 0.0 + var currentTime = 0.0 + + var onTime: ((Double) -> Void)? + var onDuration: ((Double) -> Void)? + var onFinish: (() -> Void)? + + func load(_ url: URL) { + loaded.append(url) + } + + func play() { + playCount += 1 + } + + func pause() { + pauseCount += 1 + } + + func seek(to time: Double) { + seeks.append(time) + currentTime = time + } + + func setRate(_ rate: Double) { + rates.append(rate) + } + + /// Delivers a position the way the real engine does, four times a second. + func tick(_ time: Double) { + currentTime = time + onTime?(time) + } + + func declareDuration(_ seconds: Double) { + duration = seconds + onDuration?(seconds) + } +} + +@MainActor +private final class SpySession: PlaybackSession { + private(set) var activations = 0 + private(set) var deactivations = 0 + var failure: (any Error)? + var onInterruption: ((PlaybackInterruption) -> Void)? + + func activate() throws { + if let failure { + throw failure + } + activations += 1 + } + + func deactivate() { + deactivations += 1 + } + + func interrupt(_ interruption: PlaybackInterruption) { + onInterruption?(interruption) + } +} + +private struct Fixture { + let engine: SpyEngine + let session: SpySession + let model: PlaybackModel +} + +@MainActor +private func fixture(duration: Double = 100) -> Fixture { + let engine = SpyEngine() + let session = SpySession() + let model = PlaybackModel(engine: engine, session: session) + model.load(URL(fileURLWithPath: "/tmp/track.m4a")) + engine.declareDuration(duration) + return Fixture(engine: engine, session: session, model: model) +} + +@MainActor +@Suite("Playback transport") +struct PlaybackModelTests { + // MARK: - Scrub on commit + + /// The prototype bound the scrubber straight to `player.seek`, so every + /// drag delta issued a frame-accurate seek; they cancelled each other and + /// fought the position observer writing the old value back. This is the + /// test that pins the fix, and it is about counting calls — which is the + /// whole reason the engine is behind a protocol. + @Test("A drag issues no seeks at all") + func draggingIssuesNoSeeks() { + let bench = fixture() + bench.model.beginScrub(at: 10) + for position in stride(from: 11.0, through: 40.0, by: 1) { + bench.model.scrub(to: position) + } + #expect(bench.engine.seeks.isEmpty) + // And the thumb follows the finger, not the player. + #expect(bench.model.displayTime == 40) + #expect(bench.model.isScrubbing) + } + + @Test("Committing issues exactly one seek") + func commitIssuesOneSeek() { + let bench = fixture() + bench.model.beginScrub(at: 10) + bench.model.scrub(to: 20) + bench.model.scrub(to: 30) + bench.model.endScrub() + + #expect(bench.engine.seeks == [30]) + #expect(!bench.model.isScrubbing) + #expect(bench.model.displayTime == 30) + } + + /// The other half of the fight: while the finger is down the engine is + /// still reporting from where it was, and letting that through is what + /// made the prototype's thumb jump backwards under the finger. + @Test("Engine reports are ignored while dragging") + func reportsAreIgnoredWhileDragging() { + let bench = fixture() + bench.engine.tick(3) + bench.model.beginScrub(at: 50) + bench.engine.tick(4) + bench.engine.tick(5) + + #expect(bench.model.displayTime == 50) + // And the published position is not merely overridden by the finger + // — it is not written at all, so the bar is not re-rendered once per + // second by a player that is standing still behind the drag. + #expect(bench.model.observedTime == 3) + // The exact playhead still tracks, because a drag that is abandoned + // must leave the model knowing where the player actually is. + #expect(bench.model.playhead == 5) + } + + @Test("A commit outside the track is clamped to it") + func commitIsClamped() { + let bench = fixture(duration: 60) + bench.model.beginScrub(at: 10) + bench.model.scrub(to: 500) + bench.model.endScrub() + #expect(bench.engine.seeks == [60]) + + bench.model.beginScrub(at: 10) + bench.model.scrub(to: -20) + bench.model.endScrub() + #expect(bench.engine.seeks == [60, 0]) + } + + /// A drag that never began must not move the playhead — otherwise a + /// stray gesture delivered after the finger came up seeks the track. + @Test("Scrubbing without beginning does nothing") + func scrubWithoutBeginIsIgnored() { + let bench = fixture() + bench.model.scrub(to: 42) + bench.model.endScrub() + #expect(bench.engine.seeks.isEmpty) + #expect(bench.model.displayTime == 0) + } + + // MARK: - Throttled publishing + + /// The display shows `m:ss`. The prototype published ten times a second + /// to render the same string, on the actor laying out CJK text. + @Test("The playhead is published at most once a second") + func publishingIsThrottled() { + let bench = fixture() + // Four reports inside one second: the display shows `m:ss`, so there + // is nothing new to say and nothing should reach the observed value. + bench.engine.tick(0.25) + bench.engine.tick(0.5) + bench.engine.tick(0.75) + #expect(bench.model.observedTime == 0) + + bench.engine.tick(1.0) + #expect(bench.model.observedTime == 1.0) + + // And again through the next second. + bench.engine.tick(1.25) + bench.engine.tick(1.75) + #expect(bench.model.observedTime == 1.0) + bench.engine.tick(2.5) + #expect(bench.model.observedTime == 2.5) + } + + /// The throttle is about how *often* a value is published, not about + /// rounding it: the published time is exact, there is simply at most one + /// of them per second. The stored playhead tracks every report, so a + /// reader who stops mid-sentence resumes mid-sentence rather than at the + /// last whole second. + @Test("The stored playhead tracks every report, the published one does not") + func playheadTracksEveryReport() { + let bench = fixture() + bench.engine.tick(12.4) + #expect(bench.model.playhead == 12.4) + #expect(bench.model.observedTime == 12.4) + + bench.engine.tick(12.8) + #expect(bench.model.playhead == 12.8) + // Same second, so nothing new to display. + #expect(bench.model.observedTime == 12.4) + } + + // MARK: - The session + + /// Activating a session you are not using interrupts whatever the reader + /// was listening to, for an app that might never make a sound. + @Test("The audio session is activated on first play, not on load") + func sessionIsActivatedLazily() { + let bench = fixture() + #expect(bench.session.activations == 0) + bench.model.play() + #expect(bench.session.activations == 1) + bench.model.pause() + bench.model.play() + // Still once: it stays active across a pause. + #expect(bench.session.activations == 1) + } + + @Test("A session that will not activate does not stop playback") + func sessionFailureIsNotFatal() { + let bench = fixture() + bench.session.failure = CocoaError(.featureUnsupported) + bench.model.play() + #expect(bench.model.isPlaying) + #expect(bench.engine.playCount == 1) + } + + // MARK: - Interruptions + + @Test("An interruption pauses, and its end resumes") + func interruptionResumes() { + let bench = fixture() + bench.model.play() + bench.session.interrupt(.began) + #expect(!bench.model.isPlaying) + bench.session.interrupt(.ended) + #expect(bench.model.isPlaying) + } + + /// Audio the reader paused themselves must stay paused: a phone call + /// ending is not permission to start something they stopped. + @Test("An interruption does not resume audio the reader paused") + func deliberatePauseIsNotResumed() { + let bench = fixture() + bench.model.play() + bench.model.pause() + bench.session.interrupt(.began) + bench.session.interrupt(.ended) + #expect(!bench.model.isPlaying) + } + + /// Unplugging headphones must not play the reader's audio out loud to + /// the room — and plugging them back in must not start it either. + @Test("Losing the output pauses, and does not resume") + func outputGoneDoesNotResume() { + let bench = fixture() + bench.model.play() + bench.session.interrupt(.outputGone) + #expect(!bench.model.isPlaying) + bench.session.interrupt(.ended) + #expect(!bench.model.isPlaying) + } + + // MARK: - Speech + + /// The attached audio is usually the same narration, so hearing the + /// spoken word over it is confusing rather than merely noisy. + @Test("Speaking pauses the narration and resumes it after") + func speechDucksPlayback() { + let bench = fixture() + bench.model.play() + bench.model.pauseForSpeech() + #expect(!bench.model.isPlaying) + bench.model.resumeAfterSpeech() + #expect(bench.model.isPlaying) + } + + @Test("Speaking over paused audio leaves it paused") + func speechDoesNotStartPausedAudio() { + let bench = fixture() + bench.model.pauseForSpeech() + bench.model.resumeAfterSpeech() + #expect(!bench.model.isPlaying) + #expect(bench.engine.playCount == 0) + } + + // MARK: - Transport basics + + @Test("Loading parks the playhead where the reader left it") + func loadResumesFromStoredPosition() { + let engine = SpyEngine() + let model = PlaybackModel(engine: engine, session: SpySession()) + model.load(URL(fileURLWithPath: "/tmp/track.m4a"), startingAt: 42) + + #expect(engine.seeks == [42]) + #expect(model.playhead == 42) + // And does not start playing: opening a text should not fill the + // room with sound. + #expect(!model.isPlaying) + #expect(engine.playCount == 0) + } + + @Test("Loading from the start issues no seek") + func loadFromZeroDoesNotSeek() { + let engine = SpyEngine() + let model = PlaybackModel(engine: engine, session: SpySession()) + model.load(URL(fileURLWithPath: "/tmp/track.m4a")) + #expect(engine.seeks.isEmpty) + } + + @Test("Playing a finished track starts it again") + func playAfterFinishRestarts() { + let bench = fixture(duration: 30) + bench.model.play() + bench.engine.onFinish?() + #expect(!bench.model.isPlaying) + #expect(bench.model.playhead == 30) + + bench.model.play() + #expect(bench.engine.seeks == [0]) + #expect(bench.model.isPlaying) + } + + @Test("Skipping is clamped to the track") + func skipIsClamped() { + let bench = fixture(duration: 30) + bench.model.skip(by: 10) + #expect(bench.engine.seeks == [10]) + bench.model.skip(by: 100) + #expect(bench.engine.seeks == [10, 30]) + bench.model.skip(by: -100) + #expect(bench.engine.seeks == [10, 30, 0]) + } + + @Test("Nothing moves before a track is loaded") + func emptyTransportIsInert() { + let engine = SpyEngine() + let model = PlaybackModel(engine: engine, session: SpySession()) + model.play() + model.skip(by: 10) + model.seek(to: 5) + model.beginScrub(at: 5) + #expect(engine.playCount == 0) + #expect(engine.seeks.isEmpty) + #expect(!model.isPlaying) + #expect(!model.isScrubbing) + } + + @Test("A speed change reaches the engine once") + func rateIsApplied() { + let bench = fixture() + let before = bench.engine.rates.count + bench.model.rate = .slow + #expect(bench.engine.rates.suffix(1) == [0.75]) + #expect(bench.engine.rates.count == before + 1) + // Setting the same speed again is not a change. + bench.model.rate = .slow + #expect(bench.engine.rates.count == before + 1) + } + + @Test("Unloading stops the track and releases the session") + func unloadReleasesTheSession() { + let bench = fixture() + bench.model.play() + bench.model.unload() + #expect(!bench.model.isPlaying) + #expect(bench.model.state == .empty) + #expect(bench.session.deactivations == 1) + } + + // MARK: - What the lock screen is told + + /// Now-playing info is extrapolated by the system from the last elapsed + /// time and rate it was given, so pushing on every tick is both wasted + /// work and a worse lock screen. It must be pushed on transitions. + @Test("State changes are announced on transitions, not on ticks") + func stateChangesAreAnnouncedOnTransitions() { + let bench = fixture() + var announcements = 0 + bench.model.onStateChange = { announcements += 1 } + + bench.model.play() + #expect(announcements == 1) + + for second in 1 ... 8 { + bench.engine.tick(Double(second)) + } + #expect(announcements == 1) + + bench.model.pause() + #expect(announcements == 2) + bench.model.seek(to: 12) + #expect(announcements == 3) + bench.model.rate = .fast + #expect(announcements == 4) + } +} + +@Suite("Playback rate") +struct PlaybackRateTests { + @Test("Every rate has a label and normal speed reads as 1×") + func labels() { + #expect(PlaybackRate.normal.label == "1×") + #expect(PlaybackRate.half.label == "0.5×") + #expect(PlaybackRate.allCases.allSatisfy { !$0.label.isEmpty }) + } + + /// Ordered slowest to fastest, because the menu is built from + /// `allCases` and a jumbled speed picker is a bug the compiler cannot + /// see. + @Test("Rates are ordered slowest first") + func ordering() { + let rates = PlaybackRate.allCases.map(\.rawValue) + #expect(rates == rates.sorted()) + } +} diff --git a/Tests/HanReaderUITests/AudioAttachmentTests.swift b/Tests/HanReaderUITests/AudioAttachmentTests.swift new file mode 100644 index 0000000..aeb4aac --- /dev/null +++ b/Tests/HanReaderUITests/AudioAttachmentTests.swift @@ -0,0 +1,156 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import HanReaderCore +import HanReaderPersistence +import Testing +@testable import HanReaderUI + +@MainActor +@Suite("Attaching audio") +struct AudioAttachmentTests { + private func workspace() throws -> (root: URL, locations: AppDatabase.Locations) { + let root = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("audio-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + return (root, AppDatabase.Locations( + library: root.appendingPathComponent("library.sqlite"), + dictionaries: root.appendingPathComponent("dictionaries", isDirectory: true), + audio: root.appendingPathComponent("audio", isDirectory: true), + )) + } + + private func source(_ root: URL, named name: String) throws -> URL { + let url = root.appendingPathComponent(name) + try Data("pretend this is audio".utf8).write(to: url) + return url + } + + /// The platform-fatal bug this code exists to prevent. The prototype + /// stored `dest.path`, an absolute path containing the app container's + /// UUID — which changes when an iOS app is reinstalled or restored, so + /// every attachment broke at once, silently, long afterwards. + @Test("The stored path is relative, and inside the container") + func pathIsRelative() async throws { + let space = try workspace() + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let id = try await imported(into: library) + let file = try await AudioAttachment.attach( + source(space.root, named: "narration.m4a"), + to: id, + in: space.locations, + library: library, + ) + + let track = try #require(try await library.audioTrack(for: id)) + #expect(!track.relativePath.hasPrefix("/")) + #expect(!track.relativePath.contains("/")) + #expect(track.relativePath.hasSuffix(".m4a")) + // And resolving it finds the copy, not the original. + #expect(AudioAttachment.file(for: track, in: space.locations) == file) + #expect(FileManager.default.fileExists(atPath: file.path)) + } + + /// The reader picked the file out of a security-scoped location, and + /// that grant does not survive the launch. A reference would be a + /// reference that stops working. + @Test("The file is copied, not referenced") + func fileIsCopied() async throws { + let space = try workspace() + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let id = try await imported(into: library) + let original = try source(space.root, named: "narration.m4a") + let copy = try await AudioAttachment.attach( + original, + to: id, + in: space.locations, + library: library, + ) + + #expect(copy != original) + let copied = try Data(contentsOf: copy) + + // Changed **in place**, through a file handle: `Data.write(to:)` + // replaces the file with a new one, which a hard link would survive + // — so that would pass whether or not the bytes were really copied. + let handle = try FileHandle(forWritingTo: original) + try handle.seek(toOffset: 0) + try handle.write(contentsOf: Data("something else entirely".utf8)) + try handle.close() + + #expect(try Data(contentsOf: copy) == copied) + #expect(try Data(contentsOf: copy) != Data(contentsOf: original)) + + // And deleting what the reader chose leaves the narration intact. + try FileManager.default.removeItem(at: original) + #expect(FileManager.default.fileExists(atPath: copy.path)) + } + + /// The name comes from the text, not from the file the reader picked, so + /// two texts narrated by two files both called `audio.m4a` do not + /// collide — and so no part of the reader's own filesystem is kept. + @Test("Two texts with identically named sources do not collide") + func namesDoNotCollide() async throws { + let space = try workspace() + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let first = try await imported(into: library, content: "我爱你") + let second = try await imported(into: library, content: "你好世界") + + let one = try await AudioAttachment.attach( + source(space.root, named: "audio.m4a"), + to: first, + in: space.locations, + library: library, + ) + let two = try await AudioAttachment.attach( + source(space.root, named: "audio.m4a"), + to: second, + in: space.locations, + library: library, + ) + #expect(one != two) + #expect(FileManager.default.fileExists(atPath: one.path)) + #expect(FileManager.default.fileExists(atPath: two.path)) + } + + /// Re-attaching replaces, rather than leaving the old file on disk with + /// nothing pointing at it. + @Test("Replacing a narration leaves one file behind, not two") + func replacingDoesNotOrphan() async throws { + let space = try workspace() + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let id = try await imported(into: library) + + _ = try await AudioAttachment.attach( + source(space.root, named: "first.m4a"), + to: id, + in: space.locations, + library: library, + ) + _ = try await AudioAttachment.attach( + source(space.root, named: "second.m4a"), + to: id, + in: space.locations, + library: library, + ) + + let files = try FileManager.default.contentsOfDirectory( + atPath: space.locations.audio.path, + ) + #expect(files.count == 1) + // And one row, not two: the schema is one track per text. + #expect(try await library.audioTrack(for: id) != nil) + } + + private func imported( + into library: LibraryRepository, + content: String = "我在一个山村学校当老师", + ) async throws + -> TextID + { + switch try await library.importText(title: "听力", content: content) { + case let .imported(id): id + case let .alreadyPresent(id): id + } + } +} diff --git a/Tests/HanReaderUITests/AudioBarTests.swift b/Tests/HanReaderUITests/AudioBarTests.swift new file mode 100644 index 0000000..20c9e78 --- /dev/null +++ b/Tests/HanReaderUITests/AudioBarTests.swift @@ -0,0 +1,57 @@ +// HanReader — MIT licensed. See LICENSE. + +import Foundation +import Testing +@testable import HanReaderUI + +@MainActor +@Suite("Audio transport formatting") +struct AudioBarTests { + /// `m:ss`, because a transport is read at a glance. `Duration.formatted` + /// spells this as "1 min, 4 sec", which is correct prose and unreadable + /// as a clock that changes once a second. + @Test( + "A position reads as a clock", + arguments: [ + (0.0, "0:00"), + (4.0, "0:04"), + (9.9, "0:09"), + (64.0, "1:04"), + (599.0, "9:59"), + (600.0, "10:00"), + ], + ) + func clockFormatting(seconds: Double, expected: String) { + #expect(AudioBar.clock(seconds) == expected) + } + + /// An hour-long recording is a plausible thing to attach — a whole + /// chapter read aloud — and `62:03` for an hour and two minutes is a + /// clock nobody reads correctly. + @Test("Over an hour, hours are shown") + func longRecordings() { + #expect(AudioBar.clock(3600) == "1:00:00") + #expect(AudioBar.clock(3723) == "1:02:03") + } + + /// A track whose duration has not loaded yet reports zero, and an + /// `AVPlayer` with no item reports `NaN`. Neither may put `nan:0-2147483648` + /// on screen. + @Test("Nonsense values do not reach the screen") + func degenerateValues() { + #expect(AudioBar.clock(.nan) == "0:00") + #expect(AudioBar.clock(.infinity) == "0:00") + #expect(AudioBar.clock(-5) == "0:00") + } + + /// VoiceOver gets prose, not a clock: "4:12" is announced as a time of + /// day, which is wrong twice over. + @Test("VoiceOver hears a duration, not a time of day") + func spokenValues() { + let spoken = AudioBar.spoken(252) + #expect(spoken.contains("4")) + #expect(spoken.lowercased().contains("minute")) + #expect(!spoken.contains(":")) + #expect(AudioBar.spoken(.nan) == "0 seconds") + } +} From 999182990dc3f881339a153b0a115656f72ea04a Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:46:39 +0300 Subject: [PATCH 2/3] fix: answer fifteen review findings on the audio milestone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All fifteen were real. They cluster into five. **Who may resume narration.** One flag was doing two jobs, and the two cases contradict each other. A system interruption that never invites us back left its claim standing, so the next word the reader tapped ended by resuming audio the system had taken away; and losing the output while already paused by an interruption left that claim set, so the interruption ending played into a room with no headphones in it. Now `pausedForSpeech` and `pausedByInterruption` are separate, each undone only by its own counterpart, and `.outputGone` clears both before its guard rather than after it. **Speech that never happens.** `SpeechModel.speak` declines in three ordinary situations — VoiceOver on, an empty word, no Chinese voice — and says so by never starting. The narration ducked for an utterance that never came and stayed paused until the reader pressed play, which on a Mac with no Chinese voice is every single tap. **Process-wide singletons used per window.** `MPRemoteCommandCenter` and `AVAudioSession` are one per process, so registering per window meant a lock-screen play reaching every window's transport, and one window finishing a track deactivating the session another was still playing through. Commands now route to the controller that most recently claimed the lock screen, and the session is reference counted. **Shared transport, unguarded `await`.** `loadAudio` and `attachAudio` wrote to the shared transport after asynchronous work without checking the text was still open, so changing text mid-lookup loaded one text's narration under another's name. Save tasks read the playhead when they ran rather than when they were scheduled, which could write a new track's position under the old text's id. Both now check, and the playhead is read at schedule time. **Destructive ordering.** Attaching deleted the existing narration before copying the replacement, so a failed copy left a row pointing at a file that was gone; it now stages the copy and moves it into place. Replacing an `.m4a` with an `.mp3` left the old file with nothing pointing at it. A replaced narration kept the previous one's stored playhead, so it resumed partway through a recording it had never played. A parked position was clamped only to zero, so a shorter replacement showed a slider past its own end until the duration arrived. **Migration overreach.** A reader who has used both versions has a library that is partly theirs already. Carrying prototype audio over a text that was matched by content replaced a narration they had chosen; stamping `last_opened_at` on it moved it backwards in the sidebar. Both now apply only to texts this migration created. `imported_at` is carried, so a migrated library keeps the order it was built in. And an audio file the old container will not give up is a note rather than a thrown error — it used to abort the migration before the rename that marks it done, so it would fail again on every launch for ever. Nine mutants planted against the fixes; all nine killed. Co-Authored-By: Claude Opus 5 --- .../PrototypeLibrary.swift | 80 +++++-- .../NowPlayingController.swift | 31 ++- Sources/HanReaderPlayback/PlaybackModel.swift | 58 +++-- .../HanReaderUI/Screens/ReaderScreen.swift | 60 +++++- .../Services/AudioAttachment.swift | 46 ++-- .../PrototypeLibraryTests.swift | 158 ++++++++++++++ .../PlaybackModelTests.swift | 198 ++++++++++++------ .../AudioAttachmentTests.swift | 70 +++++++ 8 files changed, 584 insertions(+), 117 deletions(-) diff --git a/Sources/HanReaderPersistence/PrototypeLibrary.swift b/Sources/HanReaderPersistence/PrototypeLibrary.swift index a797121..c7f6e1d 100644 --- a/Sources/HanReaderPersistence/PrototypeLibrary.swift +++ b/Sources/HanReaderPersistence/PrototypeLibrary.swift @@ -94,7 +94,7 @@ public enum PrototypeLibrary { let texts = try await readTexts(from: old) let progress = try await readProgress(from: old) - let run = Migration(repository: repository, locations: locations) + var run = Migration(repository: repository, locations: locations) var outcome = Outcome() let identifiers = try await run.importTexts(texts, into: &outcome) try await run.applyProgress(progress, identifiers: identifiers, into: &outcome) @@ -115,13 +115,14 @@ public enum PrototypeLibrary { private static func readTexts(from old: DatabaseQueue) async throws -> [OldText] { try await old.read { db in try Row.fetchAll(db, sql: """ - SELECT id, title, content, audio_path FROM texts ORDER BY id + SELECT id, title, content, audio_path, imported_at FROM texts ORDER BY id """).map { OldText( id: $0["id"], title: $0["title"], content: $0["content"], audioPath: $0["audio_path"], + importedAt: $0["imported_at"], ) } } @@ -151,15 +152,21 @@ private struct Migration { let repository: LibraryRepository let locations: AppDatabase.Locations + /// Which old ids this run actually created, as opposed to matched + /// against something already in the library. Several steps must act only + /// on the former. + private var newcomers: [Int64: Bool] = [:] + /// - Returns: old row id to new `TextID`, so the progress rows can find /// the text they belong to. - func importTexts( + mutating func importTexts( _ texts: [OldText], into outcome: inout PrototypeLibrary.Outcome, ) async throws -> [Int64: TextID] { var identifiers: [Int64: TextID] = [:] + newcomers = [:] for text in texts { guard let title = text.title, let content = text.content, let oldID = text.id else { outcome.notes.append("A text could not be read and was skipped.") @@ -169,17 +176,26 @@ private struct Migration { title: title, content: content, sourceName: "HanReader (prototype)", + now: text.importedAt.map { Date(timeIntervalSince1970: $0) } ?? .now, ) + let isNew: Bool switch result { case let .imported(id): identifiers[oldID] = id + isNew = true outcome.texts += 1 case let .alreadyPresent(id): identifiers[oldID] = id + isNew = false outcome.alreadyPresent += 1 } - if let path = text.audioPath, let id = identifiers[oldID] { - try await carryAudio(from: path, for: id, title: title, into: &outcome) + newcomers[oldID] = isNew + // Only for a text this migration actually created. The same + // content already in the library may well have a narration the + // reader chose themselves, and replacing it with one from a + // previous version of the app is not a migration, it is a loss. + if isNew, let path = text.audioPath, let id = identifiers[oldID] { + await carryAudio(from: path, for: id, title: title, into: &outcome) } } return identifiers @@ -199,7 +215,11 @@ private struct Migration { try await repository.reveal(String(word), in: id) outcome.revealedWords += 1 } - if let opened = row.lastOpenedAt { + // Only for a text this migration created. A text already in the + // library has its own, newer, history — stamping it with a + // prototype timestamp moves it backwards in the sidebar and can + // change which text opens on launch. + if newcomers[oldID] == true, let opened = row.lastOpenedAt { try await repository.markOpened(id, at: Date(timeIntervalSince1970: opened)) } } @@ -212,7 +232,7 @@ private struct Migration { for id: TextID, title: String, into outcome: inout PrototypeLibrary.Outcome, - ) async throws { + ) async { let original = URL(fileURLWithPath: path) guard FileManager.default.fileExists(atPath: original.path) else { // The failure the relative-path rule exists to prevent, met here @@ -224,23 +244,33 @@ private struct Migration { ) return } - try FileManager.default.createDirectory( - at: locations.audio, - withIntermediateDirectories: true, - ) - let name = "text-\(id.rawValue).\(original.pathExtension.lowercased())" - let destination = locations.audio.appendingPathComponent(name) - if FileManager.default.fileExists(atPath: destination.path) { - try FileManager.default.removeItem(at: destination) + // Never thrown out of. A file the old container will not let us read + // would otherwise abort the whole migration — and because the rename + // that marks it done never runs, it would be retried, and fail, on + // every launch for ever. + do { + try FileManager.default.createDirectory( + at: locations.audio, + withIntermediateDirectories: true, + ) + let name = "text-\(id.rawValue).\(original.pathExtension.lowercased())" + let destination = locations.audio.appendingPathComponent(name) + if FileManager.default.fileExists(atPath: destination.path) { + try FileManager.default.removeItem(at: destination) + } + try FileManager.default.copyItem(at: original, to: destination) + try await repository.attachAudio(AudioTrack( + textID: id, + relativePath: name, + duration: nil, + importedAt: Date(), + )) + outcome.audioTracks += 1 + } catch { + outcome.notes.append( + "The audio for \u{201C}\(title)\u{201D} could not be copied across.", + ) } - try FileManager.default.copyItem(at: original, to: destination) - try await repository.attachAudio(AudioTrack( - textID: id, - relativePath: name, - duration: nil, - importedAt: Date(), - )) - outcome.audioTracks += 1 } } @@ -255,6 +285,10 @@ private struct OldText: Sendable { let title: String? let content: String? let audioPath: String? + /// Carried, so a migrated library keeps the order the reader built it + /// in. Without it every text is stamped with the moment of migration + /// and they all arrive at once. + let importedAt: Double? } private struct OldProgress: Sendable { diff --git a/Sources/HanReaderPlayback/NowPlayingController.swift b/Sources/HanReaderPlayback/NowPlayingController.swift index 3979bed..0e66ee0 100644 --- a/Sources/HanReaderPlayback/NowPlayingController.swift +++ b/Sources/HanReaderPlayback/NowPlayingController.swift @@ -66,12 +66,18 @@ public final class NowPlayingController { model?.onStateChange = nil model = nil title = nil + // Only the controller that currently owns the lock screen may clear + // it. Another window may have claimed it since, and wiping its + // Now Playing item on the way out would be this bug in reverse. + guard Self.commandTarget === self else { return } + Self.commandTarget = nil info.nowPlayingInfo = nil } // MARK: - Pushing state out private func update() { + guard Self.commandTarget === self else { return } guard let model, model.state == .ready else { info.nowPlayingInfo = nil return @@ -92,7 +98,19 @@ public final class NowPlayingController { // MARK: - Taking commands in + /// The controller remote commands act on. + /// + /// `MPRemoteCommandCenter` is process-wide, so registering handlers per + /// window meant a lock-screen play reaching *every* open window's + /// transport at once. There is only one lock screen and only one Now + /// Playing item, so there is only one controller it can sensibly mean: + /// the one that most recently claimed it. Weak, so a closed window's + /// controller does not keep itself alive as the target of commands it + /// can no longer serve. + private weak static var commandTarget: NowPlayingController? + private func subscribe() { + Self.commandTarget = self guard handlers.isEmpty else { return } add(commands.playCommand) { $0.play() } add(commands.pauseCommand) { $0.pause() } @@ -107,8 +125,8 @@ public final class NowPlayingController { // drag, for the same reason the in-app scrubber commits once. let changePosition = commands.changePlaybackPositionCommand changePosition.isEnabled = true - handlers[changePosition] = changePosition.addTarget { [weak self] event in - guard let model = self?.model, + handlers[changePosition] = changePosition.addTarget { event in + guard let model = Self.commandTarget?.model, let event = event as? MPChangePlaybackPositionCommandEvent else { return .commandFailed } model.seek(to: event.positionTime) @@ -125,8 +143,13 @@ public final class NowPlayingController { private func add(_ command: MPRemoteCommand, _ action: @escaping (PlaybackModel) -> Void) { command.isEnabled = true - handlers[command] = command.addTarget { [weak self] _ in - guard let model = self?.model else { return .noActionableNowPlayingItem } + // Routed through the shared target rather than through `self`, so a + // command reaches the window that owns the lock screen rather than + // every window that ever opened a narrated text. + handlers[command] = command.addTarget { _ in + guard let model = Self.commandTarget?.model else { + return .noActionableNowPlayingItem + } action(model) return .success } diff --git a/Sources/HanReaderPlayback/PlaybackModel.swift b/Sources/HanReaderPlayback/PlaybackModel.swift index 73c3a6e..6a3ee70 100644 --- a/Sources/HanReaderPlayback/PlaybackModel.swift +++ b/Sources/HanReaderPlayback/PlaybackModel.swift @@ -93,7 +93,16 @@ public final class PlaybackModel { /// True when playback was stopped by something other than the reader — /// a phone call, headphones unplugged, a word being spoken — so it can be /// resumed without resuming audio the reader had deliberately paused. - @ObservationIgnored private var wasInterrupted = false + /// Paused so a tapped word could be heard over it. + /// + /// Two flags rather than one, and that separation is the fix for a real + /// confusion: a system interruption that never invites us back leaves its + /// flag set, and with one shared flag the next word the reader tapped + /// would end by resuming narration the system had taken away. + @ObservationIgnored private var pausedForSpeech = false + + /// Paused by the system — a call, an alarm, another app. + @ObservationIgnored private var pausedByInterruption = false @ObservationIgnored private let engine: any AudioEngine @ObservationIgnored private let session: any PlaybackSession @@ -113,6 +122,14 @@ public final class PlaybackModel { engine.onDuration = { [weak self] seconds in guard let self else { return } duration = seconds + // A parked position is clamped once the length is known. A + // narration replaced by a shorter file would otherwise show a + // slider past its own end, and tell a lock screen an elapsed + // time longer than the track. + if playhead > seconds { + playhead = seconds + observedTime = seconds + } onStateChange?() } engine.onFinish = { [weak self] in self?.finish() } @@ -133,7 +150,7 @@ public final class PlaybackModel { state = .ready duration = engine.duration isPlaying = false - wasInterrupted = false + clearPauseReasons() scrubTime = nil let start = max(0, time ?? 0) playhead = start @@ -155,7 +172,7 @@ public final class PlaybackModel { observedTime = 0 playhead = 0 scrubTime = nil - wasInterrupted = false + clearPauseReasons() deactivateSession() onStateChange?() } @@ -181,7 +198,7 @@ public final class PlaybackModel { } engine.play() isPlaying = true - wasInterrupted = false + clearPauseReasons() onStateChange?() } @@ -189,7 +206,9 @@ public final class PlaybackModel { guard isPlaying else { return } engine.pause() isPlaying = false - wasInterrupted = false + // Deliberate, so neither the system nor a finished utterance may + // start it again on the reader's behalf. + clearPauseReasons() onStateChange?() } @@ -253,15 +272,16 @@ public final class PlaybackModel { guard isPlaying else { return } engine.pause() isPlaying = false - wasInterrupted = true + pausedForSpeech = true onStateChange?() } + /// Resumes only narration that *this* paused for a word. public func resumeAfterSpeech() { - guard wasInterrupted, state == .ready else { return } + guard pausedForSpeech, state == .ready else { return } + pausedForSpeech = false engine.play() isPlaying = true - wasInterrupted = false onStateChange?() } @@ -271,20 +291,29 @@ public final class PlaybackModel { guard isPlaying else { return } engine.pause() isPlaying = false - wasInterrupted = true + pausedByInterruption = true onStateChange?() case .ended: - resumeAfterSpeech() + guard pausedByInterruption, state == .ready else { return } + pausedByInterruption = false + engine.play() + isPlaying = true + onStateChange?() case .outputGone: // Headphones pulled out. Pausing is not politeness, it is the // difference between a quiet room and the reader's audio playing // through the speaker to everyone around them — and it must not // be resumable automatically, or plugging back in starts it in // the open. + // + // The reasons are cleared *before* the guard, not after it: the + // output can be lost while already paused by an interruption, + // and leaving that reason set means the interruption ending + // resumes narration into a room with no headphones in it. + clearPauseReasons() guard isPlaying else { return } engine.pause() isPlaying = false - wasInterrupted = false onStateChange?() } } @@ -306,7 +335,7 @@ public final class PlaybackModel { private func finish() { engine.pause() isPlaying = false - wasInterrupted = false + clearPauseReasons() if duration > 0 { playhead = duration observedTime = duration @@ -333,6 +362,11 @@ public final class PlaybackModel { hasActivatedSession = false } + private func clearPauseReasons() { + pausedForSpeech = false + pausedByInterruption = false + } + private func clamp(_ time: Double) -> Double { guard duration > 0 else { return max(0, time) } return min(max(0, time), duration) diff --git a/Sources/HanReaderUI/Screens/ReaderScreen.swift b/Sources/HanReaderUI/Screens/ReaderScreen.swift index 55dfe27..eec276a 100644 --- a/Sources/HanReaderUI/Screens/ReaderScreen.swift +++ b/Sources/HanReaderUI/Screens/ReaderScreen.swift @@ -112,9 +112,11 @@ struct ReaderScreen: View { // with one in flight loses it. On iOS the app may not be // running again afterwards to write it. guard phase != .active, let model else { return } + let pending = pendingPlayhead() + let id = textID Task { await model.flushPosition() - await savePlayhead(for: textID) + await write(pending, for: id) } } // The playhead is written while the reader is using the app, not @@ -124,7 +126,13 @@ struct ReaderScreen: View { // the prototype's bug wearing a different hat. .onChange(of: playback.isPlaying) { _, playing in guard !playing else { return } - Task { await savePlayhead(for: textID) } + // Read now, written later. A task that read the playhead when + // it ran could find the shared transport already loaded with + // another text's narration, and write that position under + // this text's id. + let pending = pendingPlayhead() + let id = textID + Task { await write(pending, for: id) } } .onChange(of: playback.observedTime) { _, time in // And periodically while playing, so quitting mid-sentence @@ -135,7 +143,8 @@ struct ReaderScreen: View { abs(time - lastSavedPlayhead) >= Self.playheadSaveInterval else { return } lastSavedPlayhead = time - Task { await savePlayhead(for: textID) } + let id = textID + Task { await write(time, for: id) } } .onChange(of: speech.isSpeaking) { _, speaking in // Resumed when the utterance ends rather than after a guessed @@ -320,7 +329,13 @@ extension ReaderScreen { /// reader left off. private func loadAudio(for model: ReaderModel) async { do { - guard let track = try await services.library.audioTrack(for: textID) else { + let found = try await services.library.audioTrack(for: textID) + // The transport is shared across texts, and this is the other + // side of an `await`: the reader may have chosen another text + // while the lookup was in flight, in which case writing to it + // here loads this text's narration under that one's name. + guard isStillCurrent(model) else { return } + guard let track = found else { hasAudio = false playback.unload() return @@ -374,31 +389,62 @@ extension ReaderScreen { } private func attachAudio(_ url: URL) async { + let target = textID do { let file = try await AudioAttachment.attach( url, - to: textID, + to: target, in: services.locations, library: services.library, ) + // A narration starts at its own beginning, so the playhead + // stored for whatever was attached before has to go — otherwise + // a reader who replaces a recording and leaves finds the new one + // resuming at the old one's position, possibly past its end. + try? await services.library.clearAudioTime(of: target) + guard isStillCurrent() else { return } + lastSavedPlayhead = 0 hasAudio = true playback.load(file) nowPlaying.attach(to: playback, title: title) } catch { Log.error("reader", "could not attach audio: \(error)") + guard isStillCurrent() else { return } hasAudio = true playback.fail(error.localizedDescription) } } + /// Whether this screen still owns the shared transport. + /// + /// `model` is `@State`, so reading it after an `await` gives whichever + /// text is open *now* — not the one this call started for. + private func isStillCurrent(_ expected: ReaderModel? = nil) -> Bool { + guard let current = model else { return false } + if let expected, current !== expected { + return false + } + return current.textID == textID + } + /// Speaks a word, ducking the narration first if the reader asked for /// that — which is the default, because the attached audio is usually /// the same narration and hearing both is confusing. private func speak(_ word: String) { - if settings.pausesAudioWhileSpeaking { - playback.pauseForSpeech() + guard settings.pausesAudioWhileSpeaking else { + speech.speak(word, suppressed: voiceOverEnabled) + return } + playback.pauseForSpeech() speech.speak(word, suppressed: voiceOverEnabled) + // `speak` declines in three ordinary situations — VoiceOver is on, + // the word is empty, no Chinese voice is installed — and says so by + // never starting. Without this the narration ducks for an utterance + // that never happens and stays paused until the reader presses play, + // which on a Mac with no Chinese voice is every single tap. + if !speech.isSpeaking { + playback.resumeAfterSpeech() + } } } diff --git a/Sources/HanReaderUI/Services/AudioAttachment.swift b/Sources/HanReaderUI/Services/AudioAttachment.swift index 61ec607..739588b 100644 --- a/Sources/HanReaderUI/Services/AudioAttachment.swift +++ b/Sources/HanReaderUI/Services/AudioAttachment.swift @@ -62,21 +62,43 @@ enum AudioAttachment { // and so the name carries no part of the reader's own filesystem. let name = "text-\(text.rawValue).\(url.pathExtension.lowercased())" let destination = locations.audio.appendingPathComponent(name) + let previous = try await library.audioTrack(for: text) - if FileManager.default.fileExists(atPath: destination.path) { - try FileManager.default.removeItem(at: destination) + // Copied beside the destination first, then moved into place. The + // obvious order — delete the old file, copy the new one — destroys a + // working narration the moment the copy fails, leaving a row + // pointing at a file that is no longer there. + let staging = locations.audio + .appendingPathComponent("incoming-\(UUID().uuidString).\(url.pathExtension)") + try FileManager.default.copyItem(at: url, to: staging) + do { + if FileManager.default.fileExists(atPath: destination.path) { + _ = try FileManager.default.replaceItemAt(destination, withItemAt: staging) + } else { + try FileManager.default.moveItem(at: staging, to: destination) + } + try await library.attachAudio(AudioTrack( + textID: text, + relativePath: name, + // Left for the player to discover. Reading it here would mean + // loading the asset twice, and a duration the transport has + // not confirmed is worse than none. + duration: nil, + importedAt: Date(), + )) + } catch { + try? FileManager.default.removeItem(at: staging) + throw error } - try FileManager.default.copyItem(at: url, to: destination) - try await library.attachAudio(AudioTrack( - textID: text, - relativePath: name, - // Left for the player to discover. Reading it here would mean - // loading the asset twice, and a duration the transport has not - // confirmed is worse than none. - duration: nil, - importedAt: Date(), - )) + // The row holds one path, so a narration replaced by a file with a + // different extension leaves the old one on disk with nothing + // pointing at it and no way to find it later. + if let previous, previous.relativePath != name { + try? FileManager.default.removeItem( + at: file(for: previous, in: locations), + ) + } return destination } diff --git a/Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift b/Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift index 421007e..19e7c05 100644 --- a/Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift +++ b/Tests/HanReaderPersistenceTests/PrototypeLibraryTests.swift @@ -231,6 +231,74 @@ struct PrototypeLibraryTests { #expect(FileManager.default.fileExists(atPath: copied.path)) } + /// Review finding: a migrated library should keep the order the reader + /// built it in, rather than having every text stamped with the moment of + /// migration. + @Test("Migrated texts keep their original import dates") + func importDatesAreCarried() async throws { + let space = try workspace() + let old = try makePrototype(at: space.prototype) + let when = 1_500_000_000.0 + try await old.write { db in + try db.execute( + sql: "INSERT INTO texts (id, title, content, imported_at) VALUES (?, ?, ?, ?)", + arguments: [1, "孔乙己", "我从十二岁起", when], + ) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + _ = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + + let item = try #require(try await library.items().first) + #expect(abs(item.importedAt.timeIntervalSince1970 - when) < 1) + } + + /// Review finding: a file the old container will not let us read used to + /// throw out of the whole migration — and because the rename that marks + /// it done never ran, it would fail again on every launch for ever. + @Test("An unreadable old audio file does not abort the migration") + func unreadableAudioDoesNotAbort() async throws { + let space = try workspace() + let narration = space.root.appendingPathComponent("old.m4a") + try Data("old recording".utf8).write(to: narration) + // The audio directory's path is occupied by an ordinary file, so + // nothing can be written into it. That stands in for the real + // failure — a sandbox or permission denial on the old container — + // which cannot be staged in a test. + try Data().write(to: space.locations.audio) + + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute(sql: """ + INSERT INTO texts (id, title, content, audio_path, imported_at) + VALUES (?, ?, ?, ?, ?) + """, arguments: [1, "孔乙己", "我从十二岁起", narration.path, 1.0]) + try db.execute( + sql: "INSERT INTO texts (id, title, content, imported_at) VALUES (?, ?, ?, ?)", + arguments: [2, "立论", "我梦见自己正在小学校的讲堂上", 2.0], + ) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let outcome = try await PrototypeLibrary.migrateIfPresent( + into: library, + locations: space.locations, + ) + let result = try #require(outcome) + + // Both texts arrive; only the audio is reported as lost. + #expect(result.texts == 2) + #expect(result.notes.count == 1) + // And the migration is marked done, so it is not retried for ever. + #expect(FileManager.default.fileExists( + atPath: PrototypeLibrary.migratedFile(near: space.locations.library).path, + )) + } + @Test("No prototype database means nothing to do") func absentPrototypeIsNotAnError() async throws { let space = try workspace() @@ -280,3 +348,93 @@ private enum AudioAttachmentPath { locations.audio.appendingPathComponent(track.relativePath) } } + +/// What a migration must *not* touch. +/// +/// A reader who has used both versions has a library that is partly theirs +/// already — the same text, their own narration for it, their own reading +/// history. Migration adds what is missing; it does not get to overwrite +/// what is there. +@Suite("Prototype migration leaves existing work alone") +struct PrototypeLibraryExistingWorkTests { + /// Review finding: a text already in the library may have a narration + /// the reader chose themselves. Replacing it with one from a previous + /// version of the app is not a migration, it is a loss. + @Test("Migration does not replace audio on a text that was already there") + func existingAudioIsNotReplaced() async throws { + let space = try workspace() + let narration = space.root.appendingPathComponent("old.m4a") + try Data("old recording".utf8).write(to: narration) + + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute(sql: """ + INSERT INTO texts (id, title, content, audio_path, imported_at) + VALUES (?, ?, ?, ?, ?) + """, arguments: [1, "孔乙己", "我从十二岁起", narration.path, 1.0]) + } + + // The same content is already in the library, with its own audio. + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let existing = try await library.importText(title: "孔乙己", content: "我从十二岁起") + guard case let .imported(id) = existing else { + Issue.record("expected a new text") + return + } + try await library.attachAudio(AudioTrack( + textID: id, + relativePath: "chosen-by-the-reader.m4a", + duration: nil, + importedAt: Date(), + )) + + let outcome = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + + #expect(outcome.alreadyPresent == 1) + #expect(outcome.audioTracks == 0) + let track = try #require(try await library.audioTrack(for: id)) + #expect(track.relativePath == "chosen-by-the-reader.m4a") + } + + /// Review finding: a text already in the library has its own, newer + /// history. Stamping it with a prototype timestamp moves it backwards in + /// the sidebar and can change which text opens on launch. + @Test("Migration does not move an existing text backwards in time") + func existingRecencyIsNotRewound() async throws { + let space = try workspace() + let old = try makePrototype(at: space.prototype) + try await old.write { db in + try db.execute( + sql: "INSERT INTO texts (id, title, content, imported_at) VALUES (?, ?, ?, ?)", + arguments: [1, "孔乙己", "我从十二岁起", 1.0], + ) + try db.execute(sql: """ + INSERT INTO reading_progress + (text_id, scroll_offset, revealed_words, last_opened_at) + VALUES (?, 0, '', ?) + """, arguments: [1, 100.0]) + } + + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let existing = try await library.importText(title: "孔乙己", content: "我从十二岁起") + guard case let .imported(id) = existing else { + Issue.record("expected a new text") + return + } + let recent = Date(timeIntervalSince1970: 1_800_000_000) + try await library.markOpened(id, at: recent) + + _ = try await PrototypeLibrary.migrate( + from: space.prototype, + into: library, + locations: space.locations, + ) + + let item = try #require(try await library.items().first { $0.id == id }) + #expect(item.lastOpenedAt == recent) + } +} diff --git a/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift b/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift index fa798cc..6e22283 100644 --- a/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift +++ b/Tests/HanReaderPlaybackTests/PlaybackModelTests.swift @@ -246,65 +246,6 @@ struct PlaybackModelTests { #expect(bench.engine.playCount == 1) } - // MARK: - Interruptions - - @Test("An interruption pauses, and its end resumes") - func interruptionResumes() { - let bench = fixture() - bench.model.play() - bench.session.interrupt(.began) - #expect(!bench.model.isPlaying) - bench.session.interrupt(.ended) - #expect(bench.model.isPlaying) - } - - /// Audio the reader paused themselves must stay paused: a phone call - /// ending is not permission to start something they stopped. - @Test("An interruption does not resume audio the reader paused") - func deliberatePauseIsNotResumed() { - let bench = fixture() - bench.model.play() - bench.model.pause() - bench.session.interrupt(.began) - bench.session.interrupt(.ended) - #expect(!bench.model.isPlaying) - } - - /// Unplugging headphones must not play the reader's audio out loud to - /// the room — and plugging them back in must not start it either. - @Test("Losing the output pauses, and does not resume") - func outputGoneDoesNotResume() { - let bench = fixture() - bench.model.play() - bench.session.interrupt(.outputGone) - #expect(!bench.model.isPlaying) - bench.session.interrupt(.ended) - #expect(!bench.model.isPlaying) - } - - // MARK: - Speech - - /// The attached audio is usually the same narration, so hearing the - /// spoken word over it is confusing rather than merely noisy. - @Test("Speaking pauses the narration and resumes it after") - func speechDucksPlayback() { - let bench = fixture() - bench.model.play() - bench.model.pauseForSpeech() - #expect(!bench.model.isPlaying) - bench.model.resumeAfterSpeech() - #expect(bench.model.isPlaying) - } - - @Test("Speaking over paused audio leaves it paused") - func speechDoesNotStartPausedAudio() { - let bench = fixture() - bench.model.pauseForSpeech() - bench.model.resumeAfterSpeech() - #expect(!bench.model.isPlaying) - #expect(bench.engine.playCount == 0) - } - // MARK: - Transport basics @Test("Loading parks the playhead where the reader left it") @@ -435,3 +376,142 @@ struct PlaybackRateTests { #expect(rates == rates.sorted()) } } + +/// Everything about who is allowed to resume narration, and when. +/// +/// Its own suite because the rules are not obvious and there are several: +/// the system may take the audio away and give it back, a spoken word may +/// duck it, and the reader may simply have pressed pause — and only one of +/// those three is allowed to undo each of the others. +@MainActor +@Suite("Playback interruptions") +struct PlaybackInterruptionTests { + @Test("An interruption pauses, and its end resumes") + func interruptionResumes() { + let bench = fixture() + bench.model.play() + bench.session.interrupt(.began) + #expect(!bench.model.isPlaying) + bench.session.interrupt(.ended) + #expect(bench.model.isPlaying) + } + + /// Audio the reader paused themselves must stay paused: a phone call + /// ending is not permission to start something they stopped. + @Test("An interruption does not resume audio the reader paused") + func deliberatePauseIsNotResumed() { + let bench = fixture() + bench.model.play() + bench.model.pause() + bench.session.interrupt(.began) + bench.session.interrupt(.ended) + #expect(!bench.model.isPlaying) + } + + /// Unplugging headphones must not play the reader's audio out loud to + /// the room — and plugging them back in must not start it either. + @Test("Losing the output pauses, and does not resume") + func outputGoneDoesNotResume() { + let bench = fixture() + bench.model.play() + bench.session.interrupt(.outputGone) + #expect(!bench.model.isPlaying) + bench.session.interrupt(.ended) + #expect(!bench.model.isPlaying) + } + + /// Review finding: the output can be lost while already paused by an + /// interruption, and `.outputGone` returned early without clearing the + /// interruption's claim — so the interruption ending resumed narration + /// into a room with no headphones in it. + @Test("Losing the output while interrupted cancels the resume") + func outputLostDuringInterruption() { + let bench = fixture() + bench.model.play() + bench.session.interrupt(.began) + #expect(!bench.model.isPlaying) + + bench.session.interrupt(.outputGone) + bench.session.interrupt(.ended) + #expect(!bench.model.isPlaying) + } + + /// Review finding: an interruption the system never invites us back from + /// leaves its claim standing. With one shared flag, the next word the + /// reader tapped ended by resuming narration the system had taken away. + @Test("A word spoken after an unresumed interruption does not start the narration") + func speechDoesNotResumeAnInterruption() { + let bench = fixture() + bench.model.play() + // `.began` with no matching `.ended` — a call that was answered. + bench.session.interrupt(.began) + #expect(!bench.model.isPlaying) + + // The reader taps a word. Nothing is playing, so there is nothing to + // duck, and the utterance ending must not start the narration. + bench.model.pauseForSpeech() + bench.model.resumeAfterSpeech() + #expect(!bench.model.isPlaying) + } + + /// And the converse: ducking for a word must not be undone by a system + /// interruption ending. + @Test("An interruption ending does not resume audio ducked for a word") + func interruptionDoesNotResumeSpeechDucking() { + let bench = fixture() + bench.model.play() + bench.model.pauseForSpeech() + bench.session.interrupt(.ended) + #expect(!bench.model.isPlaying) + // The word finishing is what resumes it. + bench.model.resumeAfterSpeech() + #expect(bench.model.isPlaying) + } + + /// Review finding: a narration replaced by a shorter file left the + /// parked slider past its own end, and told a lock screen an elapsed + /// time longer than the track. + @Test("A position past a newly-known duration is pulled back to it") + func parkedPositionIsClampedWhenDurationArrives() { + let engine = SpyEngine() + let model = PlaybackModel(engine: engine, session: SpySession()) + model.load(URL(fileURLWithPath: "/tmp/short.m4a"), startingAt: 300) + #expect(model.playhead == 300) + + engine.declareDuration(45) + #expect(model.playhead == 45) + #expect(model.observedTime == 45) + } + + @Test("A position inside the duration is left alone") + func validParkedPositionSurvives() { + let engine = SpyEngine() + let model = PlaybackModel(engine: engine, session: SpySession()) + model.load(URL(fileURLWithPath: "/tmp/long.m4a"), startingAt: 30) + engine.declareDuration(300) + #expect(model.playhead == 30) + } + + // MARK: - Speech + + /// The attached audio is usually the same narration, so hearing the + /// spoken word over it is confusing rather than merely noisy. + @Test("Speaking pauses the narration and resumes it after") + func speechDucksPlayback() { + let bench = fixture() + bench.model.play() + bench.model.pauseForSpeech() + #expect(!bench.model.isPlaying) + bench.model.resumeAfterSpeech() + #expect(bench.model.isPlaying) + } + + @Test("Speaking over paused audio leaves it paused") + func speechDoesNotStartPausedAudio() { + let bench = fixture() + bench.model.pauseForSpeech() + bench.model.resumeAfterSpeech() + #expect(!bench.model.isPlaying) + #expect(bench.engine.playCount == 0) + } +} diff --git a/Tests/HanReaderUITests/AudioAttachmentTests.swift b/Tests/HanReaderUITests/AudioAttachmentTests.swift index aeb4aac..7f7cc2d 100644 --- a/Tests/HanReaderUITests/AudioAttachmentTests.swift +++ b/Tests/HanReaderUITests/AudioAttachmentTests.swift @@ -142,6 +142,76 @@ struct AudioAttachmentTests { #expect(try await library.audioTrack(for: id) != nil) } + /// Review finding: deleting the old file before copying the new one + /// destroys a working narration the moment the copy fails, leaving a row + /// that points at a file which is no longer there. + @Test("A failed replacement leaves the existing narration playable") + func failedReplacementKeepsTheOldFile() async throws { + let space = try workspace() + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let id = try await imported(into: library) + + let first = try await AudioAttachment.attach( + source(space.root, named: "first.m4a"), + to: id, + in: space.locations, + library: library, + ) + let original = try Data(contentsOf: first) + + // A source that cannot be copied. + let missing = space.root.appendingPathComponent("gone.m4a") + await #expect(throws: (any Error).self) { + try await AudioAttachment.attach( + missing, + to: id, + in: space.locations, + library: library, + ) + } + + // The narration the reader had is still there, and still the one the + // database names. + let track = try #require(try await library.audioTrack(for: id)) + let file = AudioAttachment.file(for: track, in: space.locations) + #expect(FileManager.default.fileExists(atPath: file.path)) + #expect(try Data(contentsOf: file) == original) + // And nothing half-copied was left lying about. + let files = try FileManager.default.contentsOfDirectory( + atPath: space.locations.audio.path, + ) + #expect(files == [track.relativePath]) + } + + /// Review finding: the row holds one path, so replacing an `.m4a` with + /// an `.mp3` left the old file on disk with nothing pointing at it and + /// no way to find it again. + @Test("Replacing with a different format does not orphan the old file") + func replacingAcrossFormatsCleansUp() async throws { + let space = try workspace() + let library = try LibraryRepository.inMemory(audioDirectory: space.locations.audio) + let id = try await imported(into: library) + + _ = try await AudioAttachment.attach( + source(space.root, named: "first.m4a"), + to: id, + in: space.locations, + library: library, + ) + _ = try await AudioAttachment.attach( + source(space.root, named: "second.mp3"), + to: id, + in: space.locations, + library: library, + ) + + let files = try FileManager.default.contentsOfDirectory( + atPath: space.locations.audio.path, + ) + #expect(files.count == 1) + #expect(files.first?.hasSuffix(".mp3") == true) + } + private func imported( into library: LibraryRepository, content: String = "我在一个山村学校当老师", From 4410e011b546660775f6a96f0a55d8c99a67d5f8 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Mon, 19 Oct 2026 20:04:11 +0300 Subject: [PATCH 3/3] fix: build on iOS, and actually apply the session fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things CI caught that this machine could not. `SystemPlaybackSession.deinit` reached main-actor state from a nonisolated `deinit`. The same problem as `AVPlayerAudioEngine`'s, and the same fix — a teardown closure — but the property is inside `#if os(iOS)`, so the macOS build compiled happily and only the iOS one failed. Both app targets are now built locally before pushing, which is what should have happened. The reference-counted audio session was never applied. The edit that was supposed to add it silently matched nothing and reported success, and because it changed no code the build and the tests agreed. It is here now: `AVAudioSession` is one per process, so the last transport out is the one that puts it down, and an `isActive` flag keeps the count balanced against a repeated activate or a deactivate without one. `Migration`'s synthesised memberwise initializer was private, because a `private` stored property drags it down. Xcode 26.6, which CI pins, enforces that; the newer toolchain here does not, so it built locally and failed there. Spelled out explicitly, which is correct on both. Co-Authored-By: Claude Opus 5 --- .../PrototypeLibrary.swift | 9 ++++ .../HanReaderPlayback/PlaybackSession.swift | 42 +++++++++++++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/Sources/HanReaderPersistence/PrototypeLibrary.swift b/Sources/HanReaderPersistence/PrototypeLibrary.swift index c7f6e1d..44aec96 100644 --- a/Sources/HanReaderPersistence/PrototypeLibrary.swift +++ b/Sources/HanReaderPersistence/PrototypeLibrary.swift @@ -157,6 +157,15 @@ private struct Migration { /// on the former. private var newcomers: [Int64: Bool] = [:] + /// Spelled out rather than synthesised. A `private` stored property + /// drags the memberwise initializer down to `private` too, which the + /// toolchain CI pins enforces and the newer one on this machine does + /// not — so the synthesised version built locally and failed there. + init(repository: LibraryRepository, locations: AppDatabase.Locations) { + self.repository = repository + self.locations = locations + } + /// - Returns: old row id to new `TextID`, so the progress rows can find /// the text they belong to. mutating func importTexts( diff --git a/Sources/HanReaderPlayback/PlaybackSession.swift b/Sources/HanReaderPlayback/PlaybackSession.swift index 42f1aab..9b4ff33 100644 --- a/Sources/HanReaderPlayback/PlaybackSession.swift +++ b/Sources/HanReaderPlayback/PlaybackSession.swift @@ -38,19 +38,38 @@ public final class SystemPlaybackSession: PlaybackSession { #if os(iOS) private var observers: [any NSObjectProtocol] = [] + + /// How many transports are using the shared session. + /// + /// `AVAudioSession` is one per process, so the obvious implementation + /// — deactivate when I am done — stops the *other* window's narration + /// on an iPad with two texts open. Counted instead, and released only + /// by the last one out. + private static var users = 0 + + /// Unsubscription, captured so `deinit` can run it. + /// + /// `deinit` is nonisolated and this class is not, so it cannot reach + /// the array above — the same bargain as `AVPlayerAudioEngine`'s. It + /// is needed only on iOS, which is why the macOS build compiled + /// happily and CI was the first to say otherwise. + private nonisolated(unsafe) var teardown: (() -> Void)? #endif + /// Whether this session holds an activation of its own, so a repeated + /// activate or a deactivate without one cannot unbalance the count. + private var isActive = false + public init() {} deinit { #if os(iOS) - for observer in observers { - NotificationCenter.default.removeObserver(observer) - } + teardown?() #endif } public func activate() throws { + guard !isActive else { return } #if os(iOS) let session = AVAudioSession.sharedInstance() // `.spokenAudio` rather than plain `.playback`: it tells the system @@ -58,12 +77,22 @@ public final class SystemPlaybackSession: PlaybackSession { // over it, and the "pause spoken audio" accessory controls apply. try session.setCategory(.playback, mode: .spokenAudio) try session.setActive(true) + Self.users += 1 subscribe() #endif + isActive = true } public func deactivate() { + guard isActive else { return } + isActive = false #if os(iOS) + Self.users -= 1 + // Only the last transport out puts the session down. Deactivating + // it while another window is still playing through it stops that + // window's narration, which is this fix in reverse. + guard Self.users <= 0 else { return } + Self.users = 0 // Notifying others is what lets the music the reader interrupted // start itself again. Failing here is not worth surfacing: the // session is already idle and the reader has nothing to do about it. @@ -114,6 +143,13 @@ public final class SystemPlaybackSession: PlaybackSession { else { return } self?.send(.outputGone) }) + + let registered = observers + teardown = { + for observer in registered { + NotificationCenter.default.removeObserver(observer) + } + } } private func send(_ interruption: PlaybackInterruption) {