Repository navigation
feat(sdk): report when a listener can hear every speaker who was live at join - #795
realhasanshoaib wants to merge 4 commits into
Conversation
… at join media.audioReadiness is connecting, ready or degraded. Ready needs increasing inbound packets for each remote microphone live at join (reusing the receive-stall sampler), or a live media connection when nobody speaks. Later speakers never move it back; the ceiling (default 15 s) yields degraded, which can still become ready. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ation id yet Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
realhasanshoaib
added
risk:medium
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return [...this.#remoteTracks.values()].map(({ participantId, source, publicationId }) => ({ participantId, source, publicationId, enabled: true })); | ||
| } | ||
| #publishSnapshot(phase = this.#snapshot.connection.phase, failure = this.#snapshot.failure): void { | ||
| this.#audioReadiness.connection(phase === "live"); |
There was a problem hiding this comment.
🔴 Empty-room readiness precedes media connection
On a receive-only join to an empty room, audioReadiness becomes ready without an established peer connection. #activatePreparedTracks sets phase live even when no tracks are negotiated, so the empty cohort passes the live check.
Learn more
An empty cohort can report ready only after the media peer has actually connected. With microphone and camera off, #publishPreparedTracks returns without negotiating a local track. #activatePreparedTracks nevertheless marks the media phase live. The new readiness bridge treats that phase as proof of a connected peer, so an empty Sync publication list completes readiness while the peer connection is still new.
Example: A listener joins an empty Space with { microphone: false, camera: false }. Sync sends []; media startup marks phase live; readiness changes to ready before any peer connection is established.
Recommended fix: Gate empty-cohort readiness on connectionIsLive(this.#connection), and publish the resulting transition from peer connection state changes. Preserve the existing behavior for cohorts with actual packet evidence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| this.#audioReadyCeilingTimer ??= globalThis.setTimeout(() => { | ||
| if (this.#audioReadiness.ceilingPassed()) this.#publishSnapshot(); | ||
| }, this.#audioReadyCeilingMs); |
There was a problem hiding this comment.
🔍 Readiness ceiling includes pre-entry work
start begins the ceiling before capture finishes or Sync confirms entry. A slow entrance can consume the readiness budget before the listener joins; confirm whether that timing is intended.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99cf5ffc91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const { stalled, advanced } = this.#receiveProgress.sample(performance.now(), connection, report, this.#remoteTracks.values()); | ||
| for (const publicationId of advanced) this.#remotePullRetries.delete(publicationId); | ||
| this.#audioReadiness.advanced(advanced); |
There was a problem hiding this comment.
If a listener switches tabs before readiness is established, audio can be flowing normally but this update is never reached: #canObserveReceiveProgress rejects hidden documents and #observeReceiveProgress returns before sampling. The ceiling timer still runs, so the snapshot eventually reports degraded and cannot become ready until the tab is foregrounded, even when every expected microphone is audible. Keep the hidden-tab safeguard for stall detection/recovery, but allow positive packet-progress evidence to establish readiness independently.
Useful? React with 👍 / 👎.
| this.#publicationOwners = new Map(microphones.flatMap((publication) => (publication.publicationId === null ? [] : [[publication.publicationId, publication.participantId] as const]))); | ||
| const present = new Set(microphones.map((publication) => publication.participantId)); | ||
| if (this.#cohort === null) this.#cohort = new Map([...present].map((participantId) => [participantId, false])); | ||
| else for (const participantId of this.#cohort.keys()) if (!present.has(participantId)) this.#cohort.delete(participantId); |
There was a problem hiding this comment.
If Alice's audio advances while Bob is still pending, then Alice republishes under a new publication ID, her cohort entry remains true even though #publicationOwners now references the replacement. When Bob's packets advance, the tracker reports ready without receiving any packets from Alice's replacement. This is reproducible directly with the tracker, and remote-track reconciliation retires Alice's old track when its publication is replaced, so the previous evidence no longer establishes that she can be heard. While readiness is still pending, invalidate a member's heard flag when their publication changes; preserve the intended sticky behavior after ready.
Useful? React with 👍 / 👎.
…o readiness Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…udio is ready Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Review findings, answered:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ed55d842f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| connection(live: boolean): boolean { | ||
| this.#connectionLive = live; | ||
| return this.#evaluate(); |
There was a problem hiding this comment.
If Alice has been heard while Bob is still pending and the listener's peer connection is replaced, Alice's evidence survives even though her receiver has been destroyed. Both restart() and #replaceMediaConnectionForRecovery() dispose the old peer and clear remote tracks, but only the receive-stall tracker is reset. Because remote publication IDs can remain unchanged, receiving Bob on the replacement peer then reports ready even if Alice's replacement receiver never receives packets. Invalidate pre-ready evidence on peer replacement while preserving the cohort and the intended sticky behavior after ready.
Useful? React with 👍 / 👎.
A listener can sit in a call and hear nothing, with no sign that audio is still starting. The app has no way to tell "silent room" from "audio not connected yet".
Fix
snapshot.media.audioReadiness:connecting,readyordegraded.readymeans inbound audio packets are increasing for every remote microphone that was on when this person joined. If nobody was speaking, it means the media connection is up.readylistener back toconnecting. A speaker who mutes or leaves before ready stops being waited for. A speaker who republishes before ready must be heard again on the new publication.degradedmeans a ceiling passed first (15 s, oraudioReadyCeilingMsin the client options). It keeps updating, so it can still becomeready, and recovery keeps running underneath.advancedresult of the existing receive-stall sampler (media/receive-stall.ts), so "ready" is at least two consecutive samples after the first packet.ready. Total time is unchanged; silence becomes a visible wait.What changed
Changed: SDK media client,
media.audioReadinesson the space snapshot, client options, SDK README.Untouched: Server, signalling, receive-stall recovery behavior, React components (only the preview fixture gets the new field).
Proof
diag/audio-ready-rig, SDK at99cf5ffc9; the later commits move a type export and a helper, and make a republished microphone count as unheard until its new publication delivers packets). "Audible" is the first sample with a bound, playing audio element and audio level above zero.readybefore first audiblereadyafter last speaker audible, p50 / p95degradedAn earlier run of 20 open rooms (40 cohort joins) on the commit before the last fix also had 0 early
readyand 0degraded. Each cohort join needs two consecutive packet samples, soreadytrails first audio by about 1.3 to 1.6 s.degradedpath on a real network. No join reached the ceiling, so it is covered by the fake-timer tests only.audio-ready-shaper.pydelays, jitters, drops and rate-limits UDP between a browser network namespace and the host). The Instabox kernel has no netem, so the usualtcrig could not run. The shaper counters show the loss rates applied (about 2% and about 8%). Open ran inside the same namespace for 8 rooms and on the host for the first 20.Created with Claude Sonnet 5.5 in Claude Code.
🤖 Generated with Claude Code