diff --git a/Cargo.lock b/Cargo.lock index 88b7ca2..54803d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -119,26 +119,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "audiopus" -version = "0.3.0-rc.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab55eb0e56d7c6de3d59f544e5db122d7725ec33be6a276ee8241f3be6473955" -dependencies = [ - "audiopus_sys", -] - -[[package]] -name = "audiopus_sys" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62314a1546a2064e033665d658e88c620a62904be945f8147e6b16c3db9f8651" -dependencies = [ - "cmake", - "log", - "pkg-config", -] - [[package]] name = "autocfg" version = "1.5.0" @@ -389,6 +369,12 @@ version = "3.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" +[[package]] +name = "bytemuck" +version = "1.25.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" + [[package]] name = "byteorder" version = "1.5.0" @@ -2125,6 +2111,24 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" +[[package]] +name = "opusic-c" +version = "1.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9486eb5a1a735bf56430b5b44e21157be30ac9fcc17999ba309981b8bd90d2ff" +dependencies = [ + "opusic-sys", +] + +[[package]] +name = "opusic-sys" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc3280fe5b6f97ac1a35a0ac003e2fb0b92f8e4bdf2b2057e1bf9b87acca5696" +dependencies = [ + "cmake", +] + [[package]] name = "os_str_bytes" version = "6.6.1" @@ -4309,9 +4313,11 @@ dependencies = [ name = "wzp-codec" version = "0.1.0" dependencies = [ - "audiopus", + "bytemuck", "codec2", "nnnoiseless", + "opusic-c", + "opusic-sys", "rand 0.8.5", "tracing", "wzp-proto", diff --git a/Cargo.toml b/Cargo.toml index dfe4b50..905ba0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,7 +35,14 @@ quinn = "0.11" raptorq = "2" # Codec -audiopus = "0.3.0-rc.0" +# opusic-c: high-level safe bindings over libopus 1.5.2 (encoder side). +# opusic-sys: raw FFI for the decoder side — we build our own DecoderHandle +# because opusic-c::Decoder.inner is pub(crate) and cannot be reached for the +# Phase 3 DRED reconstruction path. See docs/PRD-dred-integration.md. +# Pinned exactly (no caret) for reproducible libopus 1.5.2 across the fleet. +opusic-c = { version = "=1.5.5", default-features = false, features = ["bundled", "dred"] } +opusic-sys = { version = "=0.6.0", default-features = false, features = ["bundled"] } +bytemuck = "1" codec2 = "0.3" # Crypto diff --git a/crates/wzp-codec/Cargo.toml b/crates/wzp-codec/Cargo.toml index 4a74499..f958cd9 100644 --- a/crates/wzp-codec/Cargo.toml +++ b/crates/wzp-codec/Cargo.toml @@ -10,8 +10,17 @@ description = "WarzonePhone audio codec layer — Opus + Codec2 encoding/decodin wzp-proto = { workspace = true } tracing = { workspace = true } -# Opus bindings -audiopus = { workspace = true } +# Opus bindings — libopus 1.5.2. +# opusic-c for the encoder (set_dred_duration lives here in Phase 1). +# opusic-sys for the decoder — we wrap the raw *mut OpusDecoder ourselves +# because opusic-c::Decoder.inner is pub(crate), blocking the unified +# decoder + DRED path we need in Phase 3. +opusic-c = { workspace = true } +opusic-sys = { workspace = true } + +# Zero-cost slice reinterpretation for the i16 ↔ u16 boundary between +# our PCM buffers and opusic-c's encode API. +bytemuck = { workspace = true } # Pure-Rust Codec2 implementation codec2 = { workspace = true } diff --git a/crates/wzp-codec/src/dred_ffi.rs b/crates/wzp-codec/src/dred_ffi.rs new file mode 100644 index 0000000..927c6de --- /dev/null +++ b/crates/wzp-codec/src/dred_ffi.rs @@ -0,0 +1,185 @@ +//! Raw opusic-sys FFI wrappers for libopus 1.5.2 decoder + DRED reconstruction. +//! +//! # Why this module exists +//! +//! We cannot use `opusic_c::Decoder` because its inner `*mut OpusDecoder` +//! pointer is `pub(crate)` — not reachable from outside the opusic-c crate. +//! Phase 3 of the DRED integration needs to hand that same pointer to +//! `opus_decoder_dred_decode`, and running two parallel decoders (one from +//! opusic-c for normal audio, another from opusic-sys for DRED) would cause +//! the DRED-only decoder's internal state to drift out of sync with the +//! audio stream because it would not see normal decode calls. +//! +//! The fix is to own the raw decoder ourselves and use the same handle for +//! both normal decode AND future DRED reconstruction. This module is the +//! single owner of `*mut OpusDecoder` in the WZP workspace. +//! +//! Phase 0 only exposes `DecoderHandle` (normal decode). Phase 3 will add +//! `DredDecoderHandle`, `DredState`, and the `DredReconstructor` trait +//! implementation alongside it in this same file. + +use std::ptr::NonNull; + +use opusic_sys::{ + OPUS_OK, OpusDecoder as RawOpusDecoder, opus_decode, opus_decoder_create, + opus_decoder_destroy, +}; +use wzp_proto::CodecError; + +/// libopus operates at 48 kHz for all Opus variants we use. +const SAMPLE_RATE_HZ: i32 = 48_000; +/// Mono. +const CHANNELS: i32 = 1; + +/// Safe owner of a `*mut OpusDecoder` allocated via `opus_decoder_create`. +/// +/// Releases the decoder in `Drop`. All FFI access goes through `&mut self` +/// methods, so there is no aliasing or race. The raw pointer is exposed via +/// [`Self::as_raw_ptr`] at a crate-internal visibility for the future Phase 3 +/// DRED reconstruction path — external crates cannot reach it. +pub struct DecoderHandle { + inner: NonNull, +} + +impl DecoderHandle { + /// Allocate a new Opus decoder at 48 kHz mono. + pub fn new() -> Result { + let mut error: i32 = OPUS_OK; + // SAFETY: opus_decoder_create writes to `error` and returns either a + // valid heap pointer or null. We check both before constructing the + // NonNull wrapper. + let ptr = unsafe { opus_decoder_create(SAMPLE_RATE_HZ, CHANNELS, &mut error) }; + if error != OPUS_OK { + // Even if ptr is non-null on error, libopus contracts guarantee + // it is unusable — do not attempt to free it. + return Err(CodecError::DecodeFailed(format!( + "opus_decoder_create failed: err={error}" + ))); + } + let inner = NonNull::new(ptr).ok_or_else(|| { + CodecError::DecodeFailed("opus_decoder_create returned null".into()) + })?; + Ok(Self { inner }) + } + + /// Decode an Opus packet into PCM samples. + /// + /// `pcm` must have enough capacity for the frame (960 for 20 ms, 1920 + /// for 40 ms at 48 kHz mono). Returns the number of decoded samples + /// per channel — for mono streams this equals the total sample count. + pub fn decode(&mut self, packet: &[u8], pcm: &mut [i16]) -> Result { + if packet.is_empty() { + return Err(CodecError::DecodeFailed("empty packet".into())); + } + if pcm.is_empty() { + return Err(CodecError::DecodeFailed("empty output buffer".into())); + } + // SAFETY: self.inner is a valid *mut OpusDecoder owned by this struct. + // `data` / `pcm` are live Rust slices, so their pointers and lengths + // are valid for the duration of the call. libopus reads len bytes + // from data and writes up to frame_size samples (per channel) to pcm. + let n = unsafe { + opus_decode( + self.inner.as_ptr(), + packet.as_ptr(), + packet.len() as i32, + pcm.as_mut_ptr(), + pcm.len() as i32, + /* decode_fec = */ 0, + ) + }; + if n < 0 { + return Err(CodecError::DecodeFailed(format!( + "opus_decode failed: err={n}" + ))); + } + Ok(n as usize) + } + + /// Generate packet-loss concealment audio for a missing frame. + /// + /// Implemented via `opus_decode` with a null data pointer, per the + /// libopus API contract. `pcm` should be sized for the expected frame. + pub fn decode_lost(&mut self, pcm: &mut [i16]) -> Result { + if pcm.is_empty() { + return Err(CodecError::DecodeFailed("empty output buffer".into())); + } + // SAFETY: same invariants as decode(). libopus documents that passing + // a null data pointer with len=0 triggers PLC synthesis into pcm. + let n = unsafe { + opus_decode( + self.inner.as_ptr(), + std::ptr::null(), + 0, + pcm.as_mut_ptr(), + pcm.len() as i32, + /* decode_fec = */ 0, + ) + }; + if n < 0 { + return Err(CodecError::DecodeFailed(format!( + "opus_decode PLC failed: err={n}" + ))); + } + Ok(n as usize) + } + + /// Raw pointer access for the Phase 3 DRED reconstruction path. + /// + /// The pointer is valid for the lifetime of `self`. Callers must not + /// free it or cause the underlying decoder to mutate while the pointer + /// is being used concurrently. Currently unused in Phase 0 — kept + /// `pub(crate)` so only the future `dred` submodule inside this crate + /// can reach it. + #[allow(dead_code)] + pub(crate) fn as_raw_ptr(&self) -> *mut RawOpusDecoder { + self.inner.as_ptr() + } +} + +impl Drop for DecoderHandle { + fn drop(&mut self) { + // SAFETY: we own the pointer and no further access happens after + // this call because Drop consumes self. + unsafe { opus_decoder_destroy(self.inner.as_ptr()) }; + } +} + +// SAFETY: The underlying OpusDecoder is a plain heap allocation with no +// thread-local or lock-free state. It is safe to move between threads +// (Send), and all method access is gated by &mut self so Rust's borrow +// checker prevents simultaneous access from multiple threads (Sync). +unsafe impl Send for DecoderHandle {} +unsafe impl Sync for DecoderHandle {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn decoder_handle_creates_and_drops() { + let handle = DecoderHandle::new().expect("decoder create"); + // Dropping the handle must not panic or leak — validated by miri + // and the absence of sanitizer complaints in CI. + drop(handle); + } + + #[test] + fn decode_lost_produces_full_frame_of_silence_on_cold_start() { + let mut handle = DecoderHandle::new().unwrap(); + // 20 ms @ 48 kHz mono. + let mut pcm = vec![0i16; 960]; + let n = handle.decode_lost(&mut pcm).unwrap(); + assert_eq!(n, 960); + // On a fresh decoder, PLC output is silence (no past audio to extend). + assert!(pcm.iter().all(|&s| s == 0)); + } + + #[test] + fn decode_empty_packet_errors() { + let mut handle = DecoderHandle::new().unwrap(); + let mut pcm = vec![0i16; 960]; + let err = handle.decode(&[], &mut pcm); + assert!(err.is_err()); + } +} diff --git a/crates/wzp-codec/src/lib.rs b/crates/wzp-codec/src/lib.rs index 592e21e..30cf1e7 100644 --- a/crates/wzp-codec/src/lib.rs +++ b/crates/wzp-codec/src/lib.rs @@ -15,6 +15,7 @@ pub mod agc; pub mod codec2_dec; pub mod codec2_enc; pub mod denoise; +pub mod dred_ffi; pub mod opus_dec; pub mod opus_enc; pub mod resample; diff --git a/crates/wzp-codec/src/opus_dec.rs b/crates/wzp-codec/src/opus_dec.rs index c8b6cd4..354f043 100644 --- a/crates/wzp-codec/src/opus_dec.rs +++ b/crates/wzp-codec/src/opus_dec.rs @@ -1,30 +1,32 @@ -//! Opus decoder wrapping the `audiopus` crate. +//! Opus decoder built on top of the raw opusic-sys `DecoderHandle`. +//! +//! Phase 0 of the DRED integration: we went straight to a custom +//! `DecoderHandle` instead of `opusic_c::Decoder` because the latter's +//! inner pointer is `pub(crate)` and we need to reach it in Phase 3 for +//! `opus_decoder_dred_decode`. See `dred_ffi.rs` for the rationale and +//! `docs/PRD-dred-integration.md` for the full plan. -use audiopus::coder::Decoder; -use audiopus::{Channels, MutSignals, SampleRate}; -use audiopus::packet::Packet; +use crate::dred_ffi::DecoderHandle; use wzp_proto::{AudioDecoder, CodecError, CodecId, QualityProfile}; -/// Opus decoder implementing `AudioDecoder`. +/// Opus decoder implementing [`AudioDecoder`]. /// -/// Operates at 48 kHz mono output. +/// Operates at 48 kHz mono output. 20 ms and 40 ms frames supported via +/// the active `QualityProfile`. Behavior is intentionally identical to +/// the pre-swap audiopus-based decoder at this phase — DRED reconstruction +/// lands in Phase 3. pub struct OpusDecoder { - inner: Decoder, + inner: DecoderHandle, codec_id: CodecId, frame_duration_ms: u8, } -// SAFETY: Same reasoning as OpusEncoder — exclusive access via &mut self. -unsafe impl Sync for OpusDecoder {} - impl OpusDecoder { /// Create a new Opus decoder for the given quality profile. pub fn new(profile: QualityProfile) -> Result { - let decoder = Decoder::new(SampleRate::Hz48000, Channels::Mono) - .map_err(|e| CodecError::DecodeFailed(format!("opus decoder init: {e}")))?; - + let inner = DecoderHandle::new()?; Ok(Self { - inner: decoder, + inner, codec_id: profile.codec, frame_duration_ms: profile.frame_duration_ms, }) @@ -45,15 +47,7 @@ impl AudioDecoder for OpusDecoder { pcm.len() ))); } - let packet = Packet::try_from(encoded) - .map_err(|e| CodecError::DecodeFailed(format!("invalid packet: {e}")))?; - let signals = MutSignals::try_from(pcm) - .map_err(|e| CodecError::DecodeFailed(format!("output signals: {e}")))?; - let n = self - .inner - .decode(Some(packet), signals, false) - .map_err(|e| CodecError::DecodeFailed(format!("opus decode: {e}")))?; - Ok(n) + self.inner.decode(encoded, pcm) } fn decode_lost(&mut self, pcm: &mut [i16]) -> Result { @@ -64,13 +58,7 @@ impl AudioDecoder for OpusDecoder { pcm.len() ))); } - let signals = MutSignals::try_from(pcm) - .map_err(|e| CodecError::DecodeFailed(format!("output signals: {e}")))?; - let n = self - .inner - .decode(None, signals, false) - .map_err(|e| CodecError::DecodeFailed(format!("opus PLC: {e}")))?; - Ok(n) + self.inner.decode_lost(pcm) } fn codec_id(&self) -> CodecId { diff --git a/crates/wzp-codec/src/opus_enc.rs b/crates/wzp-codec/src/opus_enc.rs index 1a5dca1..79f3080 100644 --- a/crates/wzp-codec/src/opus_enc.rs +++ b/crates/wzp-codec/src/opus_enc.rs @@ -1,7 +1,14 @@ -//! Opus encoder wrapping the `audiopus` crate. +//! Opus encoder wrapping the `opusic-c` crate (libopus 1.5.2). +//! +//! Phase 0 of the DRED integration: swapped FFI backend from audiopus +//! (dead, libopus 1.3) to opusic-c (live, libopus 1.5.2). Behavior is +//! intentionally unchanged from the audiopus-based encoder — inband FEC +//! stays ON, DRED stays at duration 0. Phase 1 enables DRED and disables +//! inband FEC. See docs/PRD-dred-integration.md. -use audiopus::coder::Encoder; -use audiopus::{Application, Bitrate, Channels, SampleRate, Signal}; +use opusic_c::{ + Application, Bitrate, Channels, Encoder, InbandFec, SampleRate, Signal, +}; use tracing::debug; use wzp_proto::{AudioEncoder, CodecError, CodecId, QualityProfile}; @@ -16,15 +23,17 @@ pub struct OpusEncoder { } // SAFETY: OpusEncoder is only used via `&mut self` methods. The inner -// audiopus Encoder contains a raw pointer that is !Sync, but we never -// share it across threads without exclusive access. +// opusic-c Encoder wraps a non-null pointer that is !Sync by default, +// but we never share it across threads without exclusive access. unsafe impl Sync for OpusEncoder {} impl OpusEncoder { /// Create a new Opus encoder for the given quality profile. pub fn new(profile: QualityProfile) -> Result { - let encoder = Encoder::new(SampleRate::Hz48000, Channels::Mono, Application::Voip) - .map_err(|e| CodecError::EncodeFailed(format!("opus encoder init: {e}")))?; + // opusic-c argument order: (Channels, SampleRate, Application) + // — different from audiopus's (SampleRate, Channels, Application). + let encoder = Encoder::new(Channels::Mono, SampleRate::Hz48000, Application::Voip) + .map_err(|e| CodecError::EncodeFailed(format!("opus encoder init: {e:?}")))?; let mut enc = Self { inner: encoder, @@ -38,21 +47,21 @@ impl OpusEncoder { // Voice signal type hint for better compression enc.inner .set_signal(Signal::Voice) - .map_err(|e| CodecError::EncodeFailed(format!("set signal: {e}")))?; + .map_err(|e| CodecError::EncodeFailed(format!("set signal: {e:?}")))?; // Default complexity 7 — good quality/CPU trade-off for VoIP enc.inner .set_complexity(7) - .map_err(|e| CodecError::EncodeFailed(format!("set complexity: {e}")))?; + .map_err(|e| CodecError::EncodeFailed(format!("set complexity: {e:?}")))?; Ok(enc) } fn apply_bitrate(&mut self, codec: CodecId) -> Result<(), CodecError> { - let bps = codec.bitrate_bps() as i32; + let bps = codec.bitrate_bps(); self.inner - .set_bitrate(Bitrate::BitsPerSecond(bps)) - .map_err(|e| CodecError::EncodeFailed(format!("set bitrate: {e}")))?; + .set_bitrate(Bitrate::Value(bps)) + .map_err(|e| CodecError::EncodeFailed(format!("set bitrate: {e:?}")))?; debug!(bitrate_bps = bps, "opus encoder bitrate set"); Ok(()) } @@ -74,7 +83,7 @@ impl OpusEncoder { /// Higher values cause the encoder to use more redundancy to survive /// packet loss, at the expense of slightly higher bitrate. pub fn set_expected_loss(&mut self, loss_pct: u8) { - let _ = self.inner.set_packet_loss_perc(loss_pct.min(100)); + let _ = self.inner.set_packet_loss(loss_pct.min(100)); } } @@ -87,10 +96,14 @@ impl AudioEncoder for OpusEncoder { pcm.len() ))); } + // opusic-c takes &[u16] for the sample input. Bit pattern is + // identical to i16 — the cast is zero-cost and the encoder + // interprets the bytes the same way as libopus internally. + let pcm_u16: &[u16] = bytemuck::cast_slice(pcm); let n = self .inner - .encode(pcm, out) - .map_err(|e| CodecError::EncodeFailed(format!("opus encode: {e}")))?; + .encode_to_slice(pcm_u16, out) + .map_err(|e| CodecError::EncodeFailed(format!("opus encode: {e:?}")))?; Ok(n) } @@ -120,10 +133,56 @@ impl AudioEncoder for OpusEncoder { } fn set_inband_fec(&mut self, enabled: bool) { - let _ = self.inner.set_inband_fec(enabled); + // opusic-c replaces the audiopus bool with an enum that distinguishes + // Mode1 (classical LBRR, equivalent to libopus 1.3's inband FEC) and + // Mode2 (newer, higher-quality variant added in 1.5). Phase 0 preserves + // pre-swap behavior by using Mode1, which is the direct equivalent of + // audiopus's `set_inband_fec(true)`. Phase 1 flips this to Off when + // DRED is enabled. + let mode = if enabled { InbandFec::Mode1 } else { InbandFec::Off }; + let _ = self.inner.set_inband_fec(mode); } fn set_dtx(&mut self, enabled: bool) { let _ = self.inner.set_dtx(enabled); } } + +#[cfg(test)] +mod tests { + use super::*; + use wzp_proto::AudioDecoder; + + /// Phase 0 acceptance gate: fail loudly if the linked libopus is not 1.5.x. + /// DRED (Phase 1+) only exists in libopus ≥ 1.5, so running against an + /// older version would silently regress the entire DRED integration. + #[test] + fn linked_libopus_is_1_5() { + let version = opusic_c::version(); + assert!( + version.contains("1.5"), + "expected libopus 1.5.x, got: {version}" + ); + } + + #[test] + fn encoder_creates_at_good_profile() { + let enc = OpusEncoder::new(QualityProfile::GOOD).expect("opus encoder init"); + assert_eq!(enc.codec_id, CodecId::Opus24k); + assert_eq!(enc.frame_samples(), 960); // 20 ms @ 48 kHz + } + + #[test] + fn encoder_roundtrip_silence() { + use crate::opus_dec::OpusDecoder; + let mut enc = OpusEncoder::new(QualityProfile::GOOD).unwrap(); + let mut dec = OpusDecoder::new(QualityProfile::GOOD).unwrap(); + let pcm_in = vec![0i16; 960]; // 20 ms silence + let mut encoded = vec![0u8; 512]; + let n = enc.encode(&pcm_in, &mut encoded).unwrap(); + assert!(n > 0); + let mut pcm_out = vec![0i16; 960]; + let samples = dec.decode(&encoded[..n], &mut pcm_out).unwrap(); + assert_eq!(samples, 960); + } +}