Repository navigation
Conversation
Quest-Claim: c61129c68ffb843d
…ming Drop the MPEG-2 picture-header parser that trimmed open-GOP leading B-pictures on every frame, its tests, and the recovery harness's MPEG-2 profile. HEVC RASL and H.264 open-GOP trimming stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The code half of the previous commit, whose staging missed it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 27 minutes. View limit details
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. |
There was a problem hiding this comment.
ℹ️ No problems in the code. One follow-up quest still builds on the parser this PR deletes.
Reviewed changes
This review covers how the TS export removes MPEG-1/2 open-GOP trimming, plus the test and doc cleanup that goes with it.
Track::queuesimplified: every kind that isn'tKind::Videonow goes straight topush, including whole-PES MPEG-1/2. I checked thatTrack.held,recovery_gapandleading_floorare only read insidequeue's video path. Skipping their updates for MPEG-2 therefore has no effect on anything else.Mpeg2Pictureand theopen_gop/orphanstate removed:push's leading check now falls back tofalsefor every kind that isn't video.- Recovery harness trimmed:
Profile::Mpeg2,Video::Mpeg2, the whole-PES video pickup, and the PES-PTS branch ofdecodedare gone.CLIPSshrinks to 8, which matches the remainingProfilevariants indexed byself as usize. - Docs: the semantic recovery bullet in
doc/bin/ts.mdnow limits trimming to H.264 and H.265.
ℹ️ quest/m5/pes-hold-open-gop.md still plans work on the deleted parser
That quest's goal is to make the MPEG-2 export trim leading B-pictures in zero-stuffed CBR streams. Its plan extends mpeg2_orphans_are_whole_pes_of_leading_b_pictures and builds on the per-track orphan state. This PR deletes all of those. The PR says "Follow-ups: None", but if that quest is left as it is, a later run will pick it up and rebuild what this PR removes. Its second half is still valid, though: H.264 (Open) and HEVC (Rasl) recovery tests have no case with a nonzero join. Since the MPEG-2 arm went away, nothing covers a join at a group already in flight.
Technical details
# Stale follow-up quest after deleting `Mpeg2Picture`
## Affected sites
- quest/m5/pes-hold-open-gop.md — the Goal and most of the Plan target MPEG-2 orphan trimming, which this PR removes. It also names `mpeg2_orphans_are_whole_pes_of_leading_b_pictures` and the per-track `orphan` state, both now gone.
## Required outcome
- No open quest asks for MPEG-2 open-GOP trimming to come back.
- The H.264/HEVC "join at an in-flight group" coverage is kept, either rescoped into this quest or moved to a new one.
## Suggested approach
- Rewrite the quest so it covers only adding a nonzero `join` arm to `open_gop_trims_only_the_orphans` (`Profile::Open`) and `hevc_trims_only_orphaned_rasl` (`Profile::Rasl`) in `rs/moq-mux/src/container/ts/recovery_test.rs`. Alternatively, delete the quest in this PR and file that coverage on its own.claude-opus-5-5 | 𝕏

Finishes quest/m1/ts-open-gop-trim.md.
Problem
moq-dev/moq#7365 added
Mpeg2Picture, a picture-header parser that scanned every MPEG-1/2 video PES the TS export forwarded, to trim open-GOP leading B-pictures after a hole. No consumer asked for it, and decoders already skip those B-pictures. The maintainer picked deleting it on moq-dev/moq#8764.Approach
rs/moq-mux/src/container/ts/export.rs: MPEG-1/2 video takes the same path as every other whole PES inTrack::queue(straight topush, no leading-picture judgment). RemovedMpeg2Picture, theopen_gopandorphantrack fields, and its unit test. H.264 open-GOP and HEVC RASL trimming, and RADL keeping, are untouched.rs/moq-mux/src/container/ts/recovery_test.rs: removedmpeg2_open_gop_trims_only_the_orphansand the harness support fix(ts): trim open-GOP MPEG-2 leading B-pictures on export #7365 added only for it (theMpeg2profile and encoder, the whole-PES video track pickup, the PES-PTS branch ofdecoded). TheCatalog<Ext>republishing and the grader's join-group accounting stay, since they are general.doc/bin/ts.md: the semantic recovery bullet says which codecs drop orphaned leading pictures and that MPEG-2 video goes out whole.Choices made unattended (reviewable): deleting the MPEG-2 recovery profile rather than rewriting its test to assert the B-pictures go out, since its grader models open-GOP orphans as must-trim and the profile existed only to cover the parser.
Impact
Validation
just checkpasses (rs lint and tests for every changed crate, markdown, quest).cargo test -p moq-mux --lib container::ts: 761 passed, including every remaining recovery test (open_gop_trims_only_the_orphans,hevc_trims_only_orphaned_rasl,hevc_keeps_radl_after_a_hole) and the MPEG-2 round trips (mpeg2_video_rides_whole_and_survives_a_round_trip,export_resumes_whole_mpeg2_video_at_a_sequence_header).mainagainst the new export. Its grader now fails withvideo unit 23 went out without the group before it: Unit { group: 2, ..., leading: true, ..., output: true }, i.e. the join group's leading B-picture is forwarded rather than trimmed, as intended.Alternatives
Caused by
Follow-ups
None.
Models
Size: XS
Implementer: Claude, claude-opus-5-5, unknown effort (arm B)
🤖 Generated with Claude Code
Review
Rewritten by
quest reviewafter each round.Head: b2a93ab
Base: origin/main at 250a13a
Risk: medium,
the base has no RISK.md to name what is always high or customer-facing, so every change is mediumClassified: medium by default
Reviewer: Codex, gpt-6.1-sol, medium effort
Session:
01a1df49-1709-7852-9e00-a4bbdacbcc55, round 1Score: 9/10,
Source inspection found no actionable regressions in forwarding MPEG-2 PES whole while preserving H.264/H.265 recovery handling, though builds and tests were not run as instructed.Rounds: 1; first round 0; findings 0: fixed 0, refuted 0, open 0
Findings: none