fix(translate): drive talk boundaries from Opus DTX and drop silence frames

[JonathanLennox]

Problem

On /translate with a live mic, clients received sending=true but never sending=false — the synthetic-source "sending" indicator stuck on. The /v1/realtime/translations output-audio stream is continuous: a live mic's ambient/room noise keeps OpenAI emitting audio, so the output-audio-silence heuristic that ends a talk never fired, and the continuous comfort noise was forwarded to the bridge as media. Confirmed live on stage — muting the mic was the only thing that produced a stop.

Neither client-side signal distinguishes speech from ambient noise: the RFC 6464 audio level only marks hard mute, and the VAD bit, while reliably present (ext_id=1), is set on 100% of non-silent packets (including typing/room noise) — it mirrors "not muted", not "voice". So the discrimination has to come from the audio itself.

Fix

Enable Opus DTX on the output encoder and use libopus's own VAD via OPUS_GET_IN_DTX:

• Voice frames (!inDtx) are forwarded to the bridge and drive the talk (start / keep-alive).
• DTX frames — silence and the periodic comfort-noise (CNG) updates — are dropped (not forwarded) and don't extend the talk.

So the silence timer ends the talk on sustained DTX and the next voice frame starts a fresh one. One mechanism satisfies both goals: DTX-driven start/stop and no silence packets to the bridge.

Changes

• C encoders — opus_frame_encoder.c (WASM) and native/opus_addon.cc (native): OPUS_SET_DTX + OPUS_GET_IN_DTX, with OPUS_SET_VBR(1) enforced when DTX is enabled (DTX no-ops under CBR). Makefile exports the two new WASM symbols.
• Bindings/facade/interface — encodeFrame now returns EncodedFrame { data, inDtx } across both backends, the facade, and IOpusEncoder; dtx? added to OpusEncoderConfig (off by default).
• TranslatorConnection — creates the encoder with dtx: true; sendAudioFrame early-returns on DTX frames (no forward, no timer arm). The RtpTimestamper inserts the real gap when voice resumes, so contiguous RTP sequence numbers + jumping timestamps are preserved (standard silence-suppression).
• CLAUDE.md — talk-boundary section rewritten for the DTX model.

Notes / behavior changes

• Granularity: talks are now bracketed by voice runs, so a pause longer than TRANSLATION_TALK_SILENCE_TIMEOUT_MS (default 350 ms) splits a talk (~10–11 talks for the 47 s sample vs. 3 before). Tune via that env var.
• Hangover: libopus emits ~180 ms of voice-flagged frames before DTX engages, so a little trailing audio is forwarded past end-of-speech.

Testing

• DTX plumbing verified in both freshly-built backends (tone → inDtx=false, silence → inDtx=true), captured by the new OpusEncoderDtx.test.ts (runs against native and WASM).
• Boundary logic unit test: DTX frames not forwarded, don't extend the talk, stop fires on sustained DTX, voice-resume = new talk.
• Real-OpenAI e2e on both the native path and the WASM/Worker path (what stage runs): balanced start/stop, media dropped ~2060 → ~1350 (silence not forwarded), transcripts intact.
• Full unit suite 625 pass; typecheck + typecheck:worker + check:worker-safe clean. (dist/+build/ are gitignored; CI/Docker rebuild the encoders from the C changed here.)

🤖 Generated with Claude Code

[claude]

Code Review

Overview

This PR fixes a real bug: the synthetic-source sending=true indicator on /translate never cleared because OpenAI's /v1/realtime/translations stream is continuous — ambient noise keeps the model emitting audio, so the silence heuristic that previously ended a talk never fired. The fix is elegant: enable Opus DTX on the output encoder and use libopus's own VAD (OPUS_GET_IN_DTX) to distinguish voice from comfort-noise/silence frames, dropping the latter.

---

Code Quality

Strengths:

