Skip to content

feat(test/ts): grade the full T-STD buffer model - #4640

Merged
kixelated merged 10 commits into
mainfrom
quest/m1/tstd/README
Oct 6, 2026
Merged

kixelated merged 10 commits into
mainfrom
quest/m1/tstd/README

Conversation

@kixelated

@kixelated kixelated commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The TS harness graded only the T-STD transport buffer, so moq export ts output could overflow or underflow the decoder buffers behind it and still pass. The T-STD questline needs a check that can tell compliant delivery from broken delivery before the export is fixed against it.

Approach

  • test/ts/compliance.py runs every audio and video stream through the full ISO 13818-1 T-STD (TB, MB, EB, B) on the stream's own PCR clock, with buffers from the SPS HRD or the level (AVC, HEVC) and from H.222.0 / ATSC for ADTS, MPEG audio, AC-3, E-AC-3 and Opus. It stays report-only (tstd is a shape check) until fixed-delay release flips the gate.
  • Audio bytes enter B as they leave TB, so a frame ending partway through a packet is graded byte-exactly, and B is checked just before each removal as well as after each packet.
  • Complete units are graded through their decoding times after the stream's last packet, so a burst held past the STD delay bound is caught; a trailing unit cut off by the capture is counted as truncated_units, not graded.
  • EB snaps to the exact ES offset whenever MB empties, and completeness tolerates under a bit, so float drift cannot fail a long stream.
  • pcr-presence checks each declared PCR PID individually.
  • pcr-timing.py owns PCR spacing and the byte schedule; sync and continuity are graded only under --live.
  • just test ts-tstd (tstd-controls.py, wired into Interop) proves the model: a Kyrion broadcast capture passes, its PCR restamps at 0.7x/4x/15x fail as named (including audio held over 1 s), and synthetic layouts cover refusal, adaptation bursts, duplicates, a mismatched PCR PID, a straddling audio frame (pass) and a mid-packet B overflow (fail).
  • Quests: check.md is done and deleted; send-ahead.md and mux-rate-hold.md are added under the line, which now stays flat on main.

Impact

  • No public API or wire change. Test harness only: just test ts-tstd is new, run.sh now needs TSDuck's tstables, and pcr-presence fails a declared PCR PID that carries no PCR.

Alternatives

Follow-ups

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated changed the base branch from main to dev October 1, 2026 04:11
@t0ms

t0ms commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

I cross-checked the T-STD check from #4643 against an independent implementation of the same
clauses, on 35 files: one broadcast clip (H.264 High@4.0 with NAL HRD, MP2, AC-3, teletext,
SCTE-35), captures derived from it, the Kyrion fixture with tstd-controls.py's restamps, and its
synthetics. They agree on the verdict of every condition on 24 files, and where both find a violation
the first ones fall within 0.3 ms of each other. The other 11 come down to three things in the check
against H.222.0 (10/2014), and one convention. (The independent grader had six defects of its own,
which are fixed; none of the 11 is one of them.)

  1. TB to B by whole packet (2.4.2.3). Bytes leave a transport buffer at Rx and enter B as they
    leave; the check moves a packet's payload only once its last byte has left. That fails clean
    files on audio. Example: a 576 B MP2 frame whose last byte is byte 42 of a packet arriving at an
    empty 2 Mb/s TB. Byte-exact, that byte leaves TB 0.317 ms before the frame's decode time; the
    check waits for the other 146 bytes, 0.584 ms more, and flags the frame absent (−0.267 ms). Five
    clean files fail this way, by 0.27–0.45 ms.

  2. Floating-point drift in the AVC leak (2.14.3.1). At one unit's decode time the accumulated
    leak output is 286,429,515.9999984 B against a unit end of 286,429,516 B. The 10⁻⁹ B tolerance is
    below a double's resolution at that magnitude (about 6 × 10⁻⁸ B), so a complete unit underflows.
    Evaluating the leak in closed form, or comparing in integer bits, removes it. One file.

  3. Units decoded after the last packet are never assessed (2.4.2.6). The simulation stops
    removing units at the last packet's time. On the Kyrion 4× restamp, 4 s of content arrives in
    about 1 s of PCR time, and 132 MP2 units per PID then wait over 1 s in the STD (the first enters
    at 0.3152 s and decodes at 1.3222 s, 1.007 s). The check reports no STD-delay violation; at 15×
    it misses 142 per PID. It still fails both files on TB and B overflow, so the verdict is right; the
    condition is missed.

