fix(avc2mp4): produce playable mp4 when timestamps are out of order

[gmegidish]

Summary

screenrecord on real iOS devices produced mp4 files that ffmpeg decodes as 1 frame with a duration of ~353 hours, while mobilecli itself reported sane numbers (299 frames, 4.916s).

Two causes in pkg/avc2mp4:

1. Timecode attached to the wrong picture. devicekit-ios emits the MOBILENXTIMECODE SEI before the picture it stamps. groupAccessUnits assumed it trailed, so every access unit got the next frame's timestamp and SPS/PPS were flushed as a sample with no slice. The tests modelled the same wrong order, so they passed.
2. B-frames. Since the extension moved to H.264 High profile, VideoToolbox reorders frames, so timestamps arrive in decode order (e.g. …544384, …511052, …494386, …527718). Writing pts == dts from those made the unsigned deltas wrap.

Changes:

• A timecode is held until the next slice claims it; a slice with no fresh timecode is another slice of the same picture.
• SPS/PPS stay in the same access unit as their key frame.
• New sampleTimesMs: dts walks the sorted timestamps (monotonic) and every pts is pushed back by the largest reorder delay so dts <= pts. With no reordering the delay is zero and dts == pts.
• Convert reports duration from min/max timestamp rather than first/last in decode order.

The extension is also being fixed to stop emitting B-frames, but this keeps recordings from already-deployed extensions playable.

Test plan

• go test ./pkg/avc2mp4/ -race; fixtures now use the real SEI-before-slice order; new tests for parameter sets staying with their key frame and for reordered timestamps (real values from a device)
• Re-muxed a real 19s B-frame capture: 1152 frames, 1148 decoded, 19.18s, 60fps (was 1 frame / 353h)
• Real iPhone, fixed extension: 10s recording → 601 frames / 9.98s; 8s recording → 480 frames / 7.95s, I and P frames only
• Known cosmetic: ffmpeg warns about dts on the first two frames because a forced key frame is stamped with the host clock, ~1 frame ahead of the next real frame. Players cope; fix belongs in the extension.

Summary by CodeRabbit

• Bug Fixes
  • Improved AVC-to-MP4 duration calculations using the actual timestamp range.
  • Corrected timestamp association for parameter sets, slices, and SEI metadata.
  • Improved handling of reordered video frames so MP4 timestamps remain monotonic and accurate.
  • Excluded video pictures without timestamps from the output.
• Tests
  • Added coverage for reordered frames, timestamp matching, parameter-set retention, and SEI placement.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: 668aa866-62e2-4b14-ad15-6c585ee0e064

📥 Commits

Reviewing files that changed from the base of the PR and between 96bfebd and 3279f71.

📒 Files selected for processing (2)

• pkg/avc2mp4/mp4writer.go
• pkg/avc2mp4/mp4writer_test.go

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

---

📝 Walkthrough

Walkthrough

Changes

AVC timestamped MP4 conversion

Layer / File(s)	Summary
Timestamped access-unit grouping pkg/avc2mp4/mp4writer.go, pkg/avc2mp4/mp4writer_test.go	Access-unit grouping now waits for SPS data, assigns preceding timestamp SEIs to slices, retains parameter sets with keyframes, groups additional slices, and drops untimestamped pictures.
Duration and PTS/DTS generation pkg/avc2mp4/mp4writer.go, pkg/avc2mp4/mp4writer_test.go	Duration uses the minimum and maximum access-unit timestamps. Sample timing sorts decode timestamps and applies reorder delay. Tests cover reordered and in-order frames.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 3279f

No concrete merge-blocking playback, decoding, or conversion failure remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: fixing out-of-order timestamp handling so avc2mp4 produces playable MP4 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}