W12 follow-up: consume in-band FEC, drop redundant DTX
Fixes the two P2 efficacy findings from the Codex audit of the W12 profiles feature. FEC was enabled on the encoder but never used: the jitter buffer's loss path did pure PLC, so the redundancy was wasted bitrate. Now the gap path reconstructs the lost frame from the next buffered packet via Opus in-band FEC (new `AudioDecoder::decode_fec`, libopus decode with fec=true into a one-frame buffer), keeping that packet for its own normal decode and falling back to PLC if FEC decode fails. This is the documented libopus FEC pattern; receiver-side only, no wire change. DTX was enabled on BadNetwork but provided no benefit — the capture noise gate already suppresses silence transmission, and the broadcast DTX silence packets only created seq gaps that grew the jitter cushion. All profiles now set dtx=false (plumbing kept for a future revisit). Adds a jitter-buffer test proving FEC reconstruction beats pure PLC (RMS error < 0.75x) and that the FEC source packet stays buffered. 500 lib tests, clippy + fmt clean, release build clean. Co-Authored-By: Codex (gpt-5.5) <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -3139,7 +3139,7 @@ fn audio_profile_hint(profile: AudioProfile) -> &'static str {
|
|||||||
}
|
}
|
||||||
AudioProfile::Balanced => "Default: voice quality with light loss recovery.",
|
AudioProfile::Balanced => "Default: voice quality with light loss recovery.",
|
||||||
AudioProfile::BadNetwork => {
|
AudioProfile::BadNetwork => {
|
||||||
"Most resilient on a lossy/congested link: extra loss recovery, lower bitrate."
|
"Most resilient on a lossy/congested link: heavier loss recovery, lower bitrate."
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -20,6 +20,9 @@ pub trait AudioDecoder: Send {
|
|||||||
/// If `compressed` is `None` (or `Some(&[])`), it indicates packet loss,
|
/// If `compressed` is `None` (or `Some(&[])`), it indicates packet loss,
|
||||||
/// enabling the decoder to perform packet loss concealment (PLC).
|
/// enabling the decoder to perform packet loss concealment (PLC).
|
||||||
fn decode(&mut self, compressed: Option<&[u8]>) -> Result<Vec<i16>, CodecError>;
|
fn decode(&mut self, compressed: Option<&[u8]>) -> Result<Vec<i16>, CodecError>;
|
||||||
|
|
||||||
|
/// Reconstructs the previous lost frame from the next packet's in-band FEC.
|
||||||
|
fn decode_fec(&mut self, next_payload: &[u8]) -> Result<Vec<i16>, CodecError>;
|
||||||
}
|
}
|
||||||
|
|
||||||
pub mod opus_impl;
|
pub mod opus_impl;
|
||||||
|
|||||||
+16
-5
@@ -40,7 +40,7 @@ pub fn opus_params(profile: AudioProfile) -> OpusParams {
|
|||||||
bitrate: 20_000,
|
bitrate: 20_000,
|
||||||
inband_fec: true,
|
inband_fec: true,
|
||||||
packet_loss_perc: 25,
|
packet_loss_perc: 25,
|
||||||
dtx: true,
|
dtx: false,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -162,6 +162,19 @@ impl AudioDecoder for OpusDecoder {
|
|||||||
pcm.truncate(decoded_per_channel * channels_count);
|
pcm.truncate(decoded_per_channel * channels_count);
|
||||||
Ok(pcm)
|
Ok(pcm)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn decode_fec(&mut self, next_payload: &[u8]) -> Result<Vec<i16>, CodecError> {
|
||||||
|
let channels_count = self.channels_count();
|
||||||
|
let mut pcm = vec![0i16; self.frame_samples * channels_count];
|
||||||
|
|
||||||
|
let decoded_per_channel = self
|
||||||
|
.decoder
|
||||||
|
.decode(next_payload, &mut pcm, true)
|
||||||
|
.map_err(|e| CodecError::Decode(format!("Opus FEC decoding failed: {}", e)))?;
|
||||||
|
|
||||||
|
pcm.truncate(decoded_per_channel * channels_count);
|
||||||
|
Ok(pcm)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
@@ -180,10 +193,8 @@ mod tests {
|
|||||||
assert!(bal.inband_fec);
|
assert!(bal.inband_fec);
|
||||||
assert!(bad.inband_fec);
|
assert!(bad.inband_fec);
|
||||||
|
|
||||||
// BadNetwork is the only profile that enables DTX, and it expects the
|
// Capture-side gating suppresses silence; no profile adds Opus DTX.
|
||||||
// heaviest loss.
|
assert!(!low.dtx && !bal.dtx && !bad.dtx);
|
||||||
assert!(bad.dtx);
|
|
||||||
assert!(!low.dtx && !bal.dtx);
|
|
||||||
assert!(bad.packet_loss_perc > bal.packet_loss_perc);
|
assert!(bad.packet_loss_perc > bal.packet_loss_perc);
|
||||||
|
|
||||||
// BadNetwork trims base bitrate to make room for FEC redundancy.
|
// BadNetwork trims base bitrate to make room for FEC redundancy.
|
||||||
|
|||||||
+1
-1
@@ -105,7 +105,7 @@ pub enum AudioProfile {
|
|||||||
#[default]
|
#[default]
|
||||||
Balanced,
|
Balanced,
|
||||||
/// Maximum resilience on a lossy/congested link: in-band FEC tuned for heavy
|
/// Maximum resilience on a lossy/congested link: in-band FEC tuned for heavy
|
||||||
/// loss plus DTX, at a lower bitrate to leave headroom for the redundancy.
|
/// loss, at a lower bitrate to leave headroom for the redundancy.
|
||||||
BadNetwork,
|
BadNetwork,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+95
-5
@@ -202,11 +202,15 @@ impl JitterBuffer {
|
|||||||
None
|
None
|
||||||
} else {
|
} else {
|
||||||
// Gap with later packets already buffered: a packet was lost
|
// Gap with later packets already buffered: a packet was lost
|
||||||
// or reordered out of window. Conceal this frame via Opus PLC
|
// or reordered out of window. First try Opus in-band FEC from
|
||||||
// and grow the cushion — the jitter beat our current delay.
|
// the next packet; if unavailable, fall back to plain PLC.
|
||||||
self.next_seq = Some(next.wrapping_add(1));
|
self.next_seq = Some(next.wrapping_add(1));
|
||||||
self.note_disruption();
|
self.note_disruption();
|
||||||
self.decoder.decode(None).ok()
|
let next_payload = self.packets.values().next().expect("non-empty");
|
||||||
|
self.decoder
|
||||||
|
.decode_fec(next_payload)
|
||||||
|
.or_else(|_| self.decoder.decode(None))
|
||||||
|
.ok()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -221,8 +225,8 @@ impl JitterBuffer {
|
|||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::codec::AudioEncoder;
|
use crate::codec::opus_impl::{OpusDecoder, OpusEncoder, OpusParams};
|
||||||
use crate::codec::opus_impl::OpusEncoder;
|
use crate::codec::{AudioDecoder, AudioEncoder};
|
||||||
use opus::{Application, Channels};
|
use opus::{Application, Channels};
|
||||||
|
|
||||||
/// A real, decodable Opus packet for one 20ms mono frame at amplitude `amp`.
|
/// A real, decodable Opus packet for one 20ms mono frame at amplitude `amp`.
|
||||||
@@ -233,6 +237,32 @@ mod tests {
|
|||||||
enc.encode(&pcm).unwrap()
|
enc.encode(&pcm).unwrap()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn tone_frame(enc: &mut OpusEncoder, amp: i16, frame_index: usize) -> Vec<u8> {
|
||||||
|
let pcm: Vec<i16> = (0..FRAME_SAMPLES)
|
||||||
|
.map(|i| {
|
||||||
|
let sample_index = frame_index * FRAME_SAMPLES + i;
|
||||||
|
let t = sample_index as f32 / 48_000.0;
|
||||||
|
let fundamental = (t * 220.0 * 2.0 * std::f32::consts::PI).sin();
|
||||||
|
let harmonic = (t * 440.0 * 2.0 * std::f32::consts::PI).sin();
|
||||||
|
((fundamental * 0.7 + harmonic * 0.3) * amp as f32) as i16
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
enc.encode(&pcm).unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn rms_error(a: &[i16], b: &[i16]) -> f64 {
|
||||||
|
assert_eq!(a.len(), b.len());
|
||||||
|
let sum_sq: f64 = a
|
||||||
|
.iter()
|
||||||
|
.zip(b)
|
||||||
|
.map(|(&left, &right)| {
|
||||||
|
let diff = left as f64 - right as f64;
|
||||||
|
diff * diff
|
||||||
|
})
|
||||||
|
.sum();
|
||||||
|
(sum_sq / a.len() as f64).sqrt()
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn buffers_then_plays_in_order() {
|
fn buffers_then_plays_in_order() {
|
||||||
let mut enc = OpusEncoder::new(48000, Channels::Mono, Application::Voip).unwrap();
|
let mut enc = OpusEncoder::new(48000, Channels::Mono, Application::Voip).unwrap();
|
||||||
@@ -289,6 +319,66 @@ mod tests {
|
|||||||
assert!(jb.pop_frame().is_none());
|
assert!(jb.pop_frame().is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn uses_in_band_fec_from_next_packet_for_gap() {
|
||||||
|
let mut enc = OpusEncoder::new(48000, Channels::Mono, Application::Voip).unwrap();
|
||||||
|
enc.apply_params(&OpusParams {
|
||||||
|
bitrate: 20_000,
|
||||||
|
inband_fec: true,
|
||||||
|
packet_loss_perc: 60,
|
||||||
|
dtx: false,
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let dropped_seq = 5usize;
|
||||||
|
let amps = [1800, 1800, 1800, 1800, 1800, 12_000, 12_000, 12_000];
|
||||||
|
let packets: Vec<Vec<u8>> = amps
|
||||||
|
.into_iter()
|
||||||
|
.enumerate()
|
||||||
|
.map(|(seq, amp)| tone_frame(&mut enc, amp, seq))
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
let mut expected_decoder = OpusDecoder::new(48000, Channels::Mono, FRAME_SAMPLES).unwrap();
|
||||||
|
for packet in packets.iter().take(dropped_seq) {
|
||||||
|
expected_decoder.decode(Some(packet)).unwrap();
|
||||||
|
}
|
||||||
|
let expected_lost = expected_decoder
|
||||||
|
.decode(Some(&packets[dropped_seq]))
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let mut plc_decoder = OpusDecoder::new(48000, Channels::Mono, FRAME_SAMPLES).unwrap();
|
||||||
|
for packet in packets.iter().take(dropped_seq) {
|
||||||
|
plc_decoder.decode(Some(packet)).unwrap();
|
||||||
|
}
|
||||||
|
let pure_plc = plc_decoder.decode(None).unwrap();
|
||||||
|
|
||||||
|
let mut jb = JitterBuffer::new().unwrap();
|
||||||
|
for (seq, packet) in packets.iter().enumerate() {
|
||||||
|
if seq != dropped_seq {
|
||||||
|
jb.insert(seq as u32, packet.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for _ in 0..dropped_seq {
|
||||||
|
assert_eq!(jb.pop_frame().map(|frame| frame.len()), Some(FRAME_SAMPLES));
|
||||||
|
}
|
||||||
|
|
||||||
|
let recovered = jb.pop_frame().expect("gap should be reconstructed");
|
||||||
|
assert_eq!(recovered.len(), FRAME_SAMPLES);
|
||||||
|
assert!(
|
||||||
|
jb.packets.contains_key(&(dropped_seq as u32 + 1)),
|
||||||
|
"FEC source packet must remain buffered for normal decode"
|
||||||
|
);
|
||||||
|
assert_eq!(jb.pop_frame().map(|frame| frame.len()), Some(FRAME_SAMPLES));
|
||||||
|
|
||||||
|
let fec_error = rms_error(&recovered, &expected_lost);
|
||||||
|
let plc_error = rms_error(&pure_plc, &expected_lost);
|
||||||
|
assert!(
|
||||||
|
fec_error < plc_error * 0.75,
|
||||||
|
"FEC reconstruction should be materially closer than PLC (fec_error={fec_error}, plc_error={plc_error})"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn drops_packets_already_played() {
|
fn drops_packets_already_played() {
|
||||||
let mut enc = OpusEncoder::new(48000, Channels::Mono, Application::Voip).unwrap();
|
let mut enc = OpusEncoder::new(48000, Channels::Mono, Application::Voip).unwrap();
|
||||||
|
|||||||
Reference in New Issue
Block a user