And one convention. A capture's last access unit is usually cut mid-frame. The check sizes it
from its header and grades the missing bytes as an underflow; on five of our captures that decides an
audio condition (206 B short on one, 110 B on another). The standard does not speak to bytes a
capture never received, so either reading is defensible, but for captures it reports an underflow the
stream may not have. Dropping a trailing incomplete unit, or reporting it separately, would keep it
out of the verdict.

The first two fail clean streams, which matters once the check gates CI; the third under-reports STD
delay. Happy to send synthetic controls for the first two (a single-packet-straddling MP2 frame; a
long AVC run past 2²⁸ B of leak) and an STD-delay assertion for the restamps in tstd-controls.py,
if that's useful.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated changed the base branch from dev to main October 2, 2026 22:03
Conflicts:
- test/ts: keep the branch's split (compliance.py grades the T-STD model,
  pcr-timing.py owns PCR spacing), taking main's 100 ms TR 101 290 V1.4.1
  repetition default and main's description of the exporter's padding.
- test/justfile: keep both ts-tstd and drain recipes.
- quest/m1/tstd/README.md: keep the branch's Required list and main's
  Related section.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review October 5, 2026 04:10
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 126811df-cef5-4189-bc02-62af84b05b24
📥 Commits

Reviewing files that changed from the base of the PR and between 3138f10 and dae164a.

📒 Files selected for processing (12)
  • .github/workflows/interop.yml
  • quest/m1/tstd/README.md
  • quest/m1/tstd/check.md
  • quest/m1/tstd/delay.md
  • quest/m1/tstd/mux-rate-hold.md
  • quest/m1/tstd/send-ahead.md
  • test/justfile
  • test/ts/README.md
  • test/ts/compliance.py
  • test/ts/pcr-timing.py
  • test/ts/run.sh
  • test/ts/tstd-controls.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merged origin/main into the branch (1c2bec1) and marked ready for review.

Conflict resolutions:

  • test/ts: kept this line's split (compliance.py grades the full T-STD model, pcr-timing.py owns PCR spacing, with pcr-repetition/pcr-jitter/null-ratio/pcr-single-pid removed), and took main's 100 ms TR 101 290 V1.4.1 repetition default into pcr-timing.py and its README row, plus main's description of the exporter's null padding and 25 ms PCR cadence in README.md and run.sh.
  • test/justfile: kept both ts-tstd and main's drain recipe.
  • quest/m1/tstd/README.md: kept this line's Required list (send-ahead, mux-rate hold; check.md stays finished) and main's Related link to TS passthrough.

Checks: quest check ok, just check passes, just test ts-tstd passes all nine controls locally.

The earlier Interop failure was python -> js timing out on "resumed playback" in the JS pause/resume step (video stuck at 10 frames, audio stalled after resume). It is in the browser player, not in this branch's TS graders, and did not reach the new T-STD controls step.

Child PR #4645 still targets this branch and was left untouched.

(Written by Claude Opus 5.5)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Summary: This is the questline parent for T-STD compliant TS export. So far it contains the full ISO 13818-1 T-STD buffer model in test/ts/compliance.py (TB/MB/EB/B with HRD- or level-derived parameters), a tstd-controls.py positive/negative control suite wired into Interop CI, and a pcr-timing split where sync and continuity are graded only under --live and the PCR clock is chosen from the busiest PID. I spot-checked the spec tables against H.264 A-1/A-2, H.265 A.8, the ADTS buffers in H.222.0 2.4.2.3, and the AC-3/MPEG audio frame sizes, and they match. The controls are sound: each must-fail case requires the violations it names, so a model that passes or fails everything would be caught. CI is green.

