Skip to content

Commit e151dd8

Browse files
committed
fix(15.1-07a): pick PeerNextDsUpdate slot in apply visitor (random remote cutout)
User UAT on build 289: remote audio cuts out RANDOMLY during intervals (different from the every-other-interval cutout fixed in build 289). Root cause of the new symptom: build-289's mitigation (always publish to slot 0) defer-deleted any pending next_ds[0] on every publish. That silently dropped data when the run thread published 2+ updates between two audio drains — which happens under bursty network arrivals. Result: random missed intervals → random audible cutouts. The deeper architectural mistake (in 07a originally, not just my mitigation): slot selection was placed on the run-thread publisher. The 07a executor used a stateful XOR shadow because "the run thread can't read mirror state" — but this overlooked that the apply visitor (which IS where the slot is actually written into the mirror) RUNS ON THE AUDIO THREAD via drainRemoteUserUpdates() called from AudioProc. The audio thread CAN read its own state. Slot picking should always have been there. Fix: move slot selection into the apply visitor at line 3290+. Ignore the publisher's u.slot_idx field. Apply visitor logic: - if next_ds[0] is empty → fill slot 0 - else if next_ds[1] is empty → queue at slot 1 - else (both full, extremely rare) → defer-delete oldest (next_ds[0]), shift, queue new ds at slot 1 This recreates the legacy `useidx = !!next_ds[0]` semantic on the correct (audio) thread, no concurrency required (both writers to next_ds[2] — this apply visitor AND the on_new_interval shuffle — already run on the audio thread). Sequence under bursty arrivals (the failure mode of build 289): T0: run publishes A; run publishes B (both before audio drain) T0 audio drain: apply A → next_ds[0]=A; apply B → next_ds[0] occupied → next_ds[1]=B (was: defer-delete A, lose interval) T0 interval boundary: ds=A; shift → next_ds[0]=B; next_ds[1]=null T1 audio drain: (no updates) T1 interval boundary: ds=B; shift → next_ds[0]=null Both A and B audible. No cutout. Sequence under normal flow (1 publish per interval): T0: run publishes A T0 audio drain: next_ds[0]=A T0 interval: ds=A; shift → next_ds[0]=null T1: run publishes B T1 audio drain: next_ds[0]=B T1 interval: ds=B Continuous. The publisher u.slot_idx field stays in the payload (spsc_payloads.h is FINAL per Codex M-9 — DO NOT modify) but is now ignored by the apply visitor. The run-thread `run_thread_next_ds_idx` field at njclient.cpp:544 is dead code; can be removed in 15.1-10 cleanup. Build 290. Verifies on next UAT — remote audio should be continuous under both steady-rate and bursty network arrivals. The random cutouts should be gone.
1 parent e827453 commit e151dd8

2 files changed

Lines changed: 35 additions & 10 deletions

File tree

src/build_number.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
#pragma once
2-
#define JAMWIDE_BUILD_NUMBER 289
2+
#define JAMWIDE_BUILD_NUMBER 290

src/core/njclient.cpp

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3288,16 +3288,41 @@ void NJClient::drainRemoteUserUpdates()
32883288
deferDecodeStateDelete(m_deferred_delete_q, m_deferred_delete_overflows, incoming_ds);
32893289
return;
32903290
}
3291-
if (u.slot_idx < 0 || u.slot_idx > 1) {
3292-
deferDecodeStateDelete(m_deferred_delete_q, m_deferred_delete_overflows, incoming_ds);
3293-
return;
3294-
}
3291+
// 15.1-07a post-UAT build 289 fix: pick the slot HERE (audio-thread
3292+
// apply visitor), not on the run-thread publisher. The publisher's
3293+
// u.slot_idx is now ignored. Background:
3294+
// - The original 07a publisher used a stateful XOR shadow that
3295+
// alternated 0/1 per publish — but the audio thread always reads
3296+
// next_ds[0] first and shifts next_ds[1]→next_ds[0]→nullptr, so
3297+
// slot-1 publishes were unreachable until the next shift, causing
3298+
// every-other-interval audio cutout (build 288 fix tried to
3299+
// hardcode slot 0 to mitigate).
3300+
// - The build-288 always-slot-0 mitigation defer-deleted the prior
3301+
// next_ds[0] on every publish, silently dropping data when the
3302+
// run thread published 2+ updates between two audio drains
3303+
// (bursty network arrivals → random remote audio cutout).
3304+
// - Correct semantics (matches the legacy `useidx = !!next_ds[0]`):
3305+
// fill the empty slot, queue behind in slot 1 if slot 0 is
3306+
// occupied, only defer-delete the oldest if BOTH slots are full
3307+
// (extremely rare — would require the audio thread to skip 2+
3308+
// intervals while the run thread publishes).
3309+
// - This works concurrency-safely because BOTH writers to next_ds
3310+
// (this apply visitor AND the on_new_interval shuffle) run on
3311+
// the audio thread; no run-thread state read needed.
32953312
auto& chan = m_remoteuser_mirror[u.slot].chans[u.channel];
3296-
// If a previous next_ds[slot_idx] pointer was queued, defer-delete it
3297-
// before overwriting (audio thread retains exclusive ownership during
3298-
// the swap; only the now-orphaned pointer crosses to the run thread).
3299-
deferDecodeStateDelete(m_deferred_delete_q, m_deferred_delete_overflows, chan.next_ds[u.slot_idx]);
3300-
chan.next_ds[u.slot_idx] = incoming_ds;
3313+
if (chan.next_ds[0] == nullptr) {
3314+
chan.next_ds[0] = incoming_ds;
3315+
} else if (chan.next_ds[1] == nullptr) {
3316+
chan.next_ds[1] = incoming_ds;
3317+
} else {
3318+
// Both slots full — defer-delete oldest (next_ds[0]), shift, queue
3319+
// new ds at next_ds[1]. Bumps the deferred-delete overflow path
3320+
// implicitly (the captured pointer goes to defer-delete; no audio
3321+
// gap, just a backlog skip).
3322+
deferDecodeStateDelete(m_deferred_delete_q, m_deferred_delete_overflows, chan.next_ds[0]);
3323+
chan.next_ds[0] = chan.next_ds[1];
3324+
chan.next_ds[1] = incoming_ds;
3325+
}
33013326
}
33023327
else if constexpr (std::is_same_v<T, jamwide::PeerCodecSwapUpdate>) {
33033328
if (u.slot < 0 || u.slot >= MAX_PEERS) return;

0 commit comments

Comments
 (0)