fix(audio): bound playout depth to stop voice latency ratcheting up

Latency between speakers grew to multiple seconds and only reset on
rejoining voice. Root cause was the receiver playout logic, not the
codec settings: the playout clock free-ran in real time while the
sender omitted silence from its timestamps (and set no header flags),
and the only correction snapped the clock to the *oldest* buffered
frame — which could only ever add standing latency. target_depth_ms_
was computed but never enforced, so latency could only grow or reset.

Fix: bound playout against the stream's leading edge (newest frame).
(Re)seed to the leading edge on start/marker/starve (no prebuffer, so
latency stays low), and frame-skip catch-up trims any backlog beyond
target+hysteresis — the missing downward force.

Hardening: sender now stamps kFlagMarker (talkspurt start) and kFlagDtx,
consumed on recv for clean resync; adaptive late-drop window; EWMA
outlier rejection so silence gaps/stragglers don't poison the estimate;
duplicate counting and ring-underrun diagnostics.

New test_jitter_depth asserts depth stays bounded (<200ms) while
arrivals outrun playback. ctest --preset dev green (27/27).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

core/src/audio/audio_engine.cpp

@@ -18,13 +18,18 @@ int64_t now_ms() {
         .count();
 }
 
-// Playout-clock re-sync thresholds (samples @ 48 kHz). The playout clock advances every callback
+// 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 — so they drift across VAD/PTT silence gaps and late joins. Both are kept
-// under JitterBuffer::kLateDropSamples (500 ms) so the clock is snapped back before frames would
-// begin to be dropped-as-late, which is what produced the "talk indicator lit, no audio" silence.
-constexpr int32_t kResyncAheadSamples  = 48000 * 200 / 1000;  // clock 200 ms ahead → re-seed
-constexpr int32_t kResyncBehindSamples = 48000 * 500 / 1000;  // clock 500 ms behind → re-seed
+// 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
@@ -79,22 +84,37 @@ bool hex_decode_device_id(const std::string& hex, ma_device_id* out) {
 void JitterBuffer::push(Frame f) {
     std::lock_guard lk(mu_);
 
-    uint32_t ts = f.timestamp;
+    uint32_t ts     = f.timestamp;
+    bool     marker = f.marker;
 
-    // Jitter estimation (EWMA of inter-arrival gap vs expected gap).
+    // Jitter estimation (EWMA of inter-arrival gap vs the expected per-frame gap). Skip
+    // silence-gap outliers — a talkspurt restart (marker) or any gap far larger than a frame
+    // (DTX/VAD/PTT silence) is not jitter; counting it would spike the estimate and inflate the
+    // target depth for the rest of the call. The sender omits silence from its timestamp, so a
+    // restart can also arrive "behind" (negative gap) — also an outlier.
     if (!first_push_) {
-        uint32_t arrived_gap = ts - last_push_ts_;
-        uint32_t expected_gap = 960;  // 20 ms @48k; TODO: derive from params
-        uint32_t diff = (arrived_gap > expected_gap) ? (arrived_gap - expected_gap)
-                                                      : (expected_gap - arrived_gap);
-        jitter_est_ = (jitter_est_ * 7 + diff) / 8;
-        uint32_t depth = std::clamp(jitter_est_ * 2 + 960u, 960u, 48000u * 200u / 1000u);
-        target_depth_ms_.store(depth * 1000u / 48000u, std::memory_order_relaxed);
+        int32_t gap = static_cast<int32_t>(ts - last_push_ts_);
+        bool outlier = marker || gap <= 0 ||
+                       gap > static_cast<int32_t>(expected_gap_samples_ * 8);
+        if (!outlier) {
+            uint32_t diff = (static_cast<uint32_t>(gap) > expected_gap_samples_)
+                                ? (static_cast<uint32_t>(gap) - expected_gap_samples_)
+                                : (expected_gap_samples_ - static_cast<uint32_t>(gap));
+            jitter_est_ = (jitter_est_ * 7 + diff) / 8;
+            uint32_t depth = std::clamp(jitter_est_ * 2 + expected_gap_samples_,
+                                        expected_gap_samples_, 48000u * 200u / 1000u);
+            target_depth_ms_.store(depth * 1000u / 48000u, std::memory_order_relaxed);
+        }
     }
     last_push_ts_ = ts;
     first_push_   = false;
 
-    buf_.emplace(ts, std::move(f));
+    // Track the leading edge (newest timestamp), wrap-safe.
+    if (!have_newest_ || static_cast<int32_t>(ts - newest_ts_) > 0) newest_ts_ = ts;
+    have_newest_ = true;
+
+    auto res = buf_.emplace(ts, std::move(f));
+    if (!res.second) dup_.fetch_add(1, std::memory_order_relaxed);  // duplicate timestamp
 }
 
 std::optional<JitterBuffer::Frame> JitterBuffer::pop(uint32_t playout_ts) {
@@ -106,8 +126,14 @@ std::optional<JitterBuffer::Frame> JitterBuffer::pop(uint32_t playout_ts) {
     auto it = buf_.begin();
     uint32_t ts = it->first;
 
-    // Drop frames that are too old (> 500 ms late).
-    if (static_cast<int32_t>(playout_ts - ts) > static_cast<int32_t>(kLateDropSamples)) {
+    // Drop frames that are too old to play. The window tracks the (adaptive) target depth plus a
+    // margin so it never undercuts DRED's next-packet lookahead, floored/capped at the fixed
+    // 500 ms bound. Without this the only downward force on latency was a snap that *added* it.
+    constexpr uint32_t kLateMarginSamples = 48000u * 200u / 1000u;  // +200 ms over target depth
+    constexpr uint32_t kLateDropFloor     = 48000u * 200u / 1000u;  // never drop earlier than 200 ms
+    uint32_t late_window = std::clamp(target_depth_samples() + kLateMarginSamples,
+                                      kLateDropFloor, kLateDropSamples);
+    if (static_cast<int32_t>(playout_ts - ts) > static_cast<int32_t>(late_window)) {
         lost_.fetch_add(1, std::memory_order_relaxed);
         buf_.erase(it);
         return std::nullopt;
@@ -127,6 +153,23 @@ std::optional<uint32_t> JitterBuffer::peek_front_ts() const {
     return buf_.begin()->first;
 }
 
+std::optional<uint32_t> JitterBuffer::peek_back_ts() const {
+    std::unique_lock lk(mu_, std::try_to_lock);
+    if (!lk || !have_newest_) return std::nullopt;
+    return newest_ts_;
+}
+
+void JitterBuffer::drop_before(uint32_t ts) {
+    std::lock_guard lk(mu_);
+    while (!buf_.empty()) {
+        auto it = buf_.begin();  // oldest
+        if (static_cast<int32_t>(ts - it->first) > 0)
+            buf_.erase(it);  // strictly before the new playout point — stale
+        else
+            break;
+    }
+}
+
 size_t JitterBuffer::try_copy_front_payload(uint32_t expected_ts, uint8_t* out, size_t max_sz) {
     std::unique_lock lk(mu_, std::try_to_lock);
     if (!lk || buf_.empty()) return 0;
@@ -142,7 +185,10 @@ void JitterBuffer::reset() {
     std::lock_guard lk(mu_);
     buf_.clear();
     lost_.store(0);
-    first_push_ = true;
+    dup_.store(0);
+    first_push_  = true;
+    have_newest_ = false;
+    jitter_est_  = 0;
 }
 
 // ── AudioEngine ──────────────────────────────────────────────────────────────
@@ -444,6 +490,7 @@ void AudioEngine::push_recv_frame(uint32_t ssrc, JitterBuffer::Frame f) {
     std::lock_guard lk(streams_mu_);
     auto& s = streams_[ssrc];
     s.last_voice_ms.store(now_ms(), std::memory_order_relaxed);
+    if (f.marker) s.pending_marker = true;  // talkspurt start → force playout reseed
     s.jitter.push(std::move(f));
 }
 
@@ -520,6 +567,29 @@ uint32_t AudioEngine::stream_target_depth_ms(uint32_t ssrc) const {
     return (it != streams_.end()) ? it->second.jitter.target_depth_ms() : 40;
 }
 
+uint32_t AudioEngine::stream_duplicates(uint32_t ssrc) const {
+    std::lock_guard lk(streams_mu_);
+    auto it = streams_.find(ssrc);
+    return (it != streams_.end()) ? it->second.jitter.duplicates() : 0;
+}
+
+uint64_t AudioEngine::stream_underruns(uint32_t ssrc) const {
+    std::lock_guard lk(streams_mu_);
+    auto it = streams_.find(ssrc);
+    return (it != streams_.end())
+               ? it->second.underruns.load(std::memory_order_relaxed)
+               : 0;
+}
+
+int32_t AudioEngine::stream_playout_depth_samples(uint32_t ssrc) const {
+    std::lock_guard lk(streams_mu_);
+    auto it = streams_.find(ssrc);
+    if (it == streams_.end() || !it->second.playout_started) return 0;
+    auto newest = it->second.jitter.peek_back_ts();
+    if (!newest) return 0;
+    return static_cast<int32_t>(*newest - it->second.playout_ts);
+}
+
 #ifdef VOICECAT_HAS_OPUS
 void AudioEngine::init_recv_stream(uint32_t ssrc, const codec::OpusParams& p,
                                    uint32_t user_id, uint32_t stream_id) {
@@ -534,6 +604,9 @@ void AudioEngine::init_recv_stream(uint32_t ssrc, const codec::OpusParams& p,
     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);
+    // The expected inter-arrival gap = the sender's frame size in samples @48 kHz; the jitter
+    // EWMA and silence-gap outlier rejection key off it (defaults to 20 ms otherwise).
+    stream.jitter.set_expected_gap(static_cast<uint32_t>(frame_samples));
     // DRED: pre-allocate per-stream scratch (no RT-thread allocation). 4000 bytes > max Opus pkt.
     stream.dred_payload_scratch_.assign(4000, 0);
     if (!stream.dred_state_) {
@@ -620,21 +693,31 @@ 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();
 
-        // Seed / re-sync the playout clock to the arriving stream. playout_ts advances every
-        // callback (via the PLC path below) independently of whether the sender is transmitting,
-        // so across a late join or any VAD/PTT silence gap it drifts away from the sender's frame
-        // timestamps without bound. Left uncorrected, the divergence eventually exceeds the jitter
-        // buffer's late-drop window and every real frame is dropped-as-late (clock ahead) or
-        // never-due (clock behind) — permanent silence even though frames keep arriving (the talk
-        // indicator, driven by push_recv_frame, stays lit). Snap to the earliest buffered frame on
-        // the first frame and whenever the clock has drifted too far; this both seeds startup and
-        // recovers after every silence gap. u32 subtraction via int32_t handles timestamp wrap.
-        if (auto front_ts = stream.jitter.peek_front_ts()) {
-            int32_t drift = static_cast<int32_t>(stream.playout_ts - *front_ts);
-            if (!stream.playout_started || drift > kResyncAheadSamples ||
-                drift < -kResyncBehindSamples) {
-                stream.playout_ts      = *front_ts;
+        // 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);
+        if (auto newest = stream.jitter.peek_back_ts()) {
+            int32_t depth = static_cast<int32_t>(*newest - stream.playout_ts);  // wrap-safe
+            if (!stream.playout_started || stream.pending_marker || depth < -kStarveSamples) {
+                stream.playout_ts      = *newest;
                 stream.playout_started = true;
+                stream.pending_marker  = false;
+            } else if (depth > target + kCatchupSamples) {
+                stream.playout_ts = *newest - static_cast<uint32_t>(target);
+                stream.jitter.drop_before(stream.playout_ts);
             }
         }
 
@@ -713,6 +796,12 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) {
             stream.playout_ts += static_cast<uint32_t>(n);
         }
 
+        // 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
+        // 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);
+
         const float g = stream.gain;
         int16_t frame_buf[2];
         for (ma_uint32 i = 0; i < frames; ++i) {

core/src/audio/audio_engine.h

@@ -46,6 +46,7 @@ class JitterBuffer {
         uint64_t             seq;
         uint32_t             timestamp;
         bool                 fec_present;
+        bool                 marker = false;  // kFlagMarker: first frame of a talkspurt
         std::vector<uint8_t> payload;
     };
 
@@ -61,28 +62,49 @@ class JitterBuffer {
     // AudioEngine::on_playback). Uses try_lock — never blocks the real-time callback.
     std::optional<uint32_t> peek_front_ts() const;
 
+    // Timestamp of the newest (latest) buffered frame — the stream's leading edge. Lets the
+    // playout clock keep a bounded depth behind the arriving stream and catch up (frame-skip)
+    // when the backlog grows; see AudioEngine::on_playback. try_lock — never blocks the RT thread.
+    std::optional<uint32_t> peek_back_ts() const;
+
     // If the front frame's timestamp == expected_ts, copies its raw Opus payload into out
     // (caller-allocated, max_sz bytes). Returns bytes copied, or 0 (lock miss / wrong ts /
     // empty). Caller pre-allocates out to avoid RT-thread allocation. Uses try_lock.
     size_t try_copy_front_payload(uint32_t expected_ts, uint8_t* out, size_t max_sz);
 
-    uint32_t target_depth_ms()const { return target_depth_ms_.load(); }
-    uint32_t packets_lost()   const { return lost_.load(); }
+    // Erase all buffered frames whose timestamp is strictly before `ts` (oldest-first). Used by
+    // the playout catch-up so a forward clock jump doesn't flood the decode loop with stale frames.
+    void drop_before(uint32_t ts);
+
+    uint32_t target_depth_ms()      const { return target_depth_ms_.load(); }
+    uint32_t target_depth_samples() const { return target_depth_ms_.load() * 48; }  // @48 kHz
+    uint32_t packets_lost()         const { return lost_.load(); }
+    uint32_t duplicates()           const { return dup_.load(); }
+
+    // Set the expected inter-arrival gap (= sender frame size in samples @48 kHz) so the jitter
+    // EWMA and silence-gap outlier rejection are correct for non-20 ms channels. Call at init.
+    void set_expected_gap(uint32_t samples) {
+        if (samples > 0) expected_gap_samples_ = samples;
+    }
     void     reset();
 
    private:
-    static constexpr uint32_t kLateDropSamples = 48000 / 2;  // 500 ms @48 kHz
+    static constexpr uint32_t kLateDropSamples = 48000 / 2;  // 500 ms @48 kHz (hard floor/cap)
 
     mutable std::mutex       mu_;
     std::map<uint32_t, Frame> buf_;   // keyed by timestamp (u32 wraps are handled below)
 
     std::atomic<uint32_t> target_depth_ms_{40};
     std::atomic<uint32_t> lost_{0};
+    std::atomic<uint32_t> dup_{0};
 
     // Jitter estimation (EWMA).
-    uint32_t last_push_ts_  = 0;      // local clock estimate on last push
-    uint32_t jitter_est_    = 0;      // EWMA jitter in samples
-    bool     first_push_    = true;
+    uint32_t last_push_ts_        = 0;    // sender timestamp on last push
+    uint32_t jitter_est_          = 0;    // EWMA jitter in samples
+    uint32_t expected_gap_samples_ = 960;  // expected inter-arrival gap (frame size @48 kHz)
+    uint32_t newest_ts_           = 0;    // latest buffered timestamp (leading edge)
+    bool     have_newest_         = false;
+    bool     first_push_          = true;
 };
 
 // ── AudioParams ──────────────────────────────────────────────────────────────
@@ -192,6 +214,13 @@ class AudioEngine {
     // Get stats for a remote stream's jitter buffer.
     uint32_t stream_packets_lost(uint32_t ssrc) const;
     uint32_t stream_target_depth_ms(uint32_t ssrc) const;
+    uint32_t stream_duplicates(uint32_t ssrc) const;
+    uint64_t stream_underruns(uint32_t ssrc) const;
+
+    // TEST-ONLY: current playout depth in samples (newest buffered ts - playout_ts), i.e. the
+    // standing latency held in the jitter buffer. Used by test_jitter_depth to assert the
+    // bounded-depth invariant. Returns 0 if the stream is unknown or not yet playing out.
+    int32_t  stream_playout_depth_samples(uint32_t ssrc) const;
 
 #ifdef VOICECAT_HAS_OPUS
     // Configure the Opus decoder for an incoming ssrc (must be called before
@@ -422,6 +451,16 @@ class AudioEngine {
         // dropped/never-due (silent playback). false until the first frame seeds it (on_playback).
         bool             playout_started = false;
 
+        // Set by push_recv_frame when a kFlagMarker (talkspurt-start) frame arrives; consumed by
+        // on_playback to force an immediate playout-clock reseed at the new talkspurt, so the
+        // bounded-depth target is re-established cleanly across silence gaps. See on_playback.
+        bool             pending_marker = false;
+
+        // Diagnostic: times the decode/playback ring underran (produced silence because the
+        // jitter buffer had nothing due) while the stream was actively playing out — i.e. the
+        // "frames arriving but silent / latency starved" signal. Polled via stream_underruns().
+        std::atomic<uint64_t> underruns{0};
+
         // PLC cap (defense-in-depth): consecutive samples produced by packet-loss
         // concealment since the last real decoded frame. Reset to 0 on every real frame.
         // When it exceeds kPlcCapSamples (audio_engine.cpp), on_playback stops calling

core/src/core/client.cpp

@@ -904,6 +904,7 @@ void vc_client::run_udp_recv() {
         jf.seq         = hdr.seq;
         jf.timestamp   = hdr.timestamp;
         jf.fec_present = (hdr.flags & voicecat::net::kFlagFecPresent) != 0;
+        jf.marker      = (hdr.flags & voicecat::net::kFlagMarker) != 0;
         jf.payload     = std::move(plain);
         audio_engine_.push_recv_frame(hdr.ssrc, std::move(jf));
     }
@@ -1097,6 +1098,18 @@ void vc_client::encode_and_send_frame(LocalStream& ls, const int16_t* pcm, int s
     hdr.timestamp = ls.timestamp;
     ls.timestamp += static_cast<uint32_t>(samples);
 
+    // Talkspurt marker: first frame overall, or the first after a transmission gap longer than a
+    // few frame intervals (VAD/PTT closed, or DTX silence). The sender omits silence from the
+    // timestamp, so this is how the receiver knows to reseed its playout clock (see on_playback).
+    const int64_t now_ms      = client_now_ms();
+    const int64_t frame_ms    = std::max<int64_t>(1, samples / 48);  // @48 kHz
+    if (ls.last_send_ms < 0 || (now_ms - ls.last_send_ms) > frame_ms * 3)
+        hdr.flags |= voicecat::net::kFlagMarker;
+    ls.last_send_ms = now_ms;
+    // DTX: Opus emits a 1–2 byte comfort-noise packet when it gates silence. Flag it so the
+    // receiver can treat it as such (informational; the bounded-depth playout handles timing).
+    if (opus_len <= 2) hdr.flags |= voicecat::net::kFlagDtx;
+
     uint8_t header_bytes[voicecat::net::kVoiceHeaderSize];
     voicecat::net::serialize_header(hdr, header_bytes);
 

core/src/core/client.h

@@ -240,6 +240,13 @@ struct vc_client {
         std::atomic<int64_t> last_capture_ms{0};
         bool                  talking = false;
 
+        // Talkspurt marker: the sender's `timestamp` omits VAD/PTT/DTX silence, so the receiver
+        // can't tell a continuation from a post-silence restart. encode_and_send_frame stamps
+        // kFlagMarker on the first frame after a transmission gap (detected via last_send_ms) so
+        // the receiver reseeds its playout clock cleanly. -1 = no frame sent yet (first frame is
+        // always a marker).
+        int64_t               last_send_ms = -1;
+
         // Device-enumeration follow-up: the device this stream's capture should use ("" =
         // default). Only meaningful for VC_STREAM_MIC today (the real capture device); set via
         // vc_set_input_device. Opaque id from AudioEngine::enumerate_devices — see