A video sample whose NAL chain overruns it is cut, not handed to Apple's parser (AE#561)

Scripts/nal-overrun-fixture.py

@@ -0,0 +1,105 @@
+#!/usr/bin/env python3
+"""Overrun one sample's last NAL length field, the way a damaged remux carries it (AE#561).
+
+No encoder writes this file for you. A reporter's Blu-ray remux carried HEVC packets whose last
+length prefix declared 384137139 bytes with 350873 left in the packet, and the shape matters
+because the two consumers answer it differently: libavcodec logs "Invalid NAL unit size" and skips
+the frame, so the file plays in mpv, while Apple's fMP4 parser answers the whole SEGMENT with
+CoreMediaErrorDomain -19602, which ends the session and every reload onto that segment. MKVToolNix
+drops the unparsable tail on remux, which is why remuxing such a file fixes it.
+
+The patch is four bytes wide and moves nothing else: same file size, same block boundaries, same
+timestamps, so the healthy source is a true control arm rather than a second encode.
+
+    # several NALs per packet, so the truncation leaves a picture behind rather than nothing
+    ffmpeg -f lavfi -i testsrc=size=1280x720:rate=25:duration=8 -c:v libx265 -preset ultrafast \\
+           -x265-params "bframes=3:keyint=50:slices=4:aud=1:log-level=0" -pix_fmt yuv420p healthy.mkv
+    python3 Scripts/nal-overrun-fixture.py healthy.mkv damaged.mkv
+
+    aetherctl play --seconds 14 file://$PWD/healthy.mkv    # control: VERDICT: OK
+    aetherctl play --seconds 14 file://$PWD/damaged.mkv    # before the fix: -19602 at once
+
+Works on any mp4/mkv whose video track is length-prefixed (avcC / hvcC framing); Annex B sources
+carry no length fields to overrun and are rejected here.
+"""
+import argparse
+import shutil
+import struct
+import subprocess
+import sys
+
+# The reporter's own impossible length, kept verbatim so a log line from the fixture and one from
+# the field read the same.
+OVERRUN_LENGTH = 0x16E577B3
+
+
+def packet_rows(path):
+    out = subprocess.run(
+        ["ffprobe", "-v", "error", "-select_streams", "v", "-show_packets",
+         "-show_entries", "packet=pts_time,size,pos", "-of", "csv=p=0", path],
+        capture_output=True, text=True, check=True).stdout.strip().split("\n")
+    rows = []
+    for line in out:
+        pts, size, pos = line.split(",")[:3]
+        if pos == "N/A" or size == "N/A":
+            continue
+        rows.append((float(pts) if pts != "N/A" else 0.0, int(size), int(pos)))
+    return rows
+
+
+def chain(blob):
+    """The NAL lengths in `blob`, or None when it is not a clean length-prefixed run."""
+    offset, units = 0, []
+    while offset + 4 <= len(blob):
+        length = struct.unpack(">I", blob[offset:offset + 4])[0]
+        if length == 0 or offset + 4 + length > len(blob):
+            return None
+        units.append((offset, length))
+        offset += 4 + length
+    return units if offset == len(blob) else None
+
+
+def locate_payload(data, pos, size):
+    """ffprobe reports the element's position, not the frame's; the payload sits a few bytes in."""
+    for delta in range(-8, 40):
+        units = chain(data[pos + delta:pos + delta + size])
+        if units:
+            return pos + delta, units
+    return None, None
+
+
+def main():
+    ap = argparse.ArgumentParser(description=__doc__,
+                                 formatter_class=argparse.RawDescriptionHelpFormatter)
+    ap.add_argument("source")
+    ap.add_argument("destination")
+    ap.add_argument("--after", type=float, default=3.0,
+                    help="patch the first eligible packet past this presentation time (s)")
+    ap.add_argument("--min-units", type=int, default=2,
+                    help="how many NAL units the packet must hold, so the cut leaves something")
+    args = ap.parse_args()
+
+    data = open(args.source, "rb").read()
+    for pts, size, pos in packet_rows(args.source):
+        if pts < args.after:
+            continue
+        payload, units = locate_payload(data, pos, size)
+        if units and len(units) >= args.min_units:
+            break
+    else:
+        sys.exit("no length-prefixed packet with %d units past %.3fs (Annex B source?)"
+                 % (args.min_units, args.after))
+
+    last_offset, last_length = units[-1]
+    shutil.copyfile(args.source, args.destination)
+    with open(args.destination, "r+b") as handle:
+        handle.seek(payload + last_offset)
+        handle.write(struct.pack(">I", OVERRUN_LENGTH))
+
+    print("packet pts=%.3fs size=%d units=%s" % (pts, size, [n for _, n in units]))
+    print("last length %d -> %d, with %d bytes left in the packet"
+          % (last_length, OVERRUN_LENGTH, size - last_offset - 4))
+
+
+if __name__ == "__main__":
+    main()

