Skip to content

close rtcp connection when browser tab closes#353

Merged
IIamaseconds merged 1 commit into
Glimesh:mainfrom
Endeavourl:close-rtcp
Jul 10, 2025
Merged

close rtcp connection when browser tab closes#353
IIamaseconds merged 1 commit into
Glimesh:mainfrom
Endeavourl:close-rtcp

Conversation

@Endeavourl

Copy link
Copy Markdown
Contributor

server continues sending stream to client for ~30s after the tab has been closed
this fixes that (or is my attempt at it anyway)

@IIamaseconds

Copy link
Copy Markdown
Collaborator

Super cool catch!

This would be great to add, but a little change to it.

I would advise on using the window.addEventListener and window.removeEventListener.
Since I am not sure if other areas of the app is looking towards these events and possibly being overwritten by doing this. But adding this code in cases where react does not get a chance to unload and detach the connection, is really good.

I can create a branch and implement it myself if that is easier for you. Or else, if you could get your fork up-to-date with the current release and make the changes, I can review and merge this

@Endeavourl

Copy link
Copy Markdown
Contributor Author

Where would i add/remove the listener? Same useEffect call where the change is now? And remove in cleanup?

Sorry not too familiar with react/frontend in general.

@IIamaseconds

Copy link
Copy Markdown
Collaborator

No worries!
But yes! You would introduce it in the useEffect call, and as part of the cleanup, if you pull the newest main branch and merge it in, the Player.tsx has been changed since your version.
If you would like to add the changes, the useEffect would look like this.

useEffect(() => {
		const handleWindowBeforeUnload = () => {
			peerConnectionRef.current?.close();
			peerConnectionRef.current = null;
		}
		const handleOverlayTimer = (isVisible: boolean) => resetTimer(isVisible);
		const player = document.getElementById("videoPlayer");
		
		player?.addEventListener('mousemove', () => handleOverlayTimer(true))
		player?.addEventListener('mouseenter', () => handleOverlayTimer(true))
		player?.addEventListener('mouseleave', () => handleOverlayTimer(false))
		player?.addEventListener('mouseup', () => handleOverlayTimer(true))
		
		window.addEventListener("beforeunload", handleWindowBeforeUnload)
		
		peerConnectionRef.current = new RTCPeerConnection();

		return () => {
			peerConnectionRef.current?.close()
			peerConnectionRef.current = null
			
			videoRef.current?.removeEventListener("playing", setHasSignalHandler)

			player?.removeEventListener('mouseenter', () => handleOverlayTimer)
			player?.removeEventListener('mouseleave', () => handleOverlayTimer)
			player?.removeEventListener('mousemove', () => handleOverlayTimer)
			player?.removeEventListener('mouseup', () => handleOverlayTimer)
			
			window.removeEventListener("beforeunload", handleWindowBeforeUnload)
			
			clearTimeout(videoOverlayVisibleTimeoutRef.current)
		} 
	}, [])

@Endeavourl

Copy link
Copy Markdown
Contributor Author

Updated PR, thanks for the tips!

@IIamaseconds IIamaseconds merged commit a663769 into Glimesh:main Jul 10, 2025
1 check passed
@IIamaseconds

Copy link
Copy Markdown
Collaborator

@Endeavourl - Awesome! Thanks for contributing!
By the way, there is a known issue with Chromium browsers, so if the issue does persist, it could be due to Chromium browsers not releasing the connection due to some issues with the garbage collection.

https://issues.chromium.org/issues/41378764

@Endeavourl Endeavourl deleted the close-rtcp branch July 11, 2025 08:22
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