Repository navigation
Conversation
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Automated review of 343658a (quest-only: Checked against Non-blocking:
Verdict: MERGE This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 343658a
No actionable findings in this planning-only change. Deduplicating identical table runs within the same flush is a focused approach: it preserves distinct tune-in points and changed versions without redesigning the scheduler. The padded/unpadded distinction and buffer-based regression cover the important risks.
Checked both changed files against the linked #5207 discussion, current scheduler/export ordering, AAC tune-in test, and compliance model. No public API or wire changes in this PR. Static review only; tests and quest validation were not run.
(Written by OpenAI)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 343658abc1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,40 @@ | |||
| # [M] TS export never bunches repeated program tables | |||
There was a problem hiding this comment.
Merge this with the existing PSI-runs quest
The parent commit already added quest/m1/tstd/psi-runs.md and linked it from quest/m1/tstd/README.md for this exact #5207 defect: it has the same aligned-keyframe scenario, review-thread/null-padding fix, and system-buffer regression test. Creating a second M1 quest leaves one fix with two statuses ([S] versus [M]), so completing either quest leaves the other falsely open; fold the extra detail into the existing quest instead.
AGENTS.md reference: AGENTS.md:L46-L46
Useful? React with 👍 / 👎.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks. Addressed in e6bad36:
(written by Claude Opus 5.5) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe README adds “TS table bursts” to the Required list. A new proposal describes dropping immediately repeated PAT and PMT table runs, restoring padded-slot length with null packets, and checking table bursts against a 512-byte system buffer. It also specifies aligned-keyframe test cases with three and two video PIDs. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This planning-only change introduces no identified runtime risk and is ready to merge subject to normal checks. Pre-merge checks |
|
|
Thanks @t0ms, this is a careful plan. It turns out to duplicate Your extra detail is not lost: #5321 folds it into (Written by Claude Opus 5.5) |
Summary
Plans
quest/m1/ts-psi-repeat.md[M], the follow-up kixelated recommended when landing #5207:moq export tsnever lays copies of the PAT and PMT back to back where they would overrun those PIDs' 512-byte transport buffers. Every copy that marks its own tune-in point still goes out.Export carries every video rendition and writes the tables ahead of each video keyframe, so an ABR ladder of three or more renditions with aligned keyframes puts three PATs back to back today (about 526 B at 25 Mb/s), on
mainbefore #5207 as after. The quest starts from the narrower diff in the #5207 review thread, which drops only a run laid right behind an identical one. It verifies with a transport-buffer check on the system PIDs, in a unit test and incompliance.py, whose strict T-STD check models only elementary streams today.Ranked in m1 just after ATSC AC-3, beside the other TS export quests.
Public API: none (planning only). Wire: none.
quest checkpasses.Decision paper trail
compliance.py(recommended) / Those plus a three-rendition Interop arm / Unit test only(Written by Claude Opus 5.5)