Skip to content

feat(moq-audio)!: let publish_capture carry a catalog extension - #3356

Merged
kixelated merged 1 commit into
moq-dev:mainfrom
Frando:pr/audio-catalog-ext
Sep 5, 2026
Merged

feat(moq-audio)!: let publish_capture carry a catalog extension#3356
kixelated merged 1 commit into
moq-dev:mainfrom
Frando:pr/audio-catalog-ext

Conversation

@Frando

@Frando Frando commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

encode::Producer is already generic over CatalogExt, but Publication and publish_capture hardcode the default, so the turnkey capture path is closed to any consumer whose catalog carries an extension. Reaching it meant driving the capture and the encoder by hand, for no reason other than the signature.

This PR is part of a series to update iroh-live to latest moq, see n0-computer/iroh-live#45. The code and this description were written by Claude Code.

Summary

Threads the parameter through Publication::new, build, Driver, Track and EncoderOutput, which is every type between the entry point and the producer that already had it.

The video side has never had this problem: moq_video::encode::publish_capture takes the catalog producer whole. This brings the audio side level with it.

Breaking changes

Driver becomes Driver<E: CatalogExt = ()>.

The parameter defaults, so nothing that writes Driver in an expression position changes, and no call to Publication::new or publish_capture changes: the extension is inferred from the catalog producer passed in. What breaks is naming the type without the default, which is a struct field, a function signature, or an impl header holding a Driver. Those become Driver<()>.

We hit exactly one such site downstream and it was a one-word change.

Test plan

  • cargo check -p moq-audio --all-features and cargo clippy --all-targets, clean.
  • cargo test -p moq-audio: 72 pass.
  • Used downstream in iroh-live, which publishes a catalog carrying its own extension through publish_capture, which is the case that motivated this.

Note on rebasing

This was written before #3337 reworked Publication to outlive a missing input, and has been re-applied on top of that design rather than merged with the old one: the retry, the Reserved registration and Status::Starting are all upstream's, and this only adds the type parameter to them.

`encode::Producer` is already generic over `CatalogExt`, but `Publication`
and `publish_capture` hardcode the default, so the turnkey path is closed
to any consumer whose catalog carries an extension. Reaching it meant
driving the capture and the encoder by hand, for no reason other than the
signature.

Thread the parameter through `Publication::new`, `build`, `Driver`,
`Track` and `EncoderOutput`.

BREAKING CHANGE: `Driver` is now `Driver<E: CatalogExt = ()>`. The
parameter defaults, so a caller that writes `Driver` keeps compiling, but
one that names the type in a struct field, a function signature or an impl
without the default will need `Driver<()>`.
Frando added a commit to Frando/moq that referenced this pull request Sep 3, 2026
@kixelated
kixelated marked this pull request as ready for review September 5, 2026 00:29
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 83354628-34e3-4a96-b60a-bc8c728c7aa8

📥 Commits

Reviewing files that changed from the base of the PR and between 5eea9e3 and f71136a.

📒 Files selected for processing (1)
  • rs/moq-audio/src/encode/capture.rs

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

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T00:31:48.565191Z f71136a Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated

Copy link
Copy Markdown
Collaborator

@coderabbitai review

(written by Claude Opus 5)

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- PR metadata ---'
gh pr view 3356 --repo moq-dev/moq --json number,title,baseRefName,headRefName,files,commits

printf '%s\n' '--- Changed files and diff ---'
gh pr diff 3356 --repo moq-dev/moq --color=never

printf '%s\n' '--- Rust source outlines for likely affected files ---'
fd -e rs . | while IFS= read -r file; do
  if rg -q '\b(Publication|Driver|EncoderOutput|publish_capture|CatalogExt)\b' "$file"; then
    echo "### $file"
    ast-grep outline "$file" --items all --type struct,impl,function,type --match 'Publication|Driver|EncoderOutput|publish_capture|CatalogExt' || true
  fi
done

Length of output: 11673


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- capture publication and driver paths ---'
sed -n '160,390p' rs/moq-audio/src/encode/capture.rs
sed -n '390,735p' rs/moq-audio/src/encode/capture.rs
sed -n '780,900p' rs/moq-audio/src/encode/capture.rs

printf '%s\n' '--- audio producer and reservation definitions ---'
sed -n '1,280p' rs/moq-audio/src/encode/producer.rs

printf '%s\n' '--- Catalog extension trait and catalog producer ---'
sed -n '1,240p' rs/moq-mux/src/catalog/hang/ext.rs
sed -n '1,240p' rs/moq-mux/src/catalog/producer.rs

printf '%s\n' '--- Public API uses and type annotations ---'
rg -n --type rust -C 4 \
  '\b(Publication::new|publish_capture\s*\(|Driver(?:\s*<[^>]*>)?\b)' \
  rs

printf '%s\n' '--- Generic catalog producer construction and audio/video capture parity ---'
rg -n --type rust -C 4 \
  '\bmoq_mux::catalog::Producer(?:\s*<[^>]*>)?\b|\bpublish_capture\s*\(' \
  rs/moq-audio rs/moq-video rs/moq-cli

Length of output: 110611


@kixelated No issues found.

The CatalogExt type parameter is preserved from Publication::new and publish_capture through Driver, Track, Reserved, Producer, and EncoderOutput. Existing callers retain the default Driver<()> behavior.

You are interacting with an AI system.

@kixelated
kixelated merged commit f5e4ae1 into moq-dev:main Sep 5, 2026
2 checks passed
@moq-bot moq-bot Bot mentioned this pull request Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants