From 3b96ebc17a2a47eaf6db2c88122b98902f2048a0 Mon Sep 17 00:00:00 2001 From: williamliu0516 <122490794+williamliu0516@users.noreply.github.com> Date: Fri, 16 Oct 2026 05:20:23 -0700 Subject: [PATCH] fix(software): an empty read-ahead store is the end of the stream only when the source ended A seek that misses the packet cache empties the store while holding `operations`. A consumer read that began inside the seek's window left its wait at once (its reader was not current yet), then waited for `operations`. When it got the lock the seek had landed and the reader was current again, so the admission check passed, the store was empty, and `read` returned nil. The demux loop takes nil for end of stream: playback parked at the landing as "end of media". DVD-Video hits it most, as its IFO time-map seek settles at once. On a MacBook Pro (M1 Pro), aetherctl playing a test disc with six seeks into other cells, each missing the cache, ended at the landing of its first such seek in 7 of 8 runs (7 of 13 cache-miss seeks). `read` now reports the end only when the producer has seen the source's own end; an empty store before that throws `.interrupted`, which the demux loop already treats as "read again" (it then waits for the refill). With the fix, 0 of 8 runs ended: all 48 cache-miss seeks landed and played on. --- CHANGELOG.md | 11 +++- .../Native/SoftwarePacketReadAhead.swift | 8 +++ ...twarePacketReadAheadEndOfStreamTests.swift | 65 +++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 Tests/AetherEngineTests/SoftwarePacketReadAheadEndOfStreamTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 983309090..781299e91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,16 @@ the public-API contract. ## [Unreleased] -_Nothing yet._ +### Fixed + +- **A seek that misses the software path's packet cache no longer ends playback at its landing.** + The miss empties the read-ahead store while holding the store's operation lock, and a demux-loop + read that began inside the seek's window could leave its wait at once (its reader was not current + yet), get the lock only after the seek had landed, find the store empty and return nil, which the + loop takes for end of stream: "end of media" at the landing. An empty store is now the end only + when the producer has seen the source's own end; before that the read throws `.interrupted` and the + loop reads again. Seen on DVD-Video, whose time-map seek settles at once: a seek past the first + cell ended playback in 7 of 8 runs, in none of 8 with the fix (48 seeks that missed the cache). ## [7.38.1] - 2026-10-16 diff --git a/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift b/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift index 4c17996bb..029dc2529 100644 --- a/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift +++ b/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift @@ -420,8 +420,16 @@ final class SoftwarePacketReadAhead: @unchecked Sendable { } if count == 0 { let error = failure + // Only the source's own end ends the stream. The wait above also stops for a reader + // that is not current, and a seek can make it current again while this read waits for + // `operations`, which a cache miss holds to empty the store: that store is about to + // refill from the new position, and reading it as the end stopped playback at the + // landing (a DVD seek past the first cell, whose time-map seek settles at once, hit it + // in 7 of 8 runs). + let sourceEnded = ended condition.unlock() if let error { throw error } + guard sourceEnded else { throw ReadError.interrupted } return nil } condition.unlock() diff --git a/Tests/AetherEngineTests/SoftwarePacketReadAheadEndOfStreamTests.swift b/Tests/AetherEngineTests/SoftwarePacketReadAheadEndOfStreamTests.swift new file mode 100644 index 000000000..ff47d946e --- /dev/null +++ b/Tests/AetherEngineTests/SoftwarePacketReadAheadEndOfStreamTests.swift @@ -0,0 +1,65 @@ +import Foundation +import Testing +@testable import AetherEngine + +/// An empty read-ahead store is the end of the stream only when the source said so. +/// +/// A seek that misses the cache empties the store while holding `operations`. A consumer read that +/// started inside the seek's window left its wait at once (its reader was not current), then waited +/// for `operations`; by the time it got the lock the seek had landed, its reader was current again, +/// and it found the store empty and returned nil, which the host takes for the end of the media. +/// Playback stopped at the landing (a DVD seek past the first cell: 7 of 8 runs). +@Suite("Software read-ahead end of stream", .offCooperativePool, .timeLimit(.minutes(1))) +struct SoftwarePacketReadAheadEndOfStreamTests { + + /// A reader that is not current on its first check and current on every later one: the + /// seek's window closing between the read's wait and its admission check. + private final class CurrentAfterFirstCheck: @unchecked Sendable { + private let lock = NSLock() + private var checks = 0 + func check() -> Bool { lock.lock(); defer { lock.unlock() }; checks += 1; return checks > 1 } + } + + private static func readAhead(_ root: URL, + source: @escaping @Sendable (@Sendable () -> Bool) throws -> SoftwareStoredPacket? + ) throws -> SoftwarePacketReadAhead { + let fifo = try SoftwarePacketDiskFIFO(chunkTargetBytes: 64 * 1024, retainConsumed: true, + parentDirectory: root) + return SoftwarePacketReadAhead( + video: .init(index: 0, numerator: 1, denominator: 1000), audio: nil, + byteBudget: 1 << 20, forwardSeconds: 10, initialSourceClock: 0, fifo: fifo, + readSource: source) + } + + private static func temporaryRoot() throws -> URL { + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("readahead-eos-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + return root + } + + @Test("an empty store whose reader becomes current again is not the end of the stream") + func emptyStoreBeforeTheSourceEndsIsInterrupted() throws { + let root = try Self.temporaryRoot() + defer { try? FileManager.default.removeItem(at: root) } + // Not started: nothing is read, the store stays empty, the source has not ended. + let source = try Self.readAhead(root) { _ in nil } + defer { source.close() } + let reader = CurrentAfterFirstCheck() + #expect(throws: SoftwarePacketReadAhead.ReadError.interrupted) { + _ = try source.read(isCurrent: { reader.check() }) + } + } + + @Test("the source's own end is the end of the stream") + func sourceEndIsEndOfStream() async throws { + let root = try Self.temporaryRoot() + defer { try? FileManager.default.removeItem(at: root) } + let source = try Self.readAhead(root) { _ in nil } + defer { source.close() } + source.start() + // The producer has to have seen the end before there is anything to measure. + try await waitFor { source.snapshot.sourceEnded } + #expect(try source.read() == nil) + } +}