fix(preconnect): send audio buffer with resolved track id - #1061
Conversation
The buffer send was triggered by the agent becoming active, which can happen before the microphone publish resolves the local track sid. The buffer then carried an empty trackId attribute and agents dropped it, losing the prelude and falling back to live mic after a timeout. Invert the trigger: the send now starts when the recorder's track is published (sid in hand) and waits for the first active agent, mirroring the JS SDK. Exactly-once sending falls out of atomic stream ownership, replacing the racy sent flag. Move the tests to LiveKitCoreTests using an injected TestAudioTrack recorder so they run headless, and cover attributes, dedup, flush, and guard paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
E2E validation against a local server + real
|
…gered send A flush taking the stream between the publish trigger's guard and the send's atomic claim surfaced an internal "already consumed" error to the consumer's onError handler. Treat a nil stream at claim time as expected teardown and return silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Validated on iOS + Bluetooth HFP (the original repro path). Cherry-picked both commits onto 2.15.2 and ran a real connect. Fix works end-to-end. Client (Swift): the send is now triggered by publish, with the sid resolved before it goes out — No empty- Agent (Python, livekit-agents 1.5.17): the buffer now arrives with a track id and is bound instead of dropped — The This is the correct fix for the empty- |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| let claim = try state.mutate { state -> (recorder: LocalAudioTrackRecorder, audioStream: LocalAudioTrackRecorder.Stream)? in | ||
| guard let recorder = state.recorder else { | ||
| throw LiveKitError(.invalidState, message: "Recorder is nil") | ||
| } | ||
| guard let audioStream = state.audioStream.take() else { return nil } | ||
| return (recorder, audioStream) | ||
| } |
There was a problem hiding this comment.
🔴 Pre-connect audio feature relies on a helper that does not exist in the codebase
The audio stream is handed over using a helper that was never added to the project (.take() at Sources/LiveKit/Core/PreConnectAudioBuffer.swift:122 and :164), so the pre-connect audio code cannot be built at all.
Impact: The SDK fails to build, so nothing that depends on it ships.
Missing Optional.take() helper used for stream ownership transfer
The final commit of this PR ("refactor(preconnect): use Optional.take() for stream ownership transfer") introduces $0.audioStream.take() in stopRecording(flush:) (Sources/LiveKit/Core/PreConnectAudioBuffer.swift:122) and state.audioStream.take() in sendAudioData(to:trackSid:agents:on:) (Sources/LiveKit/Core/PreConnectAudioBuffer.swift:164).
Swift's standard library has no Optional.take() (that is a Rust idiom), and a repo-wide search finds no extension Optional and no func take definition anywhere in Sources/, Tests/, or test support. The two call sites in this file are the only occurrences of .take() in the repository.
The intended semantics are "read the value and set the optional to nil atomically within the same state.mutate". Either add a small mutating func take() -> Wrapped? extension on Optional under Sources/LiveKit/Support/, or inline the read-then-nil inside the existing state.mutate blocks.
Prompt for agents
The pre-connect buffer now transfers ownership of the recorded audio stream with `audioStream.take()` in two places: `stopRecording(flush:)` (Sources/LiveKit/Core/PreConnectAudioBuffer.swift:122) and the internal `sendAudioData(to:trackSid:agents:on:)` (Sources/LiveKit/Core/PreConnectAudioBuffer.swift:164). No `take()` method exists on `Optional` in the Swift standard library, and a repo-wide search shows no `extension Optional` or `func take` defined anywhere in this project, so these call sites do not resolve. Fix by either adding a small, documented `mutating func take() -> Wrapped?` extension on `Optional` in Sources/LiveKit/Support/ (returning the current value and setting self to nil), or by inlining the equivalent read-then-nil logic inside the existing `state.mutate` blocks so the ownership transfer stays atomic under the StateSync lock.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
|
Unrelated test failures fixed elsewhere. |
Fixes #1060.
Root cause
The pre-connect buffer send was triggered by the agent becoming
.active, which can happen as early as the join response — before the microphone publish resolves the local track's sid (assigned only at the end of_publish, inParticipant.add(publication:)). The buffer then went out with an emptytrackIdattribute, and agents drop such buffers (pre-connect audio received but no trackId), falling back to live mic after a 3s timeout and losing the prelude.Fix
Invert the trigger, mirroring the JS SDK: the send now starts when the recorder's track is published (
didPublishTrack, sid in hand) and waits for the first active agent (AsyncCompleter<Participant.Identity>resumed on agent activation, with a fast path viaagentParticipantsfor agents already active). The race is structurally impossible rather than guarded.Hardening that falls out of the new shape:
state.mutate(replaces the racy check-then-actsentflag); the flush path takes ownership the same way, so the stream can never have two consumers.onError).lk.publish_on_behalf) are excluded from agent selection, consistently withRoom.agentParticipants.LocalAudioTrackRecorder.renderno longer callsassertionFailureon conversion failure (crashes consumer debug builds); logs once per recording instead.Public API surface is unchanged.
sendAudioData(to:agents:on:)keeps its silent-no-op behavior for duplicate/empty calls; calling it before the track is published now throws.invalidStateinstead of sending a buffer the agent can't bind.Known limitation (pre-existing outcome): if the only agent activates and then leaves before the publish completes, the buffer is sent to the departed agent and lost — same result as the old send-at-activation behavior when an agent dies mid-flow.
Tests
Moved
PreConnectAudioBufferTestsfrom the device-onlyLiveKitAudioTeststoLiveKitCoreTests—TestAudioTrackplus an injectedLocalAudioTrackRecordermake the whole flow runnable headless (audio fed viarender(pcmBuffer:)). Coverage: attributes/data/exactly-once (including duplicatedidPublishTrackre-fire), timeout-flush error path, recorder guards, and a regression test for the #1060 ordering (agent active before publish) that fails onmainwithtrackId == "".🤖 Generated with Claude Code