Sources/AetherEngine/Video/MP4SegmentMuxer.swift

@@ -163,6 +163,12 @@ final class MP4SegmentMuxer {
     /// from codecpar alone, so they never wedge, and gating the #64 RAM-cap flush on them would needlessly
     /// weaken that memory bound. Latched at init from the audio codec_id.
     private let audioNeedsParsedPacketForMoov: Bool
+    /// Width of the video track's NAL length prefix (avcC / hvcC), nil when the track is not
+    /// length-prefixed at all. Latched at init: it is a property of the configuration record that
+    /// lands in the sample entry, and the AE#561 sanitizer walks every video sample with it.
+    private let videoNALLengthPrefixSize: Int?
+    /// How many video samples the AE#561 sanitizer has had to cut, over this muxer's life.
+    private var truncatedVideoSamples: Int = 0
 
     /// Only AC-3 / E-AC-3 / TrueHD build their mp4 sample entry from a parsed packet (dac3/dec3/dmlp),
     /// so only they can hit the "moov before audio parsed" wedge and need the #64-flush guard. Shared with
@@ -239,6 +245,24 @@ final class MP4SegmentMuxer {
         self.audioDelaySeconds = audioDelaySeconds
         self.audioNeedsParsedPacketForMoov =
             audio.map { Self.audioNeedsParsedPacketForMoov($0.codecpar.pointee.codec_id) } ?? false
+        // AE#561: the override, when there is one, is the record that reaches the sample entry. Both
+        // carry the same width (the #19 rebuild keeps the source header's first 22 bytes), so this
+        // only matters for a source whose own extradata is missing or Annex B.
+        if let override = video.extradataOverride {
+            self.videoNALLengthPrefixSize = override.withUnsafeBufferPointer {
+                NALUnitChain.lengthPrefixSize(
+                    codecID: video.codecpar.pointee.codec_id,
+                    extradata: $0.baseAddress,
+                    extradataSize: $0.count
+                )
+            }
+        } else {
+            self.videoNALLengthPrefixSize = NALUnitChain.lengthPrefixSize(
+                codecID: video.codecpar.pointee.codec_id,
+                extradata: UnsafePointer(video.codecpar.pointee.extradata),
+                extradataSize: Int(video.codecpar.pointee.extradata_size)
+            )
+        }
 
         let firstPath = Self.stagingPath(forSegmentIndex: initialSegmentIndex,
                                          in: sessionDir)
@@ -585,6 +609,37 @@ final class MP4SegmentMuxer {
             }
         }
 
+        // AE#561: a damaged source can carry a video sample whose NAL chain declares a unit that
+        // reaches past the end of the packet. Apple's fMP4 parser walks that chain by addition and
+        // answers the whole segment with -19602, which kills the session and every reload onto the
+        // same segment; libavcodec's own decoder answers such a packet by skipping the frame, which
+        // is why the file plays elsewhere. Cut the sample at its last complete NAL, which is what
+        // MKVToolNix writes when it remuxes one of these files.
+        if streamIndex == videoOutputStreamIndex,
+           let lengthPrefixSize = videoNALLengthPrefixSize,
+           let data = packet.pointee.data,
+           packet.pointee.size > 0,
+           let complete = NALUnitChain.completeRunLength(
+               UnsafeRawBufferPointer(start: data, count: Int(packet.pointee.size)),
+               lengthPrefixSize: lengthPrefixSize
+           ) {
+            truncatedVideoSamples += 1
+            if truncatedVideoSamples <= 5 || truncatedVideoSamples % 100 == 0 {
+                EngineLog.emit(
+                    "[MP4SegmentMuxer] #561 video sample at dts=\(packet.pointee.dts) carries an "
+                    + "incomplete NAL chain: \(packet.pointee.size) bytes, \(complete) of them "
+                    + "complete; cut to the last whole unit (#\(truncatedVideoSamples) this muxer)",
+                    category: .session
+                )
+            }
+            if complete == 0 {
+                // Nothing in the sample survives the walk, so there is no picture to hand over.
+                av_packet_unref(packet)
+                return (0, .none)
+            }
+            av_shrink_packet(packet, Int32(complete))
+        }
+
         // av_write_frame was tried as a leak hypothesis; no impact on 8 MB/s mallocMB growth
         // (leak was Data(d) dispatch_data aliasing in AVIOReader). Reverted to interleaved for
         // cross-stream DTS monotonicity and audio+video re-ordering via libavformat.

Sources/AetherEngine/Video/NALUnitChain.swift

@@ -0,0 +1,65 @@
+import Foundation
+import AetherLibavcodec
+import AetherLibavutil
+
+/// The length-prefixed NAL chain (avcC / hvcC framing) as Apple's fMP4 parser walks it.
+///
+/// A sample in an mp4 video track is a run of NAL units, each introduced by a big-endian length of
+/// the width the configuration record declares. The parser walks that run by addition, so a length
+/// that reaches past the end of the sample leaves it with no way to continue: VideoToolbox answers
+/// the whole segment with `CoreMediaErrorDomain -19602` and the session is over.
+///
+/// Damaged sources carry such a sample (AE#561: a Blu-ray remux whose sixth length field declared
+/// 384137139 bytes with 350873 left in the packet). libavcodec logs `Invalid NAL unit size` and
+/// skips the frame, which is why such files play in mpv, and MKVToolNix drops the unparsable tail on
+/// remux, which is why remuxing them fixes them. This does the same thing one step before the write.
+enum NALUnitChain {
+
+    /// Length-prefix width declared by an avcC (H.264) or hvcC (HEVC) configuration record.
+    ///
+    /// Nil for any other codec, for Annex B extradata (which starts with a start code, not a
+    /// configuration version), and for a record too short to hold the field: those payloads are not
+    /// length-prefixed chains and must not be walked as one.
+    static func lengthPrefixSize(
+        codecID: AVCodecID,
+        extradata: UnsafePointer<UInt8>?,
+        extradataSize: Int
+    ) -> Int? {
+        guard let extradata, extradataSize > 0, extradata[0] == 1 else { return nil }
+        switch codecID {
+        case AV_CODEC_ID_HEVC:
+            guard extradataSize >= 23 else { return nil }
+            return Int(extradata[21] & 0x03) + 1
+        case AV_CODEC_ID_H264:
+            guard extradataSize >= 5 else { return nil }
+            return Int(extradata[4] & 0x03) + 1
+        default:
+            return nil
+        }
+    }
+
+    /// Byte length of the leading run of complete NAL units, or nil when the payload already ends on
+    /// one and nothing needs to change.
+    ///
+    /// Zero means not one NAL unit in the payload is complete, which leaves the caller nothing to
+    /// write.
+    static func completeRunLength(_ bytes: UnsafeRawBufferPointer, lengthPrefixSize: Int) -> Int? {
+        let count = bytes.count
+        guard (1...4).contains(lengthPrefixSize), count >= lengthPrefixSize else { return nil }
+        // An Annex B payload is not this framing at all, and reading its start code as a length would
+        // cut every frame of a healthy stream down to nothing.
+        if count >= 3, bytes[0] == 0, bytes[1] == 0, bytes[2] == 1 { return nil }
+        if count >= 4, bytes[0] == 0, bytes[1] == 0, bytes[2] == 0, bytes[3] == 1 { return nil }
+
+        var offset = 0
+        while offset + lengthPrefixSize <= count {
+            var length = 0
+            for i in 0..<lengthPrefixSize {
+                length = (length << 8) | Int(bytes[offset + i])
+            }
+            guard length > 0, offset + lengthPrefixSize + length <= count else { break }
+            offset += lengthPrefixSize + length
+        }
+        return offset == count ? nil : offset
+    }
+}

Tests/AetherEngineTests/NALUnitChainTests.swift

@@ -0,0 +1,118 @@
+// A video sample carrying a length-prefixed NAL chain that overruns its own payload (AE#561).
+//
+// A damaged Blu-ray remux handed one HEVC packet whose sixth length field declared 384137139 bytes
+// with 350873 left in the packet. libavcodec answers such a packet with "Invalid NAL unit size" and
+// skips the frame, which is why the file plays in mpv; Apple's fMP4 parser answers the whole segment
+// with CoreMediaErrorDomain -19602 and the session is over, with the reload dying on the same
+// segment. The muxer therefore truncates the sample at the last complete NAL before it is written,
+// which is byte for byte what MKVToolNix does to that packet and what makes the file play.
+import Foundation
+import Testing
+import AetherLibavcodec
+import AetherLibavutil
+@testable import AetherEngine
+
+@Suite("Length-prefixed NAL chain sanitizer (AE#561)")
+struct NALUnitChainTests {
+
+    /// `lengthPrefixSize` bytes of big-endian length followed by `payload` bytes of body.
+    private static func nal(_ body: [UInt8], lengthPrefixSize: Int = 4) -> [UInt8] {
+        var out: [UInt8] = []
+        let n = body.count
+        for shift in stride(from: (lengthPrefixSize - 1) * 8, through: 0, by: -8) {
+            out.append(UInt8((n >> shift) & 0xFF))
+        }
+        return out + body
+    }
+
+    private static func run(_ bytes: [UInt8], lengthPrefixSize: Int = 4) -> Int? {
+        bytes.withUnsafeBytes { NALUnitChain.completeRunLength($0, lengthPrefixSize: lengthPrefixSize) }
+    }
+
+    @Test("A chain that ends on a complete NAL is left alone")
+    func healthyChainIsUntouched() {
+        let bytes = Self.nal([0x26, 0x01, 0xAA, 0xBB]) + Self.nal([0x02, 0x01, 0xCC]) + Self.nal([0x02, 0x01])
+        #expect(Self.run(bytes) == nil)
+    }
+
+    @Test("A declared length past the end truncates at the last complete NAL")
+    func overrunTruncates() {
+        let good = Self.nal([0x26, 0x01, 0xAA, 0xBB]) + Self.nal([0x02, 0x01, 0xCC, 0xDD, 0xEE])
+        // The reporter's shape: a length field far larger than what is left in the packet.
+        let broken: [UInt8] = [0x16, 0xE5, 0x77, 0xB3] + [UInt8](repeating: 0x5A, count: 64)
+        #expect(Self.run(good + broken) == good.count)
+    }
+
+    @Test("A trailing stub shorter than one length field truncates too")
+    func trailingStubTruncates() {
+        let good = Self.nal([0x26, 0x01, 0xAA])
+        #expect(Self.run(good + [0x00, 0x02]) == good.count)
+    }
+
+    @Test("A zero-length NAL ends the run")
+    func zeroLengthEndsRun() {
+        let good = Self.nal([0x26, 0x01, 0xAA])
+        #expect(Self.run(good + Self.nal([])) == good.count)
+    }
+
+    @Test("A payload whose first NAL already overruns leaves nothing to write")
+    func nothingCompleteYieldsZero() {
+        #expect(Self.run([0x00, 0x10, 0x00, 0x00, 0x01, 0x02]) == 0)
+    }
+
+    /// An Annex B payload is not a length-prefixed chain, and reading its start code as a length
+    /// would truncate every frame of a healthy stream to nothing.
+    @Test("An Annex B payload is refused, in both start-code widths")
+    func annexBIsRefused() {
+        #expect(Self.run([0x00, 0x00, 0x00, 0x01, 0x26, 0x01, 0xAA, 0xBB]) == nil)
+        #expect(Self.run([0x00, 0x00, 0x01, 0x26, 0x01, 0xAA, 0xBB, 0xCC]) == nil)
+    }
+
+    @Test("The prefix width is honoured, not assumed to be four")
+    func widthIsHonoured() {
+        let two = Self.nal([0x26, 0x01, 0xAA], lengthPrefixSize: 2)
+            + Self.nal([0x02, 0x01], lengthPrefixSize: 2)
+        #expect(Self.run(two, lengthPrefixSize: 2) == nil)
+        #expect(Self.run(two + [0xFF, 0xF0, 0x01], lengthPrefixSize: 2) == two.count)
+    }
+
+    // MARK: - The width comes out of the configuration record
+
+    @Test("hvcC declares its width in byte 21, avcC in byte 4")
+    func widthFromConfigurationRecord() {
+        var hvcC = [UInt8](repeating: 0, count: 23)
+        hvcC[0] = 1
+        hvcC[21] = 0xFC | 0x03           // naluLengthSizeMinusOne = 3
+        #expect(Self.prefixSize(.hevc, hvcC) == 4)
+        hvcC[21] = 0xFC | 0x01
+        #expect(Self.prefixSize(.hevc, hvcC) == 2)
+
+        var avcC: [UInt8] = [1, 0x64, 0x00, 0x28, 0xFF, 0xE1]
+        #expect(Self.prefixSize(.h264, avcC) == 4)
+        avcC[4] = 0xFC | 0x00
+        #expect(Self.prefixSize(.h264, avcC) == 1)
+    }
+
+    @Test("Annex B extradata, a truncated record and a foreign codec declare no width")
+    func widthAbsent() {
+        #expect(Self.prefixSize(.hevc, [0x00, 0x00, 0x00, 0x01, 0x40, 0x01]) == nil)
+        #expect(Self.prefixSize(.hevc, [UInt8](repeating: 1, count: 22)) == nil)
+        #expect(Self.prefixSize(.h264, [1, 0x64, 0x00]) == nil)
+        #expect(Self.prefixSize(.av1, [UInt8](repeating: 1, count: 40)) == nil)
+        #expect(Self.prefixSize(.hevc, []) == nil)
+    }
+
+    private static func prefixSize(_ codec: Codec, _ extradata: [UInt8]) -> Int? {
+        let id: AVCodecID
+        switch codec {
+        case .hevc: id = AV_CODEC_ID_HEVC
+        case .h264: id = AV_CODEC_ID_H264
+        case .av1: id = AV_CODEC_ID_AV1
+        }
+        return extradata.withUnsafeBufferPointer {
+            NALUnitChain.lengthPrefixSize(codecID: id, extradata: $0.baseAddress, extradataSize: $0.count)
+        }
+    }
+
+    private enum Codec { case hevc, h264, av1 }
+}

docs/architecture.md

@@ -479,6 +479,7 @@ Sources/AetherEngine/
 │   ├── RestartCoalescer.swift               Coalesces a burst of producer-restart requests into one in-flight + one settled target (rapid-seek, AetherEngine#35)
 │   ├── LiveWindow.swift                     Live path: session-relative DVR timeline (seconds since first frame), shared by the native and SW live paths
 │   ├── MP4SegmentMuxer.swift                Native path: session-long fragmented-MP4 muxer (+empty_moov+default_base_moof+frag_custom+delay_moov)
+│   ├── NALUnitChain.swift                   Final-stage payload guard before the fMP4 mux: a video sample whose length-prefixed NAL chain overruns it is cut at its last complete unit, because Apple's parser answers one such sample with -19602 for the whole segment (AE#561)
 │   ├── AudioLanguageMap.swift               Native path: the ISO 639-2/T an audio track is declared with in the master's `EXT-X-MEDIA:TYPE=AUDIO` rendition and written into its `mdhd`; ICU in front of the twenty ISO 639-2/B bibliographic codes Matroska writes, then canonicalization for the ISO 639-3 members CLDR aliases and pass-through for the ones it does not, failing closed on anything ICU can neither resolve nor name (AE#458)
 │   ├── FragmentSplitter.swift               Native path: routes mp4 muxer's avio output stream into init.mp4 (ftyp+moov) vs per-segment moof+mdat files
 │   ├── PacketRingBuffer.swift               Live path: keyframe-indexed, disk-spooled packet ring backing the SW-path DVR rewind