• Clean layering: C → N-API/WASM → TS types → facade → TranslatorConnection each carry minimal, correct changes.
• EncodedFrame { data, inDtx } is self-documenting. DTX off by default means no breakage for callers that don't care.
• VBR enforcement before OPUS_SET_DTX is the right thing to do — libopus silently no-ops DTX under CBR, and the comment explains why.
• Null safety in both opus_frame_encoder_set_dtx and opus_frame_encoder_get_last_in_dtx is correct.
• sendAudioFrame's early return on inDtx is clean and well-documented.
• CLAUDE.md updated in the same commit — correct per project conventions.

Minor observations:

1. last_in_dtx not reset on encode failure (C): In opus_frame_encode, if encoded_bytes < 0 the last_in_dtx field is left at its previous value. A subsequent get_last_in_dtx call would see stale state. In practice this is harmless because the TS layer throws on encodedBytes < 0 before querying the flag — but it's worth noting for correctness pedantry.

2. BACKENDS.md not updated: CLAUDE.md instructs both files be updated when backend behavior changes. The translation encoder path probably isn't covered by BACKENDS.md, but worth confirming the project's interpretation.

3. mockInDtx module-level variable: TranslatorConnectionTalk.test.ts uses a module-level let mockInDtx = false reset in beforeEach. It works and is consistent with the existing test style; just flag it so reviewers know it's intentional and not a missing teardown.

4. Redundant OPUS_GET_IN_DTX query when DTX is disabled (WASM path): _opus_frame_encoder_get_last_in_dtx is called on every encode regardless of whether DTX is enabled. The flag will always be 0 in that case, so it's not a bug, just a tiny no-op per frame. A conditional skip would be micro-optimization, probably not worth it.

---

Potential Issues

None critical. One worth surfacing:

• Transcript deltas also arm the silence timer. CLAUDE.md correctly notes this: armTalkSilenceTimer is called by both voice frames and transcript deltas. In theory, if OpenAI emits a transcript delta (which should only happen during/after speech) while the encoder is in DTX, the timer rearms and the talk stays open longer than the DTX alone would suggest. This is probably the desired behavior (transcript arrival = still talking), but it means the guarantee "sustained DTX ends the talk" has an asterisk: "...unless a transcript delta arrives in between." Worth documenting or at least being aware of.

---

Performance

• EncodedFrame object allocation per frame (~50/s for 24 kHz/20 ms) is negligible.
• OPUS_GET_IN_DTX is a no-cost ctl call.
• Dropping DTX frames early (return in sendAudioFrame) is a net gain on silence-heavy sessions — no forwarding, no timer arm, no RTP timestamper call.

---

Test Coverage

Good. Coverage includes:

• OpusEncoderDtx.test.ts: end-to-end DTX plumbing through both WASM and native binaries (properly gated on build artifacts).
• TranslatorConnectionTalk.test.ts: DTX frames not forwarded, don't extend the talk, talk ends on sustained DTX, voice-resume starts a fresh talk.
• OpusRoundTrip.test.ts and TranslatorConnectionTranscript.test.ts: updated to use the new EncodedFrame shape.

One gap worth noting:

• There's no assertion that the RtpTimestamper correctly inserts a gap after skipped DTX frames (i.e., that the next voice frame gets a timestamp ahead of the last voice frame by more than one frame's worth). The PR description says this is handled naturally by RtpTimestamper, which is correct — but a test asserting the timestamp jump would make the guarantee explicit.

---

Summary

The fix is correct, well-scoped, and thoughtfully implemented. The DTX approach is better than a custom RMS gate — it reuses the codec's own VAD so the voice/silence boundary is the same signal the encoder itself relies on. The ~180 ms DTX hangover is an acceptable and documented trade-off.

The breaking change to encodeFrame's return type is handled correctly across all call sites. Nothing here is a blocker; the items above are low-severity observations.

[claude]

Code Review — PR #120: fix(translate): drive talk boundaries from Opus DTX and drop silence frames

Overview

