feat(cli): add moq play

[kixelated]

Summary

• Add an opt-in play feature with a top-level moq ... play command.
• Follow catalog updates, decode the first supported audio and video renditions, and present them through the native speaker and a wgpu window.
• Synchronize video to the audio clock, support audio-only and video-only broadcasts, preserve aspect ratio, and surface transport or playback failures.
• Document feature-enabled builds, selection flags, latency control, and platform backends.

Public API changes

• Adds the feature-gated play CLI command and PCM as an audio rendition selector.
• No MoQ wire format or published library API changes.

Test plan

• nix develop --command just fix
• nix develop --command cargo nextest run -p moq-cli --features play
• nix develop --command cargo clippy -p moq-cli --features play --all-targets -- -D warnings
• nix develop --command just check

Cross-package sync

• Updated rs/moq-cli/README.md and doc/bin/cli.md.
• No draft or language-binding updates are needed because this does not change the wire format or a library API.

Related to #2481.

(Written by GPT-5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0dcea4f259

ℹ️ 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".

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

The CLI adds a feature-gated play command for native broadcast playback. It supports catalog-based rendition selection, PCM audio, latency settings, and remote relay sources. Playback subscribes to selected renditions, decodes media, buffers video, outputs audio, synchronizes timestamps, and renders video in a winit/wgpu window. Audio and video consumers now use catalog-defined Hang container framing. Documentation and parsing tests cover the command.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 31.03% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the addition of the moq play CLI feature, which is the primary change.
Description check	✅ Passed	The description accurately summarizes native playback, supported formats, documentation, testing, and related implementation changes.

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch codex/moq-play

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

rs/moq-cli/src/play.rs (2)

561-582: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move the WGSL shader into its own file.

The shader is embedded as a Rust string literal with \n\ line continuations. That form is easy to break during edits, and no editor tooling highlights or validates it. Put the shader in rs/moq-cli/src/play.wgsl and load it with include_str!, or at minimum use a raw string literal.

♻️ Proposed change

 		let shader = device.create_shader_module(wgpu::ShaderModuleDescriptor {
 			label: Some("moq play shader"),
-			source: wgpu::ShaderSource::Wgsl(
-				"struct VertexOutput {\n\
-				 ...
-				 }"
-				.into(),
-			),
+			source: wgpu::ShaderSource::Wgsl(include_str!("play.wgsl").into()),
 		});

🤖 Prompt for 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.

In `@rs/moq-cli/src/play.rs` around lines 561 - 582, Move the embedded WGSL source
from the shader module construction in play.rs into a new play.wgsl file,
preserving the existing shader behavior and contents. Replace the Rust string
literal with include_str! referencing that file, while leaving the surrounding
device.create_shader_module configuration unchanged.

---

39-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Share the catalog-format fallback with SubscribeArgs.

Args::catalog_format repeats SubscribeArgs::catalog_format in rs/moq-cli/src/subscribe.rs lines 152-157 exactly. This PR already extracted SelectArgs::selection for the same reason. Move the fallback into one helper, for example a free function or an inherent method on CatalogFormatArg, and call it from both.

♻️ Proposed shared helper

Add to rs/moq-cli/src/subscribe.rs:

/// Resolve the catalog format: the explicit flag, then the broadcast name
/// suffix, then the default.
pub(crate) fn catalog_format(arg: Option<CatalogFormatArg>, broadcast: &str) -> CatalogFormat {
	arg.map(Into::into)
		.or_else(|| CatalogFormat::detect(broadcast))
		.unwrap_or_default()
}

Then in rs/moq-cli/src/play.rs:

 impl Args {
 	fn catalog_format(&self, broadcast: &str) -> CatalogFormat {
-		self.catalog_format
-			.map(Into::into)
-			.or_else(|| CatalogFormat::detect(broadcast))
-			.unwrap_or_default()
+		crate::subscribe::catalog_format(self.catalog_format, broadcast)
 	}
 }

