Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/negotiated-data-channels.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'livekit-client': minor
---

Create the publisher data channels as negotiated (fixed IDs) in single peer connection mode when the server supports it (protocol 18), saving the data channel open round trip. Falls back to in-band channels against older servers.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
},
"dependencies": {
"@livekit/mutex": "1.1.1",
"@livekit/protocol": "1.52.1",
"@livekit/protocol": "1.53.0",
"events": "^3.3.0",
"jose": "^6.1.0",
"loglevel": "^1.9.2",
Expand All @@ -94,6 +94,7 @@
"@eslint/js": "10.0.1",
"@livekit/changesets-changelog-github": "^0.2.0",
"@livekit/throws-transformer": "^0.1.3",
"@nbilyk/downlevel-dts": "^0.15.2",
"@rollup/plugin-babel": "7.1.0",
"@rollup/plugin-commonjs": "29.0.3",
"@rollup/plugin-json": "6.1.0",
Expand All @@ -111,7 +112,6 @@
"@typescript/native": "catalog:",
"@vitest/browser": "^4.1.10",
"@vitest/browser-playwright": "^4.1.10",
"@nbilyk/downlevel-dts": "^0.15.2",
"eslint": "10.7.0",
"eslint-config-airbnb-extended": "^2.3.2",
"eslint-config-prettier": "10.1.8",
Expand Down
12 changes: 6 additions & 6 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions src/room/RTCEngine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,40 @@ describe('RTCEngine', () => {

internals.dataChannels.lossy.stopThresholdTuning();
});

it.each([
['publisher-only', 18, true],
['publisher-only', 17, false],
['publisher-only', undefined, false],
['subscriber-primary', 18, false],
])(
'mode %s with server protocol %s creates negotiated channels: %s',
(mode, protocol, expected) => {
const engine = new RTCEngine(roomOptionDefaults);
const pcManager = {
mode,
createPublisherDataChannel: vi.fn((label: string) =>
Object.assign(new FakeDataChannel(), { label }),
),
};
Object.assign(engine as unknown as Record<string, unknown>, {
pcManager,
latestJoinResponse: { serverInfo: protocol === undefined ? undefined : { protocol } },
});
const internals = engine as unknown as {
createDataChannels: () => void;
dataChannels: {
publisherChannelsNegotiated: boolean;
lossy: { stopThresholdTuning: () => void };
};
};

internals.createDataChannels();

expect(internals.dataChannels.publisherChannelsNegotiated).toBe(expected);
internals.dataChannels.lossy.stopThresholdTuning();
},
);
});

describe('handleDataChannelClose', () => {
Expand Down
31 changes: 26 additions & 5 deletions src/room/RTCEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import type { InternalRoomOptions } from '../options';
import type { NonSharedUint8Array } from '../type-polyfills/non-shared-typed-arrays';
import TypedPromise from '../utils/TypedPromise';
import { TTLMap } from '../utils/ttlmap';
import { PROTOCOL_NEGOTIATED_DATA_CHANNELS } from '../version';
import PCTransport, { PCEvents } from './PCTransport';
import { PCTransportManager, PCTransportState } from './PCTransportManager';
import type { ReconnectContext, ReconnectPolicy } from './ReconnectPolicy';
Expand Down Expand Up @@ -379,7 +380,9 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
// createInitialOffer() defers setLocalDescription (via pendingInitialOffer) until the
// answer is applied, after updateConfiguration() has set the server's TURN servers.
await this.configure();
this.applyInitialPublisherLayout();
// The server protocol is unknown until the join response, so assume negotiated data
// channels and fall back to in-band ones below if the server is too old.
this.applyInitialPublisherLayout(true);
}
const offer = await this.pcManager?.publisher.createInitialOffer();
if (offer) {
Expand Down Expand Up @@ -414,6 +417,12 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
this.subscriberPrimary = joinResponse.subscriberPrimary;
if (sendOfferWithJoin) {
this.pcManager?.updateConfiguration(this.makeRTCConfiguration(joinResponse));
if (this.dataChannels.publisherChannelsNegotiated && !this.useNegotiatedDataChannels()) {
// The SCTP section is already in the offer, so in-band channels can be opened on it
// without renegotiation.
this.log.debug('server does not support negotiated data channels, using in-band ones');
this.createDataChannels(false);
}
} else {
if (!this.pcManager) {
// Deferred path (Firefox, and V0): configure with the join response so the PC picks up
Expand Down Expand Up @@ -902,8 +911,8 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
* present in that first offer, and the offer-with-join path needs the sections to
* build a meaningful initial offer. Must be called on a configured pcManager.
*/
private applyInitialPublisherLayout() {
this.createDataChannels();
private applyInitialPublisherLayout(negotiatedDataChannels?: boolean) {
this.createDataChannels(negotiatedDataChannels);
/**
* Native libwebrtc does not support pre-populating the media sections,
* so we skip it for React Native.
Expand Down Expand Up @@ -932,7 +941,19 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
}
}

private createDataChannels() {
/**
* Whether to create the publisher data channels as negotiated (fixed SCTP stream IDs), which
* lets the server send on them without waiting for our DCEP OPEN. Only in single peer
* connection mode: with dual peer connections the server opens its own channels anyway.
*/
private useNegotiatedDataChannels() {
return (
this.pcManager?.mode === 'publisher-only' &&
(this.latestJoinResponse?.serverInfo?.protocol ?? 0) >= PROTOCOL_NEGOTIATED_DATA_CHANNELS
);
Comment on lines +949 to +953

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Publisher data stops after server downgrade

When a session resumes on an older server, useNegotiatedDataChannels changes its verdict but leaves the negotiated channels open. The older server never receives their DCEP OPEN, so published data stops reaching it.

Learn more

A single-PC session with negotiated channels keeps its peer connection during signal resume. The resume response replaces latestJoinResponse.serverInfo, but channel recreation only runs for an open channel with a null ID in resumeConnection. Negotiated channels have fixed non-null IDs, so a resume onto a pre-18 server leaves them unchanged. That server does not use the fixed negotiated IDs and receives no in-band channel open message, while the client continues sending through the old channels.

Example: A client joins a protocol-18 server using negotiated ID 100 for reliable data. Its signaling session resumes on a protocol-17 server. The response updates the protocol to 17, but the client continues sending reliable packets on ID 100 instead of opening an in-band channel.

Recommended fix: On resume, compare the active publisher channel mode with the new server's protocol before completing reconnection. If it changed, replace the channels with the appropriate type and wait for the replacement to open; handle queued reliable messages and sync-state channel IDs during this transition.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a session resume is not expected to land on an older version.

}

private createDataChannels(negotiated: boolean = this.useNegotiatedDataChannels()) {
if (!this.pcManager) {
return;
}
Expand All @@ -943,7 +964,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
// them to open.
this.publisherConnectionPromise = undefined;
}
this.dataChannels.createPublisherChannels(this.pcManager);
this.dataChannels.createPublisherChannels(this.pcManager, negotiated);
}

private handleDataChannel = async ({ channel }: RTCDataChannelEvent) => {
Expand Down
36 changes: 36 additions & 0 deletions src/room/data-channel/DataChannelManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,42 @@ describe('DataChannelManager', () => {
manager.lossy.stopThresholdTuning();
});

it('creates negotiated channels with fixed ids and can fall back to in-band ones', () => {
const { manager, pcManager } = makeManager();
const createSpy = vi.mocked(pcManager.createPublisherDataChannel);

manager.createPublisherChannels(pcManager, true);

expect(manager.publisherChannelsNegotiated).toBe(true);
expect(createSpy).toHaveBeenCalledWith('_reliable', {
ordered: true,
negotiated: true,
id: 100,
});
expect(createSpy).toHaveBeenCalledWith('_lossy', {
ordered: false,
maxRetransmits: 0,
negotiated: true,
id: 101,
});
expect(createSpy).toHaveBeenCalledWith('_data_track', {
ordered: false,
maxRetransmits: 0,
negotiated: true,
id: 102,
});
Comment on lines +136 to +152

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Is there value in including the literal numbers like id: 102 here, or would it make more sense to make these constants like id: NegotiatedDataChannelID.NDCI_DATA_TRACK? No strong preference here.


createSpy.mockClear();
manager.createPublisherChannels(pcManager, false);

expect(manager.publisherChannelsNegotiated).toBe(false);
expect(createSpy).toHaveBeenCalledWith('_reliable', { ordered: true });
manager.teardown();
expect(manager.publisherChannelsNegotiated).toBe(false);

manager.lossy.stopThresholdTuning();
});

it('adopts subscriber channels by label and rejects unknown labels', () => {
const { manager, opts } = makeManager();

Expand Down
24 changes: 21 additions & 3 deletions src/room/data-channel/DataChannelManager.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { NegotiatedDataChannelID } from '@livekit/protocol';
import type { PCTransportManager } from '../PCTransportManager';
import { FlowControlledDataChannel } from './FlowControlledDataChannel';
import { LossyDataChannel } from './LossyDataChannel';
Expand Down Expand Up @@ -58,6 +59,8 @@ export class DataChannelManager {

private opts: DataChannelManagerOptions;

private negotiated = false;

constructor(opts: DataChannelManagerOptions) {
this.opts = opts;
const flowControlOptions = (kind: DataChannelKind) => ({
Expand Down Expand Up @@ -119,14 +122,23 @@ export class DataChannelManager {
);
}

/** Whether the current publisher channels were created as negotiated (fixed IDs). */
get publisherChannelsNegotiated(): boolean {
return this.hasPublisherChannels && this.negotiated;
}

/**
* Creates the three publisher data channels on the given transport, wires their handlers, and
* attaches them to the wrappers — attaching closes any replaced channel objects and rejects the
* waiters still parked on them.
*
* With `negotiated`, the channels use fixed SCTP stream IDs agreed with the server instead of
* the in-band DCEP open handshake, so the server can send on them right away.
*/
createPublisherChannels(pcManager: PCTransportManager) {
// Recreating (the Safari null-id path): attaching the new handles below closes the old ones,
// so unhook them first — the old channel's close must not report an unexpected close.
createPublisherChannels(pcManager: PCTransportManager, negotiated: boolean = false) {
// Recreating (the Safari null-id path, or the in-band fallback after join): attaching the new
// handles below closes the old ones, so unhook them first — the old channel's close must not
// report an unexpected close.
for (const channel of [this.lossy, this.reliable, this.dataTrack]) {
const old = channel.channelHandle;
if (old) {
Expand All @@ -152,18 +164,22 @@ export class DataChannelManager {
channel.attach(dc);
};

const idFor = (id: number) => (negotiated ? { negotiated, id } : {});

wire(
this.lossy,
pcManager.createPublisherDataChannel(lossyDataChannelLabel, {
ordered: false,
maxRetransmits: 0,
...idFor(NegotiatedDataChannelID.NDCI_LOSSY),
}),
this.opts.onDataMessage,
);
wire(
this.reliable,
pcManager.createPublisherDataChannel(reliableDataChannelLabel, {
ordered: true,
...idFor(NegotiatedDataChannelID.NDCI_RELIABLE),
}),
this.opts.onDataMessage,
);
Expand All @@ -172,9 +188,11 @@ export class DataChannelManager {
pcManager.createPublisherDataChannel(dataTrackDataChannelLabel, {
ordered: false,
maxRetransmits: 0,
...idFor(NegotiatedDataChannelID.NDCI_DATA_TRACK),
}),
this.opts.onDataTrackMessage,
);
this.negotiated = negotiated;

this.lossy.startThresholdTuning();
}
Expand Down
6 changes: 5 additions & 1 deletion src/version.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { version as v } from '../package.json';

export const version = v;
export const protocolVersion = 17;
export const protocolVersion = 18;

/** First server protocol version that creates the single peer connection data channels as
* negotiated (fixed SCTP stream IDs) instead of opening them in-band. */
export const PROTOCOL_NEGOTIATED_DATA_CHANNELS = 18;
Comment on lines 3 to +8

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Use the constant in protocolVersion rather than repeating 18 twice?

Suggested change
export const version = v;
export const protocolVersion = 17;
export const protocolVersion = 18;
/** First server protocol version that creates the single peer connection data channels as
* negotiated (fixed SCTP stream IDs) instead of opening them in-band. */
export const PROTOCOL_NEGOTIATED_DATA_CHANNELS = 18;
export const version = v;
/** First server protocol version that creates the single peer connection data channels as
* negotiated (fixed SCTP stream IDs) instead of opening them in-band. */
export const PROTOCOL_NEGOTIATED_DATA_CHANNELS = 18;
export const protocolVersion = PROTOCOL_NEGOTIATED_DATA_CHANNELS;


/** Initial client protocol. */
export const CLIENT_PROTOCOL_DEFAULT = 0;
Expand Down
Loading