From 7547b8e1408230b7a86c9f4c27b1d6bcf8030523 Mon Sep 17 00:00:00 2001 From: Talon Date: Mon, 22 Jun 2026 20:18:29 +0200 Subject: [PATCH] fix(audio): wire in-band FEC into the decoder loss path The encoder set OPUS_SET_INBAND_FEC, but the decoder never invoked FEC -- the loss path went DRED -> PLC, so FEC redundancy was emitted (and paid for in bitrate) yet never consumed. Wire FEC recovery into AudioEngine::on_playback between DRED and PLC: copy the next buffered packet once, try DRED, else (if the stream negotiated FEC) decode(next_pkt, ..., fec=true), else PLC. Recovery priority is now DRED -> FEC -> PLC. Add per-stream RemoteStream::fec_enabled_, captured from OpusParams in init_recv_stream. Docs (voice.md) updated to match. ctest --preset dev: 27/27 green. Co-Authored-By: Claude Opus 4.8 --- PROGRESS.md | 8 +++++ core/src/audio/audio_engine.cpp | 57 ++++++++++++++++++++------------- core/src/audio/audio_engine.h | 5 +++ docs/voice.md | 21 ++++++++---- 4 files changed, 62 insertions(+), 29 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index a917614..38510df 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -691,6 +691,14 @@ iOS 18.0 deployment target. App Group `group.cat.voice.VoiceCat` for Keychain sh reconstructs the lost frame — otherwise falls back to standard PLC. New test: `test_dred_toggle` (ctest 22/22). Files: `voicecat.proto`, `voicecat.h`, `opus_codec.{h,cpp}`, `audio_engine.{h,cpp}`, `client.cpp`, `session.{h,cpp}`. +- [x] **In-band FEC decoder wiring** — done (2026-06-22). The encoder set `OPUS_SET_INBAND_FEC` + all along, but the decoder never invoked it — the loss path went DRED → PLC, so FEC redundancy + was emitted (and paid for in bitrate) but never consumed. Wired the FEC recovery into + `AudioEngine::on_playback`'s loss branch between DRED and PLC: copy the next buffered packet + once, try DRED, else (if the stream negotiated FEC) `decode(next_pkt, …, fec=true)`, else PLC. + Added per-stream `RemoteStream::fec_enabled_`, captured from `OpusParams` in + `init_recv_stream`. Recovery priority is now **DRED → FEC → PLC**. ctest 27/27 green. Files: + `audio_engine.{h,cpp}`, `docs/voice.md`. - [ ] **DRED toggle in client UIs** — expose the `dred` flag in all three channel-config UIs so admins can enable it per channel. Windows: `ChannelEditForm` / `vc_channel_info.audio.dred` checkbox. macOS AppKit: channel-edit sheet. iOS SwiftUI: channel-edit form. All three UIs diff --git a/core/src/audio/audio_engine.cpp b/core/src/audio/audio_engine.cpp index f05509d..3906cc7 100644 --- a/core/src/audio/audio_engine.cpp +++ b/core/src/audio/audio_engine.cpp @@ -597,6 +597,7 @@ void AudioEngine::init_recv_stream(uint32_t ssrc, const codec::OpusParams& p, auto& stream = streams_[ssrc]; stream.user_id = user_id; stream.stream_id = stream_id; + stream.fec_enabled_ = p.fec; stream.decoder.init(p); // Ring must be sized for this decoder's actual channel/frame-size — see RemoteStream::ring // comment in audio_engine.h for why this can't just be the playback callback's frame count. @@ -740,36 +741,48 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) { sizeof(int16_t)); n = frame_samples; } else { - // Try DRED recovery: if the next packet is already in the jitter buffer, - // parse its DRED extension and reconstruct the lost frame with it — producing - // better quality than PLC comfort noise. Falls back to PLC on any failure. + // Loss recovery for a missing frame, best-quality first: DRED (Opus 1.6 ML + // reconstruction) → in-band FEC (the low-bitrate copy of this frame the encoder + // embeds in the next packet) → PLC comfort noise. DRED and FEC both need the + // *next* packet already buffered, so copy it once and try each in turn. n = -1; #ifdef VOICECAT_HAS_OPUS - if (dred_dec_ && stream.dred_state_) { - uint32_t next_ts = stream.playout_ts + static_cast(frame_samples); - size_t psz = stream.jitter.try_copy_front_payload( - next_ts, stream.dred_payload_scratch_.data(), - stream.dred_payload_scratch_.size()); - if (psz > 0) { - int dred_end = 0; - int ret = opus_dred_parse( - dred_dec_, stream.dred_state_, - stream.dred_payload_scratch_.data(), - static_cast(psz), - frame_samples, static_cast(params_.sample_rate), - &dred_end, 0); - if (ret > 0) - n = stream.decoder.decode_dred(stream.dred_state_, 0, - stream.decode_scratch.data(), - frame_samples); - } + uint32_t next_ts = stream.playout_ts + static_cast(frame_samples); + size_t psz = stream.jitter.try_copy_front_payload( + next_ts, stream.dred_payload_scratch_.data(), + stream.dred_payload_scratch_.size()); + + // 1. DRED: parse the next packet's deep-redundancy extension and reconstruct. + if (psz > 0 && dred_dec_ && stream.dred_state_) { + int dred_end = 0; + int ret = opus_dred_parse( + dred_dec_, stream.dred_state_, + stream.dred_payload_scratch_.data(), + static_cast(psz), + frame_samples, static_cast(params_.sample_rate), + &dred_end, 0); + if (ret > 0) + n = stream.decoder.decode_dred(stream.dred_state_, 0, + stream.decode_scratch.data(), + frame_samples); + } + + // 2. In-band FEC: reconstruct the lost frame from the redundant copy carried in + // the next packet (decode_fec=1). Only when FEC is negotiated and the next + // packet is present; if it carries no LBRR data libopus falls back to PLC, + // so this is at worst a no-op relative to the PLC path below. + if (n <= 0 && psz > 0 && stream.fec_enabled_) { + n = stream.decoder.decode( + stream.dred_payload_scratch_.data(), static_cast(psz), + stream.decode_scratch.data(), frame_samples, /*fec=*/true); } #endif + // 3. PLC: synthesize a continuation when no redundancy is available. if (n <= 0) { n = stream.decoder.decode(nullptr, 0, stream.decode_scratch.data(), frame_samples); } - if (n > 0) stream.plc_samples_since_real += n; // track PLC streak + if (n > 0) stream.plc_samples_since_real += n; // track concealment streak } if (n <= 0) break; // decoder error/exhausted PLC; rest of this period stays silent diff --git a/core/src/audio/audio_engine.h b/core/src/audio/audio_engine.h index 8a9c9d8..d559b97 100644 --- a/core/src/audio/audio_engine.h +++ b/core/src/audio/audio_engine.h @@ -482,6 +482,11 @@ class AudioEngine { #endif std::vector dred_payload_scratch_; // pre-sized to 4000 bytes + // In-band FEC: whether the sender negotiated OPUS_SET_INBAND_FEC for this stream. + // When set, on_playback attempts decode_fec=1 from the next buffered packet to recover a + // lost frame (between DRED and PLC). Captured from OpusParams at init_recv_stream(). + bool fec_enabled_ = false; + // Source identity: stored at init_recv_stream() so the pcm_sink_ callback can receive // (user_id, stream_id) without a separate map lookup from the RT playback thread. uint32_t user_id = 0; diff --git a/docs/voice.md b/docs/voice.md index 140eade..9a7bfaf 100644 --- a/docs/voice.md +++ b/docs/voice.md @@ -125,11 +125,14 @@ Guidance baked into defaults / docs: Layered, all configurable per channel: -1. **In-band FEC** — Opus embeds a low-bitrate copy of the previous frame; the decoder - recovers a lost packet from the *next* one (costs one frame of latency on recovery). - Tuned by `expected_packet_loss`. +1. **In-band FEC** — the encoder embeds a low-bitrate copy of the current frame in the + *next* packet (`OPUS_SET_INBAND_FEC`, redundancy scaled by `expected_packet_loss`). On a + loss, the receiver decodes that copy out of the next already-buffered packet with + `opus_decode(..., decode_fec=1)` — costing one frame of latency on recovery. Gated on the + per-stream `fec` flag; if the next packet carries no redundancy libopus yields PLC output, + so it is at worst a no-op relative to (2). 2. **PLC (packet loss concealment)** — decoder synthesizes a plausible frame for an - unrecovered loss; always on, free. + unrecovered loss; always on, free. The terminal fallback when neither DRED nor FEC applies. 3. **DTX** — sender stops transmitting during silence and sends sparse comfort-noise updates; cuts bandwidth and is bandwidth-friendly on busy channels. 4. **DRED (Deep REDundancy, per-channel toggle)** — Opus 1.6's ML redundancy: the encoder @@ -137,9 +140,13 @@ Layered, all configurable per channel: default). When a packet is lost, the receiver peeks at the next already-buffered packet, parses its DRED extension (`opus_dred_parse`), and reconstructs the lost frame with `opus_decoder_dred_decode` — producing significantly better audio than PLC comfort noise - for single-frame gaps. Falls back silently to PLC if the next packet has not arrived yet - or if the sender did not embed DRED. Heavier CPU on the encoder (~5–10 % at 24 kbps); - minimal overhead on the decoder (parse is a fast header check on non-DRED packets). + for single-frame gaps. Heavier CPU on the encoder (~5–10 % at 24 kbps); minimal overhead + on the decoder (parse is a fast header check on non-DRED packets). + +When a frame is lost, `AudioEngine::on_playback` tries these recovery paths in quality order, +falling through on failure: **DRED → in-band FEC → PLC**. DRED and FEC both need the next +packet already buffered (one frame of look-ahead); when it has not arrived yet, recovery falls +straight through to PLC. ## 5. Jitter buffer