Skip to content
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

Reuse transceivers in subscribe path #51

Merged
merged 2 commits into from
Oct 14, 2021
Merged

Conversation

boks1971
Copy link
Contributor

LK-22 (https://linear.app/livekit/issue/LK-22/feature-pooled-transceivers-to-avoid-re-negotiation)

Do not stop remote tracks. That would change readyState of track
to ended and the corresponding transceiver cannot be reused.
Instead use the enabled attribute of track to enable/disable
remote tracks.

Bumping up the protocol version that informs the server that
client is capable of reusing transceivers.

Testing:

Connect from Chrome. For remote side, connect/disconnect
from Firefox several times and ensure the following

  • SDP does not grow (it has 4 sections as reported by
    chrome://webrtc-internals (version, data, audio, video) each
    time the remote side connects.
  • Media flows from remote side on every connection.

Oddities:

  • Chrome callback ontrack sends a track. When the remote side
    reconnects and the same transceiver is used, the callback does
    not contain the new track id. It retains what was returned in the
    first callback. Don't think it is an issue as we do not use that
    track id anywhere.

LK-22 (https://linear.app/livekit/issue/LK-22/feature-pooled-transceivers-to-avoid-re-negotiation)

Do not stop remote tracks. That would change `readyState` of track
to `ended` and the corresponding transceiver cannot be reused.
Instead use the `enabled` attribute of track to enable/disable
remote tracks.

Bumping up the protocol version that informs the server that
client is capable of reusing transceivers.

Testing:
--------
Connect from Chrome. For remote side, connect/disconnect
from Firefox several times and ensure the following
- SDP does not grow (it has 4 sections as reported by
chrome://webrtc-internals (version, data, audio, video) each
time the remote side connects.
- Media flows from remote side on every connection.

Oddities:
---------
- Chrome callback `ontrack` sends a track. When the remote side
reconnects and the same transceiver is used, the callback does
not contain the new track id. It retains what was returned in the
first callback. Don't think it is an issue as we do not use that
track id anywhere.
Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

This looks great!

Don't think it is an issue as we do not use that track id anywhere.

It does pass us a new stream id though? I think that's the more important piece. FWIW, we do use the track id here, only if it's not already encoded in the stream id. (Room.onTrackAdded)

const parts = unpackStreamId(stream.id);
const participantId = parts[0];
let trackId = parts[1];
if (!trackId || trackId === '') trackId = mediaTrack.id;

src/room/track/Track.ts Outdated Show resolved Hide resolved
src/room/track/Track.ts Outdated Show resolved Hide resolved
@boks1971
Copy link
Contributor Author

@davidzhao Yes I verified that the stream id and the parsing gets the new information. I will verify it again just to be doubly sure.

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

great! lg

@boks1971
Copy link
Contributor Author

boks1971 commented Oct 14, 2021

Re-confirming that the stream id shows the right stuff on a reconnect/reuse of transceivers

remote stream:  MediaStream {id: 'PA_XuF28UHg9G3C|TR_SvjS2hYhdRfH', active: true, onaddtrack: null, onremovetrack: null, onactive: null, …} PA_XuF28UHg9G3C|TR_SvjS2hYhdRfH
onTrackAdded @ Room.ts:236
(anonymous) @ Room.ts:83
emit @ events.js:153
subscriber.pc.ontrack @ RTCEngine.ts:185
wrappedCallback @ utils.js:46
Room.ts:236 remote stream:  MediaStream {id: 'PA_XuF28UHg9G3C|TR_Snaw3Gr6svq5', active: true, onaddtrack: null, onremovetrack: null, onactive: null, …} PA_XuF28UHg9G3C|TR_Snaw3Gr6svq5
onTrackAdded @ Room.ts:236
(anonymous) @ Room.ts:83
emit @ events.js:153
subscriber.pc.ontrack @ RTCEngine.ts:185
wrappedCallback @ utils.js:46
Room.ts:236 remote stream:  MediaStream {id: 'PA_REbbQ2qJpmPK|TR_2Q6eoEgCrfQC', active: true, onaddtrack: null, onremovetrack: null, onactive: null, …} PA_REbbQ2qJpmPK|TR_2Q6eoEgCrfQC
onTrackAdded @ Room.ts:236
(anonymous) @ Room.ts:83
emit @ events.js:153
subscriber.pc.ontrack @ RTCEngine.ts:185
wrappedCallback @ utils.js:46
Room.ts:236 remote stream:  MediaStream {id: 'PA_REbbQ2qJpmPK|TR_fz7yT4G5PLzZ', active: true, onaddtrack: null, onremovetrack: null, onactive: null, …} PA_REbbQ2qJpmPK|TR_fz7yT4G5PLzZ
onTrackAdded @ Room.ts:236
(anonymous) @ Room.ts:83
emit @ events.js:153
subscriber.pc.ontrack @ RTCEngine.ts:185
wrappedCallback @ utils.js:46
Room.ts:236 remote stream:  MediaStream {id: 'PA_DUknmHLikjXn|TR_kYqUhvNcnGqn', active: true, onaddtrack: null, onremovetrack: null, onactive: null, …} PA_DUknmHLikjXn|TR_kYqUhvNcnGqn
onTrackAdded @ Room.ts:236
(anonymous) @ Room.ts:83
emit @ events.js:153
subscriber.pc.ontrack @ RTCEngine.ts:185
wrappedCallback @ utils.js:46
Room.ts:236 remote stream:  MediaStream {id: 'PA_DUknmHLikjXn|TR_4irgCrcnnZ8i', active: true, onaddtrack: null, onremovetrack: null, onactive: null, …} PA_DUknmHLikjXn|TR_4irgCrcnnZ8i

@boks1971 boks1971 merged commit 6817386 into main Oct 14, 2021
@boks1971 boks1971 deleted the raja_reuse_transceiver branch October 14, 2021 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants