fix(audio): decouple Opus decode cadence from playback callback period
on_playback() was passing miniaudio's hardware playback-callback frame count to opus_decode()'s max_samples, instead of the decoder's fixed frame size (960 samples @ 20ms/48kHz). Since real packets decode to more samples than the (often smaller, e.g. ~480 on default low-latency WASAPI) hardware period, opus_decode returned OPUS_BUFFER_TOO_SMALL on nearly every callback -- packets were received/decrypted/jitter-buffered correctly but never decoded into audible PCM. Result: control-plane events and VAD worked, but zero audio in headphones. mix_for_test()'s white-box test masked this since it always called on_playback with frames == frame_samples, the one case where the bug is invisible. Fix: RemoteStream gained a small ring buffer (init_ring/push_ring/ pop_ring) that decouples decode cadence from playback-callback cadence. on_playback now tops the ring up by decoding whole Opus frames (always decoder.frame_samples(), never the hardware frame count) and drains exactly what the callback asks for, silence-padding (PLC) on underrun. Side effect: also fixes playout_ts, which was advancing by the wrong unit (hardware frames instead of decoded samples) -- it now tracks correctly against jitter-buffer timestamps. ctest --test-dir build/m1-dev: 12/12 green. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
20
PROGRESS.md
20
PROGRESS.md
@@ -10,6 +10,26 @@ up instantly. Newest status at the top.
|
|||||||
|
|
||||||
## ▶ Where we left off / next action
|
## ▶ Where we left off / next action
|
||||||
|
|
||||||
|
- **Done:** **Fixed silent-playback bug in `AudioEngine::on_playback`** (2026-06-16, found via
|
||||||
|
live manual test: two `vccli --voice` clients, control-plane events and VAD all correct, but
|
||||||
|
zero audible output). Root cause: `opus_decode()`'s `max_samples` was being passed the
|
||||||
|
*hardware playback callback's* frame count (miniaudio's own choice, frequently smaller than
|
||||||
|
one Opus frame — e.g. ~480 samples on default low-latency WASAPI periods), instead of the
|
||||||
|
decoder's fixed frame size (960 @ 20ms/48kHz). Since the real packet almost always decodes to
|
||||||
|
more samples than that, `opus_decode` returned `OPUS_BUFFER_TOO_SMALL` on nearly every
|
||||||
|
callback — frames were correctly received/decrypted/jitter-buffered, just never decoded into
|
||||||
|
audible PCM. `mix_for_test()`'s white-box test masked this because it always called
|
||||||
|
`on_playback` with `frames == frame_samples`, the one case where the bug is invisible.
|
||||||
|
Fix: `RemoteStream` (`core/src/audio/audio_engine.h`) gained a small ring buffer
|
||||||
|
(`init_ring`/`push_ring`/`pop_ring`) that decouples decode cadence from playback-callback
|
||||||
|
cadence — `on_playback` (`core/src/audio/audio_engine.cpp`) now tops the ring up by decoding
|
||||||
|
whole Opus frames (`decoder.frame_samples()`, never the hardware `frames`) and drains exactly
|
||||||
|
`frames` samples-per-channel from it each callback, silence-padding (PLC) on underrun. Also
|
||||||
|
fixes a latent `playout_ts` bug: it now advances by the actual decoded sample count per Opus
|
||||||
|
frame, not by the hardware callback's (unrelated) frame count, which was the wrong unit for
|
||||||
|
jitter-buffer timestamp comparisons. `ctest --test-dir build/m1-dev` — 12/12 green (run via
|
||||||
|
PowerShell; Git Bash exec gotcha for these binaries, see `docs/building.md`). **Not yet
|
||||||
|
confirmed audible by ear** — pending the user re-running their live two-`vccli` test.
|
||||||
- **Done:** **Post-M3 follow-up — device enumeration, VAD/PTT gate, stereo playback, WASAPI
|
- **Done:** **Post-M3 follow-up — device enumeration, VAD/PTT gate, stereo playback, WASAPI
|
||||||
loopback** ✓ complete (2026-06-16). Closes all three items M3 explicitly carried forward as
|
loopback** ✓ complete (2026-06-16). Closes all three items M3 explicitly carried forward as
|
||||||
out of scope (see the dated section below for the full file-by-file change list).
|
out of scope (see the dated section below for the full file-by-file change list).
|
||||||
|
|||||||
@@ -327,7 +327,14 @@ uint32_t AudioEngine::stream_target_depth_ms(uint32_t ssrc) const {
|
|||||||
#ifdef VOICECAT_HAS_OPUS
|
#ifdef VOICECAT_HAS_OPUS
|
||||||
void AudioEngine::init_recv_stream(uint32_t ssrc, const codec::OpusParams& p) {
|
void AudioEngine::init_recv_stream(uint32_t ssrc, const codec::OpusParams& p) {
|
||||||
std::lock_guard lk(streams_mu_);
|
std::lock_guard lk(streams_mu_);
|
||||||
streams_[ssrc].decoder.init(p);
|
auto& stream = streams_[ssrc];
|
||||||
|
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.
|
||||||
|
int channels = std::max(1, stream.decoder.channels());
|
||||||
|
int frame_samples = stream.decoder.frame_samples();
|
||||||
|
if (frame_samples <= 0) frame_samples = static_cast<int>(p.sample_rate / 1000 * p.frame_ms);
|
||||||
|
stream.init_ring(channels, frame_samples);
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
@@ -367,37 +374,46 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) {
|
|||||||
|
|
||||||
// A stream's Opus channel count (mono/stereo, per-channel AudioConfig) may differ from
|
// A stream's Opus channel count (mono/stereo, per-channel AudioConfig) may differ from
|
||||||
// the engine-wide playback channel count (now genuinely stereo — see docs/voice.md §8).
|
// the engine-wide playback channel count (now genuinely stereo — see docs/voice.md §8).
|
||||||
// Decode into a buffer sized for the *decoder's* channel count (opus_decode's
|
// dec_channels/frame_samples are bitstream properties (fixed at decoder init); `frames`
|
||||||
// frame_size parameter is samples-per-channel, not total samples — pass `frames`,
|
// below is the *hardware* playback callback's period, an independent value miniaudio
|
||||||
// not `frames * channels`), then convert at this mix boundary: true stereo decode
|
// picks on its own — opus_decode's max_samples must be frame_samples, never `frames`
|
||||||
// output is mixed in directly (no downmix); mono decode output is upmixed L=R (mirrors
|
// (see RemoteStream::ring in audio_engine.h for what went wrong when it was). The ring
|
||||||
// the capture-side upmix in vc_client::on_capture_frame).
|
// decouples the two: top it up by decoding whole Opus frames, then drain exactly
|
||||||
int dec_channels = std::max(1, stream.decoder.channels());
|
// `frames` samples-per-channel from it below (silence-padding on underrun = PLC).
|
||||||
|
const int dec_channels = std::max(1, stream.decoder.channels());
|
||||||
|
const int frame_samples = stream.decoder.frame_samples();
|
||||||
|
|
||||||
|
while (stream.ring_count < frames && frame_samples > 0) {
|
||||||
auto maybe_frame = stream.jitter.pop(stream.playout_ts);
|
auto maybe_frame = stream.jitter.pop(stream.playout_ts);
|
||||||
std::vector<int16_t> pcm(frames * static_cast<ma_uint32>(dec_channels));
|
|
||||||
int n;
|
int n;
|
||||||
|
|
||||||
if (maybe_frame) {
|
if (maybe_frame) {
|
||||||
n = stream.decoder.decode(
|
n = stream.decoder.decode(
|
||||||
maybe_frame->payload.data(),
|
maybe_frame->payload.data(),
|
||||||
static_cast<int>(maybe_frame->payload.size()),
|
static_cast<int>(maybe_frame->payload.size()),
|
||||||
pcm.data(), static_cast<int>(frames));
|
stream.decode_scratch.data(), frame_samples);
|
||||||
} else {
|
} else {
|
||||||
n = stream.decoder.decode(nullptr, 0, pcm.data(), static_cast<int>(frames));
|
n = stream.decoder.decode(nullptr, 0, stream.decode_scratch.data(), frame_samples);
|
||||||
}
|
}
|
||||||
|
if (n <= 0) break; // decoder error/exhausted PLC; rest of this period stays silent
|
||||||
|
|
||||||
if (n > 0) {
|
|
||||||
// `n` is samples-per-channel (matches the frame_samples convention used by
|
// `n` is samples-per-channel (matches the frame_samples convention used by
|
||||||
// OpusEncoder::encode elsewhere in the codebase).
|
// OpusEncoder::encode elsewhere in the codebase).
|
||||||
if (stream.recv_ns)
|
if (stream.recv_ns)
|
||||||
stream.recv_ns->process_capture(pcm.data(), n,
|
stream.recv_ns->process_capture(stream.decode_scratch.data(), n,
|
||||||
static_cast<int>(params_.sample_rate));
|
static_cast<int>(params_.sample_rate));
|
||||||
|
|
||||||
float g = stream.gain;
|
stream.push_ring(stream.decode_scratch.data(), static_cast<size_t>(n));
|
||||||
for (int i = 0; i < n; ++i) {
|
stream.playout_ts += static_cast<uint32_t>(n);
|
||||||
|
}
|
||||||
|
|
||||||
|
const float g = stream.gain;
|
||||||
|
int16_t frame_buf[2];
|
||||||
|
for (ma_uint32 i = 0; i < frames; ++i) {
|
||||||
|
stream.pop_ring(frame_buf, 2); // zero-filled if the ring underran (PLC silence)
|
||||||
if (dec_channels == 2) {
|
if (dec_channels == 2) {
|
||||||
int32_t l = static_cast<int32_t>(static_cast<float>(pcm[i * 2]) * g);
|
int32_t l = static_cast<int32_t>(static_cast<float>(frame_buf[0]) * g);
|
||||||
int32_t r = static_cast<int32_t>(static_cast<float>(pcm[i * 2 + 1]) * g);
|
int32_t r = static_cast<int32_t>(static_cast<float>(frame_buf[1]) * g);
|
||||||
if (pb_channels == 2) {
|
if (pb_channels == 2) {
|
||||||
mix[i * 2] += l;
|
mix[i * 2] += l;
|
||||||
mix[i * 2 + 1] += r;
|
mix[i * 2 + 1] += r;
|
||||||
@@ -405,14 +421,12 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) {
|
|||||||
mix[i] += (l + r) / 2; // playback device fell back to mono
|
mix[i] += (l + r) / 2; // playback device fell back to mono
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
int32_t sample = static_cast<int32_t>(static_cast<float>(pcm[i]) * g);
|
int32_t sample = static_cast<int32_t>(static_cast<float>(frame_buf[0]) * g);
|
||||||
for (uint32_t c = 0; c < pb_channels; ++c)
|
for (uint32_t c = 0; c < pb_channels; ++c)
|
||||||
mix[i * pb_channels + c] += sample; // upmix mono -> all playback channels
|
mix[i * pb_channels + c] += sample; // upmix mono -> all playback channels
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
stream.playout_ts += frames;
|
|
||||||
}
|
|
||||||
|
|
||||||
for (ma_uint32 i = 0; i < frames * pb_channels; ++i)
|
for (ma_uint32 i = 0; i < frames * pb_channels; ++i)
|
||||||
out[i] = static_cast<int16_t>(std::clamp(mix[i], -32768, 32767));
|
out[i] = static_cast<int16_t>(std::clamp(mix[i], -32768, 32767));
|
||||||
|
|||||||
@@ -12,6 +12,7 @@
|
|||||||
#ifndef VOICECAT_AUDIO_AUDIO_ENGINE_H
|
#ifndef VOICECAT_AUDIO_AUDIO_ENGINE_H
|
||||||
#define VOICECAT_AUDIO_AUDIO_ENGINE_H
|
#define VOICECAT_AUDIO_AUDIO_ENGINE_H
|
||||||
|
|
||||||
|
#include <algorithm>
|
||||||
#include <atomic>
|
#include <atomic>
|
||||||
#include <cstdint>
|
#include <cstdint>
|
||||||
#include <functional>
|
#include <functional>
|
||||||
@@ -230,6 +231,59 @@ class AudioEngine {
|
|||||||
// (already off the real-time audio thread), polled by poll_talk_transitions().
|
// (already off the real-time audio thread), polled by poll_talk_transitions().
|
||||||
std::atomic<int64_t> last_voice_ms{0};
|
std::atomic<int64_t> last_voice_ms{0};
|
||||||
bool talking = false;
|
bool talking = false;
|
||||||
|
|
||||||
|
// ── Decode/playback decoupling ring ─────────────────────────────────────
|
||||||
|
// opus_decode() must be called with max_samples == the encoder's fixed frame size
|
||||||
|
// (decoder.frame_samples(), e.g. 960 @ 20ms/48kHz) — that's a property of the bitstream,
|
||||||
|
// not a choice. miniaudio's playback callback period is a *separate*, independently
|
||||||
|
// chosen value (often smaller, e.g. ~480 @ low-latency WASAPI defaults) and must never
|
||||||
|
// be passed to opus_decode as max_samples (doing so made decode fail basically every
|
||||||
|
// callback — silent playback bug, fixed by this ring). on_playback() tops this ring up
|
||||||
|
// by decoding whole Opus frames (decoder's channel count) and drains exactly the
|
||||||
|
// hardware-requested sample count from it each callback, padding with silence (PLC) on
|
||||||
|
// underrun. Sized once in init_ring() (called off the audio thread); never resized from
|
||||||
|
// on_playback (real-time rule).
|
||||||
|
std::vector<int16_t> ring; // capacity = (frame_samples * 8) frames * ring_channels
|
||||||
|
size_t ring_channels = 1;
|
||||||
|
size_t ring_head = 0; // next frame (sample-per-channel) to read
|
||||||
|
size_t ring_count = 0; // buffered frames (samples-per-channel) ready
|
||||||
|
std::vector<int16_t> decode_scratch; // pre-sized: frame_samples * ring_channels
|
||||||
|
|
||||||
|
void init_ring(int channels, int frame_samples) {
|
||||||
|
ring_channels = static_cast<size_t>(std::max(1, channels));
|
||||||
|
size_t cap_frames = static_cast<size_t>(std::max(1, frame_samples)) * 8; // ~160ms @20ms frames
|
||||||
|
ring.assign(cap_frames * ring_channels, 0);
|
||||||
|
ring_head = 0;
|
||||||
|
ring_count = 0;
|
||||||
|
decode_scratch.assign(static_cast<size_t>(std::max(1, frame_samples)) * ring_channels, 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Appends `n_frames` samples-per-channel (ring_channels each) from `pcm`. Drops the
|
||||||
|
// tail (rather than overwriting unread data) if the ring is unexpectedly full — should
|
||||||
|
// not happen with the generous 8x sizing above.
|
||||||
|
void push_ring(const int16_t* pcm, size_t n_frames) {
|
||||||
|
if (ring.empty() || ring_channels == 0) return;
|
||||||
|
size_t cap_frames = ring.size() / ring_channels;
|
||||||
|
for (size_t i = 0; i < n_frames; ++i) {
|
||||||
|
if (ring_count >= cap_frames) return;
|
||||||
|
size_t widx = (ring_head + ring_count) % cap_frames;
|
||||||
|
for (size_t c = 0; c < ring_channels; ++c)
|
||||||
|
ring[widx * ring_channels + c] = pcm[i * ring_channels + c];
|
||||||
|
++ring_count;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Pops one frame (sample-per-channel) into `out` (sized `out_channels`, zero-filled
|
||||||
|
// first — covers both a fully-drained ring and ring_channels < out_channels).
|
||||||
|
void pop_ring(int16_t* out, size_t out_channels) {
|
||||||
|
for (size_t c = 0; c < out_channels; ++c) out[c] = 0;
|
||||||
|
if (ring_count == 0 || ring.empty() || ring_channels == 0) return;
|
||||||
|
size_t cap_frames = ring.size() / ring_channels;
|
||||||
|
for (size_t c = 0; c < ring_channels && c < out_channels; ++c)
|
||||||
|
out[c] = ring[ring_head * ring_channels + c];
|
||||||
|
ring_head = (ring_head + 1) % cap_frames;
|
||||||
|
--ring_count;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
mutable std::mutex streams_mu_;
|
mutable std::mutex streams_mu_;
|
||||||
std::unordered_map<uint32_t, RemoteStream> streams_;
|
std::unordered_map<uint32_t, RemoteStream> streams_;
|
||||||
|
|||||||
Reference in New Issue
Block a user