fix(audio): buffer capture frames to Opus encoder's fixed frame size
AudioEngine::on_capture() was passing miniaudio's hardware callback period (commonly 480 samples / 10 ms on WASAPI shared mode) directly to opus_encode(), which requires exactly frame_samples_ (960 for 20 ms @ 48 kHz). The mismatch returned OPUS_BAD_ARG and silently dropped every real mic frame, while screen share and injected test frames happened to be correctly sized and worked fine. Fix: accumulate PCM in a pre-allocated CaptureAccum buffer (mirroring the existing RemoteStream::ring fix on the playback side) and only call capture_cb_ when a full frame_samples_ chunk is ready. Same pattern applied to on_loopback(). Add test_capture_frame_accumulation() to verify the accumulator fires exactly the right number of callbacks for misaligned chunk sizes (480, 240+720, 1920 samples). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -123,6 +123,16 @@ bool AudioEngine::start(const AudioParams& p, CaptureCallback capture_cb) {
|
||||
frame_samples_ = static_cast<int>(p.sample_rate / 1000 * p.frame_ms);
|
||||
running_.store(true, std::memory_order_release);
|
||||
|
||||
#ifdef VOICECAT_HAS_AUDIO
|
||||
// Pre-allocate capture accumulators before the devices start so on_capture / on_loopback
|
||||
// never allocate on the RT thread. count=0 means "empty"; the buf is sized to exactly one
|
||||
// encoder frame so a memcpy into it can never overrun.
|
||||
capture_accum_.buf.assign(static_cast<size_t>(frame_samples_), 0);
|
||||
capture_accum_.count = 0;
|
||||
loopback_accum_.buf.assign(static_cast<size_t>(frame_samples_), 0);
|
||||
loopback_accum_.count = 0;
|
||||
#endif
|
||||
|
||||
#ifdef VOICECAT_HAS_AUDIO
|
||||
// ── Capture device ──────────────────────────────────────────────────────
|
||||
ma_device_id cap_id{};
|
||||
@@ -136,6 +146,10 @@ bool AudioEngine::start(const AudioParams& p, CaptureCallback capture_cb) {
|
||||
cap_cfg.dataCallback = capture_data_cb;
|
||||
cap_cfg.pUserData = this;
|
||||
cap_cfg.capture.pDeviceID = have_cap_id ? &cap_id : nullptr; // null = default device
|
||||
// Hint: request the encoder's frame size as the callback period. WASAPI shared mode may
|
||||
// not honor this (the hardware period is fixed), but when it is honored the accumulator
|
||||
// below becomes a zero-copy passthrough rather than a copy every two callbacks.
|
||||
cap_cfg.periodSizeInFrames = static_cast<ma_uint32>(frame_samples_);
|
||||
|
||||
if (ma_device_init(nullptr, &cap_cfg, &capture_device_) == MA_SUCCESS) {
|
||||
if (ma_device_start(&capture_device_) == MA_SUCCESS) {
|
||||
@@ -347,10 +361,26 @@ void AudioEngine::capture_data_cb(ma_device* dev, void* /*out*/,
|
||||
}
|
||||
|
||||
void AudioEngine::on_capture(const int16_t* pcm, ma_uint32 frames) {
|
||||
// The real hardware capture device is always the "primary" tap (kind 0 / MIC). A second
|
||||
// concurrent local stream (e.g. SCREEN_AUDIO) is fed via inject_capture() in M3 — there is
|
||||
// only one real capture device.
|
||||
if (capture_cb_) capture_cb_(0, pcm, static_cast<int>(frames));
|
||||
// Accumulate samples until we have exactly frame_samples_ (e.g. 960 for 20 ms @ 48 kHz),
|
||||
// then fire capture_cb_. WASAPI shared mode commonly delivers 480-sample (10 ms) callbacks
|
||||
// regardless of the periodSizeInFrames hint above; passing a sub-frame chunk directly to
|
||||
// opus_encode() returns OPUS_BAD_ARG (negative), silently dropping every mic frame.
|
||||
if (!capture_cb_ || frame_samples_ <= 0) return;
|
||||
const int16_t* src = pcm;
|
||||
auto remaining = static_cast<int>(frames);
|
||||
while (remaining > 0) {
|
||||
int space = frame_samples_ - capture_accum_.count;
|
||||
int copy = std::min(remaining, space);
|
||||
std::memcpy(capture_accum_.buf.data() + capture_accum_.count, src,
|
||||
static_cast<size_t>(copy) * sizeof(int16_t));
|
||||
capture_accum_.count += copy;
|
||||
src += copy;
|
||||
remaining -= copy;
|
||||
if (capture_accum_.count == frame_samples_) {
|
||||
capture_cb_(0, capture_accum_.buf.data(), frame_samples_);
|
||||
capture_accum_.count = 0;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void AudioEngine::playback_data_cb(ma_device* dev, void* out,
|
||||
@@ -444,13 +474,29 @@ void AudioEngine::loopback_data_cb(ma_device* dev, void* /*out*/, const void* in
|
||||
}
|
||||
|
||||
void AudioEngine::on_loopback(const int16_t* pcm, ma_uint32 frames) {
|
||||
// Same pattern as the real mic capture callback (on_capture) — call capture_cb_ directly,
|
||||
// NOT through inject_capture()'s test-only ring (see audio_engine.h).
|
||||
if (capture_cb_) capture_cb_(loopback_kind_, pcm, static_cast<int>(frames));
|
||||
// Same accumulation as on_capture — the WASAPI loopback render endpoint's callback
|
||||
// period is also hardware-driven and may not match frame_samples_.
|
||||
if (!capture_cb_ || frame_samples_ <= 0) return;
|
||||
const int16_t* src = pcm;
|
||||
auto remaining = static_cast<int>(frames);
|
||||
while (remaining > 0) {
|
||||
int space = frame_samples_ - loopback_accum_.count;
|
||||
int copy = std::min(remaining, space);
|
||||
std::memcpy(loopback_accum_.buf.data() + loopback_accum_.count, src,
|
||||
static_cast<size_t>(copy) * sizeof(int16_t));
|
||||
loopback_accum_.count += copy;
|
||||
src += copy;
|
||||
remaining -= copy;
|
||||
if (loopback_accum_.count == frame_samples_) {
|
||||
capture_cb_(loopback_kind_, loopback_accum_.buf.data(), frame_samples_);
|
||||
loopback_accum_.count = 0;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
bool AudioEngine::start_loopback_capture(int kind) {
|
||||
if (loopback_started_) return false; // already running; stop_loopback_capture() first
|
||||
loopback_accum_.count = 0; // discard any partial frame from a previous loopback session
|
||||
|
||||
ma_device_config cfg = ma_device_config_init(ma_device_type_loopback);
|
||||
cfg.capture.format = ma_format_s16;
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
|
||||
#include <algorithm>
|
||||
#include <atomic>
|
||||
#include <cstring>
|
||||
#include <cstdint>
|
||||
#include <functional>
|
||||
#include <map>
|
||||
@@ -168,6 +169,30 @@ class AudioEngine {
|
||||
// stereo mixing end-to-end (no audio hardware needed). Same logic the real playback
|
||||
// callback uses; safe to call any time after start() (no ma_device touched).
|
||||
void mix_for_test(int16_t* out, uint32_t frames) { on_playback(out, frames); }
|
||||
|
||||
// TEST-ONLY — drives the capture-side frame accumulator directly with an explicit
|
||||
// callback, bypassing capture_cb_. Call after engine.start(p) WITHOUT a capture
|
||||
// callback so the real mic (if any) never touches capture_accum_ (on_capture returns
|
||||
// early when capture_cb_ is null). The explicit `cb` is invoked only when a full
|
||||
// frame_samples_ chunk is ready — that is the invariant under test.
|
||||
void feed_capture_for_test(const int16_t* pcm, int frames, const CaptureCallback& cb) {
|
||||
if (frame_samples_ <= 0 || !cb || capture_accum_.buf.empty()) return;
|
||||
const int16_t* src = pcm;
|
||||
auto remaining = frames;
|
||||
while (remaining > 0) {
|
||||
int space = frame_samples_ - capture_accum_.count;
|
||||
int copy = std::min(remaining, space);
|
||||
std::memcpy(capture_accum_.buf.data() + capture_accum_.count, src,
|
||||
static_cast<size_t>(copy) * sizeof(int16_t));
|
||||
capture_accum_.count += copy;
|
||||
src += copy;
|
||||
remaining -= copy;
|
||||
if (capture_accum_.count == frame_samples_) {
|
||||
cb(0, capture_accum_.buf.data(), frame_samples_);
|
||||
capture_accum_.count = 0;
|
||||
}
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
private:
|
||||
@@ -199,6 +224,20 @@ class AudioEngine {
|
||||
CaptureCallback capture_cb_;
|
||||
std::atomic<bool> running_{false};
|
||||
|
||||
// Capture-side frame accumulators: miniaudio fires the capture (and loopback) callback at
|
||||
// whatever period the hardware/driver chooses — commonly 480 samples (10 ms) on WASAPI
|
||||
// shared mode, while the Opus encoder requires exactly frame_samples_ per call (960 for
|
||||
// 20 ms @ 48 kHz). Accumulate incoming PCM until a full frame is ready, then call
|
||||
// capture_cb_. This mirrors the RemoteStream::ring fix on the playback side. Both
|
||||
// accumulators are pre-allocated once in start(); never resized from the RT callback
|
||||
// thread (satisfies architecture.md §3 — no allocation on RT threads).
|
||||
struct CaptureAccum {
|
||||
std::vector<int16_t> buf; // pre-sized to frame_samples_ in start()
|
||||
int count = 0;
|
||||
};
|
||||
CaptureAccum capture_accum_; // mic / real capture device (on_capture)
|
||||
CaptureAccum loopback_accum_; // screen-audio WASAPI loopback (on_loopback)
|
||||
|
||||
// Inject ring(s): stores raw int16 PCM written by inject_capture(), one ring per local
|
||||
// stream kind so e.g. MIC and SCREEN_AUDIO can each be fed independently in tests.
|
||||
// The encode thread reads from these (no real capture device needed in tests).
|
||||
|
||||
@@ -231,6 +231,60 @@ static void test_stereo_mix() {
|
||||
}
|
||||
#endif // VOICECAT_HAS_AUDIO && VOICECAT_HAS_OPUS
|
||||
|
||||
// ── 5. Capture-frame accumulation (white-box, no audio hardware needed) ──────────
|
||||
// Regression for the capture-side analogue of the playback ring fix: miniaudio's capture
|
||||
// callback fires at the hardware period (commonly 480 samples on WASAPI shared mode), while
|
||||
// opus_encode() requires exactly frame_samples_ (960). Sub-frame chunks must be accumulated;
|
||||
// the callback must receive exactly 960-sample frames regardless of input chunk size.
|
||||
#ifdef VOICECAT_HAS_AUDIO
|
||||
static void test_capture_frame_accumulation() {
|
||||
voicecat::audio::AudioEngine engine;
|
||||
voicecat::audio::AudioParams p;
|
||||
p.sample_rate = 48000;
|
||||
p.capture_channels = 1;
|
||||
p.frame_ms = 20; // frame_samples_ = 960
|
||||
|
||||
std::atomic<int> call_count{0};
|
||||
std::atomic<bool> wrong_size{false};
|
||||
constexpr int kExpected = 960;
|
||||
|
||||
// Start WITHOUT a capture callback: the real mic (if any) fires on_capture(), but
|
||||
// on_capture() returns immediately when capture_cb_ is null, so capture_accum_ is
|
||||
// never touched by the hardware thread. feed_capture_for_test() bypasses capture_cb_
|
||||
// and drives the same accumulator directly with the explicit `cb` below — no races.
|
||||
CHECK(engine.start(p));
|
||||
|
||||
auto cb = [&](int /*kind*/, const int16_t* /*pcm*/, int samples) {
|
||||
++call_count;
|
||||
if (samples != kExpected) wrong_size.store(true);
|
||||
};
|
||||
|
||||
// 480-sample (10 ms) input — WASAPI's common hardware period on modern Windows.
|
||||
// Two 480-chunk inputs → exactly one callback at 960.
|
||||
std::vector<int16_t> h(480, 1000);
|
||||
engine.feed_capture_for_test(h.data(), 480, cb);
|
||||
CHECK(call_count.load() == 0); // half a frame — no callback yet
|
||||
engine.feed_capture_for_test(h.data(), 480, cb);
|
||||
CHECK(call_count.load() == 1); // one full frame — callback fired once
|
||||
|
||||
// Mis-aligned split: 240 then 720 → still exactly one callback.
|
||||
std::vector<int16_t> s(240, 500), l(720, 500);
|
||||
engine.feed_capture_for_test(s.data(), 240, cb);
|
||||
CHECK(call_count.load() == 1);
|
||||
engine.feed_capture_for_test(l.data(), 720, cb);
|
||||
CHECK(call_count.load() == 2);
|
||||
|
||||
// 1920-sample input (two Opus frames) → exactly two callbacks.
|
||||
std::vector<int16_t> d(1920, 800);
|
||||
engine.feed_capture_for_test(d.data(), 1920, cb);
|
||||
CHECK(call_count.load() == 4);
|
||||
|
||||
CHECK(!wrong_size.load());
|
||||
engine.stop();
|
||||
std::printf("test_capture_frame_accumulation: ok (callbacks=%d)\n", call_count.load());
|
||||
}
|
||||
#endif // VOICECAT_HAS_AUDIO
|
||||
|
||||
// ── 2/3. VAD + PTT gate, through the real ABI against a real server ─────────────
|
||||
static void test_vad_and_ptt_gate() {
|
||||
auto tmp = std::filesystem::temp_directory_path() /
|
||||
@@ -385,6 +439,9 @@ int main() {
|
||||
test_device_enumeration();
|
||||
#if defined(VOICECAT_HAS_AUDIO) && defined(VOICECAT_HAS_OPUS)
|
||||
test_stereo_mix();
|
||||
#endif
|
||||
#ifdef VOICECAT_HAS_AUDIO
|
||||
test_capture_frame_accumulation();
|
||||
#endif
|
||||
test_vad_and_ptt_gate();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user