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

Make data channels optional for a PeerConnection #2226

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gustavo-lighttwist
Copy link

@gustavo-lighttwist gustavo-lighttwist commented Nov 6, 2023

Currently a participant is required to create two data channels (named "_lossy" and "_reliable") for each PeerConnection, otherwise the LiveKit server will terminate the connection. This pull request removes this requirement by defining that a participant is "fully connected" if the PeerConnection has been established, without taking into account the state of the data channel, or even if it exists.

I really don't know what are the other side effects of this change, so this PR is mostly about triggering the discussion about it, and if this is even a use case that LiveKit wants to cover.

@davidzhao
Copy link
Member

@gustavo-lighttwist thank you for getting this started. I like the direction of this.. The original design intention was to ensure that we do not mark a participant as ACTIVE unless it is ready to receive data messages from others.

That should only apply in the receiving direction for subscribers. I think for publishers, it's entirely fine to skip the data channel check, since the sender would be responsible for ensuring they are established before sending data.

So I think if you used this new behavior only when t.IsOfferer is true.. that would work. What do you think @boks1971 @cnderrauber ?

@boks1971
Copy link
Contributor

boks1971 commented Nov 7, 2023

I will have to check this more. But, when canSubscribe is false, server might rely on publisher data channel to send data messages. In that case, feels like we would want to wait for data channel to be ready.

@davidzhao
Copy link
Member

Ah you are right, I see it in TransportManager.

So the usecase here is to allow third-party/custom clients that do not support data channels to publish to LiveKit. I know there have been a few folks looking to integrate with the publishing flow this way. Maybe we should look into the offer and see if data channel is actually being offered before waiting on it?

@boks1971
Copy link
Contributor

boks1971 commented Nov 7, 2023

Yeah that would better. Doesn't all SDKs always negotiate data?

@davidzhao
Copy link
Member

Our first party SDKs do. Third-party SDKs may not always negotiate this. I remember one user was trying to patch the offer through from Amazon Kinesis WebRTC and it didn't support data channels.

@boks1971
Copy link
Contributor

boks1971 commented Nov 7, 2023

Ah got it. Then looking at the offer sounds good.

@cnderrauber
Copy link
Contributor

Relies on the negotiated sdp is a good point, sfu will wait for data channel opened if the client (pub or sub) offer/answer data channel, otherwise it only checks peerconnection state

@boks1971
Copy link
Contributor

boks1971 commented Nov 8, 2023

Thinking a bit more about this, using SDP to decide is a bit tricky as well. First party SDKs always open a reliable and lossy channel. It is not entirely right to assume that SDP having application section means that both types of data channels will be opened.

Another option may be to have a connect option to specify how ACTIVE should be determined, but that is not pretty.

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.

None yet

4 participants