Handle data track SID reassignment#2000
Conversation
🦋 Changeset detectedLatest commit: 717f2ad 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 |
size-limit report 📦
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| case 'active': | ||
| // The SFU does not carry subscriptions across a publisher's full | ||
| // reconnect; re-request the subscription under the new SID. | ||
| this.emit('sfuUpdateSubscription', { sid: newSid, subscribe: true }); |
There was a problem hiding this comment.
is there a scenario in which the user might have unsubscribed to the data track and we're re-forcing it to subscribe here?
There was a problem hiding this comment.
One possibility that comes to mind:
- A user completes their final subscription for a data track, which calls
IncomingDataTrackManager.unSubscribeRequest(see here). but otherwise leaves the descriptor inactivetype - A SFU full reconnect occurs, and the client must re subscribe via the new SFU to all remote data tracks
- Because of the
activetype on the descriptor, the unsubscribed data track from 1 is resubscribed
@ladvoc Given this, maybe it's worth introducing a unsubscribing or similar descriptor type which can mark that the descriptor is on its way out and should be dropped during a reconnect. Or maybe another way to do this could be to only republish if descriptor.streamControllers.size > 0. Thoughts? I think this would also need to be done on the rust one too, as that also only has the three none / pending / active states.
There was a problem hiding this comment.
unSubscribeRequest doesn't leave the descriptor in the active state:
client-sdk-js/src/room/data-track/incoming/IncomingDataTrackManager.ts
Lines 407 to 409 in 3da1eb3
1egoman
left a comment
1egoman
left a comment
There was a problem hiding this comment.
Talked about it in a 1:1, generally makes sense
main squash-merged the data-channel watermark work (#2013), so its inline RTCEngine changes conflicted with this branch's refactored data-channel module. Resolved RTCEngine.ts / RTCEngine.test.ts in favour of the module version (which already carries all of that work), and took main's other additions cleanly — notably the data-track SID reassignment fix (#2000) this branch didn't yet have. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
Equivalent to: livekit/rust-sdks#1228
Closes BOT-457