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

Add liveness check to WebRTC output using data channel #91

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jpiccari
Copy link

@jpiccari jpiccari commented Aug 5, 2023

Previously, there was no way to detect if WebRTC clients were still connected to the stream. This lead to the RTC streams being kept open indefinitely when clients focibly closed the stream. This can happen with Chrome, Edge, Safari, etc when the tab is closed or on mobile devices when the screen is locked or browser closed.

This change adds a data channel to the stream and requires users to respond to ping requests. A separate thread checks which the last time each client responded to a ping request and initiates new pings for clients that have not responded in some time (half the timeout duration). If clients do not respond to these ping requests before the timeout duration the stream is closed and the client is removed from the server. This causes the client to reinitiate the connection.

The timeout defaults to 30 seconds and is configurable via the cli option --webrtc-timeout.

Fixes #55

Previously, there was no way to detect if WebRTC clients were still
connected to the stream. This lead to the RTC streams being kept
open indefinitely when clients focibly closed the stream. This can
happen with Chrome, Edge, Safari, etc when the tab is closed or on
mobile devices when the screen is locked or browser closed.

This change adds a data channel to the stream and requires users to
respond to ping requests. A separate thread checks which the last
time each client responded to a ping request and initiates new pings
for clients that have not responded in some time (half the timeout
duration). If clients do not respond to these ping requests before
the timeout duration the stream is closed and the client is removed
from the server. This causes the client to reinitiate the connection.

The timeout defaults to 30 seconds and is configurable via the cli
option --webrtc-timeout.
@QuinnDamerell
Copy link

I’m not sure about this, but i seems that webrtc should have some kind of built in connection status / heartbeat logic, does it not? At a minimum the websocket tcp socket should detect a timeout or the STUN protocol rebinds at some interval.

I know the peer connection has a status property, maybe the server can check that and cleanup if it’s set to disconnected after a connect?

@jpiccari
Copy link
Author

jpiccari commented Aug 9, 2023

As far as I can tell WebRTC has no connection status built in. I was also puzzled by this. The connection is over UDP which doesn’t track connection state and the connection is single direction.

I’m not sure what you mean about the web socket tracking connection status but to my knowledge there is no web socket involved when streaming with WebRTC.

libdatachannel does have various state event handler hooks including for connection closed and all of those are tracked with debug messages in camera-streamer today. However, only one applies to the status we are aiming to track but only works when the client cleanly closes the WebRTC connection (by calling close on the peer connection). The ghost stream happens when the browser fails to cleanly close the connection. This happens in many normal use cases but is especially apparent on mobile clients where the browser app can close unexpectedly (phone locked, battery loss, changes in networking).

@QuinnDamerell
Copy link

Ah, my bad, I was thinking of a different project where a websocket is used for the control channel, to send and receive the sdp.

The protocol is UDP, but there are a few protocols Webrtc uses on top of it that could support timeouts, but I'm not sure.

Anyways, I think this is a good implementation. Data channels use SCTP, which handles reliability and can detect disconnects as long as you feed it info.

The only thing I would note is that some clients might not be able to create a data channel, so the timeout logic needs to have a runtime flag on the http request calls to disable it, or have the logic not run the timeout logic if the data channel never connects on the server side.

@XScorpion2
Copy link

@ayufan you mentioned in #55 that there are some tweaks you plan on making to this fix. Can you provide some additional context and I might be able to help get those in for you?

@jdobes jdobes mentioned this pull request Nov 15, 2024
} else {
if (heartbeat_detla > disconnect / 2) {
LOG_DEBUG(client.get(), "Checking if client still alive.");
client->dc->send("ping");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I refresh the /webrtc page too quickly I've noticed this line throws an exception crashing the camera-streamer:

terminate called after throwing an instance of 'std::runtime_error'
  what():  DataChannel is closed
Aborted

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.

Ghost webrtc streams
4 participants