This PR fixes a real production bug: on /translate with a live mic, the synthetic-source sending=true indicator stuck on forever because OpenAI's translation output is continuous — ambient noise keeps the audio stream flowing, so the previous output-audio-silence heuristic never fired. The fix is elegant: enable Opus DTX on the output encoder and use libopus's own VAD (OPUS_GET_IN_DTX) to distinguish voice from comfort-noise/silence, dropping DTX frames rather than forwarding them. Both goals — talk boundaries and no silence forwarded to the bridge — are satisfied with one mechanism.

---

Positives

• Correct approach: Using libopus's own VAD rather than rolling a custom energy/RMS detector avoids an entire class of tuning problems. The hangover behavior (~180 ms of voice-flagged frames before DTX engages) gives smooth transitions rather than choppy ones.
• Both backends covered: WASM (opus_frame_encoder.c) and native (opus_addon.cc) get the same treatment with consistent VBR enforcement on DTX enable.
• VBR enforcement on DTX enable: The OPUS_SET_VBR(1) before OPUS_SET_DTX (in both the C and native layers) correctly guards against CBR silently disabling DTX. Good defensive coding.
• Clean API surface: EncodedFrame { data, inDtx } is a minimal, well-scoped type. Defaulting dtx to false in OpusEncoderConfig keeps the change backward-compatible.
• if (opusFrames.length > 0) removal: The old guard was unnecessary (the loop body is a no-op on an empty array) — removing it simplifies the code without changing behavior.
• Test coverage: OpusEncoderDtx.test.ts exercises the real built binaries for both backends, which is the right level for this kind of C-layer plumbing. The new TranslatorConnectionTalk scenarios (DTX frames dropped, RTP gap preserved across DTX silence) test the logic that's actually load-bearing.
• Documentation: CLAUDE.md is updated in the same change, as required.

---

Issues / Suggestions

1. OPUS_GET_IN_DTX called unconditionally in the native addon (minor overhead)

In opus_addon.cc, the OPUS_GET_IN_DTX ctl is issued on every encode call, even when DTX is disabled:

int in_dtx = 0;
opus_encoder_ctl(encoder_, OPUS_GET_IN_DTX(&in_dtx));

When DTX is disabled, libopus always returns 0, so the result is correct — but there's a per-frame syscall/ctl overhead. For the current use case this is negligible, but a bool dtxEnabled_ member guard would make it zero-cost in the common (DTX-off) path. The WASM path avoids this by calling the C wrapper only when needed. Not a blocker, just worth noting.

2. Stale last_in_dtx on encode failure in C (edge case)

In opus_frame_encoder.c:

if (encoded_bytes >= 0) {
    int in_dtx = 0;
    if (opus_encoder_ctl(ctx->encoder, OPUS_GET_IN_DTX(&in_dtx)) == OPUS_OK) {
        ctx->last_in_dtx = in_dtx;
    } else {
        ctx->last_in_dtx = 0;
    }
}
// last_in_dtx NOT reset when encoded_bytes < 0
return encoded_bytes;

If opus_frame_encode returns an error (negative), last_in_dtx retains its previous value. A subsequent get_last_in_dtx call would return stale data. In practice, the TypeScript WASM wrapper throws on negative return codes before ever calling get_last_in_dtx, so this path won't be hit — but adding ctx->last_in_dtx = 0; in the else branch of if (encoded_bytes >= 0) would make it bulletproof.

3. Dropped CNG frames and bridge decoder state

By dropping DTX/comfort-noise frames entirely (including the periodic CNG updates libopus sends), the bridge's Opus decoder doesn't receive the comfort-noise refresh packets. This is acceptable for silence periods, but worth a CLAUDE.md note so future maintainers understand the tradeoff (bridge gets silence via packet-loss concealment rather than explicit CNG). The RtpTimestamper gap-insertion on voice resume correctly models the real elapsed silence, so the client-side experience should be fine.

4. mockInDtx is module-level mutable state

In TranslatorConnectionTalk.test.ts:

