feat(mux): carry EIT present/following through a TS round-trip - #2824
feat(mux): carry EIT present/following through a TS round-trip#2824kixelated wants to merge 1 commit into
Conversation
`moq import ts` dropped EIT at the import gate, so the EPG could not survive a round-trip even though the mechanism to carry it already existed. #2800 measured the cost and found it far cheaper than assumed: the `si` map dedupes by (table_id, table_id_extension, section_number) and compares before taking the catalog's write lock, so cost tracks the revision rate rather than the transmission rate. EIT p/f was 4 distinct sections out of 592 transmitted over ten minutes. The gap was that `SI_PIDS` keyed on PID alone and `si_section` captured every section on an intercepted PID whatever its table_id. That is right for the PIDs it covered, but it made EIT all-or-nothing: p/f (0x4E, 0x4F) and schedule (0x50..=0x6F) share PID 0x0012. `Si::interval` made it concrete, being one value per PID where p/f actual wants 2s and schedule wants 10-30s. So `SI_PIDS` becomes `&[SiPid]` with a `Tables::{All, Only}` filter. Selecting on table_id keeps the sections opaque: it is the first byte under generic section syntax, the same byte `section_key` already reads to dedupe. The filter runs after reassembly because the EIT PID interleaves the two tables, and no surviving section means no entry, so a schedule-only stream leaves no empty PID behind for export to emit. Schedule stays out. The catalog is whole-state and republished on every change, so a full multi-service eight-day EPG would cost every subscriber the whole document at join, ahead of media. It belongs on its own track if it is ever wanted, the way SCTE-35 sections already ride one. TDT/TOT stays out because every section is new content, and an exporter's own clock beats a time relayed from an upstream multiplexer of unknown delay. Both rationales are now recorded next to the entries rather than being silent omissions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 6 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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. Comment |
![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: 605ee6ae88
ℹ️ 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".
| // SCTE-35 sections already ride one (see `SectionStream` in the import path). | ||
| SiPid { | ||
| pid: 0x0012, | ||
| tables: Tables::Only(&[(0x4E, 0x4F)]), |
There was a problem hiding this comment.
When PID 0x0012 contains 0x4F sections for services on different transport streams or networks that reuse the same service_id and section number, enabling EIT-other capture here feeds them to Si::upsert, whose key is only (table_id, table_id_extension, section_number). Because an EIT's table_id_extension is just its service_id, the later section silently replaces the earlier one, and the exported EIT p/f omits a service. Either exclude 0x4F or include its transport_stream_id and original_network_id in the section identity. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L98-L98
Useful? React with 👍 / 👎.
1 participant
Closes #2800 (partially: p/f and TDT/TOT, not schedule).
moq import tsdropped EIT at the import gate, so the EPG could not survive a TS round-trip even though #2440 had already built the mechanism that would carry it. #2800 measured the cost on a synthesized EPG and found it far cheaper than we assumed: thesimap dedupes by(table_id, table_id_extension, section_number)and compares before taking the catalog's write lock, so cost tracks the revision rate, not the transmission rate. Over ten minutes, EIT present/following was 4 distinct sections out of 592 transmitted.Root cause of the gap
SI_PIDSwas keyed by PID alone, andsi_sectioncaptured every completed section on an intercepted PID whatever itstable_id. That is deliberate and correct for the PIDs it covered (the SDT PID also carries the BAT; an unrecognized table is as worth preserving as a known one), but it made EIT all-or-nothing: p/f (0x4E,0x4F) and schedule (0x50..=0x6F) share PID 0x0012.Si::intervalmade that concrete rather than theoretical. It is one value per PID, but per ETSI TS 101 211 p/f actual is 2 s and schedule is 10-30 s. One PID, two cadences, one slot: there was no entry that was correct for both.The change
SI_PIDSbecomes&[SiPid](pid, tables, interval) with aTables::{All, Only}filter, and 0x0012 joins it asOnly(&[(0x4E, 0x4F)])at 2 s.Allstays the default posture and is what 0x0010 and 0x0011 use, so nothing about their capture changes.Filtering on
table_iddoes not make the sections any less opaque: it is the first byte of every section under generic section syntax, the same bytesection_keyalready reads to dedupe. This narrows what we carry without widening what we claim to parse.The filter runs after reassembly, in
si_section, because the EIT PID interleaves p/f with schedule and a section boundary is only findable by following the whole PID. When nothing survives the filter the entry is never created, so a schedule-only stream leaves no empty 0x0012 behind for export to emit.Export needed no change: it already walks
mpegts.sigenerically, honoring each PID's own interval.What is deliberately left out
EIT schedule. The catalog is whole-state and republished on every change, so cost per change is O(catalog), not O(delta), and every subscriber pays the full document at join, ahead of media. #2800's fixture yields 8 distinct schedule sections because it is one service with twelve events; a full multi-service eight-day EPG is thousands of sections whose window edges churn continuously as it rolls forward. That asymmetry does not depend on the magnitude, only on the catalog being whole-state. If schedule is ever wanted it belongs on its own track, the way SCTE-35 sections already ride one via
SectionStream, and the reasoning is recorded next to the 0x0012 entry.TDT/TOT (0x0014), which is #2800's ask 3. Every section is new content rather than a repetition, so each would be a catalog modification and a republish, and it is the table with the least to gain from being relayed: an exporter's own clock is a better source than a time forwarded from an upstream multiplexer of unknown delay. The omission was previously silent and read as an oversight; it now has the rationale next to it.
Tests
Three, in
export_test.rsalongside the existing SI coverage:eit_carries_present_following_not_schedulefeeds six sections interleaving p/f with schedule at both ends of both ranges, and asserts exactly the two p/f sections survive at a 2 s interval. This is also the regression test for the filter running post-reassembly.eit_schedule_alone_creates_no_entrycovers the empty-entry case.tdt_is_not_capturedpins the 0x0014 policy.The existing
multi_packet_si_section_is_capturedis refactored onto the newimport_sihelper rather than duplicating the setup.Not done here
#2800 offers TSDuck fixture scripts (
make-eit-fixture.sh,eit-roundtrip.sh) that synthesize an EPG onto any clip. Worth taking as a follow-up: no broadcast capture we hold carries EIT, which is why this gap survived #2440 with the mechanism in place and the table unrouted. The tests here are synthetic sections, not a real multiplex.Cross-package sync
doc/bin/cli.mddescribed TDT/TOT and EIT together as "live or bulky rather than static identity" - two correct rationales covering three tables, and the same conflation this PR splits. Updated. No draft change: thempegtscatalog section is not specified indrafts/. No js mirror:js/has no MPEG-TS support.(written by Opus 5)