Skip to content

fix(muxer): discard incomplete HEVC headers without losing valid picture or RPU data - #960

Draft
Nebr88 wants to merge 2 commits into
superuser404notfound:mainfrom
Nebr88:fix/truncated-hevc-header
Draft

Nebr88 wants to merge 2 commits into
superuser404notfound:mainfrom
Nebr88:fix/truncated-hevc-header

Conversation

@Nebr88

@Nebr88 Nebr88 commented Nov 1, 2026 •

Copy link
Copy Markdown

A length-prefixed HEVC sample can walk exactly while containing a one-byte EOS header. Apple's native path can reject its whole fMP4 segment even when the sample also contains a valid picture and following Dolby Vision RPU.

Remove only incomplete two-byte HEVC headers after framing is confirmed. Valid pictures/RPUs and timestamps survive, shared packet storage is made writable before compaction, and a header-only sample is dropped. Unconfirmed framing and H.264 retain their existing behavior.

Maintainer feedback addressed: doc-comment/property pairing, unrelated import reorder, actual muxer round-trip/COW/all-short-drop tests, changelog and format docs. Rebased onto main a6fbf0a (4 November). The optional combined NAL walk remains outside this correctness fix.

Validation on Apple Silicon, macOS26.7.1, Xcode27.2 / Swift6.4.2:

  • isolated branch: HEVC/NAL targeted16 Swift Testing +4 API XCTest pass;
  • docs links, test hygiene and diff whitespace pass;
  • full suite attempted: NOT green. APACEncodePipelineTests byte equality and StartAfterCuePrewarmTests request-shape checks fail in this run. APAC implementation and its test are unchanged by this PR; do not treat this as successful CI.
  • fresh downstream AppleTV4K2/A12 tvOS27.2 run, engine7.56.0 plus both local PR patches: original P7 transport-stream tail, EAC3, P7-to-P8.1 native route reaches ended. Exactly one incomplete header removed while preserving5724 sample bytes; no software escalation. This is combined downstream evidence, not an isolated PR-branch hardware run. No private media or logs are published.

Still draft pending isolated hardware qualification and investigation of the full-suite failures. Historical passing counts describe earlier commits, not this head.

@superuser404notfound

Copy link
Copy Markdown
Owner

Thanks for this, nice catch. A complete length chain with a one-byte EOS header is exactly the shape the #561 sanitizer cannot see, and keeping the picture and the trailing RPU instead of cutting the sample is the right call. I built the branch locally and the NAL sanitizer and new header suites pass.

Happy to take it once it is out of draft and your hardware run is done. A few small things for when you get back to it:

  1. MP4SegmentMuxer.swift: the two new properties landed between the /// Latched from VideoConfig.convertsAnnexBSamples. doc comment and convertsAnnexBVideoSamples, so that comment now documents isHEVCVideo. Moving them above or below the pair fixes it.
  2. NALUnitChain.swift: the import reorder is unrelated to the fix, please drop it to keep the diff focused.
  3. The tests cover shortHEVCHeaderRanges only. A muxer-level case (a sample with a short header in the middle gets compacted and keeps its size consistent, and a sample made only of short headers is dropped) would pin the memmove / av_shrink_packet path too.
  4. Please rebase onto current main, which has moved on since 7.54.0.

Optional: the #561 cut and this pass now walk the same chain twice per HEVC sample. Cheap enough to leave as is, just noting it in case folding them into one walk turns out simple.

@Nebr88
Nebr88 force-pushed the fix/truncated-hevc-header branch from 7c45b8e to d8b9a77 Compare November 4, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants