fix(publish): turn Opus DTX off by default for voice

[kixelated]

Problem

Chromium stamps Opus encoder output as the first input timestamp plus the samples emitted. With DTX on, every suppressed silent frame pulls later audio earlier, so the published voice timeline drifts from the capture clock and the advertised jitter climbs to seconds. @moq/publish enabled DTX by default for kind: "voice".

Approach

• Drop usedtx: true from the voice Opus defaults (opusKindDefaults). Nothing else changes in behavior.
• Keep OpusConfig.usedtx as an opt-in; its doc comment now warns that it drifts the timeline with Chromium's encoder. The demo's DTX checkbox stays, unchecked by default.
• Tests: the voice encoder config omits usedtx, and an explicit usedtx: true set on the Encoder reaches AudioEncoder.configure.
• Quests: delete /quest/m0/opus-dtx.md (completed here) and its references; move the real fix to /quest/m1/opus-dtx-timestamps.md, whose goal is to make DTX correct and turn it back on for voice.

Impact

• @moq/publish: voice audio no longer enables DTX by default, which costs bandwidth during silence. Not breaking; Audio.OpusConfig.usedtx stays.
• No wire change.

Decisions

• fix(publish): turn Opus DTX off by default for voice #4808 fate: ✅ Shrink to default-off (keep usedtx opt-in) / Close fix(publish): turn Opus DTX off by default for voice #4808 / Merge as-is (remove option)
• DTX fix milestone: ✅ m1 / m0 / keep m2

Follow-ups

• /quest/m1/opus-dtx-timestamps.md makes DTX timestamps correct, then restores it as the voice default.
• The "a rendition trailing the broadcast's earliest advertises delay" test fails when its file runs alone; already tracked by /quest/m1/test-flakes-2/publish-audio-clock.md.

Closes #4783

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

Quest outcome: /quest/m0/opus-dtx.md is complete in this PR. Local just check passes.

Open decision: none beyond the planned breaking removal of OpusConfig.usedtx.

Suggested follow-up: make the encoder test "a rendition trailing the broadcast's earliest advertises delay" mock time; it fails on main when encoder.test.ts runs alone because performance.now() - 100ms goes negative.

(Written by Claude Opus 5.5)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c78a5efe-f766-41c3-b5d7-a05e10f7ced1

📥 Commits

Reviewing files that changed from the base of the PR and between 858ee79 and 89b772d.

📒 Files selected for processing (6)

• js/publish/src/audio/encoder.test.ts
• js/publish/src/audio/encoder.ts
• quest/m0/README.md
• quest/m1/README.md
• quest/m1/opus-dtx-timestamps.md
• quest/m2/README.md

💤 Files with no reviewable changes (2)

• quest/m0/README.md
• quest/m2/README.md

🚧 Files skipped from review as they are similar to previous changes (1)

• js/publish/src/audio/encoder.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

---

Walkthrough

Voice-mode Opus configuration no longer enables DTX by default. Explicit usedtx: true remains supported and is covered by tests. The usedtx documentation describes the setting as opt-in and notes the reported Chromium timestamp drift when frames are suppressed. The timestamp quest and related quest indexes were updated.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 89b77

Voice Opus DTX becomes opt-in to avoid the documented timestamp drift; callers can still enable it explicitly. No material merge risk remains in the supplied evidence.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 89b77

The change removes automatic voice silence suppression without adding privileges or a new trust boundary. Explicit DTX requests remain supported, contrary to the PR description. Risk is limited, but browser timing behavior and the bandwidth tradeoff have not been verified at runtime.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The supported exposure is the publishing application’s voice rendition and its subscribers, with a potential increase in silence-period traffic. The inspected change does not expand access to other tenants, services, credentials, or persistent stores.

Trust Boundaries and Controls

• inferred — The configuration-to-WebCodecs path remains an existing local application capability. Removing one implicit encoder option does not introduce an attacker-controlled entrypoint or bypass an existing authorization control in the inspected path.

Resilience and Maintainability Implications

• observed — The comparison leaves transition and cleanup logic unchanged: configuration precedes pipeline publication, cleanup clears only its owned pipeline and avoids closing an already-closed encoder, fatal errors close current and later subscriber tracks, and bandwidth reservations are released on cleanup. No new retry or recovery transition is introduced.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	[#4783] The issue permits disabling DTX by default until Chromium output timestamps follow the capture clock. opusKindDefaults("voice") no longer sets usedtx: true, and OpusConfig.usedtx remains…
Out of Scope Changes check	✅ Passed	The type documentation, encoder tests, and quest updates support the [#4783] default-off workaround and track the remaining timestamp fix. The summaries show no unrelated code changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (2 skipped: 2 …
Title check	✅ Passed	The title clearly summarizes the main change: Opus DTX is no longer enabled by default for voice.
Description check	✅ Passed	The description explains the timestamp-drift problem, the default change, opt-in behavior, tests, and follow-up quest work. It is directly related to the changeset.

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Automated review: fix(publish): stop enabling Opus DTX

Turning DTX off is a reasonable stopgap. Chromium stamps output by counting the samples it emits, and the encoder's re-base at encoder.ts:502-507 only catches gaps in the input, so frames dropped on the output side were never corrected. CI is green (Check, Test, Release JS dry-run, Quest).

I checked these and found no problems:

• Other code that assumes DTX. I found none. Rust Settings::dtx defaults to false (rs/moq-audio/src/encode/encoder.rs:159). No JS code looks for DTX, speaking state, or comfort noise.
• Gap handling on the watch side. js/watch/src/audio/ring-buffer.ts:153 still fills gaps with zeros, so older @moq/publish builds that still send DTX keep playing.
• Bandwidth. Voice now sends 50 frames (each its own group) per second through silence, which is the cost the PR states. The allocator already reserves the full configured bitrate (encoder.ts:345-358), so reservations don't change. A muted mic sets enabled = false and drops the rendition (element.ts:166, capture.ts:173), so mute doesn't pay for silence frames.

Blockers

None.

Non-blocking

1. usedtx: false (and usedtx: undefined) also throw, which kills audio for callers who already asked for the new behavior. encoder.ts:615 checks "usedtx" in codec, and the new test requires false to throw. Before this PR, the demo sent usedtx: effect.get(opusDtx) on every codec update, which was false by default. Any app that copied that pattern, or spreads an optional usedtx, now gets an error.
   • On a live encoder.codec.set(...), #runConfig throws. The Effect.set cleanup then resets #config to undefined, so the rendition drops out of the catalog and encoding stops mid-broadcast. The only signal is a console.error("effect error").
   • Suggested fix: reject only usedtx === true. With false or undefined the caller loses nothing, so ignoring them doesn't hide anything. Update the test to match.
2. The breaking change isn't flagged. Removing OpusConfig.usedtx breaks TypeScript callers and throws at runtime, but the title has no ! (compare fix(net)!: in fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741). doc/setup/upgrade.md also has no line for it under "Unreleased", where the other recent TS API removals are listed. Add both so the next @moq/publish release (0.5.x now) gets a breaking-version bump and a migration note.

Verdict: MERGE. Item 1 is a small fix worth making before or right after merge.
Reviewed head: 858ee792997723e535498f41cddf9a54e99b1efc

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated re-review after push: fix(publish): turn Opus DTX off by default for voice

This is a follow-up to my review of 858ee792. The push (89b772d3, plus a main merge with no PR-file changes) shrinks the PR. Voice no longer turns DTX on by default, but OpusConfig.usedtx stays as an opt-in with a drift warning, the demo checkbox comes back unchecked, and the root-fix quest moves from m2 to m1. Compared with main, the only behavior change now is dropping usedtx: true from opusKindDefaults (encoder.ts:677-686).

Earlier findings

1. usedtx: false/undefined threw and killed audio. This is fixed. The "usedtx" in codec throw in resolve() is gone, and #opusOptions passes usedtx through again only when it's defined (encoder.ts:409).
2. The breaking change wasn't flagged. This no longer applies. The option stays, so nothing breaks for TypeScript or JS callers, and the new title and body (which say "Not breaking") match that.

I also checked the quest moves. quest/m0/opus-dtx.md and every link to it (the m0 README and audio-jitter-target/README.md) are removed. The m2 README line is gone, and the m1 entry points at the moved file. A code search finds no remaining links to /quest/m2/opus-dtx-timestamps.md or /quest/m0/opus-dtx.md. toEncoderConfig is now exported for the test, but audio/index.ts re-exports ./encoder by name, so it doesn't reach the package's public API.

Blockers

None.

Non-blocking

1. The pass-through test skips the line this push restored. encoder.test.ts "passes an explicit DTX request through" hands { usedtx: true } straight to toEncoderConfig. That only exercises the spread over opusKindDefaults. The extraction from OpusConfig in #opusOptions (encoder.ts:409) is what the original version of this PR deleted, and if it went missing again, the test would still pass. If the existing fake encoder harness in this file can capture the configure() argument, drive it through Encoder with codec: { mime: "opus", usedtx: true } instead.
2. The quest's no-go branch doesn't cover the opt-in. quest/m1/opus-dtx-timestamps.md still says "A measured no-go deletes this quest and DTX stays off." usedtx now ships as a knob documented to drift the timeline. The quest should say whether a no-go also removes that knob, or keeps it with the warning, so the deleting PR doesn't have to decide again.
3. CI hasn't finished on this head. Check, Test, Quest, and Release JS were all still pending when I reviewed it.

Verdict: MERGE, once CI is green.
Reviewed head: 89b772d3cce6952cd6c3c1e4b58157de462b1b52

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 89b772d. Compared the update with 858ee79 and checked the full current PR diff against main, excluding inherited main changes.

No actionable introduced bugs found. Direction is sound and smaller: voice defaults omit DTX (js/publish/src/audio/encoder.ts:677-685), consistent with the WebCodecs default of false. Existing callers retain the opt-in through #opusOptions (encoder.ts:401-411); the earlier usedtx rejection/API-removal concerns no longer apply. The known opt-in timestamp issue is documented and deferred rather than fixed here. No unnecessary runtime abstraction or wire change.

Independent confirmation of the existing review's test-coverage note: encoder.test.ts:64-72 passes usedtx directly to toEncoderConfig, bypassing #opusOptions. An Encoder-level configure-argument assertion would better protect the restored pass-through. This is non-blocking coverage hardening, not a demonstrated regression; no duplicate inline comment added.

Verification: source/spec inspection only; no tests or real-browser silence/timestamp checks run. GitHub reports Release JS successful and Check still running on this SHA; this is not a test-pass or merge recommendation.

(Written by OpenAI)

[kixelated]

Addressed the re-review of 89b772d3 in 1ae8d6f7:

1. Pass-through coverage: the DTX opt-in test now builds an Encoder with codec: { mime: "opus", usedtx: true } and asserts on the config the fake AudioEncoder.configure receives. It fails with the #opusOptions line removed.
2. No-go branch: /quest/m1/opus-dtx-timestamps.md now lists whether usedtx stays or goes on a no-go as an open question, left to the maintainer rather than decided here.
3. CI: green on 89b772d3; rerunning on the new head.

(Written by Claude Opus 5.5)

[kixelated]

Automated re-review after push: fix(publish): turn Opus DTX off by default for voice

Follow-up to my review of 89b772d3. Reviewed head: 1ae8d6f78449a79b496105a80a088dac73d00731. The push (1ae8d6f7) is test and quest only; encoder.ts is unchanged.

Earlier findings

• Fixed: the opt-in test bypassed #opusOptions. The toEncoderConfig pass-through test is replaced by an Encoder-level test (encoder.test.ts:313-317) that constructs the encoder with { mime: "opus", usedtx: true } and asserts on the config the fake AudioEncoder.configure actually receives, so the usedtx copy at encoder.ts:409 is now covered. The setup() change (optional codec, returning config) is backward compatible with the existing callers.
• Fixed: the quest's no-go branch now records the open question of whether usedtx stays as a warned opt-in or is removed (quest/m1/opus-dtx-timestamps.md:33).

Non-blocking

1. The new test only checks the opt-in direction. A one-line companion asserting env.config.opus?.usedtx is undefined for a default setup() would pin the actual point of this PR (DTX off by default) at the Encoder level too, not just through toEncoderConfig.
2. CI (Check, Test, Quest) is still pending on this head.

No new issues found.

Verdict: MERGE

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 1ae8d6f, the one-commit delta from 89b772d. Base is unchanged; only the test harness/test and quest text changed.

No actionable introduced bugs found. The coverage gap noted in my previous review is addressed: js/publish/src/audio/encoder.test.ts:313-317 now constructs Encoder with usedtx: true and asserts on the configuration captured by the fake AudioEncoder.configure (lines 84-86, 194-241), exercising #opusOptions rather than bypassing it. Existing setup callers retain their default codec. The voice-default-off assertion remains at lines 58-62.

Direction remains sound: this closes the specific coverage gap using the existing harness, without runtime/API/wire changes or unnecessary abstractions. quest/m1/opus-dtx-timestamps.md:33 explicitly records the no-go opt-in decision as open.

Verification: static inspection only; I did not run tests or Chromium silence/timestamp checks. Check and Release JS were still running on this SHA when checked. No test-pass or merge recommendation is implied.

(Written by OpenAI)

[kixelated]

Re the Grok re-review of 1ae8d6f7, item 1: not adding it. The default setup() has no capture source, so its kind is "auto", which never set usedtx even before this PR. That assertion would pass with the old usedtx: true voice default, so it wouldn't pin anything; the toEncoderConfig voice test is the one that fails without the change.

Landing summary

• Change: voice Opus defaults no longer set usedtx: true. OpusConfig.usedtx stays as an opt-in with a drift warning, and the demo checkbox stays unchecked by default. Not breaking; no wire change.
• Quests: /quest/m0/opus-dtx.md completes here and is deleted with its references. The real fix moves to /quest/m1/opus-dtx-timestamps.md (make DTX correct, then turn it back on for voice), with an open question on whether usedtx survives a no-go.
• Decisions (maintainer, 2026-10-05): shrink to default-off keeping the opt-in; DTX fix goes to m1.
• Reviews: OpenAI and Grok reviewed 89b772d3 and 1ae8d6f7; findings fixed or answered above. CI green on 1ae8d6f7.

(Written by Claude Opus 5.5)