-
Notifications
You must be signed in to change notification settings - Fork 278
Handle data track SID reassignment #2000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "livekit-client": patch | ||
| --- | ||
|
|
||
| Handle data track SID reassignment |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -147,6 +147,10 @@ export default class IncomingDataTrackManager extends (EventEmitter as new () => | |||||||
| let streamController: ReadableStreamDefaultController<DataTrackFrame> | null = null; | ||||||||
| const sfuSubscriptionComplete = new Future<void, DataTrackSubscribeError>(); | ||||||||
|
|
||||||||
| // Hold the descriptor by reference: SID reassignment re-keys the map, so lookups | ||||||||
| // with the SID captured at subscribe time would fail. | ||||||||
| const descriptor = this.descriptors.get(sid); | ||||||||
|
|
||||||||
| const detachSignal = () => { | ||||||||
| signal?.removeEventListener('abort', onAbort); | ||||||||
| }; | ||||||||
|
|
@@ -158,8 +162,7 @@ export default class IncomingDataTrackManager extends (EventEmitter as new () => | |||||||
| log.warn(`ReadableStream subscribed to ${sid} was not started.`); | ||||||||
| return; | ||||||||
| } | ||||||||
| const descriptor = this.descriptors.get(sid); | ||||||||
| if (!descriptor) { | ||||||||
| if (!descriptor || this.descriptors.get(descriptor.info.sid) !== descriptor) { | ||||||||
| log.warn(`Unknown track ${sid}, skipping cancel...`); | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
@@ -180,9 +183,8 @@ export default class IncomingDataTrackManager extends (EventEmitter as new () => | |||||||
| if (!streamController) { | ||||||||
| return; | ||||||||
| } | ||||||||
| const currentDescriptor = this.descriptors.get(sid); | ||||||||
| if (currentDescriptor?.subscription.type === 'active') { | ||||||||
| currentDescriptor.subscription.streamControllers.delete(streamController); | ||||||||
| if (descriptor?.subscription.type === 'active') { | ||||||||
| descriptor.subscription.streamControllers.delete(streamController); | ||||||||
| } | ||||||||
|
|
||||||||
| streamController.error(DataTrackSubscribeError.cancelled()); | ||||||||
|
|
@@ -198,8 +200,7 @@ export default class IncomingDataTrackManager extends (EventEmitter as new () => | |||||||
|
|
||||||||
| this.subscribeRequest(sid, signal) | ||||||||
| .then(async () => { | ||||||||
| const descriptor = this.descriptors.get(sid); | ||||||||
| if (!descriptor) { | ||||||||
| if (!descriptor || this.descriptors.get(descriptor.info.sid) !== descriptor) { | ||||||||
| log.error(`Unknown track ${sid}`); | ||||||||
| const err = DataTrackSubscribeError.disconnected(); | ||||||||
| controller.error(err); | ||||||||
|
|
@@ -330,7 +331,7 @@ export default class IncomingDataTrackManager extends (EventEmitter as new () => | |||||||
| descriptor.subscription = { type: 'none' }; | ||||||||
|
|
||||||||
| // Let the SFU know that the subscribe has been cancelled | ||||||||
| this.emit('sfuUpdateSubscription', { sid, subscribe: false }); | ||||||||
| this.emit('sfuUpdateSubscription', { sid: descriptor.info.sid, subscribe: false }); | ||||||||
|
|
||||||||
| if (previousDescriptorSubscription.type === 'pending') { | ||||||||
| previousDescriptorSubscription.completionFuture.reject?.( | ||||||||
|
|
@@ -458,6 +459,9 @@ export default class IncomingDataTrackManager extends (EventEmitter as new () => | |||||||
| if (this.descriptors.has(info.sid)) { | ||||||||
| continue; | ||||||||
| } | ||||||||
| if (this.handleSidReassigned(publisherIdentity, info)) { | ||||||||
| continue; | ||||||||
| } | ||||||||
| await this.handleTrackPublished(publisherIdentity, info); | ||||||||
| } | ||||||||
| publisherParticipantToSidsInUpdate.set(publisherIdentity, sidsInUpdate); | ||||||||
|
|
@@ -499,6 +503,62 @@ export default class IncomingDataTrackManager extends (EventEmitter as new () => | |||||||
| this.emit('trackPublished', { track }); | ||||||||
| } | ||||||||
|
|
||||||||
| /** | ||||||||
| * Detects and handles SID reassignment, which occurs when the publisher | ||||||||
| * republishes its tracks after a full reconnect. | ||||||||
| * | ||||||||
| * Returns `true` if an SID reassignment occurred, `false` otherwise. | ||||||||
| */ | ||||||||
| private handleSidReassigned( | ||||||||
| publisherIdentity: Participant['identity'], | ||||||||
| info: DataTrackInfo, | ||||||||
| ): boolean { | ||||||||
| // Publisher identity and pub handle are stable across republications. | ||||||||
| const existingEntry = Array.from(this.descriptors.entries()).find( | ||||||||
| ([_sid, descriptor]) => | ||||||||
| descriptor.publisherIdentity === publisherIdentity && | ||||||||
| descriptor.info.pubHandle === info.pubHandle, | ||||||||
| ); | ||||||||
| if (!existingEntry) { | ||||||||
| return false; | ||||||||
| } | ||||||||
| const [oldSid, descriptor] = existingEntry; | ||||||||
|
|
||||||||
| // Invariant: other than SID, info should not have changed. | ||||||||
| // TODO: consider refactoring to move SID out of info to allow for direct comparison. | ||||||||
| const { name, usesE2ee } = descriptor.info; | ||||||||
| if (name !== info.name || usesE2ee !== info.usesE2ee) { | ||||||||
| log.warn(`Info mismatch for ${oldSid}, treating as new publication`); | ||||||||
| return false; | ||||||||
| } | ||||||||
|
|
||||||||
| const newSid = info.sid; | ||||||||
| log.debug(`SID reassigned: ${oldSid} -> ${newSid}`); | ||||||||
|
|
||||||||
| if (!this.descriptors.delete(oldSid)) { | ||||||||
| return false; | ||||||||
| } | ||||||||
| descriptor.info.sid = newSid; | ||||||||
|
|
||||||||
| switch (descriptor.subscription.type) { | ||||||||
| case 'none': | ||||||||
| break; | ||||||||
| case 'pending': | ||||||||
| 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 }); | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a scenario in which the user might have unsubscribed to the data track and we're re-forcing it to subscribe here?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One possibility that comes to mind:
@ladvoc Given this, maybe it's worth introducing a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
client-sdk-js/src/room/data-track/incoming/IncomingDataTrackManager.ts Lines 407 to 409 in 3da1eb3
|
||||||||
| break; | ||||||||
| } | ||||||||
| if (descriptor.subscription.type === 'active') { | ||||||||
| // Keep the routing index consistent until the SFU assigns a new handle | ||||||||
| // (see `registerSubscriberHandle`). | ||||||||
| this.subscriptionHandles.set(descriptor.subscription.subcriptionHandle, newSid); | ||||||||
| } | ||||||||
| this.descriptors.set(newSid, descriptor); | ||||||||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||||||||
| return true; | ||||||||
| } | ||||||||
|
|
||||||||
| handleTrackUnpublished(sid: DataTrackSid) { | ||||||||
| const descriptor = this.descriptors.get(sid); | ||||||||
| if (!descriptor) { | ||||||||
|
|
||||||||
Uh oh!
There was an error while loading. Please reload this page.