From bb3bf4818c5e9d208c130d1424aed0a01d34a585 Mon Sep 17 00:00:00 2001 From: zenjabba <679864+zenjabba@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:19:17 +0000 Subject: [PATCH 1/3] test(decoder): account for thread buffering in progress regression --- CHANGELOG.md | 1 + CONTRIBUTING.md | 6 +++ .../Issue220SoftwareDecoderDrainTests.swift | 46 ++++++++++++------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5ade1829..c05df98f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ the public-API contract. ### Fixed +- The software-decoder progress regression now warms the frame-thread pipeline before measuring sustained output, avoiding a false failure on hosts with more decoder threads than its former fixed allowance. Production decoding and thread selection are unchanged. - Authorized native HLS uses the engine relay from the initial load, without forwarding origin credentials to the loopback asset. Optional subtitle playlist preparation shares the authorizer and has a bounded deadline across redirects and refreshes. - Static-header HLS redirects apply the shared credential policy, including Emby and MediaBrowser token headers, before contacting another origin. - Session option corrections recognize `httpRequestAuthorization` and external subtitle provider replacements by identity. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b341ab27a..da0752d60 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,6 +47,12 @@ already covered. That is how seek ended up spread over sixteen files, live over two pairs of files ended up testing the same concept under two issue numbers. Existing files are not worth renaming on their own; put a new test where the topic already lives. +**Decoder progress tests must account for frame-thread buffering.** A fixed allowance for pending +frames can fail on hosts with more decoder threads. `Issue220SoftwareDecoderDrainTests` warms the +default threaded decoder with enough fixture input for the host processor count, then checks that +another complete pass delivers one frame per new packet. Replayed passes keep continuous timestamps +and do not flush the decoder. This preserves threaded progress coverage across different host sizes. + **Wait with `waitFor` from `Support/TestWaiting.swift`, never with a sleep or a private copy.** It carries two rules that cost three rounds of red CI to learn. A step that HAS to happen before the test can measure anything gets no deadline of its own, because any finite bound can be overrun by diff --git a/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift b/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift index cccd6c2f7..736cf812e 100644 --- a/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift +++ b/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift @@ -13,7 +13,7 @@ import AetherLibavutil /// /// The wedge state itself is only reachable through decoder-internal threading, so it is the /// disposition rule that is pinned here, plus a real decode over a fixture proving the split -/// into send + `drainDecodedFrames` still delivers every frame. +/// into send + `drainDecodedFrames` sustains one output frame per input packet once warm. struct Issue220SoftwareDecoderDrainTests { // MARK: - Send disposition @@ -38,9 +38,10 @@ struct Issue220SoftwareDecoderDrainTests { // MARK: - Real decode - /// Regression guard for the send/drain split: 40 IDR+P packets, no B-frames, so the decoder - /// owes a frame per packet minus whatever its own thread pipeline still holds at the end. - @Test("every packet of a progressive fixture still reaches the frame handler") + /// Regression guard for the send/drain split: IDR+P packets, no B-frames. Frame threading + /// delays output by the thread pipeline depth, which scales with the host processor count. + /// Warm that pipeline before measuring another full pass, without flushing the decoder. + @Test("a warmed progressive decoder keeps producing one frame per packet (#220)") func decodesFixtureFrames() throws { let data = try #require(Data(base64Encoded: Self.fixtureBase64, options: .ignoreUnknownCharacters)) @@ -55,21 +56,32 @@ struct Issue220SoftwareDecoderDrainTests { try decoder.open(stream: stream) { _, _, _ in counter.increment() } defer { decoder.close() } - var packets = 0 - while let pkt = try? demuxer.readPacket() { - if pkt.pointee.stream_index == videoIndex { - packets += 1 - decoder.decode(packet: pkt) + let packetsPerPass = 40 + let warmupPasses = (ProcessInfo.processInfo.activeProcessorCount + packetsPerPass - 1) + / packetsPerPass + let passDuration = av_rescale_q(4, AVRational(num: 1, den: 1), stream.pointee.time_base) + for pass in 0...warmupPasses { + if pass > 0 { try #require(demuxer.seek(to: 0)) } + let framesBeforePass = counter.value + var packets = 0 + while let pkt = try demuxer.readPacket() { + var ownedPacket: UnsafeMutablePointer? = pkt + defer { trackedPacketFree(&ownedPacket) } + if pkt.pointee.stream_index == videoIndex { + packets += 1 + // Each pass begins at an IDR. Keep its timestamps continuous across replays + // while leaving the decoder's in-flight frames intact. + pkt.pointee.pts += Int64(pass) * passDuration + pkt.pointee.dts += Int64(pass) * passDuration + decoder.decode(packet: pkt) + } + } + try #require(packets == packetsPerPass) + if pass == warmupPasses { + #expect(counter.value - framesBeforePass == packetsPerPass, + "a warmed decoder must keep up with every new packet") } - var p: UnsafeMutablePointer? = pkt - trackedPacketFree(&p) } - - #expect(packets == 40) - // Frame threading holds a bounded number of frames back until flush; the guard is that - // the drain runs at all and keeps up, not the exact pipeline depth. - #expect(counter.value > 0) - #expect(counter.value >= packets - 16) } private final class FrameCounter: @unchecked Sendable { From 8aa270f21edc3e3a6f529b0fb5fe1751f2592529 Mon Sep 17 00:00:00 2001 From: Quick104 <31828688+Quick104@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:11:13 -0400 Subject: [PATCH 2/3] fix(decoder): cap software playback at 16 frame threads The software decoder opened one frame thread per core. Each frame thread holds back one decoded frame, so a 32-core Mac waited for 31 frames before the first one after a load or seek, and FFmpeg warns above 16 threads. `SoftwareVideoDecoder.playbackThreadCount(activeProcessorCount:)` now caps the count at 16, FFmpeg's own auto-thread ceiling. The #220 drain test goes back to one pass over the fixture and bounds the missing frames by the opened decoder's real `thread_count - 1`, so it again catches extra startup lag without warmup passes or rewritten timestamps. Its doc comment now says the EAGAIN retry is covered only by the disposition checks. The CONTRIBUTING paragraph is dropped and the CHANGELOG entry describes the production fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + .../Decoder/SoftwareVideoDecoder.swift | 16 ++++++- .../FrameDecodeThreadBudgetTests.swift | 10 ++++ .../Issue220SoftwareDecoderDrainTests.swift | 47 +++++++------------ 4 files changed, 44 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15a8cc03b..b6056f7d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ the public-API contract. ### Fixed +- The software video decoder no longer runs more than 16 frame threads. It used one per core, and each frame thread holds back one decoded frame, so a 32-core Mac waited for 31 frames before showing the first one after a load or seek (about 3 s at 10 fps). FFmpeg also warns above 16 threads. Hosts with 16 or fewer cores keep their current thread count. - A paused video no longer starts playing by itself. When the player item died while paused (`failedToPlayToEndTime`), the recovery reload bypassed the pause guard and called `play()` on the fresh item. The reload now keeps a pause made before the item died, whether it came through the engine, AVKit, Control Center or PiP, and mounts the item paused at the same position. - A dead item's recovery no longer restarts the title from where the session was first opened. When AVPlayer refused the recovery item's master (`-11868`), the media fallback reloaded at the first mount's start position, so a title opened from its beginning restarted at 0:00. The fallback now reloads where the refused item was placed. Upstream #621. - The media fallback no longer starts a paused title. When the recovery item was refused, the fallback called `play()` unconditionally, so a title paused behind the tvOS screensaver started itself. It now plays only when the refused item was playing, or was told to play, and the viewer had not paused it. A Play or Pause from AVKit, Control Center or PiP counts as well as one through the engine. diff --git a/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift b/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift index 37c421432..5c86b9543 100644 --- a/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift +++ b/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift @@ -82,6 +82,18 @@ final class SoftwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { /// still extractor is the only caller, everything on a playback path wants the parallel default. var decodesSingleThreaded = false + /// The `thread_count` libavcodec opened with. Written once in `open`, like the other open-time + /// fields. Frame threading holds back `threadCount - 1` decoded frames until flush. + private(set) var threadCount = 0 + + /// Frame threads for a playback decode. Each frame thread delays output by one frame, so one + /// per core on a 32-core Mac held 31 frames back after every load and seek. 16 is FFmpeg's own + /// auto-thread ceiling (`MAX_AUTO_THREADS`); above it libavcodec warns the count is not + /// recommended. Apple TV, iPhone and iPad have fewer cores and are unaffected. + static func playbackThreadCount(activeProcessorCount: Int) -> Int { + return max(1, min(16, activeProcessorCount)) + } + /// AE#499: what the container declared about colour, captured at `open` before a single frame /// exists. A decoded frame carries the VUI alone, and a remux whose VUI is empty would otherwise /// reach `attachColorSpace` as an untagged picture, so an HDR10 file decoded in software lost its @@ -163,7 +175,8 @@ final class SoftwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { ctx.pointee.thread_count = 1 ctx.pointee.thread_type = 0 } else { - ctx.pointee.thread_count = Int32(ProcessInfo.processInfo.activeProcessorCount) + ctx.pointee.thread_count = Int32(Self.playbackThreadCount( + activeProcessorCount: ProcessInfo.processInfo.activeProcessorCount)) ctx.pointee.thread_type = FF_THREAD_FRAME | FF_THREAD_SLICE } @@ -176,6 +189,7 @@ final class SoftwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { throw VideoDecoderError.sessionCreationFailed(status: -2) } av_dict_free(&opts) + threadCount = Int(ctx.pointee.thread_count) containerColor = ColorDescription(codecpar: codecpar) let bitsPerSample = codecpar.pointee.bits_per_raw_sample diff --git a/Tests/AetherEngineTests/FrameDecodeThreadBudgetTests.swift b/Tests/AetherEngineTests/FrameDecodeThreadBudgetTests.swift index 35396eac3..422e06b93 100644 --- a/Tests/AetherEngineTests/FrameDecodeThreadBudgetTests.swift +++ b/Tests/AetherEngineTests/FrameDecodeThreadBudgetTests.swift @@ -20,4 +20,14 @@ struct FrameDecodeThreadBudgetTests { #expect(FrameDecodeContext.stillExtractionThreadCount(activeProcessorCount: 1) >= 1) #expect(FrameDecodeContext.stillExtractionThreadCount(activeProcessorCount: 0) >= 1) } + + /// Each frame thread delays software playback output by one frame, so one thread per core + /// held 31 frames back after every load and seek on a 32-core Mac. + @Test("software playback thread count stops at FFmpeg's 16-thread ceiling") + func playbackCapsAtSixteen() { + #expect(SoftwareVideoDecoder.playbackThreadCount(activeProcessorCount: 32) == 16) + #expect(SoftwareVideoDecoder.playbackThreadCount(activeProcessorCount: 16) == 16) + #expect(SoftwareVideoDecoder.playbackThreadCount(activeProcessorCount: 6) == 6) + #expect(SoftwareVideoDecoder.playbackThreadCount(activeProcessorCount: 0) == 1) + } } diff --git a/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift b/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift index 736cf812e..8437efb03 100644 --- a/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift +++ b/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift @@ -13,7 +13,7 @@ import AetherLibavutil /// /// The wedge state itself is only reachable through decoder-internal threading, so it is the /// disposition rule that is pinned here, plus a real decode over a fixture proving the split -/// into send + `drainDecodedFrames` sustains one output frame per input packet once warm. +/// into send + `drainDecodedFrames` still delivers every frame. struct Issue220SoftwareDecoderDrainTests { // MARK: - Send disposition @@ -38,10 +38,12 @@ struct Issue220SoftwareDecoderDrainTests { // MARK: - Real decode - /// Regression guard for the send/drain split: IDR+P packets, no B-frames. Frame threading - /// delays output by the thread pipeline depth, which scales with the host processor count. - /// Warm that pipeline before measuring another full pass, without flushing the decoder. - @Test("a warmed progressive decoder keeps producing one frame per packet (#220)") + /// Regression guard for the send/drain split: 40 IDR+P packets, no B-frames, so the decoder + /// owes a frame per packet minus the `threadCount - 1` that frame threading holds until flush. + /// A drain that stops early or falls further behind leaves the count short. This synchronous + /// feed never makes `avcodec_send_packet` return EAGAIN, so the retry itself is pinned only by + /// the disposition checks above. + @Test("a progressive fixture reaches the frame handler, short only the thread pipeline") func decodesFixtureFrames() throws { let data = try #require(Data(base64Encoded: Self.fixtureBase64, options: .ignoreUnknownCharacters)) @@ -56,32 +58,19 @@ struct Issue220SoftwareDecoderDrainTests { try decoder.open(stream: stream) { _, _, _ in counter.increment() } defer { decoder.close() } - let packetsPerPass = 40 - let warmupPasses = (ProcessInfo.processInfo.activeProcessorCount + packetsPerPass - 1) - / packetsPerPass - let passDuration = av_rescale_q(4, AVRational(num: 1, den: 1), stream.pointee.time_base) - for pass in 0...warmupPasses { - if pass > 0 { try #require(demuxer.seek(to: 0)) } - let framesBeforePass = counter.value - var packets = 0 - while let pkt = try demuxer.readPacket() { - var ownedPacket: UnsafeMutablePointer? = pkt - defer { trackedPacketFree(&ownedPacket) } - if pkt.pointee.stream_index == videoIndex { - packets += 1 - // Each pass begins at an IDR. Keep its timestamps continuous across replays - // while leaving the decoder's in-flight frames intact. - pkt.pointee.pts += Int64(pass) * passDuration - pkt.pointee.dts += Int64(pass) * passDuration - decoder.decode(packet: pkt) - } - } - try #require(packets == packetsPerPass) - if pass == warmupPasses { - #expect(counter.value - framesBeforePass == packetsPerPass, - "a warmed decoder must keep up with every new packet") + var packets = 0 + while let pkt = try demuxer.readPacket() { + var ownedPacket: UnsafeMutablePointer? = pkt + defer { trackedPacketFree(&ownedPacket) } + if pkt.pointee.stream_index == videoIndex { + packets += 1 + decoder.decode(packet: pkt) } } + + try #require(packets == 40) + #expect(counter.value >= packets - (decoder.threadCount - 1), + "the drain must keep up with every packet the thread pipeline has released") } private final class FrameCounter: @unchecked Sendable { From a290e94a2abf2364120777d323f9454e9d04c076 Mon Sep 17 00:00:00 2001 From: Quick104 <31828688+Quick104@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:25:20 -0400 Subject: [PATCH 3/3] test(decoder): check the 16-thread cap reaches the opened decoder `SoftwareVideoDecoder.activeProcessorCount` is set before `open`, like `decodesSingleThreaded`, and defaults to the host's count. The #220 decode test pins it to 32 and expects the opened `thread_count` to be 16, so a regression that bypasses `playbackThreadCount` in `open` fails on any host, and every host runs the same 16-deep frame pipeline. Co-Authored-By: Claude Opus 5.5 (1M context) --- Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift | 6 +++++- .../Issue220SoftwareDecoderDrainTests.swift | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift b/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift index 5c86b9543..9529fa8f7 100644 --- a/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift +++ b/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift @@ -82,6 +82,10 @@ final class SoftwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { /// still extractor is the only caller, everything on a playback path wants the parallel default. var decodesSingleThreaded = false + /// Cores the playback thread budget is sized from. Set before `open`; tests pin it to check the + /// cap a many-core Mac gets. + var activeProcessorCount = ProcessInfo.processInfo.activeProcessorCount + /// The `thread_count` libavcodec opened with. Written once in `open`, like the other open-time /// fields. Frame threading holds back `threadCount - 1` decoded frames until flush. private(set) var threadCount = 0 @@ -176,7 +180,7 @@ final class SoftwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { ctx.pointee.thread_type = 0 } else { ctx.pointee.thread_count = Int32(Self.playbackThreadCount( - activeProcessorCount: ProcessInfo.processInfo.activeProcessorCount)) + activeProcessorCount: activeProcessorCount)) ctx.pointee.thread_type = FF_THREAD_FRAME | FF_THREAD_SLICE } diff --git a/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift b/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift index 8437efb03..0fbb1d279 100644 --- a/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift +++ b/Tests/AetherEngineTests/Issue220SoftwareDecoderDrainTests.swift @@ -55,8 +55,12 @@ struct Issue220SoftwareDecoderDrainTests { let stream = try #require(demuxer.stream(at: videoIndex)) let counter = FrameCounter() let decoder = SoftwareVideoDecoder() + // A 32-core Mac's budget: the cap must reach `open`, and every host then runs the same + // 16-deep frame pipeline. + decoder.activeProcessorCount = 32 try decoder.open(stream: stream) { _, _, _ in counter.increment() } defer { decoder.close() } + #expect(decoder.threadCount == 16) var packets = 0 while let pkt = try demuxer.readPacket() {