diff --git a/CHANGELOG.md b/CHANGELOG.md index 98d37658..44766c37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,38 @@ the public-API contract. ## [Unreleased] -_Nothing yet._ +### Added + +- `AetherEngine.softwareDisplaySize`: the size the software path's picture + presents at, the coded frame under the pixel aspect ratio the decoder attached + (#353). A host laying an overlay out over the picture had only + `sourceVideoWidth` / `sourceVideoHeight`, which are the CODED size, so + anamorphic content was laid out against the wrong rectangle (720x576 at 64:45 + presents as 1024x576), and `AVSampleBufferDisplayLayer` carries no `videoRect` + to measure instead. Nor could a host compute it: the ratio is resolved per + frame across three sources (#177) and one whose display aspect is impossible + is dropped in favour of square pixels (#290). Read off the format description + the renderer enqueues rather than recomputed from the SAR, so it cannot + disagree with the screen. nil off the software path and before the first + frame; it follows a mid-stream format change and is cleared with the session. + +### Fixed + +- Anamorphic HEVC on the software host rendered at its coded dimensions (#354). + The VT-backed decoder attached no pixel aspect ratio, and the renderer builds + its format description from the delivered pixel buffer, so nothing carried the + ratio to the layer: 720x576 declaring 64:45 presented as 720x576, a 16:9 + picture squashed into 5:4. The libavcodec decoder on the same host has + attached it since #177, so the gap was one decoder wide. Resolved once at open + from the bitstream ratio and the container's, through the same #177 and #290 + gates, and attached next to the colour metadata that is re-applied there for + the same reason. Reached in production by the interlaced-content detour and by + forward-only sources, which is where broadcast SD lands. +- The software load path cancelled every Combine sink it had already wired. + `softwareCancellables.removeAll()` stood between two groups of `.store(in:)` + calls, so the SW-PiP cue mirror never delivered a cue after the frame + compositor was armed, and subtitles in a software-path PiP window froze at + whatever was on screen when PiP started. ## [6.16.2] - 2026-08-09 diff --git a/README.md b/README.md index 4ee827fa..4b008c33 100644 --- a/README.md +++ b/README.md @@ -250,9 +250,16 @@ player.softwarePresentationTimebase // the master clock, on the so player.setSoftwareVideoFrameTimeObserver { frame in frame.presentation; frame.generation } + +// The rectangle those frames land in (#353): coded dimensions under the pixel aspect ratio the +// decoder attached, read off the format description the renderer enqueues. `sourceVideoWidth` and +// `sourceVideoHeight` are the CODED size, so anamorphic content laid out against them is off by the +// pixel aspect (720x576 at 64:45 presents as 1024x576). nil off the software path and before the +// first frame; it follows a mid-stream format change and is cleared with the session. +player.softwareDisplaySize // CGSize?, @Published ``` -Subtitle cues land in raw source PTS; render the overlay against `player.sourceTime` (see [docs/formats.md › Subtitles](docs/formats.md#subtitles)). A host compositing its own overlay onto the native path (libass and friends) needs the item axis too, since that is what the compositor pairs its samples against: `presentationAxisMap` converts arbitrary positions, `setNativeVideoFrameTimeObserver` reports the frames themselves. On the software path neither is needed: `softwarePresentationTimebase` hands out the clock the frames are presented against and `setSoftwareVideoFrameTimeObserver` reports them, both on the same axis as the cues. Both return nothing rather than a guess when no axis is established, because a defaulted shift is indistinguishable from a measured one at the call site. The 1 Hz diagnostics snapshot lives on `player.diagnostics.liveTelemetry`, off-the-engine for the same render-stability reason. Frame extraction, authored-ASS styling, and the full published surface are documented in [docs/formats.md](docs/formats.md). +Subtitle cues land in raw source PTS; render the overlay against `player.sourceTime` (see [docs/formats.md › Subtitles](docs/formats.md#subtitles)). A host compositing its own overlay onto the native path (libass and friends) needs the item axis too, since that is what the compositor pairs its samples against: `presentationAxisMap` converts arbitrary positions, `setNativeVideoFrameTimeObserver` reports the frames themselves. On the software path neither is needed: `softwarePresentationTimebase` hands out the clock the frames are presented against and `setSoftwareVideoFrameTimeObserver` reports them, both on the same axis as the cues, and `softwareDisplaySize` gives the rectangle to lay the overlay out in (the native path measures its own on `AVPlayerLayer.videoRect`). Both return nothing rather than a guess when no axis is established, because a defaulted shift is indistinguishable from a measured one at the call site. The 1 Hz diagnostics snapshot lives on `player.diagnostics.liveTelemetry`, off-the-engine for the same render-stability reason. Frame extraction, authored-ASS styling, and the full published surface are documented in [docs/formats.md](docs/formats.md). Install via Swift Package Manager: diff --git a/Sources/AetherEngine/AetherEngine+Loading.swift b/Sources/AetherEngine/AetherEngine+Loading.swift index c41db024..b09cc66b 100644 --- a/Sources/AetherEngine/AetherEngine+Loading.swift +++ b/Sources/AetherEngine/AetherEngine+Loading.swift @@ -146,6 +146,27 @@ extension AetherEngine { hasFirstFrameReadyForDisplay = true } + /// #353: mirror a software host's settled picture size onto the public `softwareDisplaySize`. + /// + /// A mirror rather than the latch `hasFirstFrameReadyForDisplay` gets, because the two answer + /// different questions. A picture that exists cannot stop existing for the rest of the load, but + /// the size it presents at can change under it: a live source that switches resolution + /// mid-stream re-shapes the rectangle a host already laid out against, and a latched first value + /// would keep the overlay on the old one. + /// + /// No `dropFirst()` here either, and that is a property of this path rather than a style choice: + /// the software path builds a new host per load (one construction site, and `stopInternal` nils + /// it), so what a fresh mirror replays is that host's own nil and not the outgoing item's size. + /// The native hosts, which are the ones reused across a load, have no size to mirror. + func mirrorSoftwareDisplaySize( + from publisher: Published.Publisher, + storeIn cancellables: inout Set + ) { + publisher + .sink { [weak self] size in self?.softwareDisplaySize = size } + .store(in: &cancellables) + } + /// `videoReadyForDisplay` is the host's raw layer level (#315); nil on the audio hosts, which /// have nothing to display. It is folded, never mirrored: the engine's published flag is latched /// for the load, so the seams that reuse a host and briefly lose the picture do not surface. @@ -1235,6 +1256,11 @@ extension AetherEngine { } activateRendererAudioSession(audioSourceStreamIndex: audioSourceStreamIndex) + // Drop the previous session's sinks BEFORE anything wires this one's. Standing further down, + // between two groups of `.store(in:)` calls, this cancelled everything wired above it: the + // SW-PiP cue mirror never delivered a cue after the frame compositor was armed. Both halves + // of such a wiring work in isolation, which is why a dead sink here reads as a working one. + softwareCancellables.removeAll() let host = SoftwarePlaybackHost() host.deinterlaceConfig = DeinterlaceConfig( mode: loadedOptions.deinterlaceMode, @@ -1251,6 +1277,9 @@ extension AetherEngine { // #311: a load builds a new host and a new renderer, so an observer installed once by the // host app has to be carried across the seam, exactly as the native session does at load. host.setVideoFrameTimeObserver(softwareVideoFrameTimeObserver) + // #353: the settled picture size, wired next to the frame times because a host laying out an + // overlay needs the rectangle as well as the clock, and both come off this renderer. + mirrorSoftwareDisplaySize(from: host.$videoDisplaySize, storeIn: &softwareCancellables) // SW-PiP: publish the bridge once the session owns its layer (the layer object is stable for // the session; the host attaches it to the view and, on PiP start, to the system window). softwarePiPSource = SoftwarePiPSource(layer: host.displayLayer, isLive: isLive, engine: self) @@ -1296,7 +1325,6 @@ extension AetherEngine { self.playlistShiftSeconds = 0 self.setPresentationAxis(PresentationAxisMap()) - softwareCancellables.removeAll() host.$currentTime .sink { [weak self] value in guard let self = self else { return } diff --git a/Sources/AetherEngine/AetherEngine.swift b/Sources/AetherEngine/AetherEngine.swift index c3898f8b..d56af418 100644 --- a/Sources/AetherEngine/AetherEngine.swift +++ b/Sources/AetherEngine/AetherEngine.swift @@ -604,6 +604,30 @@ public final class AetherEngine: ObservableObject { /// display layer, nil on teardown. Hosts build their sample-buffer PiP ContentSource from it. @Published public internal(set) var softwarePiPSource: SoftwarePiPSource? + /// #353: the size the software path's picture presents at, in pixels: the coded frame under the + /// pixel aspect ratio the decoder attached. nil on every other path, before the first frame is + /// built, and on sources with no video. + /// + /// What it is for is laying something out over the picture. A host derives the picture rect from + /// an aspect under the active `videoGravity`, and `sourceVideoWidth`/`sourceVideoHeight` are the + /// CODED dimensions, so anamorphic content lays out against the wrong rectangle: 720x576 at + /// 64:45 presents as 1024x576, and an overlay sized 5:4 sits inside a 16:9 picture. There is + /// nothing to measure on the layer either, since `AVSampleBufferDisplayLayer` has no `videoRect` + /// the way `AVPlayerLayer` does. + /// + /// Nor can a host compute it. The ratio is resolved per frame across three sources, first sane + /// wins (#177), and one whose display aspect is impossible is dropped in favour of square pixels + /// (#290), so a host reconstructing it from container metadata disagrees with the screen in + /// exactly the cases that policy exists for. This is read off the format description the + /// renderer enqueues, so it is what the layer was handed rather than a second opinion about it. + /// + /// Mirrored, not latched, unlike `hasFirstFrameReadyForDisplay`: a live source that switches + /// resolution mid-stream changes the shape of the picture under a host that already laid out + /// against it, and it is cleared with the session so the next source cannot be laid out against + /// this one's rectangle. The native and bypass paths mount an `AVPlayerLayer`, which measures its + /// own `videoRect` and carries `AVPlayerItem.presentationSize`; this stays nil there. + @Published public internal(set) var softwareDisplaySize: CGSize? + /// #288: the native-path counterpart of `softwarePiPSource.layer`. `AVPictureInPictureController` /// wants the layer, not the player, so a host presenting its own PiP on the native path (tvOS has /// no reachable AVKit affordance behind suppressed chrome) cannot get there from `currentAVPlayer`. @@ -4769,6 +4793,9 @@ public final class AetherEngine: ObservableObject { } softwareCancellables.removeAll() + // #353: the picture belongs to the session. Left standing, the next source would be laid out + // against this one's rectangle for as long as it takes its own first frame to arrive. + softwareDisplaySize = nil // #314: same detach on the software path, where the outgoing renderer's decode thread is what // can still hand a frame over while the next host comes up. softwareHost?.setVideoFrameTimeObserver(nil) diff --git a/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift b/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift index fa64f156..ae1bc4b2 100644 --- a/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift +++ b/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift @@ -60,6 +60,17 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { private var colorTransfer: CFString? private var colorMatrix: CFString? + /// #354: the stream's pixel aspect ratio, re-applied to every CVPixelBuffer for the same reason + /// the colorimetry is: nothing else puts it there. The renderer builds its format description + /// from the delivered buffer, so a ratio that is not an attachment on that buffer never reaches + /// the layer, and anamorphic content is displayed at its coded dimensions. nil for square pixels + /// and for a ratio the policy rejects, which is the case where coded dimensions ARE correct. + /// + /// Resolved once at `open()`, not per frame: VT delivers pixel buffers rather than `AVFrame`s, so + /// the per-frame source `SoftwareVideoDecoder` prefers does not exist here. That also makes the + /// #177 latch unnecessary, since one resolution cannot oscillate. + private var pixelAspectRatio: AVRational? + /// Protects `session` across the demux thread (decode), main thread (close/flush), and VT callback (delivery). private let lock = NSLock() @@ -86,6 +97,26 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { width = codecpar.pointee.width height = codecpar.pointee.height + // #354: both declared sources, because only one of them is the container's. The bitstream + // ratio reaches codecpar, while a container-declared one reaches AVStream alone (Matroska's + // DisplayWidth quotient, MP4's `pasp`), which is where every DVD remuxed to MKV carries it. + pixelAspectRatio = Self.resolvePixelAspectRatio( + bitstream: codecpar.pointee.sample_aspect_ratio, + container: stream.pointee.sample_aspect_ratio, + width: width, + height: height + ) + if let sar = pixelAspectRatio { + EngineLog.emit( + "[HWDecoder] SAR \(sar.num):\(sar.den) on \(width)x\(height) " + + "(bitstream=\(codecpar.pointee.sample_aspect_ratio.num):" + + "\(codecpar.pointee.sample_aspect_ratio.den) " + + "container=\(stream.pointee.sample_aspect_ratio.num):" + + "\(stream.pointee.sample_aspect_ratio.den))", + category: .swPlayback + ) + } + guard codecpar.pointee.codec_id == AV_CODEC_ID_HEVC else { throw VideoDecoderError.unsupportedCodec(id: codecpar.pointee.codec_id.rawValue) } @@ -319,6 +350,22 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { close() } + // MARK: - Pixel aspect ratio (#354) + + /// The ratio to attach, or nil when there is nothing to correct. Bitstream first, container + /// second (`declaredStreamSAR`), then the same two gates the libavcodec path runs: the #177 + /// component bound and the #290 display aspect the ratio produces on this frame. Square pixels + /// return nil rather than 1:1, because attaching a correction of one is a correction a consumer + /// cannot tell from a real one. + static func resolvePixelAspectRatio( + bitstream: AVRational, container: AVRational, width: Int32, height: Int32 + ) -> AVRational? { + let declared = SoftwareVideoDecoder.declaredStreamSAR(bitstream: bitstream, container: container) + guard let sane = PixelAspectPolicy.saneSAR(declared, width: width, height: height), + sane.num != sane.den else { return nil } + return sane + } + // MARK: - Callback handling (called from VT's queue) /// Invoked by `hwDecoderOutputCallback`; delivers CVPixelBuffer+PTS, honouring `skipUntilPTS` for seek-pre-roll. @@ -345,6 +392,19 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { CVBufferSetAttachment(imageBuffer, kCVImageBufferYCbCrMatrixKey, matrix, .shouldPropagate) } + // #354: without this the renderer's format description carries no pixel aspect ratio and + // anamorphic content is displayed at its coded dimensions. + if let sar = pixelAspectRatio { + let aspect: NSDictionary = [ + kCVImageBufferPixelAspectRatioHorizontalSpacingKey: Int(sar.num), + kCVImageBufferPixelAspectRatioVerticalSpacingKey: Int(sar.den), + ] + CVBufferSetAttachment(imageBuffer, kCVImageBufferPixelAspectRatioKey, aspect, .shouldPropagate) + } else { + // A recycled pool buffer can carry a stale attachment from an earlier stream. + CVBufferRemoveAttachment(imageBuffer, kCVImageBufferPixelAspectRatioKey) + } + onFrame?(imageBuffer, pts, nil) } } diff --git a/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift b/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift index f6d6b733..b415ece1 100644 --- a/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift +++ b/Sources/AetherEngine/Native/SoftwarePlaybackHost.swift @@ -69,6 +69,12 @@ final class SoftwarePlaybackHost { /// with the session. private var readyForDisplayObserver: NSObjectProtocol? + /// #353: the size this session's picture presents at, coded dimensions under the pixel aspect + /// ratio the decoder attached; nil until the renderer builds its first sample buffer and on + /// sources with no video. The engine mirrors it as `AetherEngine.softwareDisplaySize`, which is + /// what hosts read; this host is built per load, so it starts unknown by construction. + @Published private(set) var videoDisplaySize: CGSize? + /// Fires (off-main) once per session the first time HDR10+ dynamic /// metadata appears on a decoded frame. Hooked by `AetherEngine` to /// upgrade the published `videoFormat` from `.hdr10` → `.hdr10Plus`. @@ -458,6 +464,24 @@ final class SoftwarePlaybackHost { // Default to the software decoder; load() swaps it for the // VT-backed one when the source's video codec is HEVC. self.videoDecoder = SoftwareVideoDecoder() + armDisplaySizeObserver() + } + + /// #353: the renderer settles the picture size on the decode thread, where it builds the format + /// description; publish it on the main actor like every other mirror on this host. Armed in init + /// rather than at load: the renderer is this host's own and lives exactly as long as it does. + private func armDisplaySizeObserver() { + renderer.setDisplaySizeObserver { [weak self] size in + Task { @MainActor in + guard let self, self.videoDisplaySize != size else { return } + self.videoDisplaySize = size + EngineLog.emit( + "[SWHost] picture settles at \(Int(size.width))x\(Int(size.height)) " + + "after \(self.framesEnqueued) frames", + category: .swPlayback + ) + } + } } // MARK: - Audio stream resolution (#133) diff --git a/Sources/AetherEngine/Renderer/SampleBufferRenderer.swift b/Sources/AetherEngine/Renderer/SampleBufferRenderer.swift index 75818f2f..cf56868c 100644 --- a/Sources/AetherEngine/Renderer/SampleBufferRenderer.swift +++ b/Sources/AetherEngine/Renderer/SampleBufferRenderer.swift @@ -111,6 +111,33 @@ final class SampleBufferRenderer: @unchecked Sendable { return _newestEnqueuedPtsSeconds } + /// #353: the size the picture presents at, which is the coded frame under the pixel aspect ratio + /// the decoder attached; nil before the first sample buffer is built. Read off the description + /// that is enqueued rather than recomputed from the SAR: the ratio is resolved per frame across + /// three sources (#177) and a ratio whose display aspect is impossible is dropped (#290), so a + /// second computation of the same answer is a second thing that can disagree with the screen. + /// Guarded by `reorderLock`. + private var _displaySize: CGSize? + var displaySize: CGSize? { + reorderLock.lock() + defer { reorderLock.unlock() } + return _displaySize + } + + /// #353: fires when the settled display size CHANGES, on the decode thread, plus once on + /// installation if the picture already settled. Compared against the value and not against the + /// description, because `flush()` drops the cached description and every seek therefore rebuilds + /// one for a picture that never changed shape. The late-installation call is what a host relies + /// on: on a source with one format, the only report ever due has already happened. + private var _displaySizeObserver: (@Sendable (CGSize) -> Void)? + func setDisplaySizeObserver(_ observer: (@Sendable (CGSize) -> Void)?) { + reorderLock.lock() + _displaySizeObserver = observer + let settled = _displaySize + reorderLock.unlock() + if let settled { observer?(settled) } + } + init() { displayLayer = Self.makeDisplayLayer(isHDR: false) } @@ -378,6 +405,14 @@ final class SampleBufferRenderer: @unchecked Sendable { } } + /// #353: what the layer will draw the description at. Pixel aspect ratio and clean aperture are + /// extensions of the description itself, so this asks the description what it presents at + /// instead of repeating the decision that built it. + static func presentationSize(of desc: CMVideoFormatDescription) -> CGSize { + CMVideoFormatDescriptionGetPresentationDimensions( + desc, usePixelAspectRatio: true, useCleanAperture: true) + } + /// Internal (not private) for #177 regression tests: the PAR-keyed cache behavior is the fix. func createSampleBuffer(from pixelBuffer: CVPixelBuffer, pts: CMTime) -> CMSampleBuffer? { // Cache hit avoids CMVideoFormatDescriptionCreateForImageBuffer allocation + CF refcount churn on every frame. @@ -410,10 +445,17 @@ final class SampleBufferRenderer: @unchecked Sendable { formatDescriptionOut: &formatDesc ) guard status == noErr, let new = formatDesc else { return nil } + // #353: a new description is the only moment the picture can change shape, so the + // settled size is taken here and reported outside the lock. + let settled = Self.presentationSize(of: new) reorderLock.lock() cachedFormatDesc = new cachedFormatKey = key + let changed = settled != _displaySize + if changed { _displaySize = settled } + let sizeObserver = changed ? _displaySizeObserver : nil reorderLock.unlock() + sizeObserver?(settled) desc = new } diff --git a/Sources/aetherctl/PlaybackCmd.swift b/Sources/aetherctl/PlaybackCmd.swift index 609e4374..c820b07c 100644 --- a/Sources/aetherctl/PlaybackCmd.swift +++ b/Sources/aetherctl/PlaybackCmd.swift @@ -363,6 +363,11 @@ private func playSmokeTest(url: URL, seconds: Double, live: Bool, nativeHLS: Boo defer { seekEventSub?.cancel() } var overlapVerdicts: [String] = [] + // #353: sampled during the session, because the engine clears the size with the session and the + // summary below prints after teardown. Paired with the coded dimensions read at the same moment. + var observedDisplaySize: CGSize? + var observedCodedSize: (Int32, Int32) = (0, 0) + let ticks = max(1, Int(seconds)) for tick in 1...ticks { try? await Task.sleep(nanoseconds: 1_000_000_000) @@ -390,6 +395,13 @@ private func playSmokeTest(url: URL, seconds: Double, live: Bool, nativeHLS: Boo if let timebase = engine.softwarePresentationTimebase { line += String(format: " tb=%.3fs", timebase.time.seconds) } + // #353: the rectangle the frames land in. Read next to the coded dimensions on purpose: + // on anamorphic content the two differ, and that difference IS the defect being watched. + if let size = engine.softwareDisplaySize { + line += " disp=\(Int(size.width))x\(Int(size.height))" + observedDisplaySize = size + observedCodedSize = (engine.sourceVideoWidth, engine.sourceVideoHeight) + } } print(line) // DVR-seek smoke: rewind 20 s mid-session, then live-edge return 15 s later, so the @@ -500,6 +512,14 @@ private func playSmokeTest(url: URL, seconds: Double, live: Bool, nativeHLS: Boo } if let frameProbe { print("frame times: \(frameProbe.summary())") + // #353: coded next to settled. Equal on square-pixel sources, and on anamorphic content the + // gap is exactly what a host laying out against `sourceVideoWidth` would have got wrong. + if let size = observedDisplaySize { + print("display size: \(Int(size.width))x\(Int(size.height)) " + + "(coded \(observedCodedSize.0)x\(observedCodedSize.1))") + } else { + print("display size: never published (not the software path, or no frame built)") + } } if !finalSubtitleTracks.isEmpty { let listed = finalSubtitleTracks diff --git a/Tests/AetherEngineTests/Issue353SoftwareDisplaySizeTests.swift b/Tests/AetherEngineTests/Issue353SoftwareDisplaySizeTests.swift new file mode 100644 index 00000000..a5b6e1d6 --- /dev/null +++ b/Tests/AetherEngineTests/Issue353SoftwareDisplaySizeTests.swift @@ -0,0 +1,200 @@ +import Combine +import CoreMedia +import CoreVideo +import Foundation +import Testing +@testable import AetherEngine + +/// AE#353: the picture size the software path settled on, so a host overlay lays out against the +/// picture rather than the coded frame. +/// +/// The value has to be read off the format description the renderer enqueues, not recomputed from +/// the SAR policy. `SoftwareVideoDecoder` resolves the ratio per frame (frame -> codec ctx -> +/// stream, #177) and drops one whose display aspect is impossible (#290), so a second computation +/// of the same answer is a second thing that can disagree with the screen. +@Suite("Settled software display size (#353)") +struct Issue353SoftwareDisplaySizeTests { + + private final class SizeCollector: @unchecked Sendable { + private let lock = NSLock() + private var storage: [CGSize] = [] + + var values: [CGSize] { + lock.lock(); defer { lock.unlock() }; return storage + } + + func append(_ value: CGSize) { + lock.lock(); storage.append(value); lock.unlock() + } + } + + private func makeBuffer(width: Int = 720, height: Int = 576, par: (Int, Int)?) -> CVPixelBuffer { + var pb: CVPixelBuffer? + CVPixelBufferCreate( + kCFAllocatorDefault, width, height, + kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange, + [kCVPixelBufferIOSurfacePropertiesKey: NSDictionary()] as NSDictionary, + &pb + ) + let buffer = pb! + if let par { + let aspect: NSDictionary = [ + kCVImageBufferPixelAspectRatioHorizontalSpacingKey: par.0, + kCVImageBufferPixelAspectRatioVerticalSpacingKey: par.1, + ] + CVBufferSetAttachment(buffer, kCVImageBufferPixelAspectRatioKey, aspect, .shouldPropagate) + } + return buffer + } + + private func pts(_ frame: Int64) -> CMTime { + CMTime(value: frame * 3600, timescale: 90000) + } + + // MARK: - The settled value + + @Test("no size is claimed before the first frame is built") + func nothingSettledBeforeTheFirstFrame() { + #expect(SampleBufferRenderer().displaySize == nil) + } + + @Test("square pixels settle at the coded dimensions") + func squarePixelsSettleAtCodedDimensions() throws { + let renderer = SampleBufferRenderer() + + _ = try #require(renderer.createSampleBuffer(from: makeBuffer(par: nil), pts: pts(0))) + + #expect(renderer.displaySize == CGSize(width: 720, height: 576)) + } + + /// The case the whole issue is about: 720x576 at 64:45 is PAL 16:9. A host laying out against + /// the coded frame draws into a 5:4 rect inside a 16:9 picture. + @Test("an anamorphic PAR settles at the width the picture actually has") + func anamorphicPARSettlesAtTheDisplayWidth() throws { + let renderer = SampleBufferRenderer() + + _ = try #require(renderer.createSampleBuffer(from: makeBuffer(par: (64, 45)), pts: pts(0))) + + #expect(renderer.displaySize == CGSize(width: 1024, height: 576)) + } + + // MARK: - When it is reported + + @Test("the settled size is reported once, not once per frame") + func reportedOnChangeNotPerFrame() throws { + let renderer = SampleBufferRenderer() + let collector = SizeCollector() + renderer.setDisplaySizeObserver { collector.append($0) } + + for frame in 0..<5 { + _ = try #require(renderer.createSampleBuffer( + from: makeBuffer(par: (64, 45)), pts: pts(Int64(frame)))) + } + + #expect(collector.values == [CGSize(width: 1024, height: 576)]) + } + + /// A mid-stream PAR change at identical geometry is exactly what the format cache invalidates + /// on (#177), and it is the one moment the picture changes shape under a host. + @Test("a PAR change at identical geometry reports the new size") + func parChangeReportsTheNewSize() throws { + let renderer = SampleBufferRenderer() + let collector = SizeCollector() + renderer.setDisplaySizeObserver { collector.append($0) } + + _ = try #require(renderer.createSampleBuffer(from: makeBuffer(par: nil), pts: pts(0))) + _ = try #require(renderer.createSampleBuffer(from: makeBuffer(par: (64, 45)), pts: pts(1))) + + #expect(collector.values == [ + CGSize(width: 720, height: 576), + CGSize(width: 1024, height: 576), + ]) + #expect(renderer.displaySize == CGSize(width: 1024, height: 576)) + } + + /// A seek flushes the format-description cache, so the next frame rebuilds a description that + /// describes the same picture. Reporting that as a change would have every seek re-lay-out an + /// overlay that never moved. + @Test("a format rebuild after a flush reports nothing new") + func flushRebuildIsNotAChange() throws { + let renderer = SampleBufferRenderer() + let collector = SizeCollector() + renderer.setDisplaySizeObserver { collector.append($0) } + + _ = try #require(renderer.createSampleBuffer(from: makeBuffer(par: (64, 45)), pts: pts(0))) + renderer.flush(removingDisplayedImage: false) + _ = try #require(renderer.createSampleBuffer(from: makeBuffer(par: (64, 45)), pts: pts(1))) + + #expect(collector.values == [CGSize(width: 1024, height: 576)]) + #expect(renderer.displaySize == CGSize(width: 1024, height: 576)) + } + + /// An observer installed after the picture settled still has to learn the size it missed: + /// nothing else will change until the format does, which on most sources is never. + @Test("an observer installed after the fact is told the settled size") + func lateObserverIsToldTheSettledSize() throws { + let renderer = SampleBufferRenderer() + _ = try #require(renderer.createSampleBuffer(from: makeBuffer(par: (64, 45)), pts: pts(0))) + + let collector = SizeCollector() + renderer.setDisplaySizeObserver { collector.append($0) } + + #expect(collector.values == [CGSize(width: 1024, height: 576)]) + } + + // MARK: - The engine's public mirror + + /// Stands in for the software host's `@Published private(set) var videoDisplaySize`. + @MainActor + private final class HostDouble { + @Published var videoDisplaySize: CGSize? + init(_ initial: CGSize?) { videoDisplaySize = initial } + } + + @MainActor + @Test("the settled size reaches the engine's published mirror") + func settledSizeReachesTheEngine() async throws { + let engine = try AetherEngine() + var cancellables = Set() + let host = HostDouble(nil) + engine.mirrorSoftwareDisplaySize(from: host.$videoDisplaySize, storeIn: &cancellables) + + #expect(engine.softwareDisplaySize == nil) + + host.videoDisplaySize = CGSize(width: 1024, height: 576) + #expect(engine.softwareDisplaySize == CGSize(width: 1024, height: 576)) + } + + /// Mirrored rather than latched, unlike the first-frame flag of #315. The software path builds a + /// new host per load, so a new session's mirror replays nil and the previous picture cannot be + /// inherited; a latch here would hand the next source the last one's rectangle. + @MainActor + @Test("a fresh session's mirror does not inherit the previous session's picture") + func freshSessionStartsWithNoSize() async throws { + let engine = try AetherEngine() + var cancellables = Set() + let outgoing = HostDouble(nil) + engine.mirrorSoftwareDisplaySize(from: outgoing.$videoDisplaySize, storeIn: &cancellables) + outgoing.videoDisplaySize = CGSize(width: 1024, height: 576) + + let incoming = HostDouble(nil) + engine.mirrorSoftwareDisplaySize(from: incoming.$videoDisplaySize, storeIn: &cancellables) + + #expect(engine.softwareDisplaySize == nil) + } + + @MainActor + @Test("a session teardown clears the size with the session") + func teardownClearsTheSize() async throws { + let engine = try AetherEngine() + var cancellables = Set() + let host = HostDouble(nil) + engine.mirrorSoftwareDisplaySize(from: host.$videoDisplaySize, storeIn: &cancellables) + host.videoDisplaySize = CGSize(width: 1024, height: 576) + + engine.stopInternal() + + #expect(engine.softwareDisplaySize == nil, + "an overlay must not lay out the next source against this one's picture") + } +} diff --git a/Tests/AetherEngineTests/Issue354HardwareDecoderPixelAspectTests.swift b/Tests/AetherEngineTests/Issue354HardwareDecoderPixelAspectTests.swift new file mode 100644 index 00000000..be8516b1 --- /dev/null +++ b/Tests/AetherEngineTests/Issue354HardwareDecoderPixelAspectTests.swift @@ -0,0 +1,69 @@ +import Foundation +import Libavutil +import Testing +@testable import AetherEngine + +/// AE#354: the software host's VT-backed decoder attached no pixel aspect ratio, so anamorphic HEVC +/// reached the layer at its coded dimensions (measured: 720x576 at 64:45 presented as 720x576, a +/// 16:9 picture squashed into 5:4). The libavcodec decoder on the same host has attached it since +/// #177; the gap was one decoder wide. +/// +/// The decision is the same one `SoftwareVideoDecoder` makes, minus the per-frame source: VT hands +/// over pixel buffers, not `AVFrame`s, so only the two declared ratios exist here. +@Suite("VT-backed decoder pixel aspect (#354)") +struct Issue354HardwareDecoderPixelAspectTests { + + private func rational(_ num: Int32, _ den: Int32) -> AVRational { + AVRational(num: num, den: den) + } + + @Test("the bitstream ratio wins over the container's") + func bitstreamWins() { + let resolved = HardwareVideoDecoder.resolvePixelAspectRatio( + bitstream: rational(64, 45), container: rational(1, 1), width: 720, height: 576) + + #expect(resolved?.num == 64) + #expect(resolved?.den == 45) + } + + /// The case that keeps the container fallback alive: Matroska writes its DisplayWidth quotient to + /// the stream and leaves codecpar at 0:1, which is every DVD remuxed to MKV. + @Test("an unset bitstream ratio falls back to the container's") + func containerIsTheFallback() { + let resolved = HardwareVideoDecoder.resolvePixelAspectRatio( + bitstream: rational(0, 1), container: rational(64, 45), width: 720, height: 576) + + #expect(resolved?.num == 64) + #expect(resolved?.den == 45) + } + + @Test("square pixels attach nothing") + func squarePixelsAttachNothing() { + #expect(HardwareVideoDecoder.resolvePixelAspectRatio( + bitstream: rational(1, 1), container: rational(1, 1), width: 1920, height: 1080) == nil) + } + + @Test("a garbage component ratio is rejected (#177)") + func garbageComponentsRejected() { + #expect(HardwareVideoDecoder.resolvePixelAspectRatio( + bitstream: rational(1088, 1), container: rational(0, 1), width: 1920, height: 1080) == nil) + } + + /// #290: plausible numbers, impossible picture. 1080p declaring 3:1 smears into a 5.33:1 band. + @Test("a ratio whose display aspect is impossible is rejected (#290)") + func impossibleDisplayAspectRejected() { + #expect(HardwareVideoDecoder.resolvePixelAspectRatio( + bitstream: rational(3, 1), container: rational(0, 1), width: 1920, height: 1080) == nil) + } + + /// The same 2:1 that is wrong on a full-width frame is right on a half-width broadcast one, which + /// is why the gate needs the frame and not just the numbers. + @Test("2:1 on a 960x1080 broadcast frame is kept") + func halfWidthBroadcastKept() { + let resolved = HardwareVideoDecoder.resolvePixelAspectRatio( + bitstream: rational(2, 1), container: rational(0, 1), width: 960, height: 1080) + + #expect(resolved?.num == 2) + #expect(resolved?.den == 1) + } +}