let mockInDtx = false;

This is reset in beforeEach, which handles the normal case. However, if a test throws before the mock can be consumed, the flag is still correctly reset by the next beforeEach. The current pattern is fine; just flagging it as something to watch if the test file grows.

5. Inconsistent error handling between WASM and native on set_dtx failure

The native addon throws (Napi::Error::New(env, ...).ThrowAsJavaScriptException()) when OPUS_SET_VBR or OPUS_SET_DTX fails. The WASM path logs a console.warn and continues. This inconsistency exists in the original codebase (same pattern for bitrate/complexity) but DTX failure is more consequential — if DTX silently doesn't engage, inDtx will never be true and all frames will be forwarded, which means the bug this PR fixes would silently regress. A WASM throw on dtxRet < 0 (matching native) would make this failure mode loud.

---

Questions

• Is 350 ms (TRANSLATION_TALK_SILENCE_TIMEOUT_MS) the right default given the ~180 ms libopus hangover? The effective end-of-talk latency is ~530 ms (hangover + timer). Was this tuned on the stage test mentioned in the PR description?
• The TranslatorConnectionTranscript mock hardcodes inDtx: false. If a future test exercises the transcript path alongside DTX, will it pick up the module-level mockInDtx or the hardcoded value? (Currently fine since that test file has its own local mock, but a shared fixture might be cleaner long-term.)

---

Verdict

Approve with minor suggestions. The fix is correct, well-tested, and uses the right tool (libopus VAD) for the job. The issues above are minor — items 2 and 5 are the ones I'd most recommend addressing before merge (stale flag on encode failure and WASM not throwing on DTX-enable failure). Items 1, 3, and 4 are informational.

🤖 Generated with Claude Code

[claude]

Code Review — PR #120: fix(translate): drive talk boundaries from Opus DTX and drop silence frames

Overview

This PR fixes a production bug where the synthetic-source sending=true indicator on /translate with a live mic never transitioned to sending=false. The root cause is that OpenAI's /v1/realtime/translations output is continuous — ambient/room noise keeps it emitting audio — so the existing audio-silence heuristic never fires. The fix is clean and elegant: enable Opus DTX on the output encoder, let libopus's own VAD classify frames via OPUS_GET_IN_DTX, and drop comfort-noise/silence frames before they reach the bridge. One mechanism simultaneously stops the stuck sending=true and eliminates silence packets downstream.

---

Code Quality — Mostly excellent

C layer (opus_frame_encoder.c / native/opus_addon.cc)

The implementation is sound. A few observations:

• opus_frame_encoder.c correctly clears last_in_dtx on encoding failure (belt-and-suspenders, as the JS wrapper throws before reading it, but defensive).

• VBR enforcement when enabling DTX is present in both C and native paths and the rationale is documented.

• Minor inconsistency: in opus_addon.cc the return value of opus_encoder_ctl(encoder_, OPUS_GET_IN_DTX(&in_dtx)) is silently ignored:

  int in_dtx = 0;
  opus_encoder_ctl(encoder_, OPUS_GET_IN_DTX(&in_dtx));  // return not checked

  The C counterpart in opus_frame_encoder.c properly handles the failure case (ctx->last_in_dtx = 0 on failure). The silent-ignore is safe here because the initialiser int in_dtx = 0 is the correct fallback and OPUS_GET_IN_DTX reliably succeeds on a live encoder, but a comment to that effect (or matching the C pattern) would close the gap.

TypeScript layer

• EncodedFrame { data, inDtx } is a clean, minimal interface.
• The WASM path throws loudly on DTX-enable failure vs. the bitrate/complexity console.warn — the asymmetry is intentional and well-explained: a silent DTX regression would cause every frame to be forwarded and silently break the voice/silence discrimination.
• OpusRoundTrip.test.ts correctly unwraps .map(f => f.data) after the API change.
• The sendAudioFrame(opusFrame, inDtx) early-return on DTX is the right place for the gate. It correctly keeps the RtpTimestamper untouched (no nextFrameTimestamp() call), so the elapsed-silence gap is preserved when voice resumes.

