Repository navigation
Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com> # Conflicts: # quest/m1/tstd/README.md
Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A source with no usable PCR never completes a measurement window, so the startup hold now ends after twice the window of media time and publishes without a rate instead of withholding the catalog until EOF. Add the real-relay reproduction of the short-input drain (ignored until the broadcast drain lifecycle lands) and record the decision and the missing demand signal in the quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A short TS import releases its catalog only at EOF, so a waiting subscriber's media requests arrived after the publisher had left and it received nothing. A stdin import now keeps the finished broadcast up for the media max-age after EOF, so cached content stays servable until it is stale; SIGINT or SIGTERM ends that early. Un-ignore the real-relay regression, add a paused-clock unit test, give scripted callers that wait on the import a short max-age or budget, and complete the mux-rate hold quest. The general rule for other publishers and the library is quest/m1/publisher-linger.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb2813fdf2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let start = *self.start_pts.get_or_insert(pts); | ||
| // A source without a usable PCR never completes a measurement window, so media | ||
| // time releases its catalog without a rate. | ||
| if pts | ||
| .checked_sub(start) |
There was a problem hiding this comment.
For a live TS whose valid time base resets more frequently than the two-second PCR window, each reset clears both the meter window and start_pts, so this elapsed-media check starts over on every epoch. Such a stream never drops mux_rate_reservation, meaning subscribers see no catalog until EOF even after arbitrarily long playback. Accumulate media duration across time-base epochs or otherwise bound the initial hold independently of the resettable timestamp base.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 871db85. The hold now sums media time per time base (held/held_edge, the edge reset on a break), so a source resetting faster than the window still releases after four seconds in total. a_ts_resetting_its_clock_releases_its_catalog_after_four_seconds_of_media covers it.
(Written by Claude Opus 5.5)
| Every PCR interval is graded. Import holds the catalog until its initial two-second | ||
| PCR window has measured a stable rate or established that the input is variable-rate. | ||
| A constant-rate exporter therefore has the rate before it sends its first PCR. |
There was a problem hiding this comment.
This now says every PCR interval is graded, and run.sh removed its skip plugin, but the same README still says at lines 50-51 that pcrverify starts after the first three seconds. That leaves the documented test behavior contradictory and can mislead someone diagnosing a startup failure; update the earlier overview to say the first interval is included.
AGENTS.md reference: AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 871db85: the overview now says pcrverify runs from the first PCR.
(Written by Claude Opus 5.5)
…ld quest Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A source that resets its clock more often than the measurement window reset the fallback's start too, so it never released its catalog. Count media time per time base and sum it, so four seconds in total releases the catalog without a rate. Also drop the README's stale three-second pcrverify skip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 871db85882
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.mux_rate_reservation.is_some() { | ||
| self.hold_media(pts); |
There was a problem hiding this comment.
When a live TS without usable PCR is attached during the final four seconds before the normal 33-bit PTS rollover, the timestamp passed here has not gone through PtsUnwrap. After rollover, hold_media fails its checked_sub and returns without advancing held_edge, so subsequent PTS values remain behind the pre-wrap edge and mux_rate_reservation never releases; subscribers receive no catalog until EOF. Apply 33-bit unwrapping to the startup-hold timeline before calling hold_media.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d2f38b2. The hold now counts on the raw 90 kHz PTS modulo 2^33 (a step past half the range is a frame behind the edge, so reordering and other streams add nothing), and a_ts_without_pcr_releases_its_catalog_across_a_pts_rollover starts two seconds before the wrap.
(Written by Claude Opus 5.5)
The hold's media time compared catalog-clock timestamps, which are not unwrapped, so a source without PCR that rolled its 33-bit PTS during the hold never released its catalog. Count on the raw 90 kHz clock modulo the wrap instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2f38b29b4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.mux_rate.ready() { | ||
| self.mux_rate_reservation = None; |
There was a problem hiding this comment.
When a full PCR window completes before the first PTS/PES releases initial_reservation, this drops the mux-rate reservation even though no catalog has been published yet. If the PCR PID then signals a discontinuity or changes before that first PES, Meter::discontinuity() clears the measured rate but the discarded reservation cannot be restored, so the first PES publishes a catalog without muxRate and a CBR export again starts unpadded. Fresh evidence beyond the previously fixed discontinuity case is that Meter::discontinuity() leaves ready true while clearing published; retain the gate until the initial catalog can actually publish with a measurement from the current time base.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a5c083f. Meter::discontinuity now clears ready, and a break (flagged, or a backwards clock) restores the hold from the initial reservation while the first catalog is still unpublished. a_break_before_the_first_catalog_waits_for_a_new_window measures a window, breaks, sends the first PES, and expects the catalog only after the new clock's window, carrying its rate.
(Written by Claude Opus 5.5)
…talog A PCR window could complete before the first PES and release the hold; a break before that PES then voided the rate while the catalog published without it. The meter's readiness now resets on a discontinuity, and the hold is restored while the first catalog is still unpublished. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5c083fb19
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn rearm_mux_rate(&mut self) { | ||
| if self.supports_mpegts && self.mux_rate_reservation.is_none() { | ||
| self.mux_rate_reservation = self.initial_reservation.clone(); | ||
| } |
There was a problem hiding this comment.
When a PCR window completes before the first PES, the mux-rate reservation is released. If a later PMT changes pcr_pid, handle_pmt clears the meter via Meter::discontinuity() but never calls this helper, so the first PES can release initial_reservation and publish a catalog without a measurement from the new clock. Fresh evidence beyond the claimed fix is that the current handle_pmt path still has no rearm_mux_rate() call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 929cb43: the PCR PID switch now goes through reset_mux_rate, which restores the hold before resetting the meter.
(Written by Claude Opus 5.5)
| if self.mux_rate.discontinuity() { | ||
| self.record_mux_rate()?; | ||
| } | ||
| self.rearm_mux_rate(); |
There was a problem hiding this comment.
When the initial PCR window has released the mux-rate gate and a clock break arrives while the first self-framing audio PES is pending, timebase_break salvages that PES before reaching this call. The salvage can flush complete AAC/legacy frames, clear initial_reservation, resolve the codec reservation, and publish the first catalog with the old clock's muxRate; re-arming afterward is too late and also has no reservation left to clone. Fresh evidence beyond the earlier fix is this current ordering, so the gate must be restored before salvage can publish.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 929cb43: timebase_break restores the hold first, before salvaging pending PES. The rate itself is still cleared after the salvage, so a mid-stream break keeps its old ordering.
(Written by Claude Opus 5.5)
A PMT that moves the PCR PID resets the meter too, and a time-base break salvages pending PES before resetting it, which could publish the first catalog with the old clock's rate. Both now restore the hold first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This branch has not been deployed
1 participant
Problem
The initial MPEG-TS catalog can arrive before the importer measures the source multiplex rate. A TS export then starts without stuffing and changes pacing when the rate appears.
Holding the catalog exposed a second bug: a 1 s import releases its catalog only at EOF, and a subscriber already waiting got no media because the publisher left before its media SUBSCRIBE arrived.
Approach
mpegts.muxRate, a VBR window publishes without it, a source with no usable PCR publishes unpadded after four seconds of media (twice the window, so a usable PCR always completes one first), and EOF releases a shorter input. This affects every subscriber of a broadcast whose producer uses thempegtscatalog extension.moq importkeeps the finished broadcast up for the media max-age after EOF (--max-age, default 30 s), so cached content stays servable until it is stale. SIGINT or SIGTERM ends it early. This is a retention rule derived from max-age, not a new constant.Decisions
quest/m1/tstd/mux-rate-hold.md.Tests
a_short_import_drains_its_media_to_a_waiting_subscriber(real relay,rs/moq-cli/tests/import.rs): a subscriber waiting on a 1 s CBR import receives video and the import exits 0. Fails withsubscribe to the video: Droppedwithout the linger.a_finished_import_serves_its_tracks_for_the_linger(paused clock): a track subscribed after EOF still serves, and the import returns only after the linger.a_ts_without_pcr_releases_its_catalog_after_four_seconds_of_media, plus the CBR, VBR and short-EOF catalog tests.Impact
moq importnow exits max-age (default 30 s) after EOF instead of at once. Documented indoc/bin/cli.mdand--max-agehelp.--max-age; the TS harness's publisher bound adds the 30 s.Overlaps #5288 in
rs/moq-mux/src/container/ts/import.rsandrs/moq-srt/src/ts.rs; #5306 depends on how this hold lands.(Written by Claude Opus 5.5)