Fix Pixel Streaming intermittently fails to make a network connection even with correct ICE candidates (#694)

* Fix connectivity issue where sdpMid and sdpMLineIdx were causing connections to fail, these can be safely dropped as we use bundle by default and therefore these attributes are not used.

(cherry picked from commit cbec432)

Author: lukehb

.changeset/rich-sites-hear.md

@@ -0,0 +1,5 @@
+---
+'@epicgames-ps/lib-pixelstreamingfrontend-ue5.6': minor
+---
+
+This change fixes an intermittent WebRTC connection failure where even when the appropriate ICE candidates were present the conection would sometimes fail to be made. This was caused due to the order that ICE candidates were being sent (hence the intermittent nature of the issues) and the fact that ICE candidates sent from Pixel Streaming plugin contain sdpMid and sdpMLineIndex. sdpMid and sdpMLineIndex are only necessary in legacy, non bundle, WebRTC streams; however, Pixel Streaming always assumes bundle is used and these attributes can safely be set to empty strings/omitted (respectively). We perform this modification in the frontend library prior to adding the ICE candidate to the peer connection. This change was tested on a wide range of target devices and browsers to ensure there was no adverse side effects prior.

Frontend/library/src/PeerConnectionController/PeerConnectionController.ts

@@ -144,7 +144,7 @@ export class PeerConnectionController {
                         return this.peerConnection?.setLocalDescription(Answer);
                     })
                     .then(() => {
-                        this.onSetLocalDescription(this.peerConnection?.currentLocalDescription);
+                        this.onSetLocalDescription(this.peerConnection?.localDescription);
                     })
                     .catch((err) => {
                         Logger.Error(`createAnswer() failed - ${err}`);

Frontend/library/src/PixelStreaming/PixelStreaming.test.ts

@@ -322,7 +322,14 @@ describe('PixelStreaming', () => {
         triggerSdpOfferMessage();
         triggerIceCandidateMessage();
 
-        expect(rtcPeerConnectionSpyFunctions.addIceCandidateSpy).toHaveBeenCalledWith(iceCandidate)
+        // Expect ice candidate to be stripped even if passed in with sdpMid and sdpMLineIndex
+        // as these values are not required when using bundle (which we assume)
+        const strippedIceCandidate = new RTCIceCandidate({
+            candidate: iceCandidate.candidate,
+            sdpMid: ""
+        });
+
+        expect(rtcPeerConnectionSpyFunctions.addIceCandidateSpy).toHaveBeenCalledWith(strippedIceCandidate)
     });
 
     it('should emit webRtcConnected event when ICE connection state is connected', () => {

Frontend/library/src/WebRtcPlayer/WebRtcPlayerController.ts

@@ -1103,19 +1103,35 @@ export class WebRtcPlayerController {
             this.pixelStreaming._onLatencyCalculated(latencyInfo);
         };
 
-        /* When the Peer Connection wants to send an offer have it handled */
+        /* When our PeerConnection wants to send an offer call our handler */
         this.peerConnectionController.onSendWebRTCOffer = (offer: RTCSessionDescriptionInit) => {
             this.handleSendWebRTCOffer(offer);
         };
 
-        /* Set event handler for when local answer description is set */
-        this.peerConnectionController.onSetLocalDescription = (answer: RTCSessionDescriptionInit) => {
-            this.handleSendWebRTCAnswer(answer);
+        /* Set event handler for when local description is set */
+        this.peerConnectionController.onSetLocalDescription = (sdp: RTCSessionDescriptionInit) => {
+            if (sdp.type === 'offer') {
+                this.handleSendWebRTCOffer(sdp);
+            } else if (sdp.type === 'answer') {
+                this.handleSendWebRTCAnswer(sdp);
+            } else {
+                Logger.Error(
+                    `PeerConnectionController onSetLocalDescription was called with unexpected type ${sdp.type}`
+                );
+            }
         };
 
-        /* Set event handler for when remote offer description is set */
-        this.peerConnectionController.onSetRemoteDescription = (offer: RTCSessionDescriptionInit) => {
-            this.pixelStreaming._onWebRtcSdpOffer(offer);
+        /* Event handler for when PeerConnection's remote description is set */
+        this.peerConnectionController.onSetRemoteDescription = (sdp: RTCSessionDescriptionInit) => {
+            if (sdp.type === 'offer') {
+                this.pixelStreaming._onWebRtcSdpOffer(sdp);
+            } else if (sdp.type === 'answer') {
+                this.pixelStreaming._onWebRtcSdpAnswer(sdp);
+            } else {
+                Logger.Error(
+                    `PeerConnectionController onSetRemoteDescription was called with unexpected type ${sdp.type}`
+                );
+            }
         };
 
         /* When the Peer Connection ice candidate is added have it handled */
@@ -1447,23 +1463,31 @@ export class WebRtcPlayerController {
     }
 
     /**
-     * When an ice Candidate is received from the Signaling server add it to the Peer Connection Client
-     * @param iceCandidate - Ice Candidate from Server
+     * Handler for when a remote ICE candidate is received.
+     * @param iceCandidateInit - Initialization data used to make the actual ICE Candidate.
      */
-    handleIceCandidate(iceCandidate: RTCIceCandidateInit) {
-        Logger.Info('Web RTC Controller: onWebRtcIce');
+    handleIceCandidate(iceCandidateInit: RTCIceCandidateInit) {
+        Logger.Info(`Remote ICE candidate information received: ${JSON.stringify(iceCandidateInit)}`);
+
+        // We are using "bundle" policy for media lines so we remove the sdpMid and sdpMLineIndex attributes
+        // from ICE candidates as these are legacy attributes for when bundle is not used.
+        // If we don't do this the browser may be unable to form a media connection
+        // because some browsers are brittle if the bundle master (e.g. commonly mid=0) doesn't get a candidate first.
+        const remoteIceCandidate = new RTCIceCandidate({
+            candidate: iceCandidateInit.candidate,
+            sdpMid: ''
+        });
 
-        const candidate = new RTCIceCandidate(iceCandidate);
-        this.peerConnectionController.handleOnIce(candidate);
+        this.peerConnectionController.handleOnIce(remoteIceCandidate);
     }
 
     /**
      * Send the ice Candidate to the signaling server via websocket
      * @param iceEvent - RTC Peer ConnectionIceEvent) {
      */
     handleSendIceCandidate(iceEvent: RTCPeerConnectionIceEvent) {
-        Logger.Info('OnIceCandidate');
         if (iceEvent.candidate && iceEvent.candidate.candidate) {
+            Logger.Info(`Local ICE candidate generated: ` + JSON.stringify(iceEvent.candidate));
             this.protocol.sendMessage(
                 MessageHelpers.createMessage(Messages.iceCandidate, { candidate: iceEvent.candidate })
             );
@@ -1488,6 +1512,13 @@ export class WebRtcPlayerController {
      * @param offer - RTC Session Description
      */
     handleSendWebRTCOffer(offer: RTCSessionDescriptionInit) {
+        if (offer.type !== 'offer') {
+            Logger.Error(
+                `handleSendWebRTCOffer was called with type ${offer.type} - it only expects "offer"`
+            );
+            return;
+        }
+
         Logger.Info('Sending the offer to the Server');
 
         const extraParams = {
@@ -1497,13 +1528,23 @@ export class WebRtcPlayerController {
         };
 
         this.protocol.sendMessage(MessageHelpers.createMessage(Messages.offer, extraParams));
+
+        // Send offer back to Pixel Streaming main class for event dispatch
+        this.pixelStreaming._onWebRtcSdpOffer(offer);
     }
 
     /**
      * Send the RTC Offer Session to the Signaling server via websocket
      * @param answer - RTC Session Description
      */
     handleSendWebRTCAnswer(answer: RTCSessionDescriptionInit) {
+        if (answer.type !== 'answer') {
+            Logger.Error(
+                `handleSendWebRTCAnswer was called with type ${answer.type} - it only expects "answer"`
+            );
+            return;
+        }
+
         Logger.Info('Sending the answer to the Server');
 
         const extraParams = {