One unconditional overhead: _opus_frame_encoder_get_last_in_dtx is called after every encoded frame even when DTX is disabled. For ~50 frames/sec this is negligible, but a dtxEnabled flag could skip the ctl call when it's always going to return 0. Not worth changing now — just noting it.

---

Potential Issues

1. Transcript deltas re-arming the silence timer during sustained DTX

The CLAUDE.md update says the silence timer is "armed by voice frames (and, per above, transcript deltas)". If a long transcript arrives during sustained DTX audio output (all silence frames dropped), the timer could be re-armed and delay the talkStop. In the "live mic with ambient noise" scenario this seems intentional — the speaker is still producing transcribable speech — but a test that verifies this interaction explicitly might be worth adding alongside the DTX tests.

2. First frame from OpenAI being a DTX frame

If the very first audio delta from OpenAI happens to be classified as DTX (unlikely but possible on a cold start), sendAudioFrame returns early, no talk starts, and the timer is never armed. The next voice frame correctly starts the talk. This is the right behaviour, but it's an implicit contract worth documenting alongside the existing "~180 ms hangover" note.

---

Test Coverage — Good

• OpusEncoderDtx.test.ts tests both backends against real encoder binaries — tone → inDtx=false, silence (post-hangover) → inDtx=true, and DTX-off → always false.
• The slice(-10).every(f => f.inDtx) assertion on 40 silence frames correctly avoids asserting on the ~9-frame hangover (180 ms / 20 ms) while still providing strong coverage.
• TranslatorConnectionTalk.test.ts additions cover exactly the right cases: drop on DTX, talk ends on sustained DTX, RTP timeline gap preserved.
• mockInDtx as a module-level flag is the idiomatic pattern here; beforeEach reset prevents leakage.

One small nit: the RTP-gap test asserts resumeTs - firstTs > 100 * SAMPLES_PER_FRAME after vi.advanceTimersByTimeAsync(5000). With SAMPLES_PER_FRAME = 480 (24 kHz, 20 ms), 5 s = 250 frames, so > 100 frames is a wide bound. It's robust by design — no objection — but a tighter bound (e.g. > 200 * SAMPLES_PER_FRAME) would make the assertion more expressive.

---

Security / Performance

No concerns. The change tightens what's forwarded to the bridge (silence is dropped), reducing bandwidth. DTX is a well-understood Opus feature; using OPUS_GET_IN_DTX as the discriminator (rather than packet-size heuristics) is the correct, spec-blessed approach.

---

Documentation

CLAUDE.md is updated in the same commit and the new talk-boundary section is accurate and comprehensive. The note about CNG packets being dropped (bridge fills via PLC → true silence rather than a splice) is a valuable callout for future maintainers.

---

Summary

Solid fix. The design is elegant, the implementation is correct, the new tests cover the critical cases against real encoder binaries, and the documentation is thorough. The main actionable items are:

• Minor: add a comment (or match the C pattern) for the unchecked OPUS_GET_IN_DTX ctl return in opus_addon.cc.
• Optional: explicit test for transcript-delta interaction with sustained DTX if that boundary matters for the sending-indicator semantics.

Everything else is either a non-issue or a deliberate trade-off well-explained in the code.

[claude]

Code Review

Summary: This PR fixes a real bug (sending-indicator stuck on) with an elegant, codec-level solution. Using libopus's own VAD via OPUS_GET_IN_DTX is cleaner than trying to implement audio-level analysis in userspace, and the design is sound. The implementation is thorough across both backends with good test coverage. A few observations below.

---

Strengths