Findings

  1. [Blocker] Required children are still open. The README's Required list is:

    The questline's own proof isn't met either: tstd is still a shape check, and the strict netem run has no nightly. The PR body also still says "Stays a draft until every child merges," lists the already-merged check.md, and leaves out send-ahead/mux-rate-hold. Not mergeable as a parent yet.

  2. [Medium] Dropping the hard pcr-single-pid check leaves no test that PCRs ride the declared PCR PID. pcr-timing.py now grades whichever PID carries the most PCRs (keep_busiest_pcr_pid, line 162). In compliance.py, check_pcr_presence (lines 282-289) passes as long as a PCR PID is declared and some PID carries PCRs. Failure scenario: an export regression puts PCRs on an undeclared PID.

    • check_tstd looks up scan.pcr_by_pid.get(pcr_pid, []) (line 1088) and gets nothing, so it builds no time base.
    • Every stream grades 0 access units, and the result is only a shape WARN: "no access unit fell inside a modelled time base".
    • Non-strict just test ts therefore passes.

    Fix: make pcr-presence hard-fail when a declared PCR PID carries no PCR, or when PCRs appear on a PID that no program declares. That also works for multi-program streams, which was the reason for removing the old check.

  3. [Low, perf] simulate gets slow when a capture has many time bases. For each time-base segment it scans the stream's full packet list (compliance.py:940). It also uses list.pop(0) as a FIFO (mb, late, headers; lines 991, 1012, 1054). That is O(segments × packets), and quadratic in the worst case. A long capture with frequent signalled discontinuities (e.g., export restarts) will crawl. Use collections.deque, and bisect the packet list by index per segment.

Nits

  • test/justfile:120-121: the first comment block above ts-tstd is separated from the recipe by a blank line, so it's orphaned and just --list shows only the second. Merge the two.
  • read_pes steps through the file in fixed packet_size strides, while scan_packets resyncs on the next 0x47. After a sync loss their packet indices drift apart, which skews the T-STD timing. Sync loss already hard-fails, so this only affects the metrics on corrupt captures.

Verdict: ITERATE (reviewed head 1c2bec1). The harness work is good, but four Required children are open, and #2 should be closed before tstd becomes the gate.

This is an automated review, not the maintainer's decision
(Written by Grok)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 25f6a47

Direction: the PCR/T-STD split and positive/negative controls are useful, but iterate before making this model a compliance gate. This head merges main; compliance.py, pcr-timing.py and tstd-controls.py are unchanged from 1c2bec1. I reviewed the current PR diff and checked the existing reports against this head.

Existing findings independently confirmed (referencing the discussions rather than adding duplicate inline threads):

  • [P2] Byte-granular audio delivery: test/ts/compliance.py:946–955,1033–1038 delivers an entire packet only when its last byte leaves TB. A focused 576-byte-frame case completes byte-exactly before PTS but is reported 0.352 ms late. Transfer payload progressively, with transport/PES headers accounted for, and add an audio frame ending partway through a packet as a passing control.
  • [P2] Grade complete trailing units: test/ts/compliance.py:957–974 stops at the capture horizon. In a two-unit fixture, a fully received unit held for 1.299 s is silently omitted; extending only the horizon exposes the missing delay violation. Drain complete units through their decode deadlines at final EOF, separately handling truncated units and clock discontinuities. Assert STD-delay failures in the fast-restamp controls.
  • [P2] Bound numerical error: test/ts/compliance.py:932,1001–1014,1049 compares cumulative floating-point bytes with a fixed 1e-9-byte epsilon. The reported 286,429,515.9999984 versus 286,429,516 still evaluates as underflow. Use stable/exact accumulation or a justified sub-byte error bound and add a long-stream regression. These three confirm the existing independent-grader report; I did not rerun its capture set.
  • [P2] Require PCR on each declared program clock: test/ts/compliance.py:284–288 checks only that both lists are nonempty. Declaring PID 256 while only PID 999 carries PCR returns PASS; the affected program then has no modelled time base (1088–1091). Validate declared PCR PIDs individually and add a mismatched-PID negative control. This confirms the existing review discussion.

The end-to-end quest remains incomplete: #4645 is open (now ready, not draft), and this head still leaves tstd report-only. Verification: Python compilation and focused in-memory model/check reproductions passed as described; TSDuck is unavailable here, so the nine-control suite, real captures and loss/netem proof were not run. Head CI was queued when checked.

kixelated and others added 4 commits October 5, 2026 23:04
Audio bytes enter B as they leave TB, so a frame ending partway through
a packet is no longer flagged late. Complete units are graded through
their decode times after the stream's last packet, and a trailing unit
cut off by the capture is counted, not graded. EB snaps to the exact
offset whenever MB empties and completeness tolerates under a bit, so
float drift cannot fail a long stream. pcr-presence checks each declared
PCR PID. Controls cover the straddling audio frame, the STD delay on the
fast restamps, and a mismatched PCR PID.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A unit decoded partway through a packet advanced the removal pointer
before that packet's bytes were counted, hiding the peak. Count what of
the next packet has already left TB, and add a mid-packet overflow
control.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title quest(m1): T-STD compliant TS export feat(test/ts): grade the full T-STD buffer model Oct 6, 2026

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial Codex review (peer-review), branch diff against main, final head dae164a. AI review, suggestions only.

