Cleanup audio engine comments

core/src/audio/apm_processor.cpp

@@ -17,10 +17,7 @@ int64_t steady_now_ms() {
 }
 }  // namespace
 
-// ── EnergyVadProcessor ──────────────────────────────────────────────────────
-// Lightweight, dependency-free energy/RMS VAD — see apm_processor.h's create_vad() doc comment
-// for why this exists instead of a real APM. No AEC (process_render is a no-op); doesn't modify
-// the PCM it's given, only inspects it.
+
 class EnergyVadProcessor final : public ApmProcessor {
    public:
     EnergyVadProcessor(float rms_threshold, int64_t hang_time_ms)
@@ -51,8 +48,8 @@ class EnergyVadProcessor final : public ApmProcessor {
     int64_t last_voice_ms_ = 0;  // epoch start -> gate begins closed until first loud frame
 };
 
-// ── RnnoiseProcessor ─────────────────────────────────────────────────────────
-// Real noise suppression via vendored RNNoise (third_party/rnnoise; docs/voice.md §10-11).
+// RnnoiseProcessor
+// Real noise suppression via vendored RNNoise (third_party/rnnoise
 // RNNoise is a mono, 48 kHz, fixed 480-sample (10 ms) speech denoiser; our engine clock is fixed
 // at 48 kHz and every Opus frame size (480/960/1920/2880) is a multiple of 480, so we process
 // whole 480-sample chunks with no resampling and no cross-call carry. Mono only — callers gate
@@ -72,7 +69,7 @@ class RnnoiseProcessor final : public ApmProcessor {
 
     bool process_capture(int16_t* pcm, int samples, int sample_rate) override {
         // RNNoise is 48 kHz only; anything else passes through untouched (our clock is 48 kHz, so
-        // this guard never trips in practice — it's just a correctness backstop).
+        // this guard never trips in practice. it's just a correctness backstop).
         if (!st_ || sample_rate != 48000) return true;
         for (int off = 0; off + kFrame <= samples; off += kFrame) {
             for (int i = 0; i < kFrame; ++i) in_[i] = static_cast<float>(pcm[off + i]);
@@ -82,7 +79,7 @@ class RnnoiseProcessor final : public ApmProcessor {
                 pcm[off + i] = static_cast<int16_t>(std::clamp(v, -32768, 32767));
             }
         }
-        return true;  // NS doesn't gate; the send path's VAD stays a separate stage
+        return true;  // NS doesn't gate. the send path's VAD stays a separate stage
     }
 
    private:

core/src/audio/apm_processor.h

@@ -1,8 +1,6 @@
 /*
- * audio/apm_processor.h — Send-side audio processing module (AEC/NS/AGC/VAD).
- *
- * Design: docs/voice.md §11. Uses webrtc-audio-processing when VOICECAT_HAS_APM is defined;
- * falls back to a no-op passthrough (VAD always open, PCM unmodified) otherwise.
+ * audio/apm_processor.h, Send-side audio processing module (AEC/NS/AGC/VAD).
+
  */
 #ifndef VOICECAT_AUDIO_APM_PROCESSOR_H
 #define VOICECAT_AUDIO_APM_PROCESSOR_H
@@ -21,28 +19,17 @@ class ApmProcessor {
 
     // Process one capture frame in-place (AEC, NS, AGC).
     // Returns true if VAD detects speech (or always true in passthrough mode).
-    // Returns false → caller should skip encode/send (silence gate).
+    // Returns false  caller should skip encode/send (silence gate).
     virtual bool process_capture(int16_t* pcm, int samples, int sample_rate) = 0;
 
     // Update the VAD RMS threshold in-place (used by EnergyVadProcessor; no-op in passthrough).
     // Safe to call from any thread — EnergyVadProcessor stores it atomically.
     virtual void set_threshold(float) {}
 
-    // Factory for the noise-suppression backend (docs/voice.md §10-11): a real RNNoise denoiser
-    // when VOICECAT_HAS_NS is defined (third_party/rnnoise), else a no-op passthrough. Used for
-    // BOTH recv-side per-stream NR (RemoteStream::recv_ns) and send-side mic NR (vc_client's
-    // mic_ns_). The RNNoise backend is mono/48 kHz only, so callers gate it on a single channel.
-    // NS never gates (process_capture always returns true) — the send-side VAD is separate, see
-    // create_vad() below.
+    // Factory for the noise-suppression backend 
     static std::unique_ptr<ApmProcessor> create();
 
-    // Factory for the send-side input gate (docs/voice.md §11): a lightweight, dependency-free
-    // energy/RMS VAD with configurable threshold + hang-time. webrtc-audio-processing (the
-    // originally-planned APM) has no working Windows/MSVC build upstream (GCC-only Meson build,
-    // unfinished MinGW support, hard abseil-cpp dependency — see PROGRESS.md), so this is the
-    // real v1 implementation behind the same ApmProcessor interface, not a passthrough. No AEC
-    // — process_render() is a no-op here; that's a real limitation versus the originally-planned
-    // APM, not just a deferred VAD.
+    // Factory for the send-side input gate 
     //   rms_threshold: normalized 0.0-1.0 RMS-of-int16-range; default ~0.025.
     //   hang_time_ms:  how long the gate stays open after the last loud frame; default 300 ms
     //                  (matches AudioEngine's kTalkHangoverMs so "talking" and "gate open" agree).

core/src/audio/audio_engine.cpp

@@ -16,27 +16,10 @@ int64_t now_ms() {
         .count();
 }
 
-// Bounded-depth playout targeting (samples @ 48 kHz). The playout clock advances every callback
-// via the PLC path in on_playback, while the sender's frame timestamps only advance while it is
-// actually transmitting (silence is omitted) — so the clock drifts away from the stream timeline
-// across VAD/PTT/DTX gaps and late joins. Rather than snap the clock to the *oldest* buffered
-// frame (which could only add standing latency and never trim it — the cause of the "latency keeps
-// drifting backward, fixed by rejoin" bug), on_playback keeps the clock a bounded `target` behind
-// the *newest* arrival and frame-skips to catch up when the backlog grows. These are the floor and
-// hysteresis for that target; the steady-state target itself comes from JitterBuffer's adaptive
-// estimate (target_depth_samples()).
 constexpr int32_t kMinDepthSamples  = 48000 * 40 / 1000;   // floor for the catch-up target depth
 constexpr int32_t kCatchupSamples   = 48000 * 60 / 1000;   // skip when depth > target + 60 ms
 constexpr int32_t kStarveSamples    = 48000 * 120 / 1000;  // reseed when clock 120 ms past newest
 
-// PLC cap: after this many consecutive samples of pure packet-loss concealment (no real
-// packet decoded), stop calling opus_decode(nullptr,0,...) and emit silence instead. Opus
-// PLC synthesizes soft comfort noise that never "exhausts" (opus_decode always returns
-// frame_samples > 0 for PLC), so without a cap a stale stream left in the mixer after its
-// source disconnects would hiss forever. 2 s bounds the hiss to a brief gap while still
-// bridging normal network jitter/PTT silences. The primary fix for stale streams is the
-// server's UserEvent::LEFT broadcast (which triggers remove_stream); this is
-// defense-in-depth against any future regression that skips remove_stream.
 constexpr int32_t kPlcCapSamples = 48000 * 2;  // 2 s @ 48 kHz
 
 // device_id encoding (DeviceInfo::id / AudioParams::*_device_id): a hex string of the raw
@@ -75,7 +58,7 @@ bool hex_decode_device_id(const std::string& hex, ma_device_id* out) {
 }
 }  // namespace
 
-// ── JitterBuffer ─────────────────────────────────────────────────────────────
+// JitterBuffer
 
 void JitterBuffer::push(Frame f) {
     std::lock_guard lk(mu_);
@@ -115,7 +98,7 @@ void JitterBuffer::push(Frame f) {
 
 std::optional<JitterBuffer::Frame> JitterBuffer::pop(uint32_t playout_ts) {
     std::unique_lock lk(mu_, std::try_to_lock);
-    if (!lk) return std::nullopt;  // contended — caller does PLC
+    if (!lk) return std::nullopt;  // contended. caller does PLC
 
     if (buf_.empty()) return std::nullopt;
 
@@ -187,13 +170,13 @@ void JitterBuffer::reset() {
     jitter_est_  = 0;
 }
 
-// ── AudioEngine ──────────────────────────────────────────────────────────────
+//  AudioEngine
 
 AudioEngine::AudioEngine() = default;
 
 AudioEngine::~AudioEngine() {
     stop();
-    stop_loopback_capture();  // loopback has an independent lifecycle — close it before the context
+    stop_loopback_capture();  // loopback has an independent lifecycle, close it before the context
     if (context_inited_) {
         ma_context_uninit(&context_);
         context_inited_ = false;
@@ -203,8 +186,6 @@ AudioEngine::~AudioEngine() {
 
 ma_context_config AudioEngine::make_context_config() {
     ma_context_config cfg = ma_context_config_init();
-    // iOS: leave AVAudioSession entirely to the Swift layer (IOSAudioRouter). See the
-    // make_context_config() declaration in audio_engine.h for the full rationale.
     cfg.coreaudio.sessionCategory          = ma_ios_session_category_none;  // don't call setCategory
     cfg.coreaudio.noAudioSessionActivate   = MA_TRUE;  // don't setActive(true) on device init
     cfg.coreaudio.noAudioSessionDeactivate = MA_TRUE;  // don't setActive(false) on device uninit
@@ -219,13 +200,13 @@ bool AudioEngine::start(const AudioParams& p, CaptureCallback capture_cb) {
     running_.store(true, std::memory_order_release);
     if (!dred_dec_) {
         int err = 0;
-        dred_dec_ = opus_dred_decoder_create(&err);  // null on failure — DRED silently disabled
+        dred_dec_ = opus_dred_decoder_create(&err);  // null on failure, DRED silently disabled
     }
 
     // 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. The mic accumulator is sized to
-    // frame_samples_ * capture_channels (1 = mono, 2 = stereo interleaved — set via
+    // frame_samples_ * capture_channels (1 = mono, 2 = stereo interleaved, set via
     // vc_set_capture_channels, e.g. iOS stereo built-in mic). The loopback accumulator is
     // sized mono here as a safe default and re-sized to frame_samples_*channels in
     // start_loopback_capture() once the screen stream's channel mode is known (off the RT
@@ -292,7 +273,7 @@ bool AudioEngine::start(const AudioParams& p, CaptureCallback capture_cb) {
     // External capture (iOS VPIO / external feed): skip the hardware mic device — PCM is fed
     // via inject_capture / vc_stream_feed_pcm. capture_cb_ (set above) still fires for fed frames.
     if (!params_.external_capture) {
-    // ── Capture device (opened after playback so the output route is already committed) ──
+    // Capture device (opened after playback so the output route is already committed)
     ma_device_id cap_id{};
     bool have_cap_id = !p.capture_device_id.empty() &&
                        hex_decode_device_id(p.capture_device_id, &cap_id);
@@ -648,7 +629,7 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) {
         if (stream.mute || !stream.decoder.valid()) continue;
 
         // 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 
         // dec_channels/frame_samples are bitstream properties (fixed at decoder init); `frames`
         // below is the *hardware* playback callback's period, an independent value miniaudio
         // picks on its own — opus_decode's max_samples must be frame_samples, never `frames`
@@ -658,19 +639,6 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) {
         const int dec_channels   = std::max(1, stream.decoder.channels());
         const int frame_samples  = stream.decoder.frame_samples();
 
-        // Bound the playout latency against the stream's leading edge (newest buffered frame).
-        // playout_ts free-runs every callback (PLC path below) while the sender omits silence from
-        // its timestamps, so the two diverge across late joins and VAD/PTT/DTX gaps. Two cases,
-        // both measured against the *newest* frame — never the oldest, whose snap could only ever
-        // *add* standing latency (the ratchet that caused the growing-latency bug):
-        //   • (re)seed to the leading edge when starting, on a talkspurt marker, or when the clock
-        //     has run past the newest frame (starved after silence). depth becomes ~0 — no
-        //     artificial prebuffer, so latency stays as low as the old path; buffered frames still
-        //     play in order (pop() returns oldest-first), they just stop being held back.
-        //   • frame-skip catch-up: if the backlog has grown beyond target+hysteresis (clock drift,
-        //     bursty arrival, reordering), fast-forward to leave exactly `target` buffered and drop
-        //     the now-stale frames. This is the missing downward force that bounds latency.
-        // u32 subtraction via int32_t handles timestamp wrap.
         const int32_t target =
             std::max<int32_t>(static_cast<int32_t>(stream.jitter.target_depth_samples()),
                               kMinDepthSamples);
@@ -750,11 +718,7 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) {
 
             // `n` is samples-per-channel (matches the frame_samples convention used by
             // OpusEncoder::encode elsewhere in the codebase).
-            // Receive-side NR runs only on VOICE (MIC) streams — a screen-audio share is music/
-            // video, never voice, so it's left untouched (gating on dec_channels would silently
-            // skip a now-stereo mic, see docs/voice.md §10). RNNoise is mono-only, so a stereo mic
-            // is folded to mono in place (symmetric with the send-side downmix), denoised, then
-            // duplicated back across both channels — no allocation on this RT path.
+            // Receive-side NR runs only on VOICE (MIC) streams — 
             if (stream.recv_ns && stream.is_voice) {
                 int16_t* s = stream.decode_scratch.data();
                 if (dec_channels == 2) {
@@ -788,7 +752,7 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) {
         }
 
         // Diagnostic: the decode loop couldn't keep the ring fed for this hardware period while
-        // the stream was actively playing out — a genuine underrun (decoder error / exhausted
+        // the stream was actively playing out a genuine underrun (decoder error / exhausted
         // PLC), distinct from ordinary single-packet loss the loop conceals in place.
         if (stream.playout_started && stream.ring_count < frames)
             stream.underruns.fetch_add(1, std::memory_order_relaxed);
@@ -855,12 +819,8 @@ 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 accumulation as on_capture — the WASAPI loopback render endpoint's callback
-    // period is also hardware-driven and may not match frame_samples_. PCM here is
-    // interleaved across loopback_channels_ (1 = mono downmix, 2 = stereo L/R) — the
-    // accumulator was sized to frame_samples_*loopback_channels_ in start_loopback_capture,
-    // so a memcpy into it can never overrun. capture_cb_ receives samples-per-channel
-    // (frame_samples_) and the channel count explicitly.
+    // Same accumulation as on_capture
+
     if (!capture_cb_ || frame_samples_ <= 0) return;
     const int ch = std::max(1, loopback_channels_);
     const int16_t* src = pcm;
@@ -892,17 +852,8 @@ bool AudioEngine::start_loopback_capture(int kind, int channels) {
     cfg.sampleRate        = params_.sample_rate;
     cfg.dataCallback      = loopback_data_cb;
     cfg.pUserData         = this;
-    // pDeviceID left null: captures the default render endpoint (docs/voice.md §9). Windows
-    // 10 2004+ supports process-specific loopback via AUDIOCLIENT_ACTIVATION_PARAMS, but
-    // miniaudio's loopback mode only exposes whole-device capture — a future enhancement, not
-    // this pass. Whole-device loopback inherently captures this app's own incoming voice mix
-    // along with everything else playing — an accepted self-echo-loop characteristic of
-    // desktop-audio capture, not a bug.
+    // pDeviceID left null: captures the default render endpoint 
 
-    // Use the engine's owned context (shared with playback/capture) so loopback honors the
-    // same no-AVAudioSession-management config. Loopback is Windows/WASAPI-only today, where
-    // the coreaudio fields are irrelevant, but keep it consistent. start() inits the context
-    // before any loopback can be requested; fall back to NULL if it somehow isn't.
     ma_context* ctx = context_inited_ ? &context_ : nullptr;
     if (ma_device_init(ctx, &cfg, &loopback_device_) != MA_SUCCESS) {
         // Fallback: some unusual render endpoints may reject channels=2 even though WASAPI
@@ -921,9 +872,6 @@ bool AudioEngine::start_loopback_capture(int kind, int channels) {
         return false;
     }
 
-    // Size the accumulator to one full encoder frame at the actual opened channel count.
-    // Done here (off the RT thread) before any callback can fire — satisfies architecture.md
-    // §3 (no allocation on RT threads). ma_device_start() above is what arms the callback.
     loopback_channels_    = ch;
     loopback_accum_.buf.assign(static_cast<size_t>(frame_samples_ * ch), 0);
     loopback_accum_.count = 0;

core/src/audio/audio_engine.h

@@ -1,13 +1,6 @@
 /*
- * audio/audio_engine.h — capture/playback + DSP + jitter buffer + mixer.
+ * audio/audio_engine.h: capture/playback + DSP + jitter buffer + mixer.
  *
- * Design: docs/voice.md §8–11. Real-time path:
- *   capture(miniaudio) → APM(AEC/NS/AGC/VAD, send-side) → Opus encode → ...
- *   ... → Opus decode → per-user recv NS (listener-chosen) → gain/mute → mix → playback
- *
- * REAL-TIME RULE: audio-callback threads never allocate, lock, or block (architecture.md §3).
- * The JitterBuffer and per-stream maps are accessed only under a try_lock; a failed lock
- * causes PLC for that period (acceptable today; a lock-free ring buffer would remove even that).
  */
 #ifndef VOICECAT_AUDIO_AUDIO_ENGINE_H
 #define VOICECAT_AUDIO_AUDIO_ENGINE_H
@@ -25,7 +18,6 @@
 #include <unordered_map>
 #include <vector>
 
-// miniaudio single-header — MINIAUDIO_IMPLEMENTATION defined in audio_engine.cpp
 #include <miniaudio.h>
 
 #include "codec/opus_codec.h"
@@ -34,7 +26,7 @@
 
 namespace voicecat::audio {
 
-// ── JitterBuffer ─────────────────────────────────────────────────────────────
+// JitterBuffer
 // Per-ssrc adaptive jitter buffer. Thread-safe via internal mutex.
 class JitterBuffer {
    public:
@@ -103,7 +95,7 @@ class JitterBuffer {
     bool     first_push_          = true;
 };
 
-// ── AudioParams ──────────────────────────────────────────────────────────────
+// AudioParams
 struct AudioParams {
     uint32_t    sample_rate        = 48000;
     uint32_t    capture_channels   = 1;  // mic capture: 1 = mono, 2 = stereo (set via vc_set_capture_channels)
@@ -127,7 +119,7 @@ struct DeviceInfo {
     bool        is_default = false;
 };
 
-// ── AudioEngine ──────────────────────────────────────────────────────────────
+// AudioEngine
 // Owns miniaudio capture/playback, per-ssrc jitter buffers + Opus decoders, and the mixer.
 class AudioEngine {
    public:
@@ -173,6 +165,7 @@ class AudioEngine {
     // loopback device with (1 = mono downmix of the system mix, 2 = stereo capture when the
     // channel is configured stereo); the accumulator and capture_cb_ invocation are shaped to
     // match. No-op (returns false) when unsupported.
+    // This has mostly been replaced by client-specific code, so this is likely ready to be revisited. 
     bool start_loopback_capture(int kind, int channels);
     void stop_loopback_capture();
 
@@ -192,8 +185,8 @@ class AudioEngine {
     // Per-stream receive-side controls (safe from any thread).
     void set_stream_gain(uint32_t ssrc, float gain);      // 0.0–2.0, default 1.0
     void set_stream_mute(uint32_t ssrc, bool mute);
-    // Listener-chosen, local-only noise reduction on a specific remote stream (docs/voice.md
-    // §10) — lazily instantiates an ApmProcessor on first enable, frees it on disable.
+    // Listener-chosen, local-only noise reduction on a specific remote stream
+    // lazily instantiates an ApmProcessor on first enable, frees it on disable.
     void set_stream_noise_reduction(uint32_t ssrc, bool enable);
     // Read back a stream's current receive-side state. Returns true and fills *out if the
     // stream is known (even if defaults — gain=1, mute=false, nr=false), false if it has
@@ -201,7 +194,7 @@ class AudioEngine {
     bool get_stream_state(uint32_t ssrc, float& gain, bool& mute, bool& noise_reduction);
     void remove_stream(uint32_t ssrc);
 
-    // Edge-triggered talk-state transitions since the last call (docs/voice.md §7: talk state
+    // Edge-triggered talk-state transitions since the last call : talk state
     // is derived from recent frame arrival, no protocol message). Call from a lightweight
     // poller, not the audio callback thread. Returns {ssrc, now_talking} for each stream whose
     // state flipped.
@@ -250,7 +243,7 @@ class AudioEngine {
 
     // TEST-ONLY — exposes the playback mixer without a real ma_device, so tests can verify
     // 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).
+    // callback uses
     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
@@ -279,7 +272,7 @@ class AudioEngine {
     // TEST-ONLY — stereo-aware variant: drives the capture accumulator with interleaved L/R
     // PCM (channels=2) or mono (channels=1). Sizes the accumulator to frame_samples_*channels
     // and invokes `cb` with the channel count passed through — mirrors feed_loopback_for_test.
-    // Use to verify stereo mic capture (vc_set_capture_channels → on_capture's accumulator).
+    // Use to verify stereo mic capture (vc_set_capture_channels  on_capture's accumulator).
     void feed_capture_for_test(const int16_t* pcm, int frames_per_channel, int channels,
                                const CaptureCallback& cb) {
         if (frame_samples_ <= 0 || !cb) return;
@@ -353,7 +346,7 @@ class AudioEngine {
 
     // Build the ma_context config that keeps miniaudio from managing AVAudioSession on iOS.
     // With a NULL context, ma_device_init runs miniaudio's iOS "hack" (miniaudio.h ~44057)
-    // that calls setCategory()/setActive() on EVERY device open — capture →
+    // that calls setCategory()/setActive() on EVERY device open — capture 
     // AVAudioSessionCategoryRecord with zero options. That obliterates the category/mode/
     // options the Swift IOSAudioRouter configured (notably .playAndRecord and
     // .allowBluetoothA2DP), which is what killed headphone/A2DP output when stereo was