diff --git a/CHANGELOG.md b/CHANGELOG.md index cadfbc99f..36f291cd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,8 @@ the public-API contract. after its response head reports the hop that answered and its status. The observer does not alter request policy and requires hosts to redact source URLs. +- Software playback teardown no longer waits on the main actor for asynchronous video-renderer removal. The retiring output and layer remain alive until removal completes, including when the synchronizer is stopped. + - **A remote disc image on an extensionless URL is recognized by its `Content-Type` (AE#860, thanks @qoli).** The HTTP disc adapter was gated on the `.iso` / `.img` / `.udf` extension alone, so an ISO behind an opaque proxy path reached libavformat as raw filesystem bytes and failed with `Invalid data found diff --git a/Sources/AetherEngine/Audio/AudioOutput.swift b/Sources/AetherEngine/Audio/AudioOutput.swift index 3e5e0902f..36e881bea 100644 --- a/Sources/AetherEngine/Audio/AudioOutput.swift +++ b/Sources/AetherEngine/Audio/AudioOutput.swift @@ -16,6 +16,8 @@ final class AudioOutput: @unchecked Sendable { let timeline = RendererTimeline() private let lock = NSLock() + @MainActor private var attachedVideoLayer: AVSampleBufferDisplayLayer? + @MainActor private var videoLayerRemovalTask: Task? /// AE#464: the host's audio presentation offset, applied to every buffer on its way into the /// renderer. Guarded because the host writes it from the main actor while the demux thread reads @@ -136,30 +138,67 @@ final class AudioOutput: @unchecked Sendable { } } - /// Add the video renderer to the synchronizer for automatic A/V sync + frame pacing. The display layer's - /// `sampleBufferRenderer`, never the layer: addRenderer(layer) still type-checks but on tvOS 26+ fails with - /// FigVideoQueueRemote err=-12080 after the first enqueue. Taken as the renderer rather than read off the - /// layer here, because the layer is main-actor isolated in the 27 SDKs and this runs off it (#351). - func attachVideoRenderer(_ videoRenderer: AVSampleBufferVideoRenderer) { - synchronizer.addRenderer(videoRenderer) + /// Add the video display layer to the synchronizer for automatic A/V sync + frame pacing. On iOS18/tvOS18/ + /// macOS15+ Apple split the queue rendering surface onto displayLayer.sampleBufferRenderer; direct + /// addRenderer(layer) still type-checks but on tvOS 26+ fails with FigVideoQueueRemote err=-12080 after the + /// first enqueue, so attach the renderer instead. + @MainActor + func attachVideoLayer(_ displayLayer: AVSampleBufferDisplayLayer) { + precondition(videoLayerRemovalTask == nil, "Await video layer removal before attaching again") + guard attachedVideoLayer !== displayLayer else { return } + precondition(attachedVideoLayer == nil, "An audio output owns only one video layer") + if #available(tvOS 18.0, iOS 18.0, macOS 15.0, *) { + synchronizer.addRenderer(displayLayer.sampleBufferRenderer) + } else { + synchronizer.addRenderer(displayLayer) + } + attachedVideoLayer = displayLayer } - /// Remove the video renderer and block until removal completes. The synchronizer detaches asynchronously; - /// if the caller immediately assigns displayLayer.controlTimebase for a new Atmos session the layer is briefly - /// owned by both (Apple-documented UB). Symptom: first PCM->Atmos switch after launch throws FigVideoQueueRemote - /// err=-12080 and the display layer stops rendering (audio keeps going). The semaphore wait (sub-100ms) makes - /// the handoff deterministic. - func detachVideoRenderer(_ videoRenderer: AVSampleBufferVideoRenderer) { - let semaphore = DispatchSemaphore(value: 0) - synchronizer.removeRenderer(videoRenderer, at: synchronizer.currentTime()) { _ in - semaphore.signal() + /// Retire the old session's layer without blocking the caller. SoftwarePlaybackHost creates a + /// fresh renderer for every new session, so the retired layer is never reused by its successor. The task + /// retains this output, its synchronizer, and the layer until AVFoundation finishes removal; cancellation + /// must not release them early. A caller that needs to reuse this same layer must await the returned task. + /// The injected operation lets tests hold the completion while checking main-actor responsiveness/lifetime. + @MainActor + @discardableResult + func detachVideoLayer( + _ displayLayer: AVSampleBufferDisplayLayer, + removeRenderer: @escaping @MainActor (AVSampleBufferRenderSynchronizer, AVSampleBufferDisplayLayer) async -> Void = AudioOutput.removeVideoRenderer + ) -> Task? { + guard attachedVideoLayer === displayLayer else { return nil } + if let videoLayerRemovalTask { return videoLayerRemovalTask } + let task = Task { @MainActor [self, displayLayer] in + await removeRenderer(synchronizer, displayLayer) + attachedVideoLayer = nil + videoLayerRemovalTask = nil + withExtendedLifetime((self, displayLayer)) {} } - let result = semaphore.wait(timeout: .now() + .seconds(1)) - #if DEBUG - if result == .timedOut { - EngineLog.emit("[AudioOutput] detachVideoRenderer: timed out waiting for synchronizer removal", category: .swPlayback) + videoLayerRemovalTask = task + return task + } + + @MainActor + private static func removeVideoRenderer( + from synchronizer: AVSampleBufferRenderSynchronizer, + displayLayer: AVSampleBufferDisplayLayer + ) async { + await withCheckedContinuation { continuation in + // Invalid time requests immediate removal, independent of a stopped/reset timebase. + // Only the continuation crosses into AVFoundation's completion queue. + // AVFoundation reports false for an absent renderer or a superseded removal. + // This is the attachment's only remover and duplicate requests share its task, + // so no second removal can supersede it; either result finishes retirement. + if #available(tvOS 18.0, iOS 18.0, macOS 15.0, *) { + synchronizer.removeRenderer(displayLayer.sampleBufferRenderer, at: .invalid) { _ in + continuation.resume() + } + } else { + synchronizer.removeRenderer(displayLayer, at: .invalid) { _ in + continuation.resume() + } + } } - #endif } var volume: Float { diff --git a/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift b/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift index 54fe47192..1e67c2c08 100644 --- a/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift +++ b/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift @@ -1579,7 +1579,7 @@ final class SoftwarePlaybackHost { break } if !demuxLoopStarted, let aOut = audioOutput { - aOut.attachVideoRenderer(renderer.videoRenderer) + aOut.attachVideoLayer(renderer.displayLayer) } if !demuxLoopStarted { demuxLoopStarted = true @@ -2285,7 +2285,7 @@ final class SoftwarePlaybackHost { if let aOut = audioOutput { aOut.stop() - aOut.detachVideoRenderer(renderer.videoRenderer) + aOut.detachVideoLayer(renderer.displayLayer) } audioOutput = nil audioDecoder?.close() diff --git a/Tests/AetherEngineTests/AudioOutputRendererLifecycleTests.swift b/Tests/AetherEngineTests/AudioOutputRendererLifecycleTests.swift new file mode 100644 index 000000000..fe8b5f860 --- /dev/null +++ b/Tests/AetherEngineTests/AudioOutputRendererLifecycleTests.swift @@ -0,0 +1,64 @@ +import AVFoundation +import Testing +@testable import AetherEngine + +@Suite("AudioOutput video renderer retirement", .timeLimit(.minutes(1))) +@MainActor +struct AudioOutputRendererLifecycleTests { + private final class WeakOutput { + weak var value: AudioOutput? + init(_ value: AudioOutput?) { self.value = value } + } + + @Test("a stopped synchronizer removes its renderer without waiting for its clock") + func stoppedClockRemoval() async throws { + let output = AudioOutput() + let layer = AVSampleBufferDisplayLayer() + output.attachVideoLayer(layer) + output.stop() + let retirement = try #require(output.detachVideoLayer(layer)) + await retirement.value + #expect(!output.synchronizer.renderers.contains { $0 === layer.sampleBufferRenderer }) + // Reattachment is safe only after the completion. + output.attachVideoLayer(layer) + await output.detachVideoLayer(layer)?.value + } + + @Test("retirement is nonblocking, deduplicated, and retains the output through cancellation") + func lifetimeAndDuplicateRemoval() async throws { + var output: AudioOutput? = AudioOutput() + let weakOutput = WeakOutput(output) + let layer = AVSampleBufferDisplayLayer() + output?.attachVideoLayer(layer) + var continuation: CheckedContinuation? + var removals = 0 + let retirement = try #require(output?.detachVideoLayer(layer) { _, _ in + removals += 1 + await withCheckedContinuation { continuation = $0 } + }) + let duplicate = try #require(output?.detachVideoLayer(layer) { _, _ in + Issue.record("a duplicate detach must not start another removal") + }) + try await waitFor { continuation != nil } + #expect(removals == 1) + // Reaching here on the main actor proves detach did not block on the completion. + output = nil + retirement.cancel() + #expect(weakOutput.value != nil) + continuation?.resume() + continuation = nil + await retirement.value + await duplicate.value + try await waitFor { weakOutput.value == nil } + } + + @Test("detaching an unrelated layer leaves the owned attachment intact") + func unrelatedLayerIsIgnored() async throws { + let output = AudioOutput() + let owned = AVSampleBufferDisplayLayer() + output.attachVideoLayer(owned) + #expect(output.detachVideoLayer(AVSampleBufferDisplayLayer()) == nil) + #expect(output.synchronizer.renderers.contains { $0 === owned.sampleBufferRenderer }) + await output.detachVideoLayer(owned)?.value + } +} diff --git a/docs/architecture.md b/docs/architecture.md index fab48e1bc..10d9d6d26 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -738,3 +738,7 @@ be reconstructed by this policy. ### Quantized audio packet coverage `SoftwareAudioPacketCoverage` retains feasible integer sample-clock phases across admitted audio packets rather than bridging every small gap with an epsilon. Codec geometry is limited to fixed MPEG audio/AC-3 frames, declared AAC/E-AC-3 frame sizes, complete non-reservoir WMA v1/v2 blocks and exact PCM sample frames. Unknown, packed or conflicting layouts keep strict positive-duration intervals. Missing timestamps, backwards transitions and arithmetic overflow break clock evidence; a genuine missing packet or accumulated extra tick remains a hole. This changes packet-cache coverage and seek admission, not the decoded audio output clock. Duration-less audio retains the bounded successor-PTS contract, including repeated TrueHD millisecond timestamps. + +### Software renderer retirement + +Each software playback host owns a fresh display layer. On stop, `AudioOutput` requests immediate synchronizer removal with an invalid time and returns without blocking the main actor. Its shared retirement task retains the output, synchronizer and display layer until AVFoundation calls the completion handler; cancellation does not release those objects early. Repeated detach requests share that task. A caller reusing the same layer must await retirement before attaching it again.