feat(voip): add encoded audio pipeline and native Opus negotiation - #1050
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughVoIP features now separate runtime support from codec adapters. Audio profiles flow through signaling, call setup, RTP, and the media engine, with encoded endpoints, Opus/MLOW bridging, MLOW buffer reuse, and updated documentation. ChangesVoIP codec runtime and encoded audio
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant VoipFacade
participant Signaling
participant CallEngine
participant RTP
CLI->>VoipFacade: select codec and audio endpoints
VoipFacade->>Signaling: build offer or accept with audio profile
Signaling->>VoipFacade: return negotiated rates and capability
VoipFacade->>CallEngine: start with AudioConfig
CallEngine->>RTP: encode or forward audio payload
RTP->>CallEngine: receive negotiated audio
CallEngine->>CLI: deliver PCM or EncodedAudioFrame
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
| Filename | Overview |
|---|---|
| wacore/src/voip/audio.rs | New 571-line file defining AudioFormat, AudioConfig, AudioIo, AudioCodec, AudioRtpProfile, EncodedAudioFrame, and the MLOW↔Opus packet transcoding helpers. Well-tested with 12 unit tests covering round-trip, TOC rewrite, DTX mapping, and edge cases. |
| wacore/src/voip/engine.rs | Major refactor replacing the monolithic MLOW-or-ForeignAudio dispatch with AudioConfig-driven routing. MLOW PCM state moved into PcmAudioState (feature-gated), encoded I/O path added. The audio_tx_invalid_streak counter resets on valid payloads, addressing the previous one-shot-flag concern. |
| wacore/src/voip/rtp.rs | RTP_PAYLOAD_TYPE_OPUS renamed to 111 (RFC 7587) and RTP_PAYLOAD_TYPE_WHATSAPP_AUDIO/MLOW added at 120. New set_payload_type and set_mlow_profile methods on RtpStream work correctly; speech_started is now reset on DTX. |
| src/voip/facade.rs | AcceptCall/OutgoingCall builders now carry AudioEndpoints enum instead of separate source/sink arcs. PCM and encoded channels are wired independently in attach_engine; inactive channels close immediately so the driver loop retires their select arms without busy-spinning. |
| src/handlers/call.rs | Adds AudioFormatMismatch detection for inbound PreAccept/Accept stanzas. On mismatch, dismiss_outgoing_siblings is a no-op for PreAccept actions; sibling callee devices continue ringing until timeout when mismatch is caught at the PreAccept stage. |
| wacore/src/voip/driver.rs | encoded_audio_in/out channels added to CallChannels; the select loop handles encoded_audio_fut with the same closed-channel guard pattern as the mic arm. Outputs are drained at the top of the next drive iteration, consistent with existing handling. |
| wacore/src/stanza/call.rs | PreAccept/Accept parsers now extract children into CallAction variants. build_offer now takes audio_rates slice. CAPABILITY_STANDARD_OPUS_* constants are computed at compile time via without_mlow_capability const fn. |
| wacore/src/voip/mlow/encode.rs | encode_into added to MlowEncoder reusing clean and range fields; hot-path allocations eliminated. encode_smpl_frame_into is now private; encode is the public wrapper that allocates its own Vec. |
| src/voip/audio.rs | WaOpusDecoder now has pcm_scratch and packet_scratch fields; decode and decode_mlow_escape reuse them, eliminating per-frame allocation. WaOpusEncoder gains new_mlow_escape constructor for the CELT escape path. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant App as Application
participant Facade as VoIP Facade
participant Engine as CallEngine (sans-IO)
participant Driver as Driver Loop
participant Peer as Android Peer
App->>Facade: OutgoingCall.encoded_audio(OPUS_16KHZ_60MS, src, sink)
Facade->>Peer: "offer capability=STANDARD_OPUS, audio rate=16000"
Peer-->>Facade: "preaccept audio rate=16000"
Note over Facade: AudioFormatMismatch check passes
Peer-->>Facade: "accept audio rate=16000, voip_settings"
Facade->>Engine: "CallConfig audio=AudioConfig::encoded(OPUS_16KHZ_60MS)"
Engine->>Engine: "set PT=120, mlow_profile=false"
loop per 60ms frame
App->>Driver: EncodedAudio bytes via encoded_audio_in
Driver->>Engine: Input::EncodedAudio(payload)
Engine->>Engine: accepts_encoded_payload true
Engine->>Engine: pipe.protect_audio
Engine-->>Driver: Output::Transmit(srtp_packet)
Driver->>Peer: "UDP SRTP PT=120"
end
loop inbound
Peer->>Driver: "UDP SRTP PT=120"
Driver->>Engine: Input::RelayPacket
Engine->>Engine: accepts_rtp_payload_type(120) true
Engine->>Engine: pipe.unprotect_audio
Engine->>Engine: inbound_codec AudioCodec::Opus
Engine-->>Driver: Output::EncodedAudio(EncodedAudioFrame)
Driver->>App: encoded_audio_out channel
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant App as Application
participant Facade as VoIP Facade
participant Engine as CallEngine (sans-IO)
participant Driver as Driver Loop
participant Peer as Android Peer
App->>Facade: OutgoingCall.encoded_audio(OPUS_16KHZ_60MS, src, sink)
Facade->>Peer: "offer capability=STANDARD_OPUS, audio rate=16000"
Peer-->>Facade: "preaccept audio rate=16000"
Note over Facade: AudioFormatMismatch check passes
Peer-->>Facade: "accept audio rate=16000, voip_settings"
Facade->>Engine: "CallConfig audio=AudioConfig::encoded(OPUS_16KHZ_60MS)"
Engine->>Engine: "set PT=120, mlow_profile=false"
loop per 60ms frame
App->>Driver: EncodedAudio bytes via encoded_audio_in
Driver->>Engine: Input::EncodedAudio(payload)
Engine->>Engine: accepts_encoded_payload true
Engine->>Engine: pipe.protect_audio
Engine-->>Driver: Output::Transmit(srtp_packet)
Driver->>Peer: "UDP SRTP PT=120"
end
loop inbound
Peer->>Driver: "UDP SRTP PT=120"
Driver->>Engine: Input::RelayPacket
Engine->>Engine: accepts_rtp_payload_type(120) true
Engine->>Engine: pipe.unprotect_audio
Engine->>Engine: inbound_codec AudioCodec::Opus
Engine-->>Driver: Output::EncodedAudio(EncodedAudioFrame)
Driver->>App: encoded_audio_out channel
end
Reviews (3): Last reviewed commit: "fix(voip): report fallback decoder failu..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/client/voip.rs (1)
105-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument every feature profile that exposes these builders.
acceptandcallnow compile undervoip-runtime, which is enabled byvoip-encoded,voip-mlow, andvoip-libopus, not only the compatibilityvoipaggregate. The current rustdoc misdirects minimal-feature consumers.Proposed documentation fix
-/// accept) is the consumer's concern; this drives only media. Requires the `voip` feature. +/// accept) is the consumer's concern; this drives only media. Requires a VoIP profile: +/// `voip`, `voip-encoded`, `voip-mlow`, or `voip-libopus`. -/// handle is dormant until then. Requires the `voip` feature. +/// handle is dormant until then. Requires a VoIP profile: +/// `voip`, `voip-encoded`, `voip-mlow`, or `voip-libopus`.🤖 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 `@src/client/voip.rs` around lines 105 - 115, The rustdoc for the `accept` and outgoing-call builder methods in the client API must list every feature profile that exposes them. Update the documentation around `accept` and `call` to mention `voip-runtime`, `voip-encoded`, `voip-mlow`, and `voip-libopus`, rather than directing users only to the aggregate `voip` feature.src/voip/facade.rs (1)
1136-1219: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
set_muted()silently does nothing on encoded-audio calls.The
AudioEndpoints::Encodedbranch never wiresmutedinto anything — noMuteFeed, nothing reading the flag. Mute is exclusively a PCM-adapter trick (zeroing frames to trigger Opus DTX), which is fine as an implementation choice, butCallHandle::set_mutedis the one public API for both audio modes and its doc comment doesn't say a word about this. A consumer building onencoded_audio()and callingset_muted(true)will just... keep transmitting. If we're shipping a feature, it needs to actually do what it says, or at minimum tell people it won't.📝 Proposed doc fix
/// Mute or unmute the local microphone. While muted the engine sends DTX comfort-noise (the /// stream stays fed); it does not gap, so the peer doesn't re-negotiate the transport. + /// + /// Only affects PCM/MLOW audio (`.audio(...)`). For `.encoded_audio(...)` calls this has no + /// effect — muting an externally-encoded stream is the caller's responsibility. pub fn set_muted(&self, muted: bool) {🤖 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 `@src/voip/facade.rs` around lines 1136 - 1219, Update the documentation for the public CallHandle::set_muted method to explicitly state that muting affects PCM audio calls only and has no effect for encoded-audio calls. Leave the AudioEndpoints::Encoded behavior unchanged.
🤖 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 `@examples/voip-cli/src/main.rs`:
- Line 860: Replace the Tokio tasks that perform codec processing at the
indicated spawn sites with tokio::task::spawn_blocking workers, covering both
encoding and decoding operations. Keep the surrounding async call flow and
result handling intact while ensuring libopus and MLOW CPU work never runs
directly on Tokio worker threads.
- Around line 868-871: Update the Opus encoding error branch in the audio
processing loop so a single encode failure does not close the encoded source
while leaving the call connected with dead outbound audio. In the Err(error)
handling near the Opus encode operation, drop the failed frame and continue
processing, or explicitly propagate a fatal error that terminates the call; do
not break only the source loop.
In `@src/handlers/call.rs`:
- Around line 97-140: Ensure the audio-format mismatch branch performs the
existing dismiss_outgoing_siblings cleanup before returning. Reuse or move the
sibling-dismiss path from the surrounding call handling so incompatible calls
dismiss other outgoing sibling devices before termination and the early return,
without changing behavior for compatible calls.
In `@src/voip/audio.rs`:
- Around line 150-190: Update WaOpusDecoder to reuse an internal PCM output
buffer instead of allocating a new Vec<i16> for each packet. Add a decoder
method following the decode_into pattern that writes into the reusable buffer
and returns the valid decoded sample count or view, and use it from both decode
and decode_mlow_escape while preserving existing error handling and output
behavior.
In `@wacore/src/voip/mlow/analysis.rs`:
- Around line 204-205: Eliminate per-frame heap allocations in the hot path by
moving hp, x, xn, and hp_full into SmplEncoderState as reusable buffers. Update
the processing logic around smpl_filt_arma2 to resize or clear and refill these
persistent buffers with copy_from_slice or extend_from_slice, preserving the
existing contents and lengths required by each computation.
---
Outside diff comments:
In `@src/client/voip.rs`:
- Around line 105-115: The rustdoc for the `accept` and outgoing-call builder
methods in the client API must list every feature profile that exposes them.
Update the documentation around `accept` and `call` to mention `voip-runtime`,
`voip-encoded`, `voip-mlow`, and `voip-libopus`, rather than directing users
only to the aggregate `voip` feature.
In `@src/voip/facade.rs`:
- Around line 1136-1219: Update the documentation for the public
CallHandle::set_muted method to explicitly state that muting affects PCM audio
calls only and has no effect for encoded-audio calls. Leave the
AudioEndpoints::Encoded behavior unchanged.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a356b803-1a2e-43c4-8557-8f00dfec114a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (35)
Cargo.tomlREADME.mdagent_docs/voip_audio_codecs.mdexamples/voip-cli/Cargo.tomlexamples/voip-cli/src/main.rssrc/client.rssrc/client/lifecycle.rssrc/client/sessions.rssrc/client/voip.rssrc/handlers/call.rssrc/lib.rssrc/send/tctoken_lifecycle.rssrc/voip/audio.rssrc/voip/facade.rssrc/voip/mod.rssrc/voip/transport.rswacore/Cargo.tomlwacore/benches/voip_benchmark.rswacore/examples/voip_profile.rswacore/src/stanza/call.rswacore/src/types/call.rswacore/src/voip/audio.rswacore/src/voip/driver.rswacore/src/voip/engine.rswacore/src/voip/mlow/analysis.rswacore/src/voip/mlow/encode.rswacore/src/voip/mlow/mod.rswacore/src/voip/mlow/quality_tests.rswacore/src/voip/mlow/rangecoder.rswacore/src/voip/mlow/smpl_pitch_enc.rswacore/src/voip/mlow/toc.rswacore/src/voip/mod.rswacore/src/voip/registry.rswacore/src/voip/rtp.rswacore/src/voip/session.rs
📦 Binary size report
.text per crate
Baseline: |
There was a problem hiding this comment.
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 `@examples/voip-cli/src/main.rs`:
- Around line 1264-1268: Move the fallback Opus decoding in the tokio::spawn
event handler out of the async worker thread by executing
decoder.decode_mlow_escape and the resulting speaker.try_send through
tokio::task::spawn_blocking. Preserve the existing fallback_opus handling and
PCM delivery behavior while ensuring the async event loop only schedules the
blocking task.
In `@wacore/src/voip/rtp.rs`:
- Around line 109-113: Update is_mlow_dtx_payload to replace the two-part
bitwise condition with the equivalent direct check that the masked byte value
equals 0x80, while preserving the existing first-byte handling.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dd41daea-3d40-4c94-a6f1-d774d778d31b
📒 Files selected for processing (9)
examples/voip-cli/src/main.rssrc/client/voip.rssrc/handlers/call.rssrc/voip/audio.rssrc/voip/facade.rswacore/src/voip/engine.rswacore/src/voip/mlow/analysis.rswacore/src/voip/rtp.rswacore/src/voip/session.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/voip-cli/src/main.rs`:
- Around line 924-931: Update the fallback decoder flow around
WaOpusDecoder::new and decode_mlow_escape to log initialization failures instead
of silently returning via .ok()?, and log each decoding failure instead of
dropping the payload without feedback. Preserve the existing successful decode
and speaker.try_send behavior while using the surrounding logging facility for
actionable error details.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 69a28e4e-b83f-4652-8347-7edd56397f9c
📒 Files selected for processing (2)
examples/voip-cli/src/main.rswacore/src/voip/rtp.rs
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 (1)
examples/voip-cli/src/main.rs (1)
84-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the missing
Nonematch arm to prevent build failures.Look, we're building WhatsApp here. If someone compiles this CLI with both
voip-mlowandvoip-opusfeatures disabled, the build is going to completely crash with anon-exhaustive patternserror because theNonepattern isn't covered. We need our infrastructure to be bulletproof under all configurations. Add a fallback forNoneso the build doesn't break.🛠️ Proposed fix to cover all configurations
Some(other) => bail!( "WA_AUDIO_CODEC={other:?} is unavailable in this build; compile the matching \ voip-mlow/voip-opus feature" ), + #[cfg(all(not(feature = "voip-mlow"), not(feature = "voip-opus")))] + None => bail!("No audio codecs enabled in this build; compile the matching voip-mlow/voip-opus feature"), } }🤖 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 `@examples/voip-cli/src/main.rs` around lines 84 - 104, Update the from_env match to cover None when both voip-mlow and voip-opus features are disabled, preventing a non-exhaustive-patterns build failure. Add a fallback None arm that returns an appropriate unavailable-codec error, while preserving the existing feature-specific defaults and explicit codec handling.
🤖 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 `@examples/voip-cli/src/main.rs`:
- Around line 84-104: Update the from_env match to cover None when both
voip-mlow and voip-opus features are disabled, preventing a
non-exhaustive-patterns build failure. Add a fallback None arm that returns an
appropriate unavailable-codec error, while preserving the existing
feature-specific defaults and explicit codec handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca5afafb-4afa-41b6-9fa8-be2acaf1f34d
📒 Files selected for processing (1)
examples/voip-cli/src/main.rs
Reflects the whatsapp-rust feature split (voip-runtime/voip-encoded/ voip-mlow/voip-libopus, with voip as a compatibility alias), the new encoded_audio() builder for bring-your-own-codec audio, AudioFormat profiles, and native Opus negotiation from oxidezap/whatsapp-rust#1050.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70c2a0b989
ℹ️ 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".
| && !audio.iter().any(|codec| { | ||
| codec.enc.eq_ignore_ascii_case("opus") | ||
| && codec.rate == audio_config.format.signaling_rate | ||
| }) |
There was a problem hiding this comment.
When an incoming offer comes from a MLOW-only peer, this rate check still passes for .encoded_audio(AudioFormat::OPUS_16KHZ_60MS, ...) because both MLOW and native Opus advertise <audio enc="opus" rate="16000">. The engine is then configured for StandardOpus/PT120 while the caller expects MLOW, so both directions decode the wrong payload type/profile; the accept path needs to validate the capability/voip_settings profile, not just the signaling rate.
Useful? React with 👍 / 👎.
| && !audio.iter().any(|codec| { | ||
| codec.enc.eq_ignore_ascii_case("opus") | ||
| && codec.rate == expected.signaling_rate | ||
| }) |
There was a problem hiding this comment.
For an outgoing native-Opus call, a peer that ignores or cannot honor the standard-Opus capability can still answer with <audio enc="opus" rate="16000">, which satisfies this check even though it selected the MLOW RTP profile. The session then keeps expected as StandardOpus and the media engine sends/decodes Opus while the peer sends/expects MLOW, so the mismatch is never reported or terminated.
Useful? React with 👍 / 👎.
| fn offer_capability(video: bool, audio: AudioFormat) -> &'static [u8] { | ||
| let standard_opus = matches!(audio.rtp_profile, AudioRtpProfile::StandardOpus); | ||
| match (video, standard_opus) { |
There was a problem hiding this comment.
OPUS_16KHZ_60MS and the OPUS_RFC7587_* formats all have rtp_profile == StandardOpus, so this maps them to the same offer capability. If a caller selects an RFC7587 format, place_call still sends no enable_48khz_rtp_clock/PT111 signal while the engine is configured to send and accept PT111 with a 48 kHz RTP clock, leaving the peer on the default PT120 profile and causing media to be dropped or undecodable.
Useful? React with 👍 / 👎.
| { | ||
| self.source = Some(Arc::new(source)); | ||
| self.sink = Some(Arc::new(sink)); | ||
| self.audio = Some(AudioEndpoints::Pcm { |
There was a problem hiding this comment.
In voip-encoded or voip-libopus builds without voip-mlow, this .audio() path is still available and stores a PCM/MLOW endpoint. For outgoing calls, start() proceeds to send the offer and ring the peer before CallEngine::new later returns MlowUnavailable during relay attachment, so reject or cfg-gate this path before signaling when the MLOW codec is not compiled.
Useful? React with 👍 / 👎.
1 participant
Summary
voipbackward compatibleRoot cause
Clearing the MLOW capability selected the Android Opus encoder, so Android-to-Rust audio worked. The answer still omitted the directional
voip_settingsfield that selects the Android decoder, leaving Rust-to-Android packets on the MLOW decode path.Native Opus answers now send the matching capability plus a minimal uncompressed settings overlay:
encode.use_mlow_codec_v1=falseoptions.enable_48khz_rtp_clock=falseCodec selection and RTP clock selection remain independent.
Impact
WA_AUDIO_CODEC=opusnow provides full-duplex native Opusencoded_audioValidation
cargo fmt --allcargo clippy --all --testscargo test --workspace --exclude e2e-testscargo test -p whatsapp-rust-voip-cli --no-default-features --features voip-opuscargo test -p whatsapp-rust-voip-cli --no-default-features --features voip-mlowThe local E2E crate was excluded because it requires the separate mock WebSocket server.