Fix bug where a local audio track unpublish would not cause the corresponding remote audio track's AudioStreams to stop iterating#672
Draft
1egoman wants to merge 8 commits into
Draft
Conversation
🦋 Changeset detectedLatest commit: c31a6ee The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
3e273e0 to
c31a6ee
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
Right now, this pull request is based on top of another one here, and ties into some of this pull request's new infrastructure.
Before an eventual merge, this needs to be cleaned up. But keep this in mind when reviewing - the changes which are specific to this change cleanly are segmented into 28da338 and c31a6ee.
Problem
When a remote peer unpublishes an audio track, a subscriber consuming the resulting
AudioStreamviafor await (const frame of stream)hangs indefinitely - the loop never receives an end-of-stream and the process can't exit. Callingiterator.return()orstream.cancel()to force termination doesn't help either - the in flight promise returned byreader.read()never resolves.Root cause
The native FFI side only emits
eoson the audio stream when one of: (a) the stream's own FFI handle is disposed, (b) the track's FFI handle is disposed, or (c) the underlying libwebrtc receiver gracefully ends. None of these reliably fire on a remote unsubscribe - the SDK keeps the track handle alive (the consumer may still want to readtrack.sidetc.), and (according to a LLM) the libwebrtc receiver typically keeps the stream "open" with silence rather than ending it.On the SDK side,
AudioStreamSource.cancel()previously disposed the FFI handle but never closed theReadableStreamcontroller. The nativeeos(if it ever arrived) would have triggered the close - but since the event listener was removed at the top ofcancel(), even a lateeoswas silently dropped. So any pendingreader.read()had nothing to resolve it.Fix
Two changes in
packages/livekit-rtc/src/:audio_stream.ts: collapse the three cleanup paths ('eos'event, consumercancel(), and a newcloseFromTrack()entry point) into oneteardown()method that closes theReadableStreamDefaultControllerfirst. Closing the controller synchronously resolves any pendingreader.read()with{done: true}, sofor await/iterator.return()unblock without relying on the nativeeosarriving.track.ts: when aTrackis detached from itsRoom(the natural signal that no more frames will arrive on its streams), callcloseFromTrack()on each registeredAudioStream. This gives consumers a clean end-of-stream even when the native layer doesn't emit one.TODO