From ad2b4f5759cd2a775d78645da5e8ae7de301e6a1 Mon Sep 17 00:00:00 2001 From: mcottontensor <80377552+mcottontensor@users.noreply.github.com> Date: Thu, 2 Nov 2023 12:26:18 +1100 Subject: [PATCH] Merge pull request #419 from mcottontensor/fix_reconnect_flow Rewriting a bunch of reconnect and disconnect handling. --- .../src/PixelStreaming/PixelStreaming.test.ts | 6 +- .../src/PixelStreaming/PixelStreaming.ts | 34 +-- Frontend/library/src/Util/EventEmitter.ts | 6 +- .../WebRtcPlayer/WebRtcPlayerController.ts | 261 ++++++++---------- .../src/WebSockets/WebSocketController.ts | 5 - .../ui-library/src/Application/Application.ts | 63 +++-- 6 files changed, 171 insertions(+), 204 deletions(-) diff --git a/Frontend/library/src/PixelStreaming/PixelStreaming.test.ts b/Frontend/library/src/PixelStreaming/PixelStreaming.test.ts index 3c1036ba..71d86c21 100644 --- a/Frontend/library/src/PixelStreaming/PixelStreaming.test.ts +++ b/Frontend/library/src/PixelStreaming/PixelStreaming.test.ts @@ -274,7 +274,8 @@ describe('PixelStreaming', () => { type: MessageRecvTypes.STREAMER_LIST, ids: streamerIdList }), - autoSelectedStreamerId: streamerId + autoSelectedStreamerId: streamerId, + wantedStreamerId: null })); expect(webSocketSpyFunctions.sendSpy).toHaveBeenCalledWith( expect.stringMatching(/"type":"subscribe".*MOCK_PIXEL_STREAMING/) @@ -298,7 +299,8 @@ describe('PixelStreaming', () => { type: MessageRecvTypes.STREAMER_LIST, ids: extendedStreamerIdList }), - autoSelectedStreamerId: null + autoSelectedStreamerId: null, + wantedStreamerId: null })); expect(webSocketSpyFunctions.sendSpy).not.toHaveBeenCalledWith( expect.stringMatching(/"type":"subscribe"/) diff --git a/Frontend/library/src/PixelStreaming/PixelStreaming.ts b/Frontend/library/src/PixelStreaming/PixelStreaming.ts index 5b9d3908..14ccf0da 100644 --- a/Frontend/library/src/PixelStreaming/PixelStreaming.ts +++ b/Frontend/library/src/PixelStreaming/PixelStreaming.ts @@ -70,7 +70,6 @@ export class PixelStreaming { private _videoElementParent: HTMLElement; - _showActionOrErrorOnDisconnect = true; private allowConsoleCommands = false; private onScreenKeyboardHelper: OnScreenKeyboard; @@ -353,7 +352,7 @@ export class PixelStreaming { */ public reconnect() { this._eventEmitter.dispatchEvent(new StreamReconnectEvent()); - this._webRtcController.restartStreamAutomatically(); + this._webRtcController.tryReconnect("Reconnecting..."); } /** @@ -440,7 +439,6 @@ export class PixelStreaming { */ _onWebRtcAutoConnect() { this._eventEmitter.dispatchEvent(new WebRtcAutoConnectEvent()); - this._showActionOrErrorOnDisconnect = true; } /** @@ -460,30 +458,16 @@ export class PixelStreaming { /** * Event fired when the video is disconnected - emits given eventString or an override * message from webRtcController if one has been set - * @param eventString - the event text that will be emitted - */ - _onDisconnect(eventString: string) { - // if we have overridden the default disconnection message, assign the new value here - if ( - this._webRtcController.getDisconnectMessageOverride() != '' && - this._webRtcController.getDisconnectMessageOverride() !== - undefined && - this._webRtcController.getDisconnectMessageOverride() != null - ) { - eventString = this._webRtcController.getDisconnectMessageOverride(); - this._webRtcController.setDisconnectMessageOverride(''); - } - + * @param eventString - a string describing why the connection closed + * @param allowClickToReconnect - true if we want to allow the user to retry the connection with a click + */ + _onDisconnect(eventString: string, allowClickToReconnect: boolean) { this._eventEmitter.dispatchEvent( new WebRtcDisconnectedEvent({ - eventString, - showActionOrErrorOnDisconnect: - this._showActionOrErrorOnDisconnect + eventString: eventString, + allowClickToReconnect: allowClickToReconnect }) ); - if (this._showActionOrErrorOnDisconnect == false) { - this._showActionOrErrorOnDisconnect = true; - } } /** @@ -856,4 +840,8 @@ export class PixelStreaming { public get toStreamerHandlers() { return this._webRtcController.streamMessageController.toStreamerHandlers; } + + public isReconnecting() { + return this._webRtcController.isReconnecting; + } } diff --git a/Frontend/library/src/Util/EventEmitter.ts b/Frontend/library/src/Util/EventEmitter.ts index 32ee39a1..8e0d2f6d 100644 --- a/Frontend/library/src/Util/EventEmitter.ts +++ b/Frontend/library/src/Util/EventEmitter.ts @@ -144,7 +144,7 @@ export class WebRtcDisconnectedEvent extends Event { /** Message describing the disconnect reason */ eventString: string; /** true if the user is able to reconnect, false if disconnected because of unrecoverable reasons like not able to connect to the signaling server */ - showActionOrErrorOnDisconnect: boolean; + allowClickToReconnect: boolean; }; constructor(data: WebRtcDisconnectedEvent['data']) { super('webRtcDisconnected'); @@ -347,7 +347,9 @@ export class StreamerListMessageEvent extends Event { /** Streamer list message containing an array of streamer ids */ messageStreamerList: MessageStreamerList; /** Auto-selected streamer from the list, or null if unable to auto-select and user should be prompted to select */ - autoSelectedStreamerId: string | null; + autoSelectedStreamerId: string; + /** Wanted streamer id from various configurations. */ + wantedStreamerId: string; }; constructor(data: StreamerListMessageEvent['data']) { super('streamerListMessage'); diff --git a/Frontend/library/src/WebRtcPlayer/WebRtcPlayerController.ts b/Frontend/library/src/WebRtcPlayer/WebRtcPlayerController.ts index 390d701e..e445176d 100644 --- a/Frontend/library/src/WebRtcPlayer/WebRtcPlayerController.ts +++ b/Frontend/library/src/WebRtcPlayer/WebRtcPlayerController.ts @@ -104,16 +104,13 @@ export class WebRtcPlayerController { preferredCodec: string; peerConfig: RTCConfiguration; videoAvgQp: number; + locallyClosed: boolean; shouldReconnect: boolean; isReconnecting: boolean; reconnectAttempt: number; - subscribedStream: string | null; + disconnectMessage: string; + subscribedStream: string; signallingUrlBuilder: () => string; - - // if you override the disconnection message by calling the interface method setDisconnectMessageOverride - // it will use this property to store the override message string - disconnectMessageOverride: string; - autoJoinTimer: ReturnType = undefined; /** @@ -139,10 +136,7 @@ export class WebRtcPlayerController { this.onAfkTriggered.bind(this) ); this.afkController.onAFKTimedOutCallback = () => { - this.setDisconnectMessageOverride( - 'You have been disconnected due to inactivity' - ); - this.closeSignalingServer(); + this.closeSignalingServer('You have been disconnected due to inactivity'); }; this.freezeFrameController = new FreezeFrameController( @@ -202,14 +196,6 @@ export class WebRtcPlayerController { this.webSocketController.onStreamerList = ( messageList: MessageReceive.MessageStreamerList ) => this.handleStreamerListMessage(messageList); - this.webSocketController.onWebSocketOncloseOverlayMessage = (event) => { - this.pixelStreaming._onDisconnect( - `Websocket disconnect (${event.code}) ${ - event.reason != '' ? '- ' + event.reason : '' - }` - ); - this.setVideoEncoderAvgQP(0); - }; this.webSocketController.onPlayerCount = (playerCount: MessageReceive.MessagePlayerCount) => { this.pixelStreaming._onPlayerCount(playerCount.count); }; @@ -223,6 +209,19 @@ export class WebRtcPlayerController { } }); this.webSocketController.onClose.addEventListener('close', (event : CustomEvent) => { + // when we refresh the page during a stream we get the going away code. + // in that case we don't want to reconnect since we're navigating away. + // https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code + // lists all the codes. + const CODE_GOING_AWAY = 1001; + + const willTryReconnect = this.shouldReconnect + && event.detail.code != CODE_GOING_AWAY + && this.config.getNumericSettingValue(NumericParameters.MaxReconnectAttempts) > 0 + + const disconnectMessage = this.disconnectMessage ? this.disconnectMessage : event.detail.reason; + this.pixelStreaming._onDisconnect(disconnectMessage, !willTryReconnect && !this.isReconnecting); + this.afkController.stopAfkWarningTimer(); // stop sending stats on interval if we have closed our connection @@ -230,21 +229,22 @@ export class WebRtcPlayerController { window.clearInterval(this.statsTimerHandle); } + // reset the stream quality icon. + this.setVideoEncoderAvgQP(0); + // unregister all input device event handlers on disconnect this.setTouchInputEnabled(false); this.setMouseInputEnabled(false); this.setKeyboardInputEnabled(false); this.setGamePadInputEnabled(false); - // when we refresh the page during a stream we get the going away code. - // in that case we don't want to reconnect since we're navigating away. - // https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent/code - // lists all the codes. - const CODE_GOING_AWAY = 1001; - if(this.shouldReconnect && event.detail.code != CODE_GOING_AWAY && this.config.getNumericSettingValue(NumericParameters.MaxReconnectAttempts) > 0) { - this.isReconnecting = true; - this.reconnectAttempt++; - this.restartStreamAutomatically(); + if (willTryReconnect) { + // need a small delay here to prevent reconnect spamming + setTimeout(() => { + this.isReconnecting = true; + this.reconnectAttempt++; + this.tryReconnect(event.detail.reason); + }, 2000); } }); @@ -947,9 +947,9 @@ export class WebRtcPlayerController { } /** - * Restart the stream automatically without refreshing the page + * Attempt a reconnection to the signalling server */ - restartStreamAutomatically() { + tryReconnect(message: string) { // if there is no webSocketController return immediately or this will not work if (!this.webSocketController) { Logger.Log( @@ -959,33 +959,16 @@ export class WebRtcPlayerController { return; } - // if a websocket object has not been created connect normally without closing - if ( - !this.webSocketController.webSocket || - this.webSocketController.webSocket.readyState === WebSocket.CLOSED - ) { - Logger.Log( - Logger.GetStackTrace(), - 'A websocket connection has not been made yet so we will start the stream' - ); + // if the connection is open, first close it. wait some time and try again. + this.isReconnecting = true; + if (this.webSocketController.webSocket && this.webSocketController.webSocket.readyState != WebSocket.CLOSED) { + this.closeSignalingServer(`${message} Restarting stream...`); + setTimeout(() => { + this.tryReconnect(message); + }, 3000); + } else { this.pixelStreaming._onWebRtcAutoConnect(); this.connectToSignallingServer(); - } else { - // set the replay status so we get a text overlay over an action overlay - this.pixelStreaming._showActionOrErrorOnDisconnect = false; - - // set the disconnect message - this.setDisconnectMessageOverride('Restarting stream...'); - - // close the connection - this.closeSignalingServer(); - - // wait for the connection to close and restart the connection - const autoConnectTimeout = setTimeout(() => { - this.pixelStreaming._onWebRtcAutoConnect(); - this.connectToSignallingServer(); - clearTimeout(autoConnectTimeout); - }, 3000); } } @@ -1087,13 +1070,8 @@ export class WebRtcPlayerController { ); Logger.Error(Logger.GetStackTrace(), message); - // set the disconnect message - this.setDisconnectMessageOverride( - 'Stream not initialized correctly' - ); - // close the connection - this.closeSignalingServer(); + this.closeSignalingServer('Stream not initialized correctly'); return; } @@ -1176,6 +1154,9 @@ export class WebRtcPlayerController { * Connect to the Signaling server */ connectToSignallingServer() { + this.locallyClosed = false; + this.shouldReconnect = true; + this.disconnectMessage = null; const signallingUrl = this.signallingUrlBuilder(); this.webSocketController.connect(signallingUrl); } @@ -1198,10 +1179,7 @@ export class WebRtcPlayerController { Logger.GetStackTrace(), 'No turn server was found in the Peer Connection Options. TURN cannot be forced, closing connection. Please use STUN instead' ); - this.setDisconnectMessageOverride( - 'TURN cannot be forced, closing connection. Please use STUN instead.' - ); - this.closeSignalingServer(); + this.closeSignalingServer('TURN cannot be forced, closing connection. Please use STUN instead.'); return; } } @@ -1343,84 +1321,75 @@ export class WebRtcPlayerController { 6 ); - if(this.isReconnecting) { - if(messageStreamerList.ids.includes(this.subscribedStream)) { - // If we're reconnecting and the previously subscribed stream has come back, resubscribe to it - this.isReconnecting = false; - this.reconnectAttempt = 0; - this.webSocketController.sendSubscribe(this.subscribedStream); - } else if(this.reconnectAttempt < this.config.getNumericSettingValue(NumericParameters.MaxReconnectAttempts)) { - // Our previous stream hasn't come back, wait 2 seconds and request an updated stream list - this.reconnectAttempt++; - setTimeout(() => { - this.webSocketController.requestStreamerList(); - }, 2000) - } else { - // We've exhausted our reconnect attempts, return to main screen - this.reconnectAttempt = 0; - this.isReconnecting = false; - this.shouldReconnect = false; - this.webSocketController.close(); - - this.config.setOptionSettingValue( - OptionParameters.StreamerId, - "" - ); - this.config.setOptionSettingOptions( - OptionParameters.StreamerId, - [] - ); - } - } else { - const settingOptions = [...messageStreamerList.ids]; // copy the original messageStreamerList.ids - settingOptions.unshift(''); // add an empty option at the top - this.config.setOptionSettingOptions( + // add the streamers to the UI + const settingOptions = [...messageStreamerList.ids]; // copy the original messageStreamerList.ids + settingOptions.unshift(''); // add an empty option at the top + this.config.setOptionSettingOptions( + OptionParameters.StreamerId, + settingOptions + ); + + let wantedStreamerId: string = null; + let autoSelectedStreamerId: string = null; + const waitForStreamer = this.config.isFlagEnabled(Flags.WaitForStreamer); + const reconnectLimit = this.config.getNumericSettingValue(NumericParameters.MaxReconnectAttempts); + const reconnectDelay = this.config.getNumericSettingValue(NumericParameters.StreamerAutoJoinInterval); + + // first we figure out a wanted streamer id through various means + const urlParams = new URLSearchParams(window.location.search); + if (urlParams.has(OptionParameters.StreamerId)) { + // if we've set the streamer id on the url we only want that streamer id + wantedStreamerId = urlParams.get(OptionParameters.StreamerId); + } else if (this.subscribedStream) { + // we were previously subscribed to a streamer, we want that + wantedStreamerId = this.subscribedStream; + } + + // now lets see if we can pick it. + if (wantedStreamerId && messageStreamerList.ids.includes(wantedStreamerId)) { + // if the wanted stream is in the list. we pick that + autoSelectedStreamerId = wantedStreamerId; + } else if ((!wantedStreamerId || !waitForStreamer) && messageStreamerList.ids.length == 1) { + // otherwise, if we're not waiting for the wanted streamer and there's only one streamer, connect to it + autoSelectedStreamerId = messageStreamerList.ids[0]; + } + + // if we found a streamer id to auto select, select it + if (autoSelectedStreamerId) { + this.isReconnecting = false; + this.reconnectAttempt = 0; + this.config.setOptionSettingValue( OptionParameters.StreamerId, - settingOptions + autoSelectedStreamerId ); - - const urlParams = new URLSearchParams(window.location.search); - let autoSelectedStreamerId: string | null = null; - if (messageStreamerList.ids.length == 1) { - // If there's only a single streamer, subscribe to it regardless of what is in the URL - autoSelectedStreamerId = messageStreamerList.ids[0]; - } else if ( - urlParams.has(OptionParameters.StreamerId) && - messageStreamerList.ids.includes( - urlParams.get(OptionParameters.StreamerId) - ) - ) { - // If there's a streamer ID in the URL and a streamer with this ID is connected, set it as the selected streamer - autoSelectedStreamerId = urlParams.get(OptionParameters.StreamerId); - } - if (autoSelectedStreamerId !== null) { - this.config.setOptionSettingValue( - OptionParameters.StreamerId, - autoSelectedStreamerId - ); - } else { - // no auto selected streamer - if (messageStreamerList.ids.length == 0 && this.config.isFlagEnabled(Flags.WaitForStreamer)) { - this.closeSignalingServer(); - this.startAutoJoinTimer(); + } else { + // no auto selected streamer. + // if we're waiting for a streamer then try reconnecting + if (waitForStreamer) { + if (this.reconnectAttempt < reconnectLimit) { + // still reconnects available + this.isReconnecting = true; + this.reconnectAttempt++; + setTimeout(() => { + this.webSocketController.requestStreamerList(); + }, reconnectDelay); + } else { + // We've exhausted our reconnect attempts, return to main screen + this.reconnectAttempt = 0; + this.isReconnecting = false; + this.shouldReconnect = false; } } - this.pixelStreaming.dispatchEvent( - new StreamerListMessageEvent({ - messageStreamerList, - autoSelectedStreamerId - }) - ); } - } - startAutoJoinTimer() { - clearTimeout(this.autoJoinTimer); - this.autoJoinTimer = setTimeout(() => this.tryAutoJoin(), this.config.getNumericSettingValue(NumericParameters.StreamerAutoJoinInterval)); - } - - tryAutoJoin() { - this.connectToSignallingServer(); + // dispatch this event finally + this.pixelStreaming.dispatchEvent( + new StreamerListMessageEvent({ + messageStreamerList, + autoSelectedStreamerId, + wantedStreamerId + }) + ); } /** @@ -1617,9 +1586,11 @@ export class WebRtcPlayerController { /** * Close the Connection to the signaling server */ - closeSignalingServer() { + closeSignalingServer(message: string) { // We explicitly called close, therefore we don't want to trigger auto reconnect + this.locallyClosed = true; this.shouldReconnect = false; + this.disconnectMessage = message; this.webSocketController?.close(); } @@ -1634,7 +1605,7 @@ export class WebRtcPlayerController { * Close all connections */ close() { - this.closeSignalingServer(); + this.closeSignalingServer(''); this.closePeerConnection(); } @@ -2018,20 +1989,6 @@ export class WebRtcPlayerController { this.videoPlayer.resizePlayerStyle(); } - /** - * Get the overridden disconnect message - */ - getDisconnectMessageOverride(): string { - return this.disconnectMessageOverride; - } - - /** - * Set the override for the disconnect message - */ - setDisconnectMessageOverride(message: string): void { - this.disconnectMessageOverride = message; - } - setPreferredCodec(codec: string) { this.preferredCodec = codec; if (this.peerConnectionController) { diff --git a/Frontend/library/src/WebSockets/WebSocketController.ts b/Frontend/library/src/WebSockets/WebSocketController.ts index 5a52649e..313a6a55 100644 --- a/Frontend/library/src/WebSockets/WebSocketController.ts +++ b/Frontend/library/src/WebSockets/WebSocketController.ts @@ -134,7 +134,6 @@ export class WebSocketController { * @param event - Close Event */ handleOnClose(event: CloseEvent) { - this.onWebSocketOncloseOverlayMessage(event); Logger.Log( Logger.GetStackTrace(), 'Disconnected to the signalling server via WebSocket: ' + @@ -204,10 +203,6 @@ export class WebSocketController { this.webSocket?.close(); } - /** Event used for Displaying websocket closed messages */ - // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-empty-function - onWebSocketOncloseOverlayMessage(event: CloseEvent) {} - /** * The Message Contains the payload of the peer connection options used for the RTC Peer hand shake * @param messageConfig - Config Message received from he signaling server diff --git a/Frontend/ui-library/src/Application/Application.ts b/Frontend/ui-library/src/Application/Application.ts index 809186be..af9f80bd 100644 --- a/Frontend/ui-library/src/Application/Application.ts +++ b/Frontend/ui-library/src/Application/Application.ts @@ -324,8 +324,8 @@ export class Application { ); this.stream.addEventListener( 'webRtcDisconnected', - ({ data: { eventString, showActionOrErrorOnDisconnect } }) => - this.onDisconnect(eventString, showActionOrErrorOnDisconnect) + ({ data: { eventString, allowClickToReconnect } }) => + this.onDisconnect(eventString, allowClickToReconnect) ); this.stream.addEventListener('videoInitialized', () => this.onVideoInitialized() @@ -366,8 +366,8 @@ export class Application { ) this.stream.addEventListener( 'streamerListMessage', - ({ data: { messageStreamerList, autoSelectedStreamerId } }) => - this.handleStreamerListMessage(messageStreamerList, autoSelectedStreamerId) + ({ data: { messageStreamerList, autoSelectedStreamerId, wantedStreamerId } }) => + this.handleStreamerListMessage(messageStreamerList, autoSelectedStreamerId, wantedStreamerId) ); this.stream.addEventListener( 'settingsChanged', @@ -573,14 +573,14 @@ export class Application { /** * Event fired when the video is disconnected - displays the error overlay and resets the buttons stream tools upon disconnect * @param eventString - the event text that will be shown in the overlay + * @param allowClickToReconnect - true if we want to allow the user to click to reconnect. Otherwise it's just a message. */ - onDisconnect(eventString: string, showActionOrErrorOnDisconnect: boolean) { - if (showActionOrErrorOnDisconnect == false) { - this.showErrorOverlay(`Disconnected: ${eventString}`); + onDisconnect(eventString: string, allowClickToReconnect: boolean) { + const overlayMessage = 'Disconnected' + (eventString ? `: ${eventString}` : ''); + if (allowClickToReconnect) { + this.showDisconnectOverlay(`${overlayMessage} Click To Restart.`); } else { - this.showDisconnectOverlay( - `Disconnected: ${eventString}
Click To Restart
` - ); + this.showErrorOverlay(overlayMessage); } // disable starting a latency checks this.statsPanel?.onDisconnect(); @@ -667,18 +667,41 @@ export class Application { this.statsPanel?.handlePlayerCount(playerCount); } - handleStreamerListMessage(messageStreamingList: MessageStreamerList, autoSelectedStreamerId: string | null) { - if (autoSelectedStreamerId === null) { - if(messageStreamingList.ids.length === 0) { - var message = 'No streamers connected. ' + - (this.stream.config.isFlagEnabled(Flags.WaitForStreamer) - ? 'Waiting for streamer...' - : '
Click To Restart
'); + handleStreamerListMessage(messageStreamingList: MessageStreamerList, autoSelectedStreamerId: string, wantedStreamerId: string) { + const waitForStreamer = this.stream.config.isFlagEnabled(Flags.WaitForStreamer); + const isReconnecting = this.stream.isReconnecting(); + let message: string = null; + let allowRestart: boolean = true; + + if (!autoSelectedStreamerId) { + if (waitForStreamer && wantedStreamerId) { + if (isReconnecting) { + message = `Waiting for ${wantedStreamerId} to become available.`; + allowRestart = false; + } else { + message = `Gave up waiting for ${wantedStreamerId} to become available. Click to try again`; + if (messageStreamingList.ids.length > 0) { + message += ` or select a streamer from the settings menu.`; + } + allowRestart = true; + } + } else if (messageStreamingList.ids.length == 0) { + if (isReconnecting) { + message = `Waiting for a streamer to become available.`; + allowRestart = false; + } else { + message = `No streamers available. Click to try again.`; + allowRestart = true; + } + } else { + message = `Multiple streamers available. Select one from the settings menu.`; + allowRestart = false; + } + + if (allowRestart) { this.showDisconnectOverlay(message); } else { - this.showTextOverlay( - 'Multiple streamers detected. Use the dropdown in the settings menu to select the streamer' - ); + this.showTextOverlay(message); } } }