-
Notifications
You must be signed in to change notification settings - Fork 160
Fix Pixel Streaming intermittently fails to make a network connection even with correct ICE candidates #694
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
Conversation
…ections to fail, these can be safely dropped as we use bundle by default and therefore these attributes are not used.
🦋 Changeset detectedLatest commit: 41bfea8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
const remoteIceCandidate = new RTCIceCandidate({ | ||
candidate: iceCandidateInit.candidate, | ||
sdpMid: '' | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the important lines of this fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Relevant components:
Problem statement:
In some instances Pixel Streaming/WebRTC will fail to make a network connection even if the relevant ICE candidates are present. This behaviour manifests with certain connections (often Windows UE running on a slow machine where
srflx
is required on both sides) and if the connection is repeatedly retried it will intermittently connect. See: #692Solution
Through investigation it was found that:
sdpMid
andsdpMLineIndex
sdpMid
andsdpMLineIndex
are not required if usingbundle
mode (where media is multiplexed over one transport)bundle
is Pixel Streaming's default (and only supported mode) for WebRTC connectionssdpMid
can be set to an empty string""
andsdpMLineIndex
can be omitted and the connection can proceed normallyWhile the WebRTC spec says candidates can be trickled in any order, we observed that if a candidate containing
sdpMid:n
(where n is any value other than the master bundle, e.g.0
in our case) was received before the candidate containingsdpMid:m
(where m is the master bundle, e.g.0
) then the connection would fail. The fix was simply to remove this unnecessary information and the connection proceeds as it should and is able to connect.Documentation
N/A - though it should be noted with this change we explicitly are making the assumption now that Pixel Streaming must use
bundle
- which has always been an implicit assumption anyway.Test Plan and Compatibility
I tested this PR on the following browsers/devices:
Chrome + Windows
Firefox + Windows
Chrome + iPhone
Chrome + Mac
Chrome + iPad
Safari + iPhone
Safari + Mac
Safari + iPad
Chrome + Android
Android Browser + Android