diff --git a/CHANGELOG.md b/CHANGELOG.md index cadfbc99f..dff96cdea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,10 @@ 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. +- **VideoToolbox output metadata respects immutable reference buffers.** Colour and pixel-aspect + attachments are updated in place only when the decode callback marks the buffer modifiable. + Otherwise a pooled owned copy receives the attachments; matching output is passed through. + - **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/Decoder/HardwareVideoDecoder.swift b/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift index f919bf767..5d78494f6 100644 --- a/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift +++ b/Sources/AetherEngine/Decoder/HardwareVideoDecoder.swift @@ -83,6 +83,7 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { /// Color metadata from codecpar, re-applied to every CVPixelBuffer. /// VTDecompressionSession should propagate these from SPS+hvcC but has been observed not to; /// without them an HDR buffer renders as desaturated SDR on AVSampleBufferDisplayLayer. + private let outputMetadataNormalizer = VideoToolboxOutputMetadataNormalizer() private var colorPrimaries: CFString? private var colorTransfer: CFString? private var colorMatrix: CFString? @@ -734,6 +735,7 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { box.release() refConBox = nil } + outputMetadataNormalizer.reset() onFrame = nil } @@ -760,7 +762,8 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { /// Invoked by `hwDecoderOutputCallback`; delivers CVPixelBuffer+PTS, honouring `skipUntilPTS` for seek-pre-roll. fileprivate func handleDecodedFrame( imageBuffer: CVImageBuffer, - pts: CMTime + pts: CMTime, + infoFlags: VTDecodeInfoFlags ) { skipLock.lock() _framesDecoded &+= 1 @@ -779,32 +782,17 @@ final class HardwareVideoDecoder: VideoDecodingPipeline, @unchecked Sendable { clearSkip(ifStillAt: threshold) } - // Attach color metadata; without it HDR PQ content shows as desaturated SDR on AVSampleBufferDisplayLayer. - if let primaries = colorPrimaries { - CVBufferSetAttachment(imageBuffer, kCVImageBufferColorPrimariesKey, primaries, .shouldPropagate) - } - if let transfer = colorTransfer { - CVBufferSetAttachment(imageBuffer, kCVImageBufferTransferFunctionKey, transfer, .shouldPropagate) - } - if let matrix = colorMatrix { - 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) - } - - reportDecodedFormat(imageBuffer) - onFrame?(imageBuffer, pts, hdr10PlusData) + // A VT output may remain a reference frame after delivery. Only change its metadata + // in place when the decoder explicitly grants permission; otherwise use an owned copy. + let outputBuffer = outputMetadataNormalizer.applyingOutputMetadata( + to: imageBuffer, + isModifiable: infoFlags.contains(.imageBufferModifiable), + colorPrimaries: colorPrimaries, + transferFunction: colorTransfer, + yCbCrMatrix: colorMatrix, + pixelAspectRatio: pixelAspectRatio) + reportDecodedFormat(outputBuffer) + onFrame?(outputBuffer, pts, hdr10PlusData) } /// VideoToolbox decodes straight into the display buffer, so the buffer IS the decoded picture; its @@ -842,7 +830,8 @@ private func hwDecoderOutputCallback( .fromOpaque(refCon).takeUnretainedValue() box.decoder?.handleDecodedFrame( imageBuffer: imageBuffer, - pts: presentationTimeStamp + pts: presentationTimeStamp, + infoFlags: infoFlags ) } diff --git a/Sources/AetherEngine/Decoder/VideoToolboxOutputMetadataNormalizer.swift b/Sources/AetherEngine/Decoder/VideoToolboxOutputMetadataNormalizer.swift new file mode 100644 index 000000000..d74be25be --- /dev/null +++ b/Sources/AetherEngine/Decoder/VideoToolboxOutputMetadataNormalizer.swift @@ -0,0 +1,290 @@ +import Foundation +import CoreVideo +import VideoToolbox +import AetherLibavutil + +/// Supplies codec-declared display metadata without mutating immutable VideoToolbox output. +/// +/// Frames with matching attachments return unchanged. A mismatch draws +/// into a decoder-owned pool through VideoToolbox's pixel-transfer path, avoiding CPU mapping and +/// per-frame IOSurface allocation. The lock serializes the transfer session and all mutable pool +/// state across VideoToolbox callback queues. +// All mutable session/pool state is protected by lock. Callers own the returned buffer's +// metadata writes; only immutable VT buffers enter the copy path, and each checkout is distinct. +final class VideoToolboxOutputMetadataNormalizer: @unchecked Sendable { + private struct PoolFormat: Equatable { + let width: Int + let height: Int + let pixelFormat: OSType + } + + private let lock = NSLock() + private var transferSession: VTPixelTransferSession? + private var pixelBufferPool: CVPixelBufferPool? + private var poolFormat: PoolFormat? + private var loggedFailure = false + + func applyingOutputMetadata( + to imageBuffer: CVPixelBuffer, + isModifiable: Bool, + colorPrimaries: CFString?, + transferFunction: CFString?, + yCbCrMatrix: CFString?, + pixelAspectRatio: AVRational? + ) -> CVPixelBuffer { + if Self.outputMetadataMatches( + imageBuffer, + colorPrimaries: colorPrimaries, + transferFunction: transferFunction, + yCbCrMatrix: yCbCrMatrix, + pixelAspectRatio: pixelAspectRatio + ) { + return imageBuffer + } + + let destination: CVPixelBuffer + if isModifiable { + destination = imageBuffer + } else { + guard let copiedBuffer = transferToOwnedBuffer(imageBuffer) else { + logFailureOnce() + return imageBuffer + } + destination = copiedBuffer + CVBufferRemoveAllAttachments(destination) + CVBufferPropagateAttachments(imageBuffer, destination) + } + + Self.applyOutputMetadata( + to: destination, + colorPrimaries: colorPrimaries, + transferFunction: transferFunction, + yCbCrMatrix: yCbCrMatrix, + pixelAspectRatio: pixelAspectRatio + ) + return destination + } + + private static func applyOutputMetadata( + to imageBuffer: CVPixelBuffer, + colorPrimaries: CFString?, + transferFunction: CFString?, + yCbCrMatrix: CFString?, + pixelAspectRatio: AVRational? + ) { + if let colorPrimaries { + CVBufferSetAttachment( + imageBuffer, + kCVImageBufferColorPrimariesKey, + colorPrimaries, + .shouldPropagate + ) + } + if let transferFunction { + CVBufferSetAttachment( + imageBuffer, + kCVImageBufferTransferFunctionKey, + transferFunction, + .shouldPropagate + ) + } + if let yCbCrMatrix { + CVBufferSetAttachment( + imageBuffer, + kCVImageBufferYCbCrMatrixKey, + yCbCrMatrix, + .shouldPropagate + ) + } + if let pixelAspectRatio { + let aspect: NSDictionary = [ + kCVImageBufferPixelAspectRatioHorizontalSpacingKey: Int(pixelAspectRatio.num), + kCVImageBufferPixelAspectRatioVerticalSpacingKey: Int(pixelAspectRatio.den), + ] + CVBufferSetAttachment(imageBuffer, kCVImageBufferPixelAspectRatioKey, aspect, .shouldPropagate) + } else { + CVBufferRemoveAttachment(imageBuffer, kCVImageBufferPixelAspectRatioKey) + } + } + + private static func outputMetadataMatches( + _ imageBuffer: CVPixelBuffer, + colorPrimaries: CFString?, + transferFunction: CFString?, + yCbCrMatrix: CFString?, + pixelAspectRatio: AVRational? + ) -> Bool { + func attachment(_ key: CFString) -> CFTypeRef? { + CVBufferCopyAttachment(imageBuffer, key, nil) + } + + if let colorPrimaries, + attachment(kCVImageBufferColorPrimariesKey) as? String != colorPrimaries as String { + return false + } + if let transferFunction, + attachment(kCVImageBufferTransferFunctionKey) as? String != transferFunction as String { + return false + } + if let yCbCrMatrix, + attachment(kCVImageBufferYCbCrMatrixKey) as? String != yCbCrMatrix as String { + return false + } + if let pixelAspectRatio { + guard let aspect = attachment(kCVImageBufferPixelAspectRatioKey) as? NSDictionary, + aspect[kCVImageBufferPixelAspectRatioHorizontalSpacingKey] as? Int + == Int(pixelAspectRatio.num), + aspect[kCVImageBufferPixelAspectRatioVerticalSpacingKey] as? Int + == Int(pixelAspectRatio.den) else { + return false + } + } else if attachment(kCVImageBufferPixelAspectRatioKey) != nil { + return false + } + return true + } + + func reset() { + lock.lock() + let retiredSession = transferSession + transferSession = nil + pixelBufferPool = nil + poolFormat = nil + loggedFailure = false + lock.unlock() + if let retiredSession { + VTPixelTransferSessionInvalidate(retiredSession) + } + } + + private func transferToOwnedBuffer(_ source: CVPixelBuffer) -> CVPixelBuffer? { + lock.lock() + defer { lock.unlock() } + + if transferSession == nil { + var createdSession: VTPixelTransferSession? + let status = VTPixelTransferSessionCreate( + allocator: kCFAllocatorDefault, + pixelTransferSessionOut: &createdSession + ) + guard status == noErr, let createdSession else { return nil } + VTSessionSetProperty( + createdSession, + key: kVTPixelTransferPropertyKey_RealTime, + value: kCFBooleanTrue + ) + transferSession = createdSession + } + + let format = PoolFormat( + width: CVPixelBufferGetWidth(source), + height: CVPixelBufferGetHeight(source), + pixelFormat: CVPixelBufferGetPixelFormatType(source) + ) + if pixelBufferPool == nil || poolFormat != format { + var createdPool: CVPixelBufferPool? + let pixelBufferAttributes: NSDictionary = [ + kCVPixelBufferPixelFormatTypeKey: format.pixelFormat, + kCVPixelBufferWidthKey: format.width, + kCVPixelBufferHeightKey: format.height, + kCVPixelBufferIOSurfacePropertiesKey: NSDictionary(), + kCVPixelBufferMetalCompatibilityKey: true, + ] + let status = CVPixelBufferPoolCreate( + kCFAllocatorDefault, + nil, + pixelBufferAttributes, + &createdPool + ) + guard status == kCVReturnSuccess, let createdPool else { return nil } + pixelBufferPool = createdPool + poolFormat = format + } + + guard let transferSession, let pixelBufferPool else { return nil } + var destination: CVPixelBuffer? + guard CVPixelBufferPoolCreatePixelBuffer( + kCFAllocatorDefault, + pixelBufferPool, + &destination + ) == kCVReturnSuccess, let destination else { + return nil + } + let transferStatus = VTPixelTransferSessionTransferImage( + transferSession, + from: source, + to: destination + ) + guard transferStatus == noErr || copyPixels(from: source, to: destination) else { + return nil + } + return destination + } + + /// Failure-only CPU fallback. The normal path stays GPU-side through `VTPixelTransferSession`; + /// this preserves metadata correctness if a platform rejects a same-format transfer. + private func copyPixels( + from source: CVPixelBuffer, + to destination: CVPixelBuffer + ) -> Bool { + guard CVPixelBufferLockBaseAddress(source, .readOnly) == kCVReturnSuccess else { + return false + } + defer { CVPixelBufferUnlockBaseAddress(source, .readOnly) } + guard CVPixelBufferLockBaseAddress(destination, []) == kCVReturnSuccess else { + return false + } + defer { CVPixelBufferUnlockBaseAddress(destination, []) } + + let planeCount = CVPixelBufferGetPlaneCount(source) + guard planeCount == CVPixelBufferGetPlaneCount(destination) else { return false } + if planeCount == 0 { + guard let sourceBase = CVPixelBufferGetBaseAddress(source), + let destinationBase = CVPixelBufferGetBaseAddress(destination), + CVPixelBufferGetDataSize(source) == CVPixelBufferGetDataSize(destination) else { + return false + } + memcpy(destinationBase, sourceBase, CVPixelBufferGetDataSize(source)) + return true + } + + for plane in 0.. CVPixelBuffer { + var buffer: CVPixelBuffer? + let result = CVPixelBufferCreate(kCFAllocatorDefault, width, height, + pixelFormat, + [kCVPixelBufferIOSurfacePropertiesKey: NSDictionary()] as NSDictionary, &buffer) + #expect(result == kCVReturnSuccess) + return try #require(buffer) + } + + private func normalize(_ buffer: CVPixelBuffer, with normalizer: VideoToolboxOutputMetadataNormalizer, + modifiable: Bool = false, ratio: AVRational? = nil, + primaries: CFString? = nil) -> CVPixelBuffer { + normalizer.applyingOutputMetadata(to: buffer, isModifiable: modifiable, + colorPrimaries: primaries, transferFunction: nil, yCbCrMatrix: nil, + pixelAspectRatio: ratio) + } + + private func aspect(_ buffer: CVPixelBuffer) -> NSDictionary? { + CVBufferCopyAttachment(buffer, kCVImageBufferPixelAspectRatioKey, nil) as? NSDictionary + } + + @Test("immutable output is copied before adding display metadata") + func immutableOutputRetainsItsAttachments() throws { + let input = try makeOutput() + let customKey = "test.preserved" as CFString + CVBufferSetAttachment(input, customKey, "value" as CFString, .shouldPropagate) + let normalizer = VideoToolboxOutputMetadataNormalizer() + let output = normalize(input, with: normalizer, ratio: rational(64, 45), + primaries: kCVImageBufferColorPrimaries_ITU_R_709_2) + #expect(output !== input) + #expect(aspect(input) == nil) + #expect(CVBufferCopyAttachment(input, kCVImageBufferColorPrimariesKey, nil) == nil) + #expect(aspect(output)?[kCVImageBufferPixelAspectRatioHorizontalSpacingKey] as? Int == 64) + #expect(CVBufferCopyAttachment(output, customKey, nil) as? String == "value") + } + + @Test("modifiable output gets metadata in place") + func modifiableOutputIsReused() throws { + let input = try makeOutput() + let output = normalize(input, with: VideoToolboxOutputMetadataNormalizer(), + modifiable: true, ratio: rational(64, 45)) + #expect(output === input) + #expect(aspect(input)?[kCVImageBufferPixelAspectRatioVerticalSpacingKey] as? Int == 45) + } + + @Test("matching metadata needs no copy even on an immutable output") + func matchingOutputIsReused() throws { + let input = try makeOutput() + let normalizer = VideoToolboxOutputMetadataNormalizer() + _ = normalize(input, with: normalizer, modifiable: true, ratio: rational(64, 45)) + let output = normalize(input, with: normalizer, ratio: rational(64, 45)) + #expect(output === input) + } + + @Test("square-pixel output removes stale PAR without mutating an immutable source") + func staleRatioIsRemovedFromOwnedCopy() throws { + let input = try makeOutput() + let normalizer = VideoToolboxOutputMetadataNormalizer() + _ = normalize(input, with: normalizer, modifiable: true, ratio: rational(64, 45)) + let output = normalize(input, with: normalizer) + #expect(output !== input) + #expect(aspect(output) == nil) + #expect(aspect(input) != nil) + } + + @Test("owned copies retain pixels and follow format changes") + func copiedPixelsSurvivePoolReconfiguration() throws { + let normalizer = VideoToolboxOutputMetadataNormalizer() + for width in [16, 32, 16] { + let input = try makeOutput(width: width) + #expect(CVPixelBufferLockBaseAddress(input, []) == kCVReturnSuccess) + let base = try #require(CVPixelBufferGetBaseAddress(input)) + memset(base, 0x59, CVPixelBufferGetDataSize(input)) + CVPixelBufferUnlockBaseAddress(input, []) + let output = normalize(input, with: normalizer, ratio: rational(64, 45)) + #expect(output !== input) + #expect(CVPixelBufferGetWidth(output) == width) + #expect(CVPixelBufferLockBaseAddress(output, .readOnly) == kCVReturnSuccess) + let copied = try #require(CVPixelBufferGetBaseAddress(output)) + #expect(copied.load(as: UInt8.self) == 0x59) + CVPixelBufferUnlockBaseAddress(output, .readOnly) + } + normalizer.reset() + let input = try makeOutput() + #expect(normalize(input, with: normalizer, ratio: rational(64, 45)) !== input) + } + @Test("owned copies preserve bi-planar decoder output", + arguments: [kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange, + kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange]) + func biPlanarPixelsArePreserved(pixelFormat: OSType) throws { + let input = try makeOutput(pixelFormat: pixelFormat) + #expect(CVPixelBufferLockBaseAddress(input, []) == kCVReturnSuccess) + for plane in 0..