Round 1 (head d11b534), needs-attention:

  • [high] Audio B overflow was checked only once a whole packet left TB, so a unit decoded mid-packet advanced out first and hid the peak. Fixed in 862d877: B now also counts what of the next packet has left TB just before each removal, with a mid-packet overflow negative control (fails on the previous model, x20 B overflow now).

Round 2 (862d877, same test code as the final head), needs-attention:

  • [medium] A time base with fewer than two PCRs after a signalled discontinuity is dropped silently, so its units go ungraded and the check can PASS. Not fixed here: pre-existing, needs a capture tail cut right after a discontinuity, and tstd is still report-only. Listed as a follow-up for before the gate flips in #4645, since warning on it could also fail clean captures that end just after an export restart.

Earlier findings on this PR, all addressed on this head:

  • Byte-granular audio delivery (t0ms 1, OpenAI review): audio bytes enter B as they leave TB; straddling audio control (576 B frames ending at byte 41 of a packet, decoded 0.3 ms after) passes, fails before.
  • Trailing complete units (t0ms 3): units are graded through their decoding times after the stream's last packet. The 4x and 15x restamps now report held over 1 s x132 and x142 per audio PID, matching t0ms's independent counts, and the controls require it.
  • Float drift (t0ms 2): EB snaps to the exact ES offset whenever MB empties, and completeness tolerates under one bit. No discriminating control: neither the Kyrion capture nor the synthetic produce a partial leak at a unit boundary even with offsets moved past 2^46 B. t0ms's offered long-AVC control would close that.
  • Truncated last unit (t0ms convention): counted as truncated_units, not graded.
  • Per-PID PCR presence (Grok 2, OpenAI review): pcr-presence fails any declared PCR PID (other than 0x1FFF) with no PCR; PCR PID mismatch control.
  • Perf (Grok 3): FIFOs are deques and each segment bisects its packet range.
  • Justfile orphaned comment (Grok nit): merged.
  • read_pes stride vs resync (Grok nit): not changed; sync loss already hard-fails.
  • Open children (Grok 1): questlines are flat now, so the line lands on main and #4645 retargets to main.

Checks on this head: just check, just test ts-tstd (12/12), just test interop --all (all rows pass), quest check.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 6, 2026 06:35
@kixelated

Copy link
Copy Markdown
Collaborator Author

Follow-up review after the push to dae164a3 (new PR commits d11b534d and 862d8772; the rest are origin/main merges). Since the last review on 1c2bec1b, the PR has been re-scoped: it now lands the T-STD check on main as a flat questline instead of waiting as a parent for every child, and the two commits tighten the model. Audio is now byte-exact into B, units are drained to their DTS at EOF, MB/EB snaps to the exact offset, pcr-presence checks each declared PID, and B is checked just before each audio removal. Each fix comes with a control that fails without it. I checked that the mid-packet overflow case (3,592 B against a 3,584 B B, where whole packets alone peak at 3,496 B) can only fail through the new pre-removal check. CI is still queued on this head.

Earlier findings

  1. [Blocker] Required children still open. Resolved by the re-scope. The quest README is already on main. This PR deletes check.md as done and adds send-ahead.md and mux-rate-hold.md as separate quests, and the body no longer says it stays a draft. One thing to watch: feat(moq-mux)!: fixed-delay jitter buffer and per-PID T-STD admission for TS export #4645 still targets quest/m1/tstd/README, so it needs retargeting to main when this merges (GitHub does that automatically only if the head branch is deleted on merge).
  2. [Medium] PCRs on an undeclared PID pass. Fixed. check_pcr_presence (compliance.py:283-296) now fails any declared PCR PID that carries no PCR and exempts 0x1FFF, and the new "PCR PID mismatch" control covers it.
  3. [Low, perf] O(segments × packets) and list.pop(0). Fixed. Packets are bisected per segment (:947) and the FIFOs are deques. The units loop still scans every unit per segment (:997), but that's cheap.
  • Nit: orphaned ts-tstd comment in test/justfile. Fixed.
  • Nit, still open: read_pes (:550) still steps through the file in fixed strides while scan_packets resyncs on 0x47. This only matters on captures that already fail sync.

