Move frame processor url/token/stream info to client sdk#1881
Conversation
|
1egoman
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| private createStream(track: RemoteTrack): ReadableStream<AudioFrame> { | ||
| return new AudioStream(track, { | ||
| sampleRate: this.sampleRate, | ||
| numChannels: this.numChannels, | ||
| noiseCancellation: this.frameProcessor || this.noiseCancellation, | ||
| // Don't let the AudioStream close the processor when the track switches — | ||
| // this input stream owns the processor across track changes and closes it | ||
| // itself in close(). | ||
| autoCloseNoiseCancellation: false, | ||
| // TODO(AJS-269): resolve compatibility issue with node-sdk to remove the forced type casting | ||
| }) as unknown as ReadableStream<AudioFrame>; |
There was a problem hiding this comment.
🚩 Removal of onStreamInfoUpdated/onCredentialsUpdated depends on rtc-node handling them internally
The old code explicitly called this.frameProcessor?.onStreamInfoUpdated(...) and this.frameProcessor?.onCredentialsUpdated(...) when a track was subscribed (_input.ts:156-164 in old code), and refreshed credentials on TokenRefreshed events. The new code removes all of these, relying on the AudioStream constructor in @livekit/rtc-node to handle stream info and credential propagation internally when it receives the frameProcessor via the noiseCancellation option. This is a correctness assumption that can't be verified from this repo alone — it depends on the AudioStream implementation in @livekit/rtc-node@^0.13.27 actually performing these calls. If the SDK version doesn't handle this internally, the frame processor (e.g., server-side noise cancellation) would not receive the participant/room/credential context it needs to function.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Don't let the AudioStream close the processor when the track switches — | ||
| // this input stream owns the processor across track changes and closes it | ||
| // itself in close(). | ||
| autoCloseNoiseCancellation: false, |
There was a problem hiding this comment.
🚩 autoCloseNoiseCancellation is a new option that must be supported by the rtc-node SDK
The autoCloseNoiseCancellation: false option at _input.ts:167 is passed to the AudioStream constructor. This option doesn't appear anywhere else in this repository — it's a new SDK feature. The catalog pins @livekit/rtc-node to ^0.13.27. If this option was only added in a newer minor/patch version not yet published, the option would be silently ignored (since JS doesn't validate extra constructor options), and the AudioStream would still auto-close the processor on track switches, leading to a use-after-close when the next track tries to use the same processor. Worth confirming the SDK version supports this flag.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Description
Ports livekit/agents#5867 to node. For more details about this change, read the pull request body / comments there.
TODO
node-sdksto the ^ above, which will fix the ci build errorPre-Review Checklist
Testing
restaurant_agent.tsandrealtime_agent.tswork properly (for major changes)Additional Notes
Note to reviewers: Please ensure the pre-review checklist is completed before starting your review.