fix(video): preserve immutable VideoToolbox output buffers

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

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
     )
 }
 

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..<planeCount {
+            guard let sourceBase = CVPixelBufferGetBaseAddressOfPlane(source, plane),
+                  let destinationBase = CVPixelBufferGetBaseAddressOfPlane(destination, plane) else {
+                return false
+            }
+            let rows = min(
+                CVPixelBufferGetHeightOfPlane(source, plane),
+                CVPixelBufferGetHeightOfPlane(destination, plane)
+            )
+            let sourceBytesPerRow = CVPixelBufferGetBytesPerRowOfPlane(source, plane)
+            let destinationBytesPerRow = CVPixelBufferGetBytesPerRowOfPlane(destination, plane)
+            let bytesPerRow = min(sourceBytesPerRow, destinationBytesPerRow)
+            for row in 0..<rows {
+                memcpy(
+                    destinationBase.advanced(by: row * destinationBytesPerRow),
+                    sourceBase.advanced(by: row * sourceBytesPerRow),
+                    bytesPerRow
+                )
+            }
+        }
+        return true
+    }
+
+    private func logFailureOnce() {
+        lock.lock()
+        let shouldLog = !loggedFailure
+        loggedFailure = true
+        lock.unlock()
+        if shouldLog {
+            EngineLog.emit(
+                "[HardwareVideoDecoder] unable to normalize immutable frame metadata",
+                category: .swPlayback
+            )
+        }
+    }
+
+    deinit {
+        reset()
+    }
+}