[sunghyun-k]

Summary

An MP4 whose writer dropped ctts and then forced the decode-order presentation times to be monotonic plays with its B pictures out of order on both routes (and in QuickTime and the Files app): every sample reports PTS == DTS, but the ladder has no single step, so the #409 probe skips it as "decode ladder is not uniform". This lets that probe rebuild both axes from the picture order and the stream's picture rate, under the same picture-order checks.

The bitstream itself is fine: an FFmpeg-based player (IINA, on mpv) reorders by picture order count and plays the same file correctly, so only the container's timestamps are wrong, and AVFoundation trusts them.

Refs #409 (this is the shape its uniform-ladder probe declines).

What changed

• H264CompositionOffsetRepair.classify takes an optional nominalStep (from avg_frame_rate and time_base). For a non-uniform ladder it is used only when the sampled span matches the time its pictures take at that rate (within the reorder delay plus two pictures, plus a tick per picture for rounding) and every existing check still passes (PTS == DTS, POC 0 keyframe start, picture order regresses, ranks distinct and fill the window).
• The step is remeasured on the anchors when the writer left them on the presentation axis. When it pushed the anchors too (all three reporting files did), the rate is the one the bitstream's VUI timing states, if that is within 1% of the average rate, snapped to a standard rate within 0.01%; without a VUI rate, the standard rate (whole rates up to 240, NTSC rates) nearest the average rate within 1%. Pictures are placed on that rate as an exact fraction of a tick (Plan.pictureCadence), so 23 fps (3913.04 ticks) does not lose a tick every 25 pictures.
• Plan.synthesizesDecodeLadder: the rewriter rebuilds DTS from each picture's decode slot instead of shifting it. A pushed B picture's DTS can sit after its own presentation time, which PTS >= DTS would reject.
• The writer pushes keyframes too, only ever later (0 to 4 pictures past the GOP before). A later IDR continues the sequence before it, up to the reorder delay plus two and a half pictures (Plan.keyframeAdvanceLimit); an earlier one keeps its own time. A continued sequence also leans toward the earliest origin the pictures before it point to, by at most an eighth of a picture per GOP, because the writer's clock is not exactly the stated rate (one file states 29.0002 fps and writes 29.003). A seek landing holds its first 8 pictures and takes the earliest origin they point to.
• A head that opens on a run of P pictures (one file opens every GOP on twelve) is sampled on to its first regression plus a mini-GOP, up to 48 pictures.
• HLS: a keyframe moved back onto its sequence sits before its index entry, and the segment plan is built from the index. VODSegmentCutter and the restart gate now reach back for the keyframe each boundary names (Demuxer.indexedKeyframeAdvanceTicks), bounded per boundary below the keyframe before it (HLSVideoEngine.keyframeReachTicks). 0 for every other source, so nothing else moves.
• Shift for this case is start_time - first index DTS, clamped to maximumNominalShiftPictures (16) steps.
• Order in decide(): the first pass runs without nominalStep, so the existing uniform, quantized and missing-head (fix(video): repair partial H.264 MP4 composition offsets #513) paths keep first claim. The log line gains ladder=nominal-rate.
• Tests: H264ForcedMonotonicLadderRepairTests (embedded 64x64 twin pairs: clamped, open-GOP, pushed anchors, pushed keyframe, leading P run; each demuxes to its healthy twin's timestamps, also after a seek; rate selection; the lean; a moved keyframe stays within the reach) and RepairedKeyframeSegmentReachTests (cutter, per-boundary reach, restart gate). DocumentedConstantsTests pins 16, 48 and 8.
• Scripts/make-clamped-ladder-fixture.sh generates 1280x720 twins (clamped-ladder, pushed-anchors, and p-run with the x264 CLI) and, with --check, prints the probe verdict and swdecode's backward steps.
• docs/formats.md, docs/architecture.md, CHANGELOG.md.

Test plan

• Device / OS: iPhone Duo, iOS 27.2 RC (a host app playing through the native route); macOS 27 (aetherctl).

• Source media: three real H.264 MP4s from writers with this shape (not shareable; the script reproduces the shape):

  1. 720p, 29.97 fps, 23 min, GOP 90, AAC. Anchors pushed as well.
  2. 23 fps, 53 min, GOP 90. Every GOP opens on twelve P pictures; keyframes pushed 0 to 4 pictures.
  3. 29 fps (VUI 29.0002, writer clock 29.003), 19 min, GOP 90, video only. Keyframes pushed four pictures for long stretches.

  Plus generated twins, and a sweep of 144 generated pairs: H.264 and HEVC, 23.976 / 24 / 25 / 29.97 / 50 / 59.94 / 23 / 29 fps, GOP 30 / 90 / 250, three push patterns (B only, B and anchors, every third picture including keyframes).

• Result:

  • Real files on device: out of order before, in order after (all three, at 0bed68c, 8bbb7cd and 734477e).
  • Real files, aetherctl pktdump (whole file): no duplicate or out-of-order presentation time, no gap or overlap except at the final picture of files 2 and 3; the timeline keeps a constant distance to the writer's own (lower envelope of its timestamps) from start to end. Before the lean, file 3 drifted 61 ms behind and overlapped 3 times.
  • Real files, aetherctl segverify: 40/40 segments independently decodable from 0, and 3/3 from a restart at segments 60, 150 and 230, on all three; Scripts/segment-spans.py contiguous. Without the cutter reach, 25 and 22 of 40 segments of files 2 and 1 opened mid-GOP.
  • Sweep: every pair demuxes in the healthy twin's order with no gap (144/144), and all 1392 segments verified are independent. Seeks match the twin exactly in 323 of 432; the rest are pairs whose keyframes were pushed, where a seek cannot know the push: median error 0.2 ms, p90 27.8 ms, max 72.5 ms.
  • Generated twin, AVPlayer route (aetherctl serve, frame number read from each picture): 169 backward steps before, 0 after. Software route (aetherctl swdecode --frames 300): 44 of 105 before, 0 after.
  • Unchanged verdicts and timestamps on the 15 MP4 test media files (files without ctts that already repaired, partially missing ctts, HEVC, healthy, interlaced).
  • swift test on macOS 27: XCTest 801 passed; Swift Testing 5050 with 1 failure in Issue848AtmosActivatorPauseGraceTests (timing, unrelated, passes alone 3 of 3). Scripts/check-doc-links.py clean. The all-traits configuration was not run locally.

Checklist

• CHANGELOG.md updated
• Commit messages follow Conventional Commits (feat(...), fix(...), chore(...))
• The fix lives in the engine, not in a host-side workaround
• Public API changes are intentional and documented (none; all new members are internal)

🤖 Generated with Claude Code

[github-actions]

Automated review

The PR extends the #409 missing-ctts probe to a writer that forced its decode-order timestamps to be monotonic. It takes the step from avg_frame_rate, remeasures it on the anchors and rebuilds DTS from decode slots. The direction is right: the fix is in the engine, it addresses the cause, it runs only after the existing uniform, quantized and missing-head paths have declined, and it keeps every existing picture-order check.

Findings

• Should fix: After a seek that lands on an open-GOP keyframe, the synthesized-ladder re-anchor is off by the number of leading pictures. The landing keyframe's container DTS is its presentation time here, not its decode slot. (Sources/AetherEngine/Video/H264CompositionOffsetRepair.swift:613)
• Should fix (question): If anchorStep declines, the plan goes ahead on the rounded avg_frame_rate step, which the PR itself describes as unreliable. Consider failing closed there. (Sources/AetherEngine/Video/H264CompositionOffsetRepair.swift:323)

Checklist: CHANGELOG yes, docs yes, tests yes (twin fixtures, seek, anchor step, variable-rate head; nothing yet covers an open-GOP landing or a failed anchor measurement), traits both ways n.a.

Needs a device run: an AVPlayer-route check of a seek into an open-GOP file of this shape, if one exists in the wild. The closed-GOP twin is already covered by the PR's own measurements.

Thanks for the careful write-up, the twin fixture and the script that reproduces the timestamp shape.

_{Automated review by Claude of c040521. It reads the code, it does not build or run it; a maintainer reviews and merges.}

[github-actions]

On a synthesized ladder this re-anchor uses the regular ladder's formula, and that formula does not hold here. It computes anchor = dts - (landingIndex - leading) * step. That is right when the container DTS is the landing's decode slot. But in the shape this PR targets, an anchor keeps its own presentation time: an open-GOP I picture displays after everything decoded before it, so max(PTS, PREV_OUTDTS+1001) leaves it alone. Its container DTS is then trueAnchor + landingIndex * step, so the computed anchor comes out leading * step too late. Every PTS in that sequence would then be shifted by the number of leading pictures, and so would the new DTS through slot.

It only bites after a seek onto a non-IDR keyframe with leading pictures (POC != 0, noteLanding(leadingPictures:) > 0). The closed-GOP fixtures do not reach it. A possible fix: when plan.synthesizesDecodeLadder, set anchor = dts - landingIndex * step and keep decodeOrdinal = anchorSlots. A unit test on Rewriter with a hand-built open-GOP landing would pin it.

[github-actions]

When anchorStep returns nil, step stays at the rounded avg_frame_rate value. The comment just above says that value can be wrong (29.80 for a 29.97 clip, i.e. 3020 against 3003). Within a coded video sequence PTS is anchor + displayIndex * step and DTS is anchor + slot * step, so the error grows by 17 ticks a picture until the next IDR. On a long GOP that reaches more than a frame, which could misplace the pictures that pass the pts >= newDTS guard against the audio and against the next sequence's anchor.

The anchors are the only evidence this path has that the stream really runs at one rate. Should a nil here return .inconclusive("anchors do not agree on a step"), so the probe fails closed like the rest of classify? If there is a measured case where anchors disagree and the nominal step still repairs correctly, a comment quoting it would help.

[sunghyun-k]

Thanks, both findings were real. Fixed in the new commit.

Open-GOP landing. Reproduced with an open-GOP twin pair (open-gop=1, the second keyframe is a non-IDR I picture with a leading B): after a seek to 1.2 s the clamped twin did not match the healthy one. Two things were wrong:

• The re-anchor used the landing's decode slot. On this ladder the keyframe kept its own presentation time, so the origin is now landingIndex slots back, and the decode slot stays landingIndex - leading. Reverting only that line makes the new seek test fail again.
• After the seek the parser is reset, so the H.264 non-IDR landing reads its picture order relative to itself (it came out at -4, its leading B at -6), and the pictureOrderCount >= 0 guard left both in decode order. A synthesized-ladder sequence anchored on a landing now accepts relative counts. The pts >= dts check still guards every picture.

The new tests embed the open-GOP twins (64x64, 60 pictures): a full play-through and the seek onto the open-GOP keyframe both match the healthy twin packet for packet.

Failed anchor measurement. Agreed. If the anchors do not agree on a step, the head is now left untouched (anchors do not agree on a step) instead of falling back to the rounded rate. There is a test for that, and docs/formats.md says so.

Not covered: a device run of an open-GOP file of this shape. The open-GOP case is only checked with the generated twins.

[github-actions]

Automated review

This is a re-review at d6bd646. The PR extends the #409 missing-ctts probe to a writer that forced its decode ladder to be monotonic. The direction is still right: the repair is in the engine, runs only after the existing uniform, quantized and missing-head paths have declined, and keeps every picture-order check.

Findings

• Fixed: the open-GOP landing re-anchor. originSlots is now landingIndex on a synthesized ladder, and the decode slot stays landingIndex - leading. A play-through test and a seek test on an open-GOP twin cover it.
• Fixed: a failed anchor measurement. The probe now returns anchors do not agree on a step instead of falling back to the rounded rate. A test covers it, and docs/formats.md documents it.
• No new findings. One change to note: on a synthesized ladder anchored on a seek landing, the pictureOrderCount >= 0 guard is relaxed. The relaxation is limited to that case (synthesizesDecodeLadder && anchoredOnLanding), it is reset at the next IDR and on noteSeek, and every picture still passes the pts >= newDTS check.

Checklist: CHANGELOG yes, docs yes, tests yes, traits both ways n.a.

Needs a device run: the PR's device and AVPlayer-route measurements cover the closed-GOP case. Only the generated twins check the open-GOP case, so a device run of a real open-GOP file of this shape would settle it, if one exists.

Thanks for the quick, well-tested follow-up on both points.

_{Automated review by Claude of d6bd646. It reads the code, it does not build or run it; a maintainer reviews and merges.}

[sunghyun-k]

Four more commits, from two more real files of the same writer family and a generated sweep. The description is updated; the short version:

One decision reversed. d6bd646 left a head untouched when the anchors disagree on a step. The first reporting file turned out to push its P pictures too (each anchor 1001 ticks past the picture before it), so no two anchors agree and the file was no longer repaired. 4530d68 takes the picture rate instead; 8bbb7cd reads it from the bitstream's VUI timing (trusted only within 1% of the average rate), because the average of a short clip with a pushed tail cannot tell 24 from 23.976. A head whose span does not fit the rate is still left alone.

What the other two files showed.

• Keyframes are pushed too, 0 to 4 pictures past the GOP before, which left a gap or an overlap at most GOP boundaries (522 of 823 on the second file). A later IDR now continues the sequence before it (dea36f4, bounds widened in 0bed68c).
• The second file opens every GOP on twelve P pictures, so the 12-picture head never reordered. The sample now extends to the first regression plus a mini-GOP, up to 48.
• The third file's writer clock runs at 29.003 fps against a stated 29.0002. Counting from the rate drifted 61 ms behind over 19 minutes and overlapped where an earlier keyframe pulled it back, so a continued sequence leans toward the earliest origin its pictures point to, at most an eighth of a picture per GOP (8bbb7cd).

HLS cut. A keyframe continued this way sits up to the reorder delay plus two and a half pictures before its index entry, which the segment plan is built from, so segverify found 25 of 40 segments of the second file opening mid-GOP (continuous playback was fine, cold starts were not). VODSegmentCutter and the restart gate now reach back for the keyframe each boundary names, never as far as the keyframe before it. It is 0 for every other source.

Measured on the three files and 144 generated pairs (H.264 and HEVC, eight rates, three GOP lengths, three push patterns): demuxed timelines in order with no gaps, every verified segment independent, sequential and restarted. Numbers are in the description. The open-GOP device run the last review asked for is still open.

[quint-aether]

Automated review

This is a re-review at 8bbb7cd. Since d6bd646 the PR adds keyframe continuation with a clock lean, an extended head sample for a GOP that opens on a run of P pictures, a VUI or standard-rate cadence, and a per-boundary reach for the HLS cutter and restart gate. The direction is still right. The repair stays in the engine, keeps every picture-order check, and leaves every other source with a reach of 0.

Findings

• Earlier findings: the open-GOP re-anchor fix still holds. The "fail closed when anchors disagree" point was reversed with evidence (the first reporting file pushes its P pictures too). The replacement uses the VUI rate within 1%, or else a standard rate within 1%, and stays gated by nominalLadderFits. That answer holds. Whether this second fallback is acceptable is the maintainer's call.
• Should fix: planKeyframeReachTicks is computed from the plan before collapseShortSegments merges it, but it is indexed against the collapsed segmentPlan. Two keyframes a few pictures apart are exactly what makes a sub-second segment that gets folded. After the fold, the reaches near that point are off by one boundary. One boundary loses the reach it needs, and the one before gets a reach that lets the earlier keyframe open it. (Sources/AetherEngine/Video/HLSVideoEngine.swift:1566)

Checklist: CHANGELOG yes, docs yes, tests yes (nothing covers the reach after a collapsed plan), traits both ways n.a.

Needs a device run: the PR already lists the pending device run of 8bbb7cd on the three files. A real open-GOP file of this shape is also still unchecked. A segverify run on the 23 fps asset over the range that holds its four-picture keyframe pair would confirm the reach fix.

Thanks for the follow-up and for bringing measurements to back the reversed decision.

_{Automated review by Quint (Claude) of 8bbb7cd. It reads the code, it does not build or run it; a maintainer reviews and merges.}

[quint-aether]

The reach array is built from plan before line 1893 runs collapseShortSegments. makeProducer then indexes the reach array against the collapsed segmentPlan (dropFirst(baseIndex), then local index = boundary index). collapseShortSegments removes the start boundary of every segment shorter than 1 s. So after the first merge, each reach belongs to the boundary one later in the original plan.

The case this PR describes triggers exactly that. buildKeyframeSegmentPlan lags segIdx behind a 15 s GOP, so every keyframe becomes a boundary. A pair K_a, K_b four pictures apart therefore gives a 0.17 s segment, and K_a is folded away. After the fold:

• K_b's slot reads K_a's reach (the full advance). K_a's DTS + 17 608 is past K_b, so K_a opens K_b's segment, four pictures before its advertised start.
• The next boundary, 15 s later, reads K_b's reach (gap - 1, about 15 651). A keyframe the writer pushed by more than that stays in the previous segment, which is the mid-GOP open this change exists to prevent.
• Each later boundary is also shifted by one. That is harmless only while every reach there is advance.

Suggested fix: compute the reach after the collapse from the final boundaries, e.g. Self.keyframeReachTicks(advance:keyframes:boundaries: segmentPlan.map(\.startPts)), keeping advance and the sorted keyframes from this branch. A test that feeds keyframeReachTicks a plan that collapseShortSegments shortens would pin it.

[sunghyun-k]

Fixed in 734477e, thanks. The reach is now read off the plan collapseShortSegments leaves (finalizedPlan does both in that order), and RepairedKeyframeSegmentReachTests covers a fold: the folded boundary keeps the later keyframe of the pair and its own reach, and the test shows the slot the old order handed it.

On the 23 fps asset the plan folds three times (791 to 788 segments). segverify over all 788 passes at both 8bbb7cd and 734477e, because the neighbouring reaches there happen to be close, so the file did not show the defect; restarts at 536 to 539 (the four-picture pair at 2162 s) and around the other two folds are independent too. The device run of the three files is still pending.

[quint-aether]

Automated review

This is a re-review at 734477e. The PR extends the #409 missing-ctts probe to a writer that forced its decode ladder to be monotonic. The direction is still right: the repair is in the engine, and every other source keeps a reach of 0.

Findings

• Fixed: the reach is now built from the plan that collapseShortSegments returns. finalizedPlan folds the plan first, then calls keyframeReachTicks on the boundaries that are left. The engine assigns both in one statement (HLSVideoEngine.swift:1895). I checked that the fold puts the short segment into the one before it, so the later keyframe of a pair stays as the boundary. That matches what reachFollowsCollapsedPlan expects. The reach is recomputed on every load, and it is empty for a live session or a plan with no advance.
• No new findings.

Checklist: CHANGELOG yes, docs yes, tests yes (the reach after a fold is now covered), traits both ways n.a.

Needs a device run: the PR still lists the device run of the three reporting files as pending. A real open-GOP file of this shape has also not been checked on a device.

Thanks for the quick fix and the fold test.

_{Automated review by Quint (Claude) of 734477e. It reads the code, it does not build or run it; a maintainer reviews and merges.}

[sunghyun-k]

Device run done: all three reporting files play in order at 734477e (the description is updated). An open-GOP file of this shape is still unchecked on a device, since none of the reporting files is open-GOP.