From 52b8fe543f04435ef3bc970203f5e1179f450711 Mon Sep 17 00:00:00 2001 From: williamliu0516 <122490794+williamliu0516@users.noreply.github.com> Date: Sat, 17 Oct 2026 02:59:21 -0700 Subject: [PATCH] fix(hdr10+): the software path's VideoToolbox decoder carries HDR10+, and both decoders attach it in Core Media's shape A PQ HEVC title with ST 2094-40 SEI played on the software path as plain HDR10: HardwareVideoDecoder never read the SEI (its onFirstHDR10PlusDetected was declared and never fired), so videoFormat stayed .hdr10 and no frame carried kCMSampleAttachmentKey_HDR10PlusPerFrameData. It now takes each packet's validated T.35 message (HDR10PlusMetadataScan.coreMediaPayload, PQ streams only), keeps it under the packet's presentation time until VideoToolbox outputs that frame, and passes it to the renderer, which already attaches it. SoftwareVideoDecoder attached a payload per frame, but serialised with av_dynamic_hdr_plus_to_t35, which omits the six T.35 header bytes the attachment is documented to start with; the header is put back. The loopback path is unchanged and needs nothing: with a two-scene 1080p HDR10+ fixture, every one of 144 messages is in the served fragments from MKV, MP4 and MPEG-TS (hdr10plus_tool extract identical to the source), and AVPlayerItemVideoOutput's buffers carry HDR10+ per frame, scene 2's values on scene 2. `aetherctl play --sw` on the same file now logs the .hdr10Plus upgrade and the renderer's attachments. --- CHANGELOG.md | 11 ++++- .../Decoder/HardwareVideoDecoder.swift | 47 +++++++++++++++++-- .../Decoder/SoftwareVideoDecoder.swift | 15 ++---- .../Video/HDR10PlusMetadataScan.swift | 46 ++++++++++++++++++ .../HDR10PlusMetadataScanTests.swift | 38 +++++++++++++++ 5 files changed, 142 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e869ccc3..a99dccdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,16 @@ the public-API contract. ## [Unreleased] -_Nothing yet._ +### Fixed + +- **HDR10+ reaches the display on the software path.** The VideoToolbox decoder there never read the + ST 2094-40 SEI, so a PQ HEVC title played as plain HDR10 and `videoFormat` stayed `.hdr10`; it now + reports `.hdr10Plus` on the first message and hands each frame its payload as + `kCMSampleAttachmentKey_HDR10PlusPerFrameData`. The libavcodec decoder attached one per frame already, + but from `av_dynamic_hdr_plus_to_t35`, which leaves out the six T.35 header bytes the attachment must + start with (`0xB5 0x00 0x3C 0x00 0x01 0x04`); they are put back. The loopback path needed nothing: + the SEI is in every served fragment from MKV, MP4 and MPEG-TS sources, and AVPlayer's decoded buffers + carry the per-frame metadata (measured on macOS 27 with a two-scene fixture). ## [7.39.0] - 2026-10-17 diff --git a/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift b/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift index 086526c1..41c4b19d 100644 --- a/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift +++ b/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift @@ -21,9 +21,26 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { set { skipLock.lock(); _onFrame = newValue; skipLock.unlock() } } private var _onFrame: DecodedFrameHandler? - /// Not yet wired on the VT side (follow-up: read AV_PKT_DATA_DYNAMIC_HDR10_PLUS before decode, - /// mirror SoftwareVideoDecoder.extractHDR10PlusBytes). Flag kept so host wiring stays identical to SW path. + /// Fires once (decode thread) on the first packet carrying a valid ST 2094-40 message, as the + /// libavcodec decoder does; the engine then moves `videoFormat` to `.hdr10Plus`. var onFirstHDR10PlusDetected: (@Sendable () -> Void)? + /// HDR10+ per frame. VideoToolbox outputs in presentation order and the packets arrive in decode + /// order, so each packet's payload waits here under its presentation time until its frame comes + /// out, and leaves as that frame's `kCMSampleAttachmentKey_HDR10PlusPerFrameData`. Only a PQ + /// stream is scanned; HDR10+ describes nothing else. Its own lock: the output callback must not + /// take `lock` (see `decode`). + private var scansHDR10Plus = false + private var hdr10PlusFraming = VideoNALFraming.lengthPrefixed(size: 4) + private var seenHDR10Plus = false + private let hdr10PlusLock = NSLock() + private var hdr10PlusByPTS: [HDR10PlusKey: Data] = [:] + private struct HDR10PlusKey: Hashable { + let value: CMTimeValue + let timescale: CMTimeScale + init(_ time: CMTime) { value = time.value; timescale = time.timescale } + } + /// Far past any reorder depth: entries for frames VideoToolbox never output do not pile up. + private static let hdr10PlusPendingLimit = 64 var onA53Captions: (@Sendable ([CCDataParser.CCTriplet], Double) -> Void)? var onDecodedFormat: (@Sendable (DecodedVideoFormat) -> Void)? private var streamColor = ColorDescription.unspecified @@ -293,6 +310,10 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { streamColor = ColorDescription(codecpar: codecpar) streamCodecID = codecpar.pointee.codec_id streamProfile = codecpar.pointee.profile + scansHDR10Plus = codecpar.pointee.color_trc == AVCOL_TRC_SMPTE2084 + hdr10PlusFraming = .lengthPrefixed(size: NALUnitChain.lengthPrefixSize( + codecID: streamCodecID, extradata: extradata, + extradataSize: Int(codecpar.pointee.extradata_size)) ?? 4) self.colorPrimaries = ColorAttachments.primaries(codecpar.pointee.color_primaries) self.colorTransfer = ColorAttachments.transfer(codecpar.pointee.color_trc) self.colorMatrix = ColorAttachments.matrix(codecpar.pointee.color_space) @@ -583,6 +604,19 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { ) guard sbStatus == noErr, let sb = sampleBuffer else { return } + if scansHDR10Plus, + let payload = HDR10PlusMetadataScan.coreMediaPayload( + packet, codecID: streamCodecID, framing: hdr10PlusFraming) { + hdr10PlusLock.lock() + if hdr10PlusByPTS.count >= Self.hdr10PlusPendingLimit { hdr10PlusByPTS.removeAll() } + hdr10PlusByPTS[HDR10PlusKey(pts)] = payload + hdr10PlusLock.unlock() + if !seenHDR10Plus { + seenHDR10Plus = true + onFirstHDR10PlusDetected?() + } + } + // Tag non-keyframes as DependsOnOthers so VT can drop pre-seek RASL frames after a flush. if (packet.pointee.flags & AV_PKT_FLAG_KEY) == 0 { if let attachArray = CMSampleBufferGetSampleAttachmentsArray(sb, createIfNecessary: true), @@ -672,6 +706,7 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { // Drain in-flight frames then signal a discontinuity so VT drops its reference picture state. VTDecompressionSessionWaitForAsynchronousFrames(session) VTDecompressionSessionFinishDelayedFrames(session) + hdr10PlusLock.lock(); hdr10PlusByPTS.removeAll(); hdr10PlusLock.unlock() } func close() { @@ -716,6 +751,12 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { imageBuffer: CVImageBuffer, pts: CMTime ) { + var hdr10PlusData: Data? + if scansHDR10Plus { + hdr10PlusLock.lock() + hdr10PlusData = hdr10PlusByPTS.removeValue(forKey: HDR10PlusKey(pts)) + hdr10PlusLock.unlock() + } if let threshold = skipUntilPTS { if CMTimeCompare(pts, threshold) < 0 { return @@ -749,7 +790,7 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { } reportDecodedFormat(imageBuffer) - onFrame?(imageBuffer, pts, nil) + onFrame?(imageBuffer, pts, hdr10PlusData) } /// VideoToolbox decodes straight into the display buffer, so the buffer IS the decoded picture; its diff --git a/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift b/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift index 43266051..0ae2f6dd 100644 --- a/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift +++ b/Sources/AetherEngine/Decoder/SoftwareVideoDecoder.swift @@ -517,17 +517,10 @@ final class SoftwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { guard let entry = sideData[i] else { continue } guard entry.pointee.type == AV_FRAME_DATA_DYNAMIC_HDR_PLUS else { continue } guard let raw = entry.pointee.data, entry.pointee.size > 0 else { continue } - return raw.withMemoryRebound( - to: AVDynamicHDRPlus.self, - capacity: 1 - ) { recordPtr -> Data? in - var dataPtr: UnsafeMutablePointer? = nil - var size: Int = 0 - let result = av_dynamic_hdr_plus_to_t35(recordPtr, &dataPtr, &size) - guard result >= 0, let buf = dataPtr, size > 0 else { return nil } - let data = Data(bytes: buf, count: size) - av_free(buf) // use av_free, not plain free(): libavutil allocator contract - return data + // Core Media reads the attachment from the T.35 country code on; libavutil's serialiser + // leaves out the six header bytes, so the payload attached before did not have that shape. + return raw.withMemoryRebound(to: AVDynamicHDRPlus.self, capacity: 1) { + HDR10PlusMetadataScan.coreMediaPayload($0) } } return nil diff --git a/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift b/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift index 4dd233c1..17604346 100644 --- a/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift +++ b/Sources/AetherEngine/Video/HDR10PlusMetadataScan.swift @@ -57,6 +57,52 @@ enum HDR10PlusMetadataScan { } } + /// The packet's ST 2094-40 message in the shape `kCMSampleAttachmentKey_HDR10PlusPerFrameData` + /// documents: the registered T.35 bytes from `itu_t_t35_country_code` (0xB5) on. The VideoToolbox + /// route decodes into a pixel buffer and builds the display layer's sample buffer itself, so the + /// per-frame metadata reaches the layer only through this attachment. H.264 / HEVC SEI and + /// Matroska's packet side data; the first message the full validation accepts wins. + static func coreMediaPayload( + _ packet: UnsafePointer, codecID: AVCodecID, framing: VideoNALFraming + ) -> Data? { + var sideDataSize = 0 + if let sideData = av_packet_get_side_data(packet, AV_PKT_DATA_DYNAMIC_HDR10_PLUS, &sideDataSize), + validSideData(sideData, size: sideDataSize) { + var allocationSize = 0 + guard let metadata = av_dynamic_hdr_plus_alloc(&allocationSize) else { return nil } + defer { av_free(metadata) } + memcpy(metadata, sideData, allocationSize) + return coreMediaPayload(metadata) + } + guard codecID == AV_CODEC_ID_H264 || codecID == AV_CODEC_ID_HEVC, + let data = packet.pointee.data, packet.pointee.size > 0, + mayContainT35Header(data, size: Int(packet.pointee.size)) else { return nil } + var payload: Data? + _ = scanNALs( + UnsafeBufferPointer(start: data, count: Int(packet.pointee.size)), + codecID: codecID, framing: framing + ) { bytes, size in + guard validT35(bytes, size) else { return false } + if payload == nil { payload = Data(bytes: bytes, count: size) } + return true + } + return payload + } + + /// `av_dynamic_hdr_plus_to_t35` writes the message from the application version on, without the + /// six header bytes Core Media's attachment has to start with, so they go back in front here. + static func coreMediaPayload(_ metadata: UnsafePointer) -> Data? { + var buffer: UnsafeMutablePointer? + var size = 0 + guard av_dynamic_hdr_plus_to_t35(metadata, &buffer, &size) >= 0, let buffer, size > 0 else { + return nil + } + defer { av_free(buffer) } + var payload = Data(t35Header) + payload.append(buffer, count: size) + return payload + } + /// Audit BIT-4: exact prefilter. The playback scan runs on every video packet until its first hit, /// so a source without HDR10+ paid a byte-wise NAL walk for the whole session. The T.35 header /// holds no `00 00` pair, so no emulation-prevention byte can land inside it and AV1 does not diff --git a/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift b/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift index 3be54aa0..9634fca9 100644 --- a/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift +++ b/Tests/AetherEngineTests/HDR10PlusMetadataScanTests.swift @@ -471,4 +471,42 @@ struct HDR10PlusMetadataScanTests { } } } + + @Test("Core Media's per-frame payload starts at the T.35 country code, from SEI and from side data") + func coreMediaPayload() throws { + // SEI: the validated message, verbatim and header first, whatever the framing. + for hevc in [true, false] { + let codec = hevc ? AV_CODEC_ID_HEVC : AV_CODEC_ID_H264 + try withPacket(Self.lengthPrefixed(Self.sei(hevc: hevc), width: 4)) { packet in + let payload = HDR10PlusMetadataScan.coreMediaPayload( + packet, codecID: codec, framing: .lengthPrefixed(size: 4)) + #expect(payload.map { [UInt8]($0) } == Self.t35()) + } + try withPacket(Self.annexB(Self.sei(hevc: hevc))) { packet in + let payload = HDR10PlusMetadataScan.coreMediaPayload(packet, codecID: codec, framing: .annexB) + #expect(payload.map { [UInt8]($0) } == Self.t35()) + } + } + // Side data: libavutil's serialiser omits the six header bytes, which have to come back. + for source in [Self.t35(), Self.t35(windows: 3, grids: true, toneMapping: true)] { + try withPacket { packet in + try addSideData(packet, payload: source) + let payload = try #require(HDR10PlusMetadataScan.coreMediaPayload( + packet, codecID: AV_CODEC_ID_VP9, framing: .annexB)) + #expect(Array(payload.prefix(6)) == Self.header) + #expect(payload.count == source.count) + let metadata = try #require(av_dynamic_hdr_plus_alloc(nil)) + defer { av_free(metadata) } + let parsed = payload.withUnsafeBytes { + av_dynamic_hdr_plus_from_t35( + metadata, $0.bindMemory(to: UInt8.self).baseAddress! + 6, payload.count - 6) + } + #expect(parsed >= 0) + } + } + // Nothing to attach: a packet without the message, and a slice that only looks like one. + try withPacket(Self.annexB([0x26, 0x01] + Self.header)) { packet in + #expect(HDR10PlusMetadataScan.coreMediaPayload(packet, codecID: AV_CODEC_ID_HEVC, framing: .annexB) == nil) + } + } }