feat: add moq-audio crate, raw-audio FFI, and rename moq-codec to moq-video#1484
Merged
Conversation
kixelated
added a commit
that referenced
this pull request
May 24, 2026
… libopus CI for #1484 failed because `audiopus_sys` (pulled in by `moq-audio`'s `opus` feature) couldn't find a system libopus via pkg-config and fell back to building the vendored copy via CMake. That copy's CMakeLists.txt asks for `cmake_minimum_required(VERSION < 3.5)`, which modern CMake rejects. Two changes: - Add `libopus` to `flake.nix` `rustDeps` so pkg-config finds a system copy. This is the path used by the default `opus` feature and by `cargo check --workspace`. - Set `CMAKE_POLICY_VERSION_MINIMUM=3.5` in `.cargo/config.toml [env]` so that the vendored CMake build still works when `opus-static` is enabled (CI's `cargo check --workspace --all-features` exercises this path). The cmake crate inherits cargo's env when invoking CMake, so the var propagates without further plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request introduces 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rs/moq-ffi/src/error.rs (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
#[non_exhaustive]toMoqError
rs/moq-ffi/src/error.rsdefinespub enum MoqErroras a publicthiserrorenum without#[non_exhaustive], reducing forward-compatibility for downstream exhaustive matches.Suggested fix
#[derive(Debug, thiserror::Error, uniffi::Error)] +#[non_exhaustive] #[uniffi(flat_error)] pub enum MoqError {🤖 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-ffi/src/error.rs` at line 4, The public error enum MoqError is missing the #[non_exhaustive] attribute which prevents downstream crates from exhaustively matching its variants; add #[non_exhaustive] above the pub enum MoqError declaration (and keep existing derives/thiserror attributes) so consumers must use a wildcard arm when matching, then run cargo check to ensure no internal exhaustive matches break.
🤖 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/libmoq/src/audio.rs`:
- Line 235: The FFI parameter input_format is ignored in
moq_publish_raw_audio_opus (and the similar block at lines ~251-258); either
validate it at the FFI boundary or propagate it into the producer creation. Fix
by checking the moq_audio_format value inside moq_publish_raw_audio_opus,
returning an error (via the existing FFI error path) for unsupported/invalid
formats, or modify AudioProducer::new_opus to accept a format parameter and pass
input_format through when creating the producer so the provided format is
enforced; reference moq_publish_raw_audio_opus, input_format, moq_audio_format,
and AudioProducer::new_opus when making the change.
- Around line 173-184: The run function currently snapshots entry.callback while
holding State::lock then drops the lock and calls callback, allowing
consume_close to revoke the entry concurrently and causing a potential FFI
use-after-free; fix by atomically removing/taking the callback from the task
entry while holding the lock (e.g., use
State::audio.consumer_tasks.get_mut(task_id) and Option::take on entry.callback
or otherwise replace it with a no-op) so that after dropping the lock you hold
ownership of the callback and calling it cannot race with consume_close; ensure
consume_close likewise removes/clears the map entry (or checks for already-taken
callbacks) so both run and consume_close use the same ownership semantics to
prevent callbacks firing after close.
In `@rs/moq-audio/src/consumer.rs`:
- Around line 26-29: The pending Vec<f32> is never written to so flush() is a
no-op; modify the resample/merge code path (the logic that currently drains
pending and immediately returns the merged Vec) to append any leftover samples
that do not fit into the returned chunk into the pending buffer instead of
discarding them, and ensure flush() consumes and returns pending; alternatively,
if the resampler truly never produces leftovers, remove the pending field and
simplify/clean up flush() and the merge code paths where pending is referenced
(look for the pending field and the flush() implementation to update).
- Around line 74-85: The resampler is being created with decoder.channel_count()
for both input and output channels causing silent channel-mismatch bugs; update
the logic in Consumer so Resampler::new is only constructed when both sample
rate and channel count match Resampler's requirement (i.e., output_rate ==
decoder.sample_rate() AND output_channels == decoder.channel_count()), otherwise
do not instantiate Resampler and instead branch to perform explicit channel
remapping/mixing after decoding (or add a separate channel-conversion step) so
the produced interleaved PCM is actually laid out for output_channels; adjust
the resampler creation/condition around Resampler::new, referencing Consumer,
Resampler::new, decoder.channel_count(), output_channels and output_rate to
locate and fix the code.
In `@rs/moq-audio/src/error.rs`:
- Around line 26-60: Replace the string-typed Resample variant and manual impl
From conversions with typed #[from] variants so the original rubato errors are
preserved: inside AudioError (under cfg(feature = "resample")) add two
transparent/from variants such as ResamplerConstruction(#[from]
rubato::ResamplerConstructionError) and Resample(#[from] rubato::ResampleError)
(or rename as you prefer) and remove the impl
From<rubato::ResamplerConstructionError> and impl From<rubato::ResampleError>
blocks; keep #[error(transparent)] on those variants so the original error types
are propagated.
In `@rs/moq-audio/src/producer.rs`:
- Around line 82-94: The resampler is only given input_channels so it won't
convert channel count when input_channels != encoder.channel_count(); update the
Resampler construction in producer.rs (the resampler variable / Resampler::new
call) to pass both input and target output channels (use encoder.channel_count()
as the output channel argument) along with the existing sample rates and
chunk_frames so the resampler performs channel conversion as well as sample-rate
conversion.
In `@rs/moq-audio/src/resample.rs`:
- Around line 44-50: Reject zero chunk_frames early in the resampler
constructors to avoid an infinite processing loop: in with_channels (and the
other resampler constructor around the 143-167 block) validate that chunk_frames
> 0 and return an appropriate AudioError (e.g. an InvalidArgument/BadParameter
error with a clear message like "chunk_frames must be > 0") instead of
continuing; this prevents chunk_samples == 0 and the subsequent while/drain loop
from never making progress in process().
---
Outside diff comments:
In `@rs/moq-ffi/src/error.rs`:
- Line 4: The public error enum MoqError is missing the #[non_exhaustive]
attribute which prevents downstream crates from exhaustively matching its
variants; add #[non_exhaustive] above the pub enum MoqError declaration (and
keep existing derives/thiserror attributes) so consumers must use a wildcard arm
when matching, then run cargo check to ensure no internal exhaustive matches
break.
🪄 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: a665cff4-df65-408e-8469-dc58f0d56e14
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
CLAUDE.mdCargo.tomlrs/libmoq/Cargo.tomlrs/libmoq/src/audio.rsrs/libmoq/src/consume.rsrs/libmoq/src/error.rsrs/libmoq/src/lib.rsrs/libmoq/src/publish.rsrs/libmoq/src/state.rsrs/moq-audio/Cargo.tomlrs/moq-audio/src/codec/mod.rsrs/moq-audio/src/codec/opus.rsrs/moq-audio/src/consumer.rsrs/moq-audio/src/error.rsrs/moq-audio/src/format.rsrs/moq-audio/src/lib.rsrs/moq-audio/src/producer.rsrs/moq-audio/src/resample.rsrs/moq-audio/src/samples.rsrs/moq-audio/tests/roundtrip.rsrs/moq-codec/README.mdrs/moq-codec/src/lib.rsrs/moq-ffi/Cargo.tomlrs/moq-ffi/src/audio.rsrs/moq-ffi/src/consumer.rsrs/moq-ffi/src/error.rsrs/moq-ffi/src/lib.rsrs/moq-ffi/src/producer.rsrs/moq-video/CHANGELOG.mdrs/moq-video/Cargo.tomlrs/moq-video/README.mdrs/moq-video/src/lib.rs
💤 Files with no reviewable changes (2)
- rs/moq-codec/README.md
- rs/moq-codec/src/lib.rs
kixelated
added a commit
that referenced
this pull request
May 24, 2026
Address review feedback on #1484: - `AudioConsumer` and `AudioProducer` now reject `output_channels` / `input_channels` that don't match the codec's channel count. The resampler was being constructed with `decoder.channel_count()` for both input and output, so a caller-requested channel remap silently did nothing and the PCM was mislabelled. Channel upmix/downmix stays as a TODO; the error message points there. - Remove the dead `pending` / `next_timestamp_us` / `flush()` plumbing from `AudioConsumer`. The fields were never written, so `flush()` was permanently a no-op and the merge branch was unreachable. - Use typed `#[from]` variants for `rubato::ResamplerConstructionError` and `rubato::ResampleError` in `AudioError` so the source error isn't lost to `to_string()`. - Reject `chunk_frames == 0` in `Resampler` construction; otherwise `process()` would spin forever on a zero-length drain loop. - Drop the ignored `input_format` parameter from `moq_publish_raw_audio_opus` (both moq-ffi and libmoq) since the per-write `format` field is the source of truth and accepting it at publish time was misleading. - Add `#[non_exhaustive]` to `moq_ffi::error::MoqError` to match the project's stated convention for public thiserror enums. Skipped: the use-after-close concern on `libmoq` callbacks. Same race exists in every existing libmoq callback path (`run_catalog`, `run_track`); a fix here would have to be a libmoq-wide pattern change, out of scope for this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rs/moq-ffi/src/audio.rs (1)
109-112:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the producer docstring to match current API behavior.
This text still says format is fixed at construction time, but the API now carries format per write via
MoqRawAudio.format.Doc fix
/// Built via [`MoqBroadcastProducer::publish_raw_audio_opus`]. Each -/// [`write`](Self::write) accepts PCM in the format supplied at -/// construction time, resamples/encodes through libopus, and publishes -/// to the underlying moq broadcast. +/// [`write`](Self::write) accepts PCM where the layout is provided by +/// `MoqRawAudio.format`, resamples/encodes through libopus, and +/// publishes to the underlying moq broadcast.🤖 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-ffi/src/audio.rs` around lines 109 - 112, The docstring for the producer (above MoqBroadcastProducer::publish_raw_audio_opus and the write method) is out of date: it states that PCM format is fixed at construction time but the API now carries format per-call via MoqRawAudio.format. Update the producer docstring to say each write accepts PCM described by the MoqRawAudio.format field (not a construction-time format), and that write/resampling/encoding are done per-write according to that format before publishing to the underlying moq broadcast; mention MoqRawAudio.format explicitly to guide readers.rs/libmoq/src/audio.rs (1)
24-33:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftAvoid exposing
moq_audio_formatdirectly in the C ABI; validate from an integer discriminator
moq_audio_formatis used directly in the C ABI surface (moq_raw_audio.format: moq_audio_formatandmoq_consume_raw_audio_opus(..., output_format: moq_audio_format, ...)). A C caller can pass an invalid discriminant, which can lead to undefined behavior before any Rust-side conversion logic runs.Use a primitive (e.g.,
u32) in the ABI struct/fn signature and perform a checked conversion at the boundary (e.g.,TryFrom<u32>) before converting tomoq_audio::AudioFormat.🤖 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/libmoq/src/audio.rs` around lines 24 - 33, Change the C ABI to stop exposing the moq_audio_format enum directly: update the ABI spots that use it (notably the moq_raw_audio.format field and the moq_consume_raw_audio_opus(...) parameter named output_format) to accept a primitive integer type (e.g., u32), then perform a checked conversion at the Rust FFI boundary using TryFrom<u32> into moq_audio::AudioFormat (or return an error/NULL if conversion fails). Locate the enum declaration moq_audio_format and the FFI uses moq_raw_audio.format and moq_consume_raw_audio_opus and replace direct uses with the integer discriminator plus a TryFrom-based validation step before any downstream logic uses moq_audio::AudioFormat.
🤖 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/libmoq/src/audio.rs`:
- Around line 24-33: Change the C ABI to stop exposing the moq_audio_format enum
directly: update the ABI spots that use it (notably the moq_raw_audio.format
field and the moq_consume_raw_audio_opus(...) parameter named output_format) to
accept a primitive integer type (e.g., u32), then perform a checked conversion
at the Rust FFI boundary using TryFrom<u32> into moq_audio::AudioFormat (or
return an error/NULL if conversion fails). Locate the enum declaration
moq_audio_format and the FFI uses moq_raw_audio.format and
moq_consume_raw_audio_opus and replace direct uses with the integer
discriminator plus a TryFrom-based validation step before any downstream logic
uses moq_audio::AudioFormat.
In `@rs/moq-ffi/src/audio.rs`:
- Around line 109-112: The docstring for the producer (above
MoqBroadcastProducer::publish_raw_audio_opus and the write method) is out of
date: it states that PCM format is fixed at construction time but the API now
carries format per-call via MoqRawAudio.format. Update the producer docstring to
say each write accepts PCM described by the MoqRawAudio.format field (not a
construction-time format), and that write/resampling/encoding are done per-write
according to that format before publishing to the underlying moq broadcast;
mention MoqRawAudio.format explicitly to guide readers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e112a6a5-e5ab-49e1-840b-c31d5f787e93
📒 Files selected for processing (7)
rs/libmoq/src/audio.rsrs/moq-audio/src/consumer.rsrs/moq-audio/src/error.rsrs/moq-audio/src/producer.rsrs/moq-audio/src/resample.rsrs/moq-ffi/src/audio.rsrs/moq-ffi/src/error.rs
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rs/libmoq/src/audio.rs (2)
70-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the consume-side sample lifetime docs consistent.
moq_raw_audiosays consume-sidedatais only valid for the callback, butmoq_consume_raw_audio_samplekeeps that pointer alive untilmoq_consume_raw_audio_sample_free, andmoq_consume_raw_audio_closedoes not release already-delivered samples. Please document that sample IDs outlive the callback and must be freed explicitly. As per coding guidelines, "Comments must reflect the current state of the code, not its history." (Written by CodeRabbit)Also applies to: 350-360
🤖 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/libmoq/src/audio.rs` around lines 70 - 74, Update the documentation for the consume-side lifetime to match current behavior: state that in moq_raw_audio the data pointer passed to the consume callback remains valid only for the callback itself but that any sample retained via moq_consume_raw_audio_sample is owned by the consumer until they call moq_consume_raw_audio_sample_free; also clarify that moq_consume_raw_audio_close does not implicitly free already-delivered samples and consumers must free sample IDs explicitly. Modify the doc comments near moq_raw_audio, moq_consume_raw_audio_sample, moq_consume_raw_audio_sample_free and moq_consume_raw_audio_close so they consistently reflect these ownership and lifetime semantics.
21-48:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftDo not expose
moq_audio_formatas an ABI-visible C enum.Receiving
moq_audio_formatby value (for example inextern "C"signatures or#[repr(C)]struct fields) is unsound because a C caller can supply an invalid discriminant, which can lead to undefined behavior on the Rust side. Replace the ABI type with a primitive integer (for exampleu32), validate explicitly, and only then convert tomoq_audio::AudioFormatusing a fallible conversion.🤖 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/libmoq/src/audio.rs` around lines 21 - 48, The C-exposed enum moq_audio_format is unsafe as an ABI-discriminant; stop exposing moq_audio_format in extern/#[repr(C)] APIs and replace any ABI-facing parameters/fields with a primitive (e.g. u32/u8) and perform an explicit, fallible conversion to moq_audio::AudioFormat inside Rust (add a TryFrom<u32> or from_abi(u32) that validates discriminants and returns Result<AudioFormat, Error>); remove or keep the current impl From<moq_audio_format> only for internal use, update any functions that accepted moq_audio_format by value to accept the primitive and call the validation/conversion before using moq_audio::AudioFormat.
🤖 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/libmoq/src/audio.rs`:
- Around line 70-74: Update the documentation for the consume-side lifetime to
match current behavior: state that in moq_raw_audio the data pointer passed to
the consume callback remains valid only for the callback itself but that any
sample retained via moq_consume_raw_audio_sample is owned by the consumer until
they call moq_consume_raw_audio_sample_free; also clarify that
moq_consume_raw_audio_close does not implicitly free already-delivered samples
and consumers must free sample IDs explicitly. Modify the doc comments near
moq_raw_audio, moq_consume_raw_audio_sample, moq_consume_raw_audio_sample_free
and moq_consume_raw_audio_close so they consistently reflect these ownership and
lifetime semantics.
- Around line 21-48: The C-exposed enum moq_audio_format is unsafe as an
ABI-discriminant; stop exposing moq_audio_format in extern/#[repr(C)] APIs and
replace any ABI-facing parameters/fields with a primitive (e.g. u32/u8) and
perform an explicit, fallible conversion to moq_audio::AudioFormat inside Rust
(add a TryFrom<u32> or from_abi(u32) that validates discriminants and returns
Result<AudioFormat, Error>); remove or keep the current impl
From<moq_audio_format> only for internal use, update any functions that accepted
moq_audio_format by value to accept the primitive and call the
validation/conversion before using moq_audio::AudioFormat.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfa8932d-755d-42f5-aaa1-02a0a4ebe72b
📒 Files selected for processing (1)
rs/libmoq/src/audio.rs
kixelated
added a commit
that referenced
this pull request
May 24, 2026
… libopus CI for #1484 failed because `audiopus_sys` (pulled in by `moq-audio`'s `opus` feature) couldn't find a system libopus via pkg-config and fell back to building the vendored copy via CMake. That copy's CMakeLists.txt asks for `cmake_minimum_required(VERSION < 3.5)`, which modern CMake rejects. Two changes: - Add `libopus` to `flake.nix` `rustDeps` so pkg-config finds a system copy. This is the path used by the default `opus` feature and by `cargo check --workspace`. - Set `CMAKE_POLICY_VERSION_MINIMUM=3.5` in `.cargo/config.toml [env]` so that the vendored CMake build still works when `opus-static` is enabled (CI's `cargo check --workspace --all-features` exercises this path). The cmake crate inherits cargo's env when invoking CMake, so the var propagates without further plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kixelated
added a commit
that referenced
this pull request
May 24, 2026
Address review feedback on #1484: - `AudioConsumer` and `AudioProducer` now reject `output_channels` / `input_channels` that don't match the codec's channel count. The resampler was being constructed with `decoder.channel_count()` for both input and output, so a caller-requested channel remap silently did nothing and the PCM was mislabelled. Channel upmix/downmix stays as a TODO; the error message points there. - Remove the dead `pending` / `next_timestamp_us` / `flush()` plumbing from `AudioConsumer`. The fields were never written, so `flush()` was permanently a no-op and the merge branch was unreachable. - Use typed `#[from]` variants for `rubato::ResamplerConstructionError` and `rubato::ResampleError` in `AudioError` so the source error isn't lost to `to_string()`. - Reject `chunk_frames == 0` in `Resampler` construction; otherwise `process()` would spin forever on a zero-length drain loop. - Drop the ignored `input_format` parameter from `moq_publish_raw_audio_opus` (both moq-ffi and libmoq) since the per-write `format` field is the source of truth and accepting it at publish time was misleading. - Add `#[non_exhaustive]` to `moq_ffi::error::MoqError` to match the project's stated convention for public thiserror enums. Skipped: the use-after-close concern on `libmoq` callbacks. Same race exists in every existing libmoq callback path (`run_catalog`, `run_track`); a fix here would have to be a libmoq-wide pattern change, out of scope for this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kixelated
force-pushed
the
claude/sleepy-mahavira-3d84af
branch
from
72bd325 to
3dfc893
Compare
kixelated
commented
May 24, 2026
kixelated
commented
May 24, 2026
kixelated
commented
May 24, 2026
kixelated
left a comment
kixelated
left a comment
Collaborator
Author
There was a problem hiding this comment.
And if `publish_audio
kixelated
added a commit
that referenced
this pull request
May 24, 2026
Address kixelated's design feedback on #1484 in a single pass. moq-audio: - Rename `AudioSamples` → `Frame`; drop per-frame format / sample rate / channel count fields. Each `Frame` is now just bytes + timestamp. - Bake PCM layout into the producer/consumer via new `EncoderConfig` / `DecoderConfig` structs (input/output format, rates, channels, bitrate, frame_duration). Caller can no longer drift the layout mid-stream. - Drop the `Encoder` / `Decoder` traits and the `OpusEncoder` / `OpusDecoder` prefix; the codec module now exposes concrete `Encoder` / `Decoder` types tied to Opus. When a second codec lands the trait can come back, but introducing it for a single impl was premature. - Drop the `opus` and `resample` Cargo features; they were a single- codec gate and a tiny resampler. Always enabled now. - Make Opus `frame_duration` configurable (default 20ms) via `EncoderConfig::frame_duration`. Validates against libopus's supported set (2.5/5/10/20/40/60 ms). - Register the catalog rendition in `AudioProducer::new` instead of on first write, so subscribers see the track even before frames arrive. - `AudioFormat::as_interleaved_f32` returns `Cow<[f32]>` so the no-op fast path (F32 interleaved, aligned bytes) avoids the per-frame allocation. moq-ffi (UniFFI): - `MoqRawAudio` → `MoqAudioFrame` (just `timestamp_us` + `data`). - `MoqRawAudioProducer` → `MoqAudioProducer`, `MoqRawAudioConsumer` → `MoqAudioConsumer`. - `publish_raw_audio_opus` → `publish_audio(name, MoqAudioEncoderConfig)`. - `subscribe_raw_audio_opus` → `subscribe_audio(name, catalog, MoqAudioDecoderConfig)`. - New `MoqAudioEncoderConfig` / `MoqAudioDecoderConfig` / `MoqAudioCodec` (currently `Opus` only) records / enums. libmoq (C ABI): - `moq_raw_audio` → `moq_audio_frame` (timestamp + data only). - New `moq_audio_encoder_config` / `moq_audio_decoder_config` structs; encoder config takes a `codec` UTF-8 string ("opus"). - Functions: `moq_publish_audio_raw[_frame|_close]` and `moq_consume_audio_raw[_close|_frame|_frame_free]`. The `_raw` suffix avoids name collisions with the existing `moq_consume_audio_*` (encoded path). CI: - Set `OPUS_NO_PKG=1` in `.cargo/config.toml [env]` so audiopus_sys skips pkg-config; otherwise it picks up the transitive libopus that ffmpeg drags into the nix shell and emits static-link instructions for a `.a` it never built. Keep `CMAKE_POLICY_VERSION_MINIMUM=3.5` so the vendored CMake build still passes. CLAUDE.md: - Shorten the moq-audio architecture line to one sentence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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-audio/Cargo.toml`:
- Around line 23-31: The Cargo manifest currently lists audiopus_sys, opus, and
rubato unconditionally; make them optional and add feature flags to gate them:
mark the dependencies audiopus_sys, opus, and rubato with optional = true and
remove unconditional features, then add a [features] section that defines e.g.
"opus" (enables the opus crate and audiopus_sys), "opus-static" (enables
audiopus_sys with its "static" feature), and "resample" (enables rubato); ensure
default features do not force these (leave default empty or minimal) so
consumers can opt into "opus", "opus-static", or "resample" as needed.
In `@rs/moq-audio/src/format.rs`:
- Around line 151-154: The doc comment for from_interleaved_f32 incorrectly
mentions returning a `Cow::Borrowed`; update the documentation to reflect the
actual return type (an owned Vec<u8>) and remove any reference to
`Cow::Borrowed`, while keeping the existing behavioral notes (e.g., that integer
formats clamp out-of-range samples and that no conversion is needed for F32
interleaved input); ensure the function name `from_interleaved_f32` and its
Result<Vec<u8>, AudioError> return contract are stated clearly.
In `@rs/moq-audio/src/producer.rs`:
- Around line 45-47: The calculation of chunk_frames in producer.rs truncates
sub-millisecond frame durations by using frame_duration.as_millis(); change it
to use microsecond precision: compute chunk_frames as
(encoder.config().input_sample_rate as usize *
encoder.config().frame_duration.as_micros() as usize) / 1_000_000 (or equivalent
integer math), ensuring the result uses frame_duration.as_micros() to correctly
handle values like 2_500us; update the expression that references chunk_frames
accordingly and ensure casts are safe.
In `@rs/moq-audio/tests/roundtrip.rs`:
- Around line 81-91: The async loops call consumer.read().await without a
timeout and can hang; wrap each consumer.read().await in
tokio::time::timeout(...) using a reasonable Duration (bring Duration into
scope) and handle the Result/Err from timeout by treating timeout as a test
failure or breaking the loop. Update both occurrences of consumer.read() in
roundtrip.rs (the loop around total_frames/total_energy and the later loop at
line ~161) to unwrap the timeout result, map the Ok(inner) to the existing logic
and handle Err(_) to fail the test or break, ensuring Duration and tokio::time
are imported.
🪄 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: 824b0f8d-dd17-4f86-9cad-b8b686914cfa
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.cargo/config.tomlCLAUDE.mdCargo.tomlrs/libmoq/Cargo.tomlrs/libmoq/src/audio.rsrs/libmoq/src/consume.rsrs/libmoq/src/error.rsrs/libmoq/src/lib.rsrs/libmoq/src/publish.rsrs/libmoq/src/state.rsrs/moq-audio/Cargo.tomlrs/moq-audio/src/codec.rsrs/moq-audio/src/consumer.rsrs/moq-audio/src/error.rsrs/moq-audio/src/format.rsrs/moq-audio/src/frame.rsrs/moq-audio/src/lib.rsrs/moq-audio/src/producer.rsrs/moq-audio/src/resample.rsrs/moq-audio/tests/roundtrip.rsrs/moq-codec/README.mdrs/moq-codec/src/lib.rsrs/moq-ffi/Cargo.tomlrs/moq-ffi/src/audio.rsrs/moq-ffi/src/consumer.rsrs/moq-ffi/src/error.rsrs/moq-ffi/src/lib.rsrs/moq-ffi/src/producer.rsrs/moq-video/CHANGELOG.mdrs/moq-video/Cargo.tomlrs/moq-video/README.mdrs/moq-video/src/lib.rs
💤 Files with no reviewable changes (2)
- rs/moq-codec/src/lib.rs
- rs/moq-codec/README.md
✅ Files skipped from review due to trivial changes (5)
- rs/moq-video/README.md
- rs/moq-ffi/src/lib.rs
- rs/moq-video/Cargo.toml
- CLAUDE.md
- rs/moq-video/src/lib.rs
kixelated
added a commit
that referenced
this pull request
May 24, 2026
CodeRabbit feedback on #1484: - producer.rs: derive `chunk_frames` from `frame_duration.as_micros()` instead of `as_millis()`. Opus accepts 2.5 ms frames; the old code truncated those to 2 ms and skewed resampler chunking. - format.rs: drop the stale `Cow::Borrowed` line from `from_interleaved_f32`'s doc — it always returns owned `Vec<u8>`. - tests/roundtrip.rs: wrap both `consumer.read()` loops in a 5 s `tokio::time::timeout` so CI can't hang if the stream never reaches EOF. (Skipped: CodeRabbit's suggestion to re-add `opus` / `opus-static` / `resample` Cargo features. Removing those was a deliberate call from kixelated — single codec, small resampler, not worth gating.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-video Add `rs/moq-audio`, a new crate sitting on top of `moq-mux` and `hang` that adds the missing native codec layer: real Opus encode/decode of raw PCM samples, with a rubato-backed resampler so callers can pass any WebCodecs `AudioData.format` and sample rate. Today `moq-mux` only parses codec configuration headers and passes raw bitstreams through, so Python/Swift/Kotlin (`moq-ffi`) and C (`libmoq`) callers had no way to publish from a microphone buffer or render to a speaker buffer without bringing their own codec library. The new crate exposes: - `AudioFormat` mirroring WebCodecs `AudioData.format`, with helpers to convert any layout to/from interleaved f32 (the codec boundary). - `AudioSamples` carrier with timestamp + rate + channels + bytes. - Generic `Encoder` / `Decoder` traits, with an Opus implementation over the `opus` crate at fixed 20 ms frames. - `AudioProducer` / `AudioConsumer` that wire PCM in/out through `moq_mux::container::Producer<legacy::Wire>` and register or read the `hang::catalog::AudioConfig`. - Cargo features: `opus` (default, system libopus), `opus-static` (vendors libopus via `audiopus_sys/static`, used by libmoq and moq-ffi distributable artifacts), `resample` (default). `moq-ffi` gains `MoqAudioFormat`, `MoqRawAudio`, `publish_raw_audio_opus`, and `subscribe_raw_audio_opus`. `libmoq` gains the equivalent C surface (`moq_audio_format`, `moq_raw_audio`, `moq_publish_raw_audio_opus/_write/ _close`, `moq_consume_raw_audio_opus/_close/_sample/_sample_free`). Both stay on the existing name-based (moq-ffi) and catalog-index-based (libmoq) track selection; ABR-style auto-selection is a follow-up. Bundled size cost on the moq-ffi cdylib (what ships in Python wheels): ~340 KB stripped, almost all libopus. On the staticlib it's ~900 KB because the linker can't drop unused symbols pre-link. On macOS the `opus-static` feature is effectively a no-op because `audiopus_sys` vendors libopus whenever there's no system copy; the feature still matters on Linux. Also rename the previously empty `moq-codec` placeholder crate to `moq-video`. Audio and video have different enough I/O shapes that keeping them in one crate is more friction than reuse; the future video crate can mirror moq-audio's structure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… libopus CI for #1484 failed because `audiopus_sys` (pulled in by `moq-audio`'s `opus` feature) couldn't find a system libopus via pkg-config and fell back to building the vendored copy via CMake. That copy's CMakeLists.txt asks for `cmake_minimum_required(VERSION < 3.5)`, which modern CMake rejects. Two changes: - Add `libopus` to `flake.nix` `rustDeps` so pkg-config finds a system copy. This is the path used by the default `opus` feature and by `cargo check --workspace`. - Set `CMAKE_POLICY_VERSION_MINIMUM=3.5` in `.cargo/config.toml [env]` so that the vendored CMake build still works when `opus-static` is enabled (CI's `cargo check --workspace --all-features` exercises this path). The cmake crate inherits cargo's env when invoking CMake, so the var propagates without further plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review feedback on #1484: - `AudioConsumer` and `AudioProducer` now reject `output_channels` / `input_channels` that don't match the codec's channel count. The resampler was being constructed with `decoder.channel_count()` for both input and output, so a caller-requested channel remap silently did nothing and the PCM was mislabelled. Channel upmix/downmix stays as a TODO; the error message points there. - Remove the dead `pending` / `next_timestamp_us` / `flush()` plumbing from `AudioConsumer`. The fields were never written, so `flush()` was permanently a no-op and the merge branch was unreachable. - Use typed `#[from]` variants for `rubato::ResamplerConstructionError` and `rubato::ResampleError` in `AudioError` so the source error isn't lost to `to_string()`. - Reject `chunk_frames == 0` in `Resampler` construction; otherwise `process()` would spin forever on a zero-length drain loop. - Drop the ignored `input_format` parameter from `moq_publish_raw_audio_opus` (both moq-ffi and libmoq) since the per-write `format` field is the source of truth and accepting it at publish time was misleading. - Add `#[non_exhaustive]` to `moq_ffi::error::MoqError` to match the project's stated convention for public thiserror enums. Skipped: the use-after-close concern on `libmoq` callbacks. Same race exists in every existing libmoq callback path (`run_catalog`, `run_track`); a fix here would have to be a libmoq-wide pattern change, out of scope for this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI clippy (Rust 1.95) flagged this as 8/7 arguments. The function is purely internal wiring between the C FFI entry point and `moq_audio::AudioConsumer::subscribe_opus`, and bundling the parameters into a struct just to please the lint would add an indirection that doesn't carry semantic meaning. Allow the lint on this one helper instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI #2 surfaced two issues: 1. Swift CI failed to link `MoqPackageTests` against `libmoq_ffi.a` because the libopus C symbols weren't in the archive. Adding libopus to the Nix shell let `audiopus_sys` find a system copy via pkg-config and *dynamic*-link instead of vendoring. The Swift XCFramework only ships `libmoq_ffi.a`, so the libopus symbols needed to be self-contained. Revert the libopus addition to flake.nix. With `CMAKE_POLICY_VERSION_MINIMUM=3.5` in `.cargo/config.toml`, audiopus_sys's vendored libopus CMake build works on every host, producing a libmoq_ffi.a that downstream linkers (Swift, libmoq C callers, Python wheels) can use without a system libopus. 2. CodeRabbit flagged that exposing `moq_audio_format` directly in the libmoq C ABI is UB when a C caller passes an unknown discriminant — the value materializes as an invalid Rust enum *before* any conversion code runs. Change `moq_raw_audio.format` and `moq_consume_raw_audio_opus`'s `output_format` parameter to `u32`, and validate via two helpers (`audio_format_from_u32` / `_to_u32`). The enum stays as a typedef for header readability so C callers still get named constants. Also refreshes the stale `MoqRawAudioProducer` docstring in moq-ffi to reflect the per-write format model now that publish-time format is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit flagged that `moq_raw_audio`'s doc said "data only valid for the call/callback", which is true for the publish side but wrong for consume: `moq_consume_raw_audio_sample` writes a pointer that stays valid until the sample ID is freed via `moq_consume_raw_audio_sample_free`, and `moq_consume_raw_audio_close` doesn't drop already-delivered sample IDs. Tighten the docs on `moq_raw_audio`, `moq_consume_raw_audio_sample`, `moq_consume_raw_audio_sample_free`, and `moq_consume_raw_audio_close` so they consistently describe the dual lifetime and remind callers that each delivered sample must be released explicitly. (The other CodeRabbit comment in this batch — "don't expose moq_audio_format as a C ABI enum" — was already addressed in bc4c311; the review ran against the older 1efcc2b snapshot.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Main landed #1485 (`hang: non_exhaustive VideoConfig/AudioConfig with constructors`) while this PR was open. The struct-literal calls in `moq-audio` and `moq-ffi` are no longer allowed; switch to `AudioConfig::new(codec, sample_rate, channel_count)` and assign the optional `bitrate` / `description` / `container` fields afterward. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address kixelated's design feedback on #1484 in a single pass. moq-audio: - Rename `AudioSamples` → `Frame`; drop per-frame format / sample rate / channel count fields. Each `Frame` is now just bytes + timestamp. - Bake PCM layout into the producer/consumer via new `EncoderConfig` / `DecoderConfig` structs (input/output format, rates, channels, bitrate, frame_duration). Caller can no longer drift the layout mid-stream. - Drop the `Encoder` / `Decoder` traits and the `OpusEncoder` / `OpusDecoder` prefix; the codec module now exposes concrete `Encoder` / `Decoder` types tied to Opus. When a second codec lands the trait can come back, but introducing it for a single impl was premature. - Drop the `opus` and `resample` Cargo features; they were a single- codec gate and a tiny resampler. Always enabled now. - Make Opus `frame_duration` configurable (default 20ms) via `EncoderConfig::frame_duration`. Validates against libopus's supported set (2.5/5/10/20/40/60 ms). - Register the catalog rendition in `AudioProducer::new` instead of on first write, so subscribers see the track even before frames arrive. - `AudioFormat::as_interleaved_f32` returns `Cow<[f32]>` so the no-op fast path (F32 interleaved, aligned bytes) avoids the per-frame allocation. moq-ffi (UniFFI): - `MoqRawAudio` → `MoqAudioFrame` (just `timestamp_us` + `data`). - `MoqRawAudioProducer` → `MoqAudioProducer`, `MoqRawAudioConsumer` → `MoqAudioConsumer`. - `publish_raw_audio_opus` → `publish_audio(name, MoqAudioEncoderConfig)`. - `subscribe_raw_audio_opus` → `subscribe_audio(name, catalog, MoqAudioDecoderConfig)`. - New `MoqAudioEncoderConfig` / `MoqAudioDecoderConfig` / `MoqAudioCodec` (currently `Opus` only) records / enums. libmoq (C ABI): - `moq_raw_audio` → `moq_audio_frame` (timestamp + data only). - New `moq_audio_encoder_config` / `moq_audio_decoder_config` structs; encoder config takes a `codec` UTF-8 string ("opus"). - Functions: `moq_publish_audio_raw[_frame|_close]` and `moq_consume_audio_raw[_close|_frame|_frame_free]`. The `_raw` suffix avoids name collisions with the existing `moq_consume_audio_*` (encoded path). CI: - Set `OPUS_NO_PKG=1` in `.cargo/config.toml [env]` so audiopus_sys skips pkg-config; otherwise it picks up the transitive libopus that ffmpeg drags into the nix shell and emits static-link instructions for a `.a` it never built. Keep `CMAKE_POLICY_VERSION_MINIMUM=3.5` so the vendored CMake build still passes. CLAUDE.md: - Shorten the moq-audio architecture line to one sentence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI clippy (Rust 1.95) flagged this as 10/7 arguments — same shape as `consume` got in c87a3d5. Apply the same fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per kixelated's feedback: `#[allow(clippy::too_many_arguments)]` was hiding the real fix. `Audio::publish` and `Audio::consume` now take `moq_audio::EncoderConfig` / `DecoderConfig` directly, instead of exploding the struct into individual parameters and re-packing it inside. The C entry points still translate the on-the-wire `moq_audio_encoder_config` / `moq_audio_decoder_config` into the Rust types, but the helper signatures stay short and clippy stays happy without any allows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit feedback on #1484: - producer.rs: derive `chunk_frames` from `frame_duration.as_micros()` instead of `as_millis()`. Opus accepts 2.5 ms frames; the old code truncated those to 2 ms and skewed resampler chunking. - format.rs: drop the stale `Cow::Borrowed` line from `from_interleaved_f32`'s doc — it always returns owned `Vec<u8>`. - tests/roundtrip.rs: wrap both `consumer.read()` loops in a 5 s `tokio::time::timeout` so CI can't hang if the stream never reaches EOF. (Skipped: CodeRabbit's suggestion to re-add `opus` / `opus-static` / `resample` Cargo features. Removing those was a deliberate call from kixelated — single codec, small resampler, not worth gating.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`cargo deny` (added in #1486) flags audiopus_sys as unmaintained: last commit 5 years ago, maintainer unresponsive to PRs fixing the exact CMake 4 issue we already work around with CMAKE_POLICY_VERSION_MINIMUM=3.5. There's no safe upgrade and the candidate replacements (`magnum-opus`/`opusic-sys`) all wrap the same vendored libopus, so swapping doesn't fix the underlying problem. Add the advisory to deny.toml's ignore list with rationale. Drop when libopus's CMakeLists gets updated upstream or a maintained opus binding emerges. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kixelated
force-pushed
the
claude/sleepy-mahavira-3d84af
branch
from
068c4ca to
7c98ac8
Compare
…bopus You were right that libopus itself isn't unmaintained, just the Rust wrapper. unsafe-libopus (https://crates.io/crates/unsafe-libopus) is a c2rust transpilation of libopus 1.3.1 with active maintenance, tested against IETF test vectors, and crucially needs no CMake toolchain at all. Replace the codec backend with a thin internal wrapper around its exported C-ABI symbols (~60 lines of unsafe in codec.rs, fully contained in Encoder/Decoder constructors and Drop impls). Every public moq-audio API stays the same; the round-trip tests pass unchanged. Cleanups this lets us drop: - `.cargo/config.toml [env]`: OPUS_NO_PKG and CMAKE_POLICY_VERSION_MINIMUM are no longer needed since there's no native compile step. - `deny.toml`: drop the RUSTSEC-2026-0150 (audiopus_sys unmaintained) ignore. The advisory no longer applies. - `rs/moq-audio/Cargo.toml`: drop the `audiopus_sys` direct dep, the matching cargo-shear ignore, and the `opus` crate. Trade-off: about 20% slower than C-with-intrinsics libopus per the upstream README. For the 20 ms Opus frames moq-audio publishes that still sits well under any real-time budget; if it becomes a problem we can revisit with SIMD or a different binding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ppers
Two pieces of feedback from the AI review:
1. **Missing congestion-control knob on subscribe_audio.**
`MediaConsumer` has had `max_latency_ms` since day one, but the
new audio path didn't thread it through. Added
`DecoderConfig::max_latency: Option<Duration>` and forwarded it
to `moq_mux::container::Consumer::with_latency`. Mirrored as
`MoqAudioDecoderConfig.max_latency_ms: Option<u64>` in moq-ffi
and `moq_audio_decoder_config.max_latency_ms: u64` in libmoq
(0 = aggressive skip, matching the existing convention). Without
this knob the consumer falls back to moq-mux's default of zero,
which skips stalled groups instantly — fine for tests, terrible
under real network jitter.
2. **No Python wrapper.** The FFI types were there but
`py/moq-rs/moq/` didn't expose them. Added:
- `AudioProducer` / `AudioConsumer` Python classes mirroring
`MediaProducer` / `MediaConsumer` (async iterator, cancel,
finish).
- `BroadcastProducer.publish_audio(name, config)` and
`BroadcastConsumer.subscribe_audio(name, catalog_audio, config)`
methods.
- `AudioFrame` / `AudioCodec` / `AudioFormat` /
`AudioEncoderConfig` / `AudioDecoderConfig` re-exports in
`types.py`.
Existing 33 pytest tests still pass; pyright clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror the existing MoqMediaConsumer extension on both wrappers so audio frames can be iterated the same way as media frames. - swift/Sources/Moq/AsyncSequences.swift: `MoqAudioConsumer.frames -> AsyncThrowingStream<MoqAudioFrame>` - kt/moq/.../Flows.kt: `MoqAudioConsumer.frames() -> Flow<MoqAudioFrame>` Both auto-cancel the native consumer when the task/Flow is cancelled, matching the structured-concurrency pattern already in those files. Go is unaffected: its bindings are entirely auto-generated by uniffi-bindgen-go from moq-ffi, so the new MoqAudio* types ship there without any hand-written wrapper. `just swift check` passes locally (2 tests); kt/go checks need their toolchains, deferred to CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kixelated
commented
May 24, 2026
Follow-up on kixelated's review feedback. The single
EncoderConfig/DecoderConfig mixed input and output knobs awkwardly and
left no place to express "encode at a different rate than I'm
feeding". Split into three symmetric structs across every layer.
moq-audio:
- `EncoderInput { format, sample_rate, channels }` — caller-side PCM
layout.
- `EncoderOutput { codec, sample_rate: Option<u32>,
channels: Option<u32>, bitrate, frame_duration }` — codec-side
config; `None` rate/channels means "match input, snapping the rate
up to a libopus-supported value".
- `DecoderOutput { format, sample_rate, channels, max_latency }` —
caller-side delivery shape (no more `output_` prefix; the struct
name already says it).
- `Encoder::new(EncoderInput, EncoderOutput)`,
`AudioProducer::new(broadcast, catalog, name, EncoderInput, EncoderOutput)`,
`AudioConsumer::new(broadcast, catalog, name, DecoderOutput)`.
- New `Codec::from_str` / `Display` / `as_str` (canonical lowercase
identifier), so libmoq's C-string parsing no longer needs a custom
match.
- Rename `Encoder::catalog_config()` → `catalog()` (nit from review).
- Drop the redundant `SUPPORTED.contains()` early-return in
`pick_opus_rate` — the `find(|&r| r >= input)` falls through to the
exact match anyway.
moq-ffi (UniFFI):
- `MoqAudioEncoderInput`, `MoqAudioEncoderOutput`,
`MoqAudioDecoderOutput` records.
- `publish_audio(name, input, output)` /
`subscribe_audio(name, catalog_audio, output)`.
libmoq (C ABI):
- `moq_audio_encoder_input`, `moq_audio_encoder_output`,
`moq_audio_decoder_output` structs (0-sentinel for "derive from
input" on the output rate/channels).
- `moq_publish_audio_raw(broadcast, name, name_len, input, output)`,
`moq_consume_audio_raw(catalog, index, output, cb, user)`.
Python wrapper:
- `AudioEncoderInput`, `AudioEncoderOutput`, `AudioDecoderOutput`
re-exports.
- `BroadcastProducer.publish_audio(name, input, output)`.
- `BroadcastConsumer.subscribe_audio(name, catalog_audio, output)`.
Swift / Kotlin: no changes needed — they just iterate
`MoqAudioConsumer` and don't reference the record types directly.
CLAUDE.md: dropped the moq-video stub line (not worth documenting).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per kixelated: putting the qualifier as a suffix leaves room for a companion `latency_min` (jitter-buffer floor) later. Also reads better as a related pair (`latency_min` / `latency_max`). Only the new audio API is renamed; the existing `subscribe_media(max_latency_ms)` and `moq_consume_audio_ordered(max_latency_ms)` parameters stay as-is (separate APIs, already public). - moq-audio: `DecoderOutput.max_latency` → `latency_max` - moq-ffi: `MoqAudioDecoderOutput.max_latency_ms` → `latency_max_ms` - libmoq: `moq_audio_decoder_output.max_latency_ms` → `latency_max_ms` - py wrapper: docstring updated Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Consistency follow-up to the prior commit: rename the parameter on the older encoded-path APIs too. Parameter names aren't part of the binary ABI (C function args are positional, UniFFI-generated bindings re-derive each language's keyword-arg names from the Rust source), so this is safe to do here. Now every consumer subscription on every layer uses the same `latency_max_ms` name, leaving `latency_min_ms` open for the jitter-buffer floor. - moq-ffi: `MoqBroadcastConsumer::subscribe_media(name, container, latency_max_ms)` - libmoq: `moq_consume_video_ordered(..., latency_max_ms, ...)` and `moq_consume_audio_ordered(..., latency_max_ms, ...)` - py wrapper: `BroadcastConsumer.subscribe_media(name, container, latency_max_ms)` - README updates for both libmoq and moq-rs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e_*_ordered Reverting the parameter rename on the pre-existing encoded-path APIs to avoid a semver break for Swift / Kotlin / Python kwarg callers (Swift labels are part of the function signature; named-arg Python / Kotlin code would have to change). Only the new audio API keeps `latency_max_ms`: - `MoqAudioDecoderOutput.latency_max_ms` - `moq_audio_decoder_output.latency_max_ms` - moq-audio's `DecoderOutput.latency_max` So the names are mixed on the FFI surface for now, but every layer is internally consistent and the existing API is unchanged. Once we ship a 0.3 of moq-ffi for some other reason we can sweep them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rs/moq-audiocrate: native Opus encode/decode of raw PCM on top ofmoq-mux+hang, with a rubato resampler so callers can pass any WebCodecsAudioData.formatand sample rate.moq-ffiandlibmoqgain raw-audio publish/subscribe APIs so Python/Swift/Kotlin and C callers can drive a microphone or speaker without bringing their own codec.moq-codecplaceholder crate tomoq-videoso we squat on the right name; audio and video have different enough I/O shapes to live in separate crates.Why
Today, audio in moq flows as already-encoded bitstreams.
moq-muxparses codec configuration headers (OpusHead, AudioSpecificConfig) and passes raw Opus/AAC packets through tomoq-netunchanged. The browser path uses WebCodecs; native callers (moq-ffi,libmoq) had to bolt on a codec library themselves. This PR closes that gap for Opus.What's in
moq-audioAudioFormatenum mirroring WebCodecsAudioData.format(U8/S16/S32/F32 ± planar), with helpers to convert any layout to/from interleaved f32 (the codec boundary).AudioSamplescarrier: format + sample rate + channels + timestamp + bytes.Encoder/Decodertraits, with an Opus implementation over theopuscrate at fixed 20 ms frames.AudioProducer/AudioConsumerthat wire PCM throughmoq_mux::container::Producer<legacy::Wire>and register/read thehang::catalog::AudioConfig.opus(default): system libopus.opus-static: vendor libopus viaaudiopus_sys/static. Enabled bymoq-ffiandlibmoqso distributable artifacts (Python wheels, C staticlib) don't require libopus on the host.resample(default): rubato wrapper with a passthrough mode when rates match.FFI surface
moq-ffi (
rs/moq-ffi/src/audio.rs):MoqAudioFormat,MoqRawAudiorecords.MoqBroadcastProducer::publish_raw_audio_opus(name, format, rate, channels, bitrate?)returningMoqRawAudioProducer { write, finish }.MoqBroadcastConsumer::subscribe_raw_audio_opus(name, audio_config, format, output_rate?, output_channels?)returningMoqRawAudioConsumer { next, cancel }.libmoq (
rs/libmoq/src/audio.rs):moq_audio_formatenum,moq_raw_audiostruct.moq_publish_raw_audio_opus/_write/_close.moq_consume_raw_audio_opus/_close/_sample/_sample_free.Both stay on the existing track-selection model (name on moq-ffi, catalog index on libmoq). ABR-style auto-selection is a TODO at the entry points.
Size impact on shipped artifacts
Measured by building
libmoq+moq-ffiin--releasewith vs. withoutmoq-audio:libmoq.alibmoq_ffi.alibmoq_ffi.dylib(unstripped)libmoq_ffi.dylib(stripped)The cdylib is what ships in
py/moq-rsPython wheels, so the practical cost is ~340 KB stripped, almost all libopus.Note: on macOS the
opus-staticCargo feature is effectively a no-op becauseaudiopus_sysalways vendors libopus when there's no system copy to link against. The feature still matters on Linux distributions that ship libopus, where it forces vendoring instead of dynamic-linking.What's deliberately out of scope
moq-videois a renamed-but-still-empty placeholder. Once the input/output story for video samples is settled it can mirrormoq-audio's structure.Test plan
cargo test -p moq-audio(14 unit tests + 2 end-to-end round-trip tests, including a 44.1 kHz S16 → Opus → 48 kHz catalog → 44.1 kHz S16 path through the resampler).cargo test -p moq-ffi,cargo test -p libmoq, fulljust testsuite (Rust + JS + Python).just check(cargo check + clippy -D warnings + fmt + doc + shear + sort + remark).cargo check -p moq-audio --no-default-features --features opus,opus-static,resampleto confirm the static-libopus build path.py/moq-rsis rebuilt.🤖 Generated with Claude Code