Synopsis

We are currently trying to replace Coturn with this implementation. But currently it does not have alternatives to specific Coturn's functionality e.g. providing metrics, TCP fallback, external management capabilities. So we want to elaborate on that starting with exposing some management interfaces.

Here is the Coturn's telnet protocol for reference:

TURN Server
Coturn-4.5.2 'dan Eider'

  ?, h, help - print this text

  quit, q, exit, bye - end CLI session

  stop, shutdown, halt - shutdown TURN Server

  pc - print configuration

  sr <realm> - set CLI session realm

  ur - unset CLI session realm

  so <origin> - set CLI session origin

  uo - unset CLI session origin

  tc <param-name> - toggle a configuration parameter
     (see pc command output for togglable param names)

  cc <param-name> <param-value> - change a configuration parameter
     (see pc command output for changeable param names)

  ps [username] - print sessions, with optional exact user match

  psp <usernamestr> - print sessions, with partial user string match

  psd <file-name> - dump ps command output into file on the TURN server system

  pu [udp|tcp|dtls|tls]- print current users

  lr - log reset

  aas ip[:port} - add an alternate server reference
  das ip[:port] - delete an alternate server reference
  atas ip[:port] - add a TLS alternate server reference
  dtas ip[:port] - delete a TLS alternate server reference

  cs <session-id> - cancel session, forcefully

The particular function that we need is being able to forcefully close specific allocations.

Solution

Add the ability to communicate with the applications main loop, running in the Server::read_loop() method.

Add Server.delete_allocations_by_username(&self, username: String) function.

Also, there are some low-hanging fruits in terms of optimizations, so this pull request includes minor refactoring aimed at improving overall performance.

Hi, first of all, I am a Pion lover. It's great to see movement in the Rust world. Since this project is very recent, how serious is this project? I am planning long-term for some of our software components. Rust might be valuable for us but we need WebRTC support. So don't get my question offensive in any way. Just curious if this is something I could base our stack on in the future.

Cheers Chris

Per comments here, this prevents a RTCDataChannel id from updating on open unless a) the value was not negotiated, and b) the value is equal to zero (i.e., the null value by existing crate convention). This fixes most of the existing error cases and does not introduce new ones. It does, however, miss one specific case (where a user creates a non-negotiated data channel with id = 0). Fixing that case would be a sizeable refactor. There is a TODO comment for addressing it.

This PR modifies read behaviour to permit partial reads and no longer return ErrShortBuffer if the buffer is too small to fit a whole message. The buffer will be filled up to its size and the rest of the msg will be returned upon the next call to read.

Closes #273

Sockets opened in ICEGatherer seem to be leaked. They will never be closed even when the PeerConnections are closed. Finally, my server becomes unavailable with too many open files error.

I've investigated into webrtc's source code, and found that CandidateBase.close() is an no-op. Of course, it is because tokio's UdpSocket does not provide close(). Despite all, the sockets should be closed when it is dropped. Therefore, I guess that the socket is not dropped.

RTCPeerConnection holds Arc<PeerConnectionInternal> and there are other holders too. In v0.2.1, the peerconnection has no problem, dropped well. However, other holders seems not to drop it. I have no idea where the others are, but internal_rtcp_writer (peer_connection.rs, line 170) or pci (peer_connection.rs, line 1163, 1345, 1393) may have created reference cycle.

If possible, those references should be replaced by std::sync::Weak to break the cycle. Pion or other webrtc libraries written in garbage-collected languages may not suffer from this issue because the GC can detect and free those circular references well. Because Rust does not have one, we should use weak references to avoid this problem. It will also fix other memory leaks too.

hi

This is an issue reopen from 174, as state before the code below will leave some socket unreleased in the task/thread/process

let pc = peer_connection.clone();
peer_connection
        .on_peer_connection_state_change(Box::new(move |s: RTCPeerConnectionState| {
            info!("Peer Connection State has changed: {}", s);
            let is_failed = s == RTCPeerConnectionState::Failed;
            let pc1 = pc.clone();
            Box::pin(async move {
                if is_failed {
                    pc1.close().await.ok();
                }
            })
        }))
        .await;

So I try to make it async way, as it did get fixed, for some time

    let (box_sdr, mut box_rcvr) = tokio::sync::mpsc::channel::<VCError>(1);
    peer_connection
        .on_peer_connection_state_change(Box::new(move |s: RTCPeerConnectionState| {
            info!("Peer Connection State has changed: {}", s);
            let is_failed = s == RTCPeerConnectionState::Failed;
            if is_failed {
                info!("Peer Connection has gone to failed");
            }
            let box_sdr = box_sdr.clone();
            Box::pin(async move {
                if is_failed {
                    box_sdr.send(VCError::biz("Connection failed", 10)).await;
                }
            })
        }))
        .await;

    let peer_connection = peer_connection.clone();
    rt::spawn(async move {
        loop {
            let box_result = box_rcvr.try_recv();
            if box_result.is_ok() {
                tokio::time::sleep(Duration::from_secs(8)).await;
                peer_connection.close().await;
                break;
            } 
            tokio::time::sleep(Duration::from_secs(4)).await;
        }
    });

But when the task went heavy and cpu get big burden, say, 50 rtc jobs delivering large bitrate stream, I found the issue reappear from time to time. Leaving udp sockets unreleased.

I have one audio tranceiver channel, one video tranceiver channel and a data channel initialized from client side, the task get notified(started) after data channel is established. Every event handler of the peer_connection are independent, which means they are not chained. After some log working, I am pretty sure all the data I created are dropped. So once more I need some help here, just like the error before, maybe there's some cons I need to watch out?

Maybe there's other part I neglected? Hope you could help.

Hi there,

I'm trying to get webrtc-rs to interact with the browser-webrtc available in web_sys.RtcPeerConnection. I started with the example here:

https://github.com/webrtc-rs/examples/blob/5a0e2861c66a45fca93aadf9e70a5b045b26dc9e/examples/data-channels/data-channels.rs

So I'm creating an APIBuilder::new()[...].new_peer_connection(config) that should create an offer, which will be sent to the web_sys.RtcPeerConnection implementation.

The web_sys then sends back an answer that I would like to feed to the peer_connection, and here is my problem: web_sys only gives me a String, not an RTCSessionDescription as needed by the peer_connection. And the RTCSessionDescription has a private field, so I cannot do:

peer_connection.set_local_description(RTCSessionDescription{
  sdp_type: RTCSdpType::Answer,
  sdp: answer_string,
  parsed: None
}

I looked around at the methods of peer_connection but couldn't find an easy way to feed the answer-string to it.

As discussed in #360 the pending queue can grow indefinitly if the sender writes packets faster than the association is able to transmit them.

This PR solves this by enforcing a limit on the pendig queue. This blocks the sender until enough space is free.

Pion's simulcast example can receives two streams with rid = "f" and "h" ~~and play-from-disk-renegotiation example has video grid when click add video multiple times.~~

mind if i create a PR that makes all callback setters (e.g. RTCPeerConnection::on_ice_candidate()) non async? Currently callbacks are stored behind tokio::Mutex'es, i believe that ArcSwap can be used here, so we can use atomic load/store. It wont affect performance in any way, just some quality of life improvements.

All good with me. I'd like to get rid of tokio::Mutex, replacing it with sync variants, where that's possible

So i've refactored all public callback setters here.

Regarding removing tokio::Mutexes that you've mentioned, well, i've replaced some of those, but i don't want this PR to become even bigger, and it would be nice if you provide some feedback first. I've used both ArcSwap and std::Mutex, i guess its ok to use blocking mutex in simple cases, when you don't need to hold it, e.g. copy, store, swap, take one-liners. I will prepare follow-up PRs on this issue if this PR looks good to you.

And one more thing regarding the callbacks, maybe it makes sense to make them just dumb Box<dyn Fn(_)>? So Box<dyn (FnMut(ConnectionState) -> Pin<Box<dyn Future<Output = ()> + Send + 'static>>) + Send + Sync> => Box<dyn (Fn(ConnectionState)) + Send + Sync>. Yeah, that might hurt users in some rare cases, but it would make everything simpler.

ack @k0nserv

Hey,

I noticed that, at least in my modified reflect example, Firefox throws a DOM exception when applying the answer SDP.

v=0
o=- 4985367504486208344 356629000 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 D5:67:46:98:F8:D8:05:4B:D5:42:4E:FD:04:C7:03:93:B9:90:C8:AD:A2:82:3E:AE:24:E3:4A:B8:54:2D:76:31
a=group:BUNDLE 1 2
m=audio 0 UDP/TLS/RTP/SAVPF 0
a=candidate:167090039 1 udp 2130706431 :: 50760 typ host
a=candidate:167090039 2 udp 2130706431 :: 50760 typ host
a=candidate:3498147984 1 udp 2130706431 192.168.1.76 59816 typ host
a=candidate:3498147984 2 udp 2130706431 192.168.1.76 59816 typ host
a=candidate:3122685691 1 udp 2130706431 192.168.1.33 57564 typ host
a=candidate:3122685691 2 udp 2130706431 192.168.1.33 57564 typ host
a=end-of-candidates
m=video 9 UDP/TLS/RTP/SAVPF 120
c=IN IP4 0.0.0.0
a=setup:active
a=mid:1
a=ice-ufrag:nzCUobTruNCKeWEl
a=ice-pwd:pSowjNuaNylpeKBFgTsFUNwQRdrzDQxe
a=rtcp-mux
a=rtcp-rsize
a=rtpmap:120 VP8/90000
a=fmtp:120 max-fs=12288;max-fr=60
a=rtcp-fb:120 nack
a=rtcp-fb:120 nack pli
a=rtcp-fb:120 ccm fir
a=rtcp-fb:120 goog-remb
a=rtcp-fb:120 transport-cc
a=extmap:7 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=recvonly
m=application 9 UDP/DTLS/SCTP webrtc-datachannel
c=IN IP4 0.0.0.0
a=setup:active
a=mid:2
a=sendrecv
a=sctp-port:5000
a=ice-ufrag:nzCUobTruNCKeWEl
a=ice-pwd:pSowjNuaNylpeKBFgTsFUNwQRdrzDQxe

This causes the following exception to be thrown in Firefox:

Uncaught (in promise) DOMException: SIPCC Failed to parse SDP: SDP Parse Error on line 39:  c= connection line not specified for every media level, validation failed.

This makes sense, the c line is missing for the audio track.

This is the relevant part of the Firefox source code: https://searchfox.org/mozilla-central/source/third_party/sipcc/sdp_main.c#955

The method by which we detected non-signaled SSRCs would silently drop a few packets. This is particularly problematic for a video stream since these initial packets usually contain the first part of a keyframe to start the stream.

See: #386

Note: The first packet read for each RTP stream is not processed by interceptors unfortunately.

We seem to lose the first packet when doing unknown SSRC detection. For example, when identifying simulcast tracks via rid + mid extension the first packet is never delivered to the track.

Example

This image showcases the issue. On the left is a packet capture from Chrome's perspective and on the right is the result of querying a packet capture taken from an interceptor. Note that the initial packet, sequence number 24698, is missing in the capture while the second packet, sequence number 24699, does appear in the capture.

Notes

The read that consumes the initial packet happens here https://github.com/webrtc-rs/webrtc/blob/e60607fa269f20fc002347f32e0d5b756f5b92e3/webrtc/src/peer_connection/peer_connection_internal.rs#L964 I believe

Hello everyone, First of all, thank you all for this incredible project and all your efforts !

I have a question about the use of the HashMap behind the Mutex. Could this be replaced by some concurrent HashMap, like dashMap for example ?

I am fairly new to rust, not to mention rust async and executor like tokio, so surely my thoughts are naive.

If the use of a Mutex HahsMap is only usefull for controlling multi threading access to the Hashmap, from the tokio's tasks executor. Can we gain some speed with the use of a concurrent hashmap, with less coordination/contention on the tokio runtime ?

What is your opinion on this subject ?