feat(moq-cli): per-sink frame-drop latency for the export gateways#1998
Conversation
`export --latency-max` previously only reached the stdout container path (#1985). The gateway egress sinks built their own moq-mux exports and never set a latency, so they fell back to 0 (drop stale groups aggressively) with no way to configure the frame-drop budget. Thread the moq-level frame-drop latency (the OrderedConsumer skip threshold, `moq_mux::container::Consumer::with_latency`) into each export sink, and give each sink its own knob so the default fits the transport rather than exposing a global flag that silently applies to only some sinks: - Container stdout (fmp4/mkv/ts/flv): `--latency-max` (500ms), via a shared `Container` args struct. - rtmp: `--latency-max` (500ms) on a new export-only `ExportArgs`, wired to the new `moq_rtmp::Play::with_latency` / `Client::with_latency` / `Config.latency` (additive; the FLV export now sets it on both the serve and dial paths). - hls: `--latency-max` (10s, generous so live GOPs aren't skipped while segments build), exposing the existing `moq_hls::export::Config.latency` the CLI never wired up. - srt: reuses its transport `--latency` as the skip threshold (SRT paces egress on the media clock, so the receive buffer and the skip threshold are the same budget); threaded internally, no public API change. - rtc: none. WebRTC is real-time and doesn't buffer. The moq-rtmp `Config` field is additive (`#[non_exhaustive]`, defaults to 0), so `run()` stays compatible for embedders. The relay does not embed `moq_rtmp::run` / `moq_srt::run` in this repo. Closes #1987. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
…d9097 # Conflicts: # rs/moq-cli/src/hls.rs
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis change adds per-transport latency-max configuration for export sinks. Stdout container args now carry ChangesDocumentation: Added stalled-group latency behavior and per-transport default knobs to Sequence Diagram(s)sequenceDiagram
participant ExportArgs
participant listen_export
participant Play
participant connect_export
participant Client
ExportArgs->>listen_export: latency_max
listen_export->>Play: play.with_latency(latency)
Play->>Play: FlvExport.with_latency(self.latency)
ExportArgs->>connect_export: latency_max
connect_export->>Client: with_latency(latency)
Client->>Client: FlvExport.with_latency(self.latency)
Related PRs: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rs/moq-srt/src/ts.rs`:
- Around line 78-91: Update the doc comment on Subscriber::new in ts.rs to
soften the latency description: it should describe latency as the locally
configured SRT receive latency used as the skip threshold, not a negotiated
end-to-end budget. Keep the existing behavior in
OriginConsumer::announced_broadcast and ts::Export::new untouched, and revise
the wording so it says the muxer reuses the configured SRT latency budget rather
than claiming it always matches the peer’s actual buffer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4213d19-8d2f-4e3f-b58b-f76be31d6bb6
📒 Files selected for processing (11)
doc/bin/cli.mdrs/moq-cli/src/args.rsrs/moq-cli/src/hls.rsrs/moq-cli/src/main.rsrs/moq-cli/src/rtmp.rsrs/moq-rtmp/src/dial.rsrs/moq-rtmp/src/listen.rsrs/moq-rtmp/src/server.rsrs/moq-srt/src/dial.rsrs/moq-srt/src/server.rsrs/moq-srt/src/ts.rs
…tiated, SRT value srt-tokio doesn't expose the post-handshake negotiated latency, so the skip threshold reuses the locally configured receive latency. A peer that requests a higher receive latency gets a larger actual buffer than the skip threshold. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
export --latency-maxpreviously only reached the stdout container path (added in #1985). The gateway egress sinks (rtmp,srt,hls) build their own moq-mux exports internally and never set a latency, so they fell back to the default (0= drop stale groups aggressively) with no way to configure the frame-drop budget. Closes #1987.The frame-drop latency is the moq-level "how long to wait for a stalled group before skipping to a newer one" — the
moq_mux::container::Consumer::with_latencyknob (the ordered/skip consumer). It's media-agnostic, and every egress pulls frames through it, so all of them can carry it.Rather than a global
exportflag that silently applies to only some sinks, each sink owns its own--latency-maxso the default fits the transport:fmp4/mkv/ts/flv(stdout)--latency-max500msContainerargsrtmp--latency-max500msExportArgs)hls--latency-max10sConfig.latency; generous so live GOPs aren't skipped while segments buildsrt--latency200msrtcChanges
moq-rtmp (additive public API):
Play::with_latency()andClient::with_latency()builders +listen::Config.latency(defaults to0= today's behavior,#[non_exhaustive]Config sorun()stays compatible). Wired intoFlvExport::with_latencyon both the serve and dial egress paths.moq-srt (internal only, no public API change):
Server::bind/dial::publish/Config.latency) now also drives the egressts::Exportskip threshold. SRT paces egress on the media clock, so the receive buffer and the skip threshold are the same end-to-end budget.moq-cli:
Export-level latency flag; pushed--latency-maxdown onto each stdout container (new sharedContainerargs struct;Fragmentedflattens it) and ontohls.rtmp::Argsinto the shared endpoint (Args, used by import) andrtmp::ExportArgs(flattensArgs+--latency-max) so the frame-drop knob only appears onexport rtmp, notimport rtmp.srtneeded no CLI change — the library now consumes the--latencyit was already passing.doc/bin/cli.md: documents the per-sink latency knobs.
Notes for reviewers
Play::with_latency,Client::with_latency, andConfig.latency— all additive, non-breaking. moq-srt has no public API change. moq-cli renames/reshapes CLI flags (--latency-maxis now per-sink; the flag added in moq-cli: unified endpoint grammar (binary renamed tomoq) #1985 was not yet released). Nothing wire-level. Targeting main.moq_rtmp::run/moq_srt::runin this repo, so no relay blast radius; the additiveConfig.latencykeepsrun()compatible for any out-of-tree embedder.0latency; it now uses the SRT receive latency (default 200ms). Arguably a fix for a paced egress.Test plan
cargo clippy+cargo teston moq-cli / moq-rtmp / moq-srt (green)-D warnings(no broken intra-doc links)cargo fmt --check,taplo format --checkmoq export <sink> --helpshows each sink's own latency flag;export rtmphas--latency-max,import rtmpdoes not; the--connect|--listengroup is still enforced through the flatten(Written by Claude Opus 4.8)