• Correct approach. Using the encoder's built-in DTX flag rather than analysing PCM energy / RTP extension bits means no false positives from room noise, and no new heuristics to tune.
• VBR enforcement before DTX. Both backends guard OPUS_SET_DTX with OPUS_SET_VBR(1), preventing a later CBR change from silently disabling voice detection. Good defensive programming.
• Conservative fallback for OPUS_GET_IN_DTX failure. Pre-initialising in_dtx = 0 in the native addon (and clearing on ctl failure in the WASM C code) means a failure can never produce a false silence drop — it only risks forwarding a comfort-noise frame, not dropping a voice frame. The asymmetry is intentional and well-commented.
• RTP timeline preserved. Dropping DTX frames but letting RtpTimestamper insert the real silence gap on voice resume is the right design. The gap-preservation test verifies this end-to-end.
• Test coverage is solid. OpusEncoderDtx.test.ts runs against real codec binaries for both backends, handles missing artifacts gracefully, and accounts for the DTX hangover in its assertions. The new talk-boundary tests cover the three key scenarios.

---

Observations / Minor Issues

1. Error handling asymmetry between WASM and native for set_dtx failure

In OpusEncoderWasm.ts:

const dtxRet = this.module._opus_frame_encoder_set_dtx(this.ctx, 1);
if (dtxRet < 0) throw new Error(`OpusEncoder: set_dtx(1) failed (${dtxRet})`);

In OpusEncoderNative.ts, the setDtx call can throw (the N-API addon calls Napi::Error::New(...).ThrowAsJavaScriptException() on failure), and that propagates through init(). So both backends actually throw on init failure — the symmetry is there. The comment in the WASM code ("unlike bitrate/complexity, a failed DTX enable is consequential") is accurate and useful. No action needed, just noting the paths are equivalent.

2. dtx: true is hardcoded in TranslatorConnection

// TranslatorConnection.ts:430
dtx: true,

DTX is always on for translation connections with no way to opt out per-connection. Given the PR description, this is intentional (the whole /translate path needs it), but if a future debugging scenario requires seeing all frames (including silence), there's no config knob. Consider whether a TRANSLATE_ENCODER_DTX env var or runtime config field would be useful, or document explicitly that this is by design. Not a blocker.

3. Removal of if (opusFrames.length > 0) guard

-if (opusFrames.length > 0) {
-    for (const frame of opusFrames) {
-        this.sendAudioFrame(frame);
-    }
+for (const frame of opusFrames) {
+    this.sendAudioFrame(frame.data, frame.inDtx);
 }

This is correct — iterating over an empty array is a no-op. Minor clean-up, no issue.

4. CNG drop acknowledged but bridge decoder behaviour is deployment-dependent

CLAUDE.md correctly notes: "dropping DTX frames also drops libopus's periodic comfort-noise (CNG) refresh packets, so during silence the bridge's decoder gets no explicit comfort noise — it fills the gap via packet-loss concealment." PLC is standard and JVB implements it, but deployments using a different bridge/decoder that lacks PLC could hear artefacts. The documentation is clear, which is sufficient.

5. replay-dump.cjs event parsing

if (ts != null) { if (parsed.event === 'start') talkStartEvents++; else talkStopEvents++; }

This compact form is fine for a debug script. One minor note: the else branch will also fire if parsed.event were any non-'start' value that made it past the outer if — but the outer condition already gates on 'start' || 'stop', so the else is always 'stop'. No bug, just slightly implicit.

6. OpusEncoderDtx.test.ts — voice tone frame count

const voice = enc.encodeFrame(tone(15)); // 15 frames of 440 Hz

15 frames = 300ms of tone. The test then checks voice.every((f) => !f.inDtx). libopus has a short analysis window and 300ms of tone should reliably be all non-DTX. This seems robust, but if CI ever flakes on this assertion it might be worth bumping to 20–25 frames. Not a current concern.

---

Summary

This is a well-engineered fix. The core change (DTX in sendAudioFrame) is small and the surrounding plumbing is thorough. CLAUDE.md and the inline comments clearly explain the design trade-offs. The tests cover both the codec-level guarantee and the connection-level boundary logic. Looks good to merge.