New, non-blocking

  1. truncated_units is audio-only, but the README describes it generally. video_units sets the last unit's end to stream.es_len (:877), so unit.end > stream.es_len (:1000) never fires for video. With the new EOF drain (last, :993/1012), the cut-off final video AU is graded as complete even when its DTS falls past the end of the capture. It can't false-fail, since all of its captured bytes arrived, but the metric always reads 0 for video, and test/ts/README.md says the last access unit "is counted as truncated_units and not graded." Either say this applies to audio only, or treat the last video AU (one with no following AU start) as truncated.
  2. pcr-timing.py still grades the busiest PCR PID (keep_busiest_pcr_pid, :162/677), not the declared one. Now that pcr-presence guarantees the declared PID carries PCR, a stream that also carries more PCRs on an undeclared PID would have its spacing and byte schedule graded on the wrong clock. That's an unlikely layout. If you want to close it, pick the declared PID for the program and fall back to the busiest only when there are several programs.
  3. Minor overcount in the pre-removal B check (:1110-1117). partial adds every byte of the next packet that has already left TB, including payload below out that belongs to a unit already removed after an underflow. That only inflates B peak on streams that already fail with B underflow. Clamping the payload part to bytes at or above out would make it exact.

Verdict: MERGE once CI is green (reviewed head dae164a3e45e5449159dc26d410eda60d0bdba32). The earlier blocker and the medium finding are resolved, and what's left is non-blocking.

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: dae164a
Incremental review since 25f6a47, separating the merged main changes.

The four previous findings are addressed: per-declared-PID PCR validation (test/ts/compliance.py:283–296), byte-granular audio completion and pre-removal B peaks (:974–988,1109–1122), complete trailing-unit removal (:990–1015), and exact-offset resets plus sub-bit completion tolerance (:1047–1068,1105). The new controls cover straddling audio, mid-packet overflow, PCR mismatch, and held-over-1-s failures (test/ts/tstd-controls.py:245–256). No new actionable defect found in this delta.

Direction: these changes make the report-only model substantially more credible. Retain the documented follow-up for ungraded short PCR time bases (:1160–1162) before promoting it to a hard gate; this is an existing limitation, not a new finding.

Verification: Python compilation passed. Directly ran the current synthetic audio builder/parser/model: straddling audio passed with 79 graded/1 truncated unit; the overflow control recorded 20 B overflows; a 1.3-s delay recorded 79 held-over-1-s violations. The mismatched-PID check failed as expected. TSDuck and real-capture/long-stream/netem validation were not run here. Current-head Interop fails earlier at max_age_relay_javascript (max_age.rs:145, JS publisher not ready), so T-STD controls were skipped; CI has not validated this model yet.

@kixelated
kixelated merged commit 8714109 into main Oct 6, 2026
7 of 8 checks passed
@kixelated
kixelated deleted the quest/m1/tstd/README branch October 6, 2026 07:39
@t0ms

t0ms commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Re-graded the 35-file corpus from my earlier comment against compliance.py as merged (87141092). The two checks now agree on all 35 files, on every condition both grade. The false MP2 underflows on the five clean files, the leak drift on ffa-L600-w60, the 1 s audio delay on the 4× and 15× restamps (132 and 142 units per PID) and the truncated last units all match ts-tstd.py now. Thanks for taking them all.

kixelated added a commit that referenced this pull request Oct 6, 2026
The T-STD questline (#4640) landed on main, so this branch now targets main.

- Harness files merged against the questline head the branch started from.
- Quests: delay.md and byte-schedule.md are done and deleted here; references
  repointed, and the T-STD README records t0ms's CNN grading.
- #4577's per-stream export stats ported onto the slot schedule: a unit counts
  once the slot carrying its first packet is returned. Export::stats returns
  ts::stats::Export (rows plus the release clock's dropped, drift and
  out_of_tolerance), with From<stats::Export> for ts::Stats for stats::Log;
  ts::export is private again. moq export ts logs the release counters.
- #4652 skips a stalled group once the next group's start falls the budget
  behind, so the export's sources skip at half the delay: with the whole
  delay the group after a skip arrived at its deadline.
- The jitter buffer waits only for audio, video and PES tracks, not sparse
  sections.
- At end of stream a past-due unit ignores its decoder buffer, so an
  oversized last unit cannot hold the stream open.
- Tests: a timeline restarting at zero fails the export (#4725); main's new
  export tests moved to with_delay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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