As per coding guidelines: "extend existing primitives, and generalize rather than duplicate helpers."

🤖 Prompt for 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.

In `@rs/moq-cli/src/play.rs` around lines 39 - 46, The catalog-format fallback is
duplicated between Args::catalog_format and SubscribeArgs::catalog_format.
Extract the shared resolution logic into one helper, such as a crate-visible
function or CatalogFormatArg method, then update both methods to call it while
preserving the explicit argument, broadcast detection, and default fallback
order.

Source: Coding guidelines

🤖 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-cli/src/main.rs`:
- Around line 127-148: Move `play` command dispatch out of the
`#[tokio::main]`-driven `run_play` path so `play::run` executes on the process
main thread. Keep the Tokio runtime available for the asynchronous setup in
`run_play`, but ensure `winit::EventLoop::build()` and `run_app()` invoked by
`play::run` never run on a Tokio worker thread.

In `@rs/moq-cli/src/play.rs`:
- Around line 165-169: Update the playback task-joining loop around
tasks.join_next() and the post-loop media join so it drains every task before
returning, while returning early only when a joined task reports an error. Add a
regression test that starts two playback tasks, completes one first, and
verifies the loop remains active until the second completes.
- Around line 176-208: Update the rendition loops in the playback flow so
failures from source.resolve are handled like Consumer::new failures: log a
warning with the rendition name and error, then continue trying subsequent
renditions. Apply this change to both the video and audio loops, while
preserving the existing successful resolution and decoder behavior.
- Around line 337-340: Update the next_redraw assignment in the play loop to use
checked_add when adding the computed duration to Instant::now(). Preserve the
existing clock and next_timestamp matching, but return None when checked_add
overflows so hostile or corrupt timestamps cannot panic.

---

Nitpick comments:
In `@rs/moq-cli/src/play.rs`:
- Around line 561-582: Move the embedded WGSL source from the shader module
construction in play.rs into a new play.wgsl file, preserving the existing
shader behavior and contents. Replace the Rust string literal with include_str!
referencing that file, while leaving the surrounding device.create_shader_module
configuration unchanged.
- Around line 39-46: The catalog-format fallback is duplicated between
Args::catalog_format and SubscribeArgs::catalog_format. Extract the shared
resolution logic into one helper, such as a crate-visible function or
CatalogFormatArg method, then update both methods to call it while preserving
the explicit argument, broadcast detection, and default fallback order.

🪄 Autofix

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 Plus

Run ID: d4cf0e47-4fd8-4af1-b0cd-4a2297681eb7

📥 Commits

Reviewing files that changed from the base of the PR and between f67ac9c and 0dcea4f.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (7)

• doc/bin/cli.md
• rs/moq-cli/Cargo.toml
• rs/moq-cli/README.md
• rs/moq-cli/src/args.rs
• rs/moq-cli/src/main.rs
• rs/moq-cli/src/play.rs
• rs/moq-cli/src/subscribe.rs

[kixelated]

Review

Nice shape overall: the winit/wgpu app is self-contained, the feature gate keeps the default binary clean, and factoring SelectArgs::selection out of SubscribeArgs is the right way to share the rendition flags. Two things block, plus some correctness and doc follow-ups.

Blocking

1. CI is red on cargo deny, and it fails before anything compiles.

winit's default wayland-csd-adwaita feature pulls sctk-adwaita -> ab_glyph -> owned_ttf_parser -> ttf-parser, which is RUSTSEC-2026-0192 (unmaintained, "No safe upgrade is available"). Worth knowing: in rs/justfile, cargo deny check runs ahead of cargo check / cargo clippy / cargo doc / nextest, so CI has never compiled a line of play.rs on this branch. The green-looking earlier steps are cargo fmt / cargo sort / cargo shear only.

I verified locally on macOS that the code is in fact fine:

• cargo clippy -p moq-cli --features play --all-targets -- -D warnings -> clean
• cargo nextest run -p moq-cli --features play -> 18 passed (including play_verb, letterboxes_without_changing_aspect_ratio, clock_advances_from_its_media_anchor)

For the advisory itself, two options:

• Preferred: add RUSTSEC-2026-0192 to the [advisories] ignore list in deny.toml with the usual justification comment. It's an unmaintained notice, not a vulnerability, and the crate only parses the system font used to draw Wayland client-side decorations.

• Alternative: trim winit's defaults. I confirmed this drops ttf-parser, ab_glyph, sctk-adwaita, and tiny-skia from Cargo.lock entirely and makes cargo deny check advisories pass:

  winit = { version = "0.30.13", optional = true, default-features = false, features = [
      "x11",
      "wayland",
      "wayland-dlopen",
      "rwh_06",
  ] }

  The cost is real though: without wayland-csd-adwaita, a GNOME/Wayland playback window gets no titlebar or close button (Escape still works). That's why I'd lean on the ignore instead.

Either way it'd be good to land the fix and let CI actually compile the feature once, since Linux is the only platform CI covers and the winit x11/wayland paths are exactly what my macOS run didn't touch.

2. The first playback track to finish tears down the whole window.

In media() (rs/moq-cli/src/play.rs), both exit paths return on the first completed task:

result = tasks.join_next(), if !tasks.is_empty() => {
    return joined(result.expect("guarded by is_empty"));
}

and after the loop:

let result = tasks.join_next().await;
joined(result.context("all playback tracks stopped")?)

play_video / play_audio return Ok(()) when their consumer reads None, i.e. when the track ends. So if the publisher ends the video track while audio is still live (a rendition swap, an encoder restart, a source that drops one role), media() returns Ok(()), run_media sends Event::Finished, and the window closes on a broadcast that is still playing. The "all playback tracks stopped" context on the second call reads like it waits for all of them, but it doesn't.

Draining the JoinSet and only exiting once it's empty (returning early on the first Err) would match the intent.

Correctness

3. Rendition selection is one-shot, which undercuts the "follows catalog updates" claim.

video_started / audio_started are set once and never cleared. Combined with (2), a catalog update that replaces the playing rendition doesn't re-select, it ends playback. The catalog stream is only polled at all while !video_started || !audio_started. That's a defensible v1, but doc/bin/cli.md ("play follows catalog updates and starts the first supported audio and video renditions") reads as if it keeps following after start. Worth either narrowing the sentence or resetting the flag when a track's task ends.

4. play_audio hardcodes F32 on the sink but never asks the decoder for it.

let mut decode = moq_audio::decode::Config::new();
decode.latency_max = Some(args.latency_max);

then

let samples = frame.data.len() / size_of::<f32>() / channels as usize;
...
format: moq_audio::Format::F32,

Both the sample-count math and the sink layout assume decode::Config::format is F32. It is today, only because Format::default() is F32. Set decode.format = moq_audio::Format::F32; explicitly so a default change is a compile-time no-op instead of silently mis-scaling the audio clock and playing garbage.

5. Audio drives a full GPU present per decoded frame.

play_audio sends Event::Wake on every write, which user_event turns into request_redraw(). At 20 ms Opus frames that's ~50 presents/sec, including on an audio-only broadcast where every one of them re-renders the same black window. Once a clock exists the video schedule (next_redraw / about_to_wait) already covers redraws; audio only needs to wake the loop for the None -> Some(clock) transition.

6. Nothing renders until the first media frame arrives.

resumed() builds the Display but never requests a redraw, and redraw() is the only thing that clears to black. A broadcast that is slow to announce, or whose renditions are all unsupported, leaves undefined window contents. Related: the "the catalog contains no playable audio or video renditions" error only fires when the catalog track ends, so a live catalog full of unsupported codecs just logs warn! per snapshot behind a blank window forever. A request_redraw() at the end of resumed() fixes the visual half cheaply.

Structure / docs

7. doc/bin/cli.md is stale on the new codec value. Line 444 still reads `--audio-codec <aac|opus>` after AudioCodecArg::Pcm was added. Same section's lead-in ("Stdout exports can also select one rendition per media role") is now shared with play, per the SelectArgs doc comment change.

8. run_play duplicates run_export's MoQ-side wiring verbatim. Client consume + notify_ready + serve_consume + run_web is now written out three times in main.rs. Per the repo's "Refactor As You Go" rule, the consume half is worth pulling into a helper in this PR rather than making it a third copy.

9. The main-thread invariant is load-bearing and unstated. run() builds a winit EventLoop, which only works on the process main thread; it happens to be correct because #[tokio::main]'s block_on polls the main future there. It fails as an Err rather than UB, so it's not a bug, but a one-line comment on run() would stop a future refactor from tokio::spawning it. Worth noting in the same breath: blocking there means the jemalloc arm of the tokio::select! in main never gets polled under --features play,jemalloc.

Nits

• MAX_VIDEO_FRAMES and VIDEO_EARLY_TOLERANCE have no comment explaining the numbers, which the surrounding code (e.g. playback::sink's LATENCY/CAPACITY) does consistently.
• "First supported rendition" is BTreeMap order, i.e. alphabetical track name, not quality. 1080p/480p/720p picks 1080p, high/low picks high. It matches the moq-rtc egress precedent, so no objection, but the doc section could say the pick is arbitrary and point at --video-name.
• SelectArgs::selection(video_codec) takes an override and falls back to self.video_codec; the SubscribeArgs caller has already folded that in, so the fallback only ever fires from play. Reads like a double source of truth.

(written by Opus 5)

[kixelated]

Pushed 3c93420 addressing the review above.

CI. Added RUSTSEC-2026-0192 to deny.toml's ignore list rather than trimming winit's features: it's an unmaintained notice on a font parser reached only through sctk-adwaita, and dropping wayland-csd-adwaita would cost the playback window its titlebar on GNOME. cargo deny check advisories passes locally. Worth repeating that this is what kept CI from ever reaching the compile steps, so this push is the first time play.rs gets built on Linux.

One track ending no longer tears down the window. media() now drains the JoinSet and returns only once every track that started has ended, instead of returning on the first join_next(). The catalog branch is disarmed by a catalog_ended flag once the stream returns None, since re-polling a finished stream would spin.

Audio decoder is asked for f32 explicitly (decode.format = Format::F32). Both the sink layout and the sample-count feeding the A/V clock assumed it and were only correct via Format::default().

Audio wakes the render loop once, on the None -> Some clock transition, instead of per 20 ms frame. Also added a request_redraw() in resumed() so the window is black while the broadcast resolves.

Docs: --audio-codec <aac|opus|pcm>, the selection-flag section now says it covers play, and the play section no longer implies catalog updates are followed after both roles have started (it also now says the "first" rendition is the alphabetically first track name, and points at --video-name/--audio-name).

Structure: hoisted the duplicated MoQ consume-side wiring out of run_export/run_play into spawn_moq_consume, and left a comment on play::run explaining why it blocks the #[tokio::main] future (winit's main-thread requirement).

Left alone deliberately: re-selecting a rendition mid-stream when the catalog swaps one out. That's a real feature rather than a fix, and the doc no longer claims it.

Verified locally on macOS:

• cargo deny check advisories -> ok
• cargo clippy -p moq-cli --features play --all-targets -- -D warnings -> clean
• cargo nextest run -p moq-cli --features play -> 18 passed
• just fix + just check -> clean

(written by Opus 5)

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@doc/bin/cli.md`:
- Around line 134-138: Update the `play` catalog-selection description to say it
waits for any playable rendition, starts audio and video independently, and
continues following catalog updates until each available media role has started.
Preserve the existing explanation of alphabetical selection, name overrides,
clock behavior, and playback completion.
- Around line 436-437: Update the CLI documentation around the rendition option
guidance and the `play` example to distinguish syntax by subcommand: keep
rendition flags before the sink subcommand for stdout exports, but document them
in the correct position for `play` as shown by its actual command syntax. Ensure
the guidance and example are consistent.

🪄 Autofix

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 Plus

Run ID: 37fd9c9f-8df8-4bdd-90fe-d72e6fc1bf15

📥 Commits

Reviewing files that changed from the base of the PR and between 0dcea4f and 3c93420.

📒 Files selected for processing (5)

• deny.toml
• doc/bin/cli.md
• rs/moq-cli/src/main.rs
• rs/moq-cli/src/play.rs
• rs/moq-cli/src/subscribe.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• rs/moq-cli/src/subscribe.rs
• rs/moq-cli/src/main.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c9342014e

ℹ️ 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".

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

rs/moq-cli/src/play.rs (2)

176-183: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Group the media inputs in a private context type.

media has six positional parameters. Create a private role-specific context struct and pass it as one parameter. This keeps the call site and future pipeline changes coherent.

As per coding guidelines: "Refactor awkward internal shapes while making changes: replace functions with four or more arguments or repeated value groups with structs or reusable primitives, and avoid copied one-off helpers."

🤖 Prompt for 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.

In `@rs/moq-cli/src/play.rs` around lines 176 - 183, Introduce a private
role-specific context struct containing the six inputs currently accepted by
media, then update media to receive that struct as its single parameter and
adjust all call sites to construct and pass it. Preserve the existing field
types and behavior while grouping the media pipeline state coherently.

Source: Coding guidelines

---

219-251: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add regression tests for both recovered failure paths.

Add a test where the first rendition fails to resolve and a later rendition starts playback. Add a deterministic test that uses an overflowing redraw duration and verifies that scheduling returns no deadline without a panic. The current listed tests do not cover either behavior.

As per coding guidelines: "Before fixing a bug, reproduce and explain the root-cause mechanism, fix the lowest layer containing the cause, and add a regression test that fails without the fix."

Also applies to: 394-398

🤖 Prompt for 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.

In `@rs/moq-cli/src/play.rs` around lines 219 - 251, Add regression coverage for
both recovery paths in the relevant playback tests: verify that when the first
video or audio rendition fails during source.resolve, a later rendition is still
attempted and starts playback, and verify that an overflowing redraw duration
causes scheduling to return no deadline without panicking. Reproduce each
failure deterministically, then place the fix at the lowest layer responsible
for the overflow while preserving the existing rendition fallback behavior.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@rs/moq-cli/src/play.rs`:
- Around line 176-183: Introduce a private role-specific context struct
containing the six inputs currently accepted by media, then update media to
receive that struct as its single parameter and adjust all call sites to
construct and pass it. Preserve the existing field types and behavior while
grouping the media pipeline state coherently.
- Around line 219-251: Add regression coverage for both recovery paths in the
relevant playback tests: verify that when the first video or audio rendition
fails during source.resolve, a later rendition is still attempted and starts
playback, and verify that an overflowing redraw duration causes scheduling to
return no deadline without panicking. Reproduce each failure deterministically,
then place the fix at the lowest layer responsible for the overflow while
preserving the existing rendition fallback behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b90fa18-aeba-4bd4-939c-698f2064aa6c

📥 Commits

Reviewing files that changed from the base of the PR and between 3c93420 and 854bc5a.

📒 Files selected for processing (4)

• doc/bin/cli.md
• rs/moq-cli/src/args.rs
• rs/moq-cli/src/main.rs
• rs/moq-cli/src/play.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• rs/moq-cli/src/main.rs
• rs/moq-cli/src/args.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 854bc5a2f5

ℹ️ 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".

[kixelated]

Pushed 854bc5a and 07b97e0. Replies are on each inline thread; the two that matter most:

The startup race is real, and worse than "a race". media() called Source::broadcast() immediately, which lands on origin::Consumer::request_broadcast. With no session yet, Requests::insert sees handlers == 0 and the request resolves Unroutable on the spot rather than parking. moq-net already has a test pinning that precondition (dynamic_request_unroutable_without_handler), and the Dynamic handler is only registered by a live session's subscriber, so run_play spawning the media task right after kicking off the reconnect loop loses to the handshake essentially every time, not occasionally. Now it waits for the announcement first, like run_stdout does. The wait lives inside media() rather than run_play so the window is already up: it reads as a black frame with a working close button instead of a process that looks hung.

Undecodable codec selectors are rejected up front. The selection flags are shared with the stdout exports, which pass bytes through and so accept every codec the catalog can name. moq-video decodes h264/h265/av1 and moq-audio decodes opus/pcm, so play --video-codec vp9 used to filter the catalog down to a rendition that never opens and then sit on a blank window. play::Args::validate now fails before anything dials.

Also in these two commits:

• A rendition whose broadcast reference is unroutable no longer takes the player down; both loops warn and fall through to the next rendition, matching how an unsupported codec was already handled.
• Instant::now() + duration panics rather than saturating, and that duration is derived from a wire timestamp. Switched to checked_add.
• Documented the Linux install as --no-default-features --features "iroh,quinn,websocket,play". play enables moq-video, which the default pipewire feature then wires up for display capture, so cargo install moq-cli --features play wanted libpipewire-0.3 and libclang for a backend playback never touches. I left pipewire in default rather than folding it into capture, since that would silently drop import capture --display for anyone building with defaults today. Say the word if you'd rather change the default.
• Doc corrections: the play section claimed it waits for both roles two lines above saying single-role broadcasts work, and the export section claimed the selection flags precede the subcommand, which is only true for the stdout sinks.

Not done, deliberately: switching renditions mid-stream when the catalog replaces the one being played. Naively re-selecting on track end spins, because the catalog still lists the rendition and we resubscribe to a track that immediately ends again. Doing it properly means diffing snapshots against the playing rendition and aborting that role's task, which is adaptive-bitrate work rather than a fix for this PR. The docs now state the limitation.

Verified locally on macOS: cargo deny check advisories, cargo clippy -p moq-cli --features play --all-targets -- -D warnings, cargo nextest run -p moq-cli --features play (19 passed), just fix, just check.

(written by Opus 5)

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

rs/moq-cli/src/play.rs (1)

99-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace positional playback state with a context type.

Use an internal context struct to group playback inputs and shared state. This prevents argument-order mistakes and removes repeated parameter groups.

• rs/moq-cli/src/play.rs#L99-L103: Group origin, broadcast, args, and network into a playback request or runner type.
• rs/moq-cli/src/play.rs#L161-L168: Pass the context instead of six positional values.
• rs/moq-cli/src/play.rs#L177-L184: Reuse the same context for media.

As per coding guidelines: "Refactor awkward internal shapes while making changes: replace functions with four or more arguments or repeated value groups with structs or reusable primitives."

🤖 Prompt for 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.

In `@rs/moq-cli/src/play.rs` around lines 99 - 103, Introduce an internal playback
context type in rs/moq-cli/src/play.rs to group origin, broadcast, args, and
network, then update run at lines 99-103 to accept that context instead of
positional inputs. At lines 161-168, pass the context rather than six separate
values, and at lines 177-184, reuse the same context when invoking media.

Source: Coding guidelines

🤖 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-cli/src/play.rs`:
- Around line 185-194: In the play test suite, add a regression test for delayed
matching announcements: hold the announcement until after playback is requested,
assert that the media future remains pending, then announce the broadcast and
assert playback starts successfully. Exercise the existing origin/session setup
and announcement APIs around Source::new and announced_broadcast so the test
fails when Source::broadcast runs before registration.

---

Nitpick comments:
In `@rs/moq-cli/src/play.rs`:
- Around line 99-103: Introduce an internal playback context type in
rs/moq-cli/src/play.rs to group origin, broadcast, args, and network, then
update run at lines 99-103 to accept that context instead of positional inputs.
At lines 161-168, pass the context rather than six separate values, and at lines
177-184, reuse the same context when invoking media.

🪄 Autofix

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 Plus

Run ID: 2c7dc746-a708-4155-bbca-390920d9802c

📥 Commits

Reviewing files that changed from the base of the PR and between 854bc5a and 07b97e0.

📒 Files selected for processing (2)

• rs/moq-cli/src/main.rs
• rs/moq-cli/src/play.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• rs/moq-cli/src/main.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07b97e04a4

ℹ️ 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".

[kixelated]

Pushed c6b5baf. Replies on each thread; the substance:

A catalog this build can't play now says so. The earlier validate only catches selectors no build can decode. The decoders are gated by platform and cargo feature (av1 only via nvdec, VideoToolbox only on macOS), so --video-codec av1 on a Mac still filtered the catalog down to a rendition that never opens. Mirroring the decode::backend cfg matrix inside moq-cli would rot, so instead media records why each rendition was rejected and errors when a snapshot offered renditions, none started, and nothing was already playing. Partial failure still degrades: video-only or audio-only keeps running.

The tail plays out. Sink::write never blocks and drops what won't fit, so a burst delivered faster than real time lost samples, and dropping the sink at EOF cut off whatever was still queued. play_audio now waits when it is more than a second ahead of the speaker, and drains before returning (bounded, since a device that never opens reports a full queue forever). Queued video gets the same: decoder EOF is Event::Ended, which presents what is left before exiting, while Ctrl-C and transport loss still stop immediately via Event::Finished.

V4L2: confirmed, documented, not fixed here. v4l is an unconditional dependency in moq-video's cfg(target_os="linux") block, so --no-default-features drops PipeWire but not V4L2, and a Linux build wants libclang and the V4L2 headers whatever features are picked. My previous install note implied otherwise; it now states the requirement. Putting moq-video's capture module and its per-platform deps behind a feature is the actual fix, but that reshapes a published library's feature surface across three platforms I can't compile here, so I'd rather it be its own change than ride along with a CLI verb. Happy to open it if you want.

Also folded in the "six parameters" note from the out-of-diff review: media is now a Media struct with a run.

Verified locally on macOS: clippy -D warnings with --features play, cargo nextest run -p moq-cli --features play (19 passed), just fix, just check, cargo deny.

Still unverified anywhere: the Linux x11/wayland build of play.rs. cargo-deny runs ahead of every compile step in just rs ci, so CI had never built this feature before the deny fix landed, and the queue has been slow enough that no run has completed on it yet.

(written by Opus 5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6b5bafac6

ℹ️ 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".

[kixelated]

Pushed 1f2ef05 and c224cda. All review threads are resolved.

• Lost surface is recoverable. CurrentSurfaceTexture::Lost was closing the window with an error. wgpu documents it as "the surface has been lost and needs to be recreated", which is stronger than the Outdated path: Instance::create_surface then configure. Display keeps the instance so it can do that; Validation stays fatal.
• The announce-wait now has a regression test. Split out as a subscribe helper so it's reachable without an event loop. The test asserts the pre-fix path directly (subscribing to an unannounced broadcast resolves Unroutable on the spot, which pins the mechanism), then that subscribe stays parked through a 60s timeout on a paused clock, then that it resolves once announced. Verified it fails with the wait removed.
• A long PCM frame is no longer truncated. The pacing wait only looked at what was already buffered, so an empty sink accepted any single write whole. Opus caps at 120ms but PCM only has to be sample-aligned, so an oversized frame lost its tail, and worse, the clock advanced by the frame's full duration against a buffered() that never held those samples, putting video permanently ahead. Writes are chunked to a second with headroom re-checked between them, cut on the sample stride so alignment is unchanged.

CI still hasn't run. The only run this branch has produced since the deny fix is for 3c93420, and it has been queued for hours without starting; the four commits since haven't been assigned runs at all. That looks like the repo's Actions queue rather than anything in the workflow, and it means play.rs has still never been compiled on Linux by CI. Everything above is verified on macOS only: clippy -D warnings with --features play, 20 tests passing, just fix, just check, cargo deny. Worth a look at the runner situation before merging on green.

(written by Opus 5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c224cda665

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55fafa5d5f

ℹ️ 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".

[kixelated]

Ran it locally against a real relay, and it found the thing eight rounds of static review didn't: moq play could not play anything published through import fmp4, including the project's own bbb.mp4. Fixed in 17c51d2.

Repro: local moq-relay, ffmpeg | moq --broadcast x.hang import fmp4, then moq --broadcast x.hang play. Both renditions selected, VideoToolbox opened, then ~25ms later:

Error: annexb: truncated length-prefixed NAL unit

Root cause: decode::Consumer in both moq-video and moq-audio hardcoded container::legacy::Wire, so it read every track as a varint timestamp plus a codec payload. import fmp4 publishes Container::Cmaf, where a frame is a whole moof+mdat fragment. Parsing that as legacy produced garbage rather than failing, and the first frame reached the H.264 decoder as a malformed access unit. moq export h264 worked throughout, because the export path reads the container off the catalog.

catalog::hang::Container already exists for this, dispatching Legacy/Cmaf/Loc at runtime via TryFrom<&hang::catalog::Container>, and the catalog entry was already a parameter. Three lines per crate. It hits moq-transcode's decode path the same way, so only legacy-wire sources worked there too.

What the run then confirmed, which nothing in this PR had been able to before:

• The announce wait works: connect, wait, resolve, subscribe, no Unroutable.
• Rendition selection picks video and audio and reports the decoder.
• Video renders through VideoToolbox and audio plays through CoreAudio at 48kHz stereo, in sync, for ~18s until the window was closed.
• Clean exit on window close, and separately on SIGINT.
• Zero audio overflow/underflow warnings, so the pacing and the chunked writes behave, and zero "gave up re-presenting", so the surface path is stable.
• The AAC rejection path is real: on bbb.mp4 it logged cannot play audio rendition ... unsupported audio codec: mp4a.40.2 and kept playing video, which is the degradation this PR added.

Scope note: this last commit touches moq-video and moq-audio rather than moq-cli. No public signature changes (the field is private, Consumer::new is unchanged), and it is a straight bug fix, but say the word if you would rather it were split into its own PR. It is also the only part of this branch verified end to end, so I would not want it dropped silently.

No automated regression test for it: reproducing needs a live broadcast plus a platform decoder. The per-wire parsing itself is already covered in moq-mux.

(written by Opus 5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17c51d2bad

ℹ️ 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".

[kixelated]

CI red here is not from this branch. just ci fails at the shell lint step with error: justfile does not contain recipe _shell-files``, inherited from main: #2650 changed the `ci` recipe to call a recipe that has never existed. Opened #2711 to fix it; this PR should go green on its next run once that lands.

Everything else on 17c51d2 passed on Linux in that same run, which is the first time this branch has been compiled there: cargo-deny, cargo check --no-default-features, clippy --workspace --all-targets --all-features -D warnings (so the winit x11/wayland and wgpu paths), cargo doc with -D warnings, just rs wasm, and 2763 tests passing.

(written by Opus 5)

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da2fec3777

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38ae42982b

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9456d0f8b

ℹ️ 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".