diff --git a/CHANGELOG.md b/CHANGELOG.md index 84f69ff1..023b6eda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ the public-API contract. ## [Unreleased] +### Fixed + +- Duration-less audio cache coverage rejects an overflowing successor timestamp difference instead of trapping. Positive-duration packets, the existing gap bound and source-epoch handling are unchanged. +- A fully prewarmed source no longer issues a redundant suffix request when its adopted head already covers the entire file. Partial warms retain their existing tail prefetch. + ### Changed - **The documentation no longer says TrueHD Atmos objects are always lost.** The Known limitations diff --git a/Sources/AetherEngine/Demuxer/AVIOReader.swift b/Sources/AetherEngine/Demuxer/AVIOReader.swift index c54dcdf9..065f8334 100644 --- a/Sources/AetherEngine/Demuxer/AVIOReader.swift +++ b/Sources/AetherEngine/Demuxer/AVIOReader.swift @@ -1297,7 +1297,10 @@ final class AVIOReader: AVIOProvider, @unchecked Sendable { // rather than queueing behind it. It asks for a suffix range, which needs no size and // therefore needs nothing this open has learned yet. A warm that already carries the // trailing object has no use for it. - if warm?.tail == nil { + // A fully resident head already contains the suffix too. Do not start an asynchronous + // request for bytes this open has adopted in their entirety. + let fullyWarmed = warm.map { $0.head.end >= $0.contentLength } ?? false + if warm?.tail == nil, !fullyWarmed { startTailPrefetch() } // Playback path. The persistent connection's `Range: bytes=0-` request is itself diff --git a/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift b/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift index 4c17996b..eb24a907 100644 --- a/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift +++ b/Sources/AetherEngine/Native/SoftwarePacketReadAhead.swift @@ -635,10 +635,12 @@ final class SoftwarePacketReadAhead: @unchecked Sendable { audioCoverage.insert(pts: pts, duration: duration) return } - guard let previous, pts > previous, stream.numerator > 0, stream.denominator > 0, - Double(pts - previous) * Double(stream.numerator) / Double(stream.denominator) + guard let previous, pts > previous, stream.numerator > 0, stream.denominator > 0 else { return } + let (delta, overflow) = pts.subtractingReportingOverflow(previous) + guard !overflow, + Double(delta) * Double(stream.numerator) / Double(stream.denominator) <= Self.maxAudioSuccessorGapSeconds else { return } - audioCoverage.insert(pts: previous, duration: pts - previous) + audioCoverage.insert(pts: previous, duration: delta) } private func shouldParkLocked() -> Bool { diff --git a/Tests/AetherEngineTests/SoftwarePacketReadAheadAudioCoverageTests.swift b/Tests/AetherEngineTests/SoftwarePacketReadAheadAudioCoverageTests.swift index 0fb0070c..41f9e214 100644 --- a/Tests/AetherEngineTests/SoftwarePacketReadAheadAudioCoverageTests.swift +++ b/Tests/AetherEngineTests/SoftwarePacketReadAheadAudioCoverageTests.swift @@ -87,3 +87,48 @@ struct SoftwarePacketReadAheadAudioCoverageTests { #expect(try seekHits(source, to: 2.0) == false) } } +extension SoftwarePacketReadAheadAudioCoverageTests { + private final class Packets: @unchecked Sendable { + private let lock = NSLock() + private var index = 0 + let values: [SoftwareStoredPacket] + init(_ values: [SoftwareStoredPacket]) { self.values = values } + func next() -> SoftwareStoredPacket? { + lock.lock(); defer { lock.unlock() } + guard index < values.count else { return nil } + defer { index += 1 } + return values[index] + } + } + + @Test("A huge increasing audio PTS cannot trap in the read-ahead worker", .timeLimit(.minutes(1))) + func overflowingSuccessor() async throws { + func packet(_ pts: Int64, stream: Int32, duration: Int64) -> SoftwareStoredPacket { + SoftwareStoredPacket(pts: pts, dts: pts, duration: duration, position: 0, + streamIndex: stream, flags: 1, timeBaseNumerator: 1, timeBaseDenominator: 1000, + bytes: Data(repeating: 1, count: 16), sideData: []) + } + let root = FileManager.default.temporaryDirectory.appendingPathComponent("audio-overflow-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let packets = Packets([ + packet(0, stream: 0, duration: 40), + packet(Int64.min + 1, stream: 1, duration: 0), + packet(Int64.max - 1, stream: 1, duration: 0), + packet(40, stream: 0, duration: 40), + ]) + let fifo = try SoftwarePacketDiskFIFO(chunkTargetBytes: 64 * 1024, retainConsumed: true, + parentDirectory: root) + let reader = SoftwarePacketReadAhead( + video: .init(index: 0, numerator: 1, denominator: 1000), + audio: .init(index: 1, numerator: 1, denominator: 1000), + byteBudget: 1 << 20, forwardSeconds: 100, initialSourceClock: 0, fifo: fifo + ) { _ in packets.next() } + defer { reader.close() } + reader.start() + try await waitFor { reader.snapshot.sourceEnded } + let token = reader.beginSeek(to: 0.02) + #expect(try reader.prepareSeek(token, to: 0.02) == false, + "Overflow must not invent audio coverage over the video interval") + } +} diff --git a/Tests/AetherEngineTests/SourcePrewarmAdoptionTests.swift b/Tests/AetherEngineTests/SourcePrewarmAdoptionTests.swift index 56be5787..170fb6d1 100644 --- a/Tests/AetherEngineTests/SourcePrewarmAdoptionTests.swift +++ b/Tests/AetherEngineTests/SourcePrewarmAdoptionTests.swift @@ -124,10 +124,10 @@ extension PrewarmStoreSuites { /// A source that fits entirely inside the budget leaves nothing for a data connection to /// fetch, so none is opened. The guard matters because a connection asked for the range past /// the last byte is a 416, and the open would then look like a refusal. - @Test("a fully warmed source opens without any connection at all") - func fullyWarmedSourceOpensWithNoConnection() async throws { + @Test("a fully warmed source opens without any connection at all", arguments: [40 * 1024, 128 * 1024]) + func fullyWarmedSourceOpensWithNoConnection(size: Int) async throws { SourcePrewarmStore.shared.clear() - let small: Int64 = 40 * 1024 + let small = Int64(size) let server = try #require(ThrottledOriginServer(totalSize: small)) defer { server.stop() } try await warm(server, bytes: 1 << 20) @@ -139,10 +139,39 @@ extension PrewarmStoreSuites { #expect(read(reader, Int(small)) == Int32(small)) #expect(reader.resolvedByteSize == small) + // A tail request is asynchronous: counting immediately after open/read can pass before + // the redundant bytes=-65536 request reaches the origin. This bound asserts absence; + // it is not a deadline for a prerequisite event. + let extraRequest = try await waitFor(upTo: .milliseconds(500)) { + server.rangeRequestCount > warmRequests + } + #expect(!extraRequest, + "a fully adopted source issued another request: \(server.requestHeaders)") #expect(server.rangeRequestCount == warmRequests, "the open connected for a source it already held whole: \(server.requestedRanges)") } + @Test("a partial warm still prefetches the missing suffix") + func partialWarmStillFetchesSuffix() async throws { + SourcePrewarmStore.shared.clear() + let server = try #require(ThrottledOriginServer(totalSize: fileSize)) + defer { server.stop() } + try await warm(server, bytes: warmBytes) + let warmRequests = server.rangeRequestCount + #expect(SourcePrewarmStore.shared.isWarm(for: url(server))) + + let reader = AVIOReader(url: url(server)) + defer { reader.markClosed(); reader.close() } + try reader.open() + try await waitFor { + server.requestHeaders.dropFirst(warmRequests).contains { + $0["range"] == "bytes=-65536" + } + } + #expect(read(reader, 32 * 1024) == 32 * 1024) + #expect(reader.resolvedByteSize == fileSize) + } + /// A source nobody warmed keeps the cold path exactly as it was, byte for byte. @Test("an unwarmed source still opens from byte zero") func coldOpenIsUnchanged() async throws { diff --git a/docs/api.md b/docs/api.md index 16362779..9e857ff3 100644 --- a/docs/api.md +++ b/docs/api.md @@ -663,6 +663,8 @@ Three limits are part of the contract rather than implementation detail: The URL is the key, matched exactly, **and so are the headers**. A signed URL warmed under one signature is not adopted under another, and a warm fetched with different `httpHeaders` than the load carries is not adopted either: an origin that varies on Referer, User-Agent or Authorization answers a different body, and a different size, under one URL, and nothing about the bytes themselves would show it. Pass the load's headers to the warm. +When an adopted head already covers the entire source, opening it issues neither a data connection nor a speculative suffix request. A partial warm still fetches missing data and, when needed, the tail. + Warms are serialised, one at a time across the process. A second `prewarm` while one is in flight waits its turn rather than being refused, which costs the origin nothing because a queued warm holds no request slot and has issued nothing. What never queues is a request against the origin. `IOReader` is the custom-source protocol: `read`, `seek`, `close` are required; `cancel()`, `makeIndependentReader()` and `discImageProbeEnabled` have defaults that unlock teardown-unblocking, embedded subtitles plus scrub stills, and ISO/UDF probing respectively. Calls arrive on the engine's demux thread, each inside an autorelease pool the engine opens, so a reader built on `FileHandle` or `NSData` does not strand one autoreleased object per read for the length of a session. Full contract in [formats.md](formats.md).