Skip to content

fix(video): recover periodic rational H264 composition offsets (#409 follow-up) - #419

Closed
orut34iop wants to merge 2 commits into
superuser404notfound:mainfrom
orut34iop:codex/issue-409-quantized-cadence
Closed

fix(video): recover periodic rational H264 composition offsets (#409 follow-up)#419
orut34iop wants to merge 2 commits into
superuser404notfound:mainfrom
orut34iop:codex/issue-409-quantized-cadence

Conversation

@orut34iop

@orut34iop orut34iop commented Aug 25, 2026

Copy link
Copy Markdown

Summary

Retesting the released #413 + #415 implementation on the original reporting asset still showed continuous judder from the first frame, without seeking. Its decode ladder is constant-rate but periodically quantized, so the current uniform-integer-step classifier fails closed and repairs no packets.

This follow-up recovers the exact rational cadence and sampling phase from the observed repeated DTS cycle while leaving ambiguous, malformed, aliased, and genuinely variable timing untouched.

Closes #409

What changed

  • Derive a rational cadence only from an adjacent-integer DTS pattern that repeats for at least two complete periods and has one unique phase.
  • Treat avg_frame_rate as the stronger whole-stream corroboration and r_frame_rate only as its fallback, so an exact nominal short-period alias cannot override contradictory average-rate evidence.
  • For an approximate metadata match, require a known AVStream.nb_frames span and prove with exact integer arithmetic that the cadence error cannot accumulate beyond half a tick across the complete stream. Missing span, overflow, or excessive drift fails closed.
  • Reject rational cadences with fewer than two ticks per frame, where a tolerated one-tick anomaly is indistinguishable from an adjacent exact frame ordinal.
  • Keep quantization phase separate from the edit-list/reorder offset, preserve one global ordinal across IDRs, and dry-run the whole sample before arming repair.
  • Resynchronize after parser misses, seeks, and only uniquely placeable one-tick anomalies; reject larger or ambiguous deviations without mixing raw and repaired axes.
  • Map container keyframe indexes through the same rational lattice used for packets.
  • Add identity-free regression coverage for the physical timing shape, long-period prefix aliasing, the exact half-tick full-stream boundary, missing span, dense-cadence ambiguity, phase, VFR, POC depth, parser misses, seeks, and timestamp anomalies.

The physical signature is PTS == DTS for 12/12 sampled packets, video_delay=1, time_base=1/1200000, and the repeating DTS-step cycle 40040,40041,40040,40040,40041. The recovered cadence is 200202/5; the nominal 30000/1001 coded rate rounds to an integer 40040-tick cadence, while the average rate corroborates the recovered cadence within the full-stream half-tick bound.

Test plan

  • Device / OS: Apple TV 4K (3rd generation), tvOS 26.6 beta (23L773), physical device, native AVPlayer path with hardware decode.
  • Source media: seekable MP4 / progressive H.264 with reordered pictures / no usable ctts / SDR; audio was not involved in the defect.
  • Before: direct playback on the released fix(video): rebuild the presentation axis an MP4 dropped with its ctts table (#409) #413 + A seek drops the packets the repair verdict left held (#409 follow-up) #415 implementation juddered continuously; the sample was classified as a nonuniform decode ladder and 0 packets were repaired.
  • After the final follow-up: the device reported stream_frames=261363, classified the source as repairing, and repaired all 12 sampled packets with 0 unrepaired. Direct playback remained smooth; scrubbing and release resumed playback and remained smooth; a separate unaffected video remained unchanged.
  • DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer xcrun swift build --target AetherEngine passed.
  • Focused Swift Testing run: 42 tests / 2 suites passed; 2 optional A/53-fixture tests were skipped because that fixture was not installed locally.
  • PublicAPIDocumentationTests: 4/4 passed.
  • Host integration contract tests: 22/22 passed.
  • Physical candidate gates passed: package, signing, install, launch/first frame, crash, and process checks.

No source URL, media identity, request headers, device identifier, or raw device log is included.

Checklist

  • CHANGELOG.md updated
  • Commit messages follow Conventional Commits
  • The fix lives in the engine, not in a host-side workaround
  • Public API changes are intentional and documented (no public API change)

@superuser404notfound

Copy link
Copy Markdown
Owner

The retest diagnosis is right, and it is what the fix is built on. Thank you for going back to the asset and coming back with the measurement instead of a symptom.

Reproduced here before touching anything, on a purpose-built twin pair with the reported shape (1000000/33367 fps in a 1200000 timescale, so 200202/5 ticks per picture, ladder cycling 40041,40040,40040,40041,40040): the released implementation passes the whole file through with PTS == DTS, every picture still in decode order, verdict decode ladder is not uniform. Exactly as described.

One line in this PR was worth more than the rest of the review put together:

a real MP4's STTS phase need not coincide with semantic frame offset (the affected physical file starts at phase 2 with videoDelay 1)

My first cut derived the phase from the reorder delay. It reproduced both of my own fixtures bit-exactly, and it would have missed the reporting asset, because phase 2 is not -1 mod 5. The phase is now read from the ladder itself and there is a test for a sample taken away from the head, which is a second thing that assumption was hiding: measured before the change, sampling the same file after a seek classified it as unrepairable.

The engine fix is #427, merged as 69daa3d3. It takes a different route than this PR, and it is worth being specific about why rather than leaving it at "smaller".

Packet placement should not require the ladder to sit on the lattice. In this PR every packet is placed by reading its own timestamp back into an ordinal, so a packet more than a tolerated tick off the lattice, or ambiguously on it, is emitted with its raw timestamps. That is the defect returning in the middle of a file, on the axis a consumer is already reading. In #427 the lattice is consulted once per coded video sequence, at its IDR; the pictures inside it are placed by rank, and a sequence whose IDR cannot be read back onto the lattice falls back to the rounded step anchored on the container's own timestamp, which describes the stream to within a tick and cannot drift because it re-anchors at every IDR. Nothing is ever handed on in decode order because an arithmetic path did not close, and there is a test that says so.

The same holds for the container index. This PR maps each index entry through the lattice and omits the entries it cannot place. The rationale in the comment is sound as far as it goes, but a keyframe silently leaving the segment plan changes where segments are cut, and nothing downstream can see that it happened. #427 keeps the index folded by the one constant the packets themselves move by, so an entry cannot disagree with the packet it points at, and none can go missing.

Those two together are what removes the need for the ordinal bookkeeping, the resynchronisation rules and the full-stream drift proof, and the change comes to about 180 lines rather than 1327. That is the reason this PR is being closed rather than merged and hardened: the difference is architectural, not a list of findings.

Two things from your work stay in the repo: the diagnosis above, and the fixtures from #411, which are still the regression test for the whole-tick half of this and now sit next to a fractional pair built the same way:

ffmpeg -f lavfi -i 'color=c=gray:s=96x64:rate=1000000/33367' -frames:v 33 \
  -c:v libx264 -preset ultrafast -pix_fmt yuv420p -bf 3 -b_strategy 0 -g 16 \
  -sc_threshold 0 -crf 40 -video_track_timescale 1200000 -r 1000000/33367 \
  -movflags +faststart healthy.mp4
ffmpeg -i healthy.mp4 -map 0:v:0 -c:v copy -bsf:v 'setts=pts=DTS' \
  -movflags +faststart missing.mp4

33 packets, three coded video sequences, both writer shapes, from the head and after a seek: every repaired packet carries the healthy twin's PTS and DTS exactly.

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.

Progressive H.264 MP4 without CTTS judders on the native path

2 participants