Enable Sender and Receiver Reports by default #1684

Sean-Der · 2021-02-18T05:57:35Z

The play-from-disk examples sees the average bitrate using
Chromium 90.0.4412.3 when enabled on loopback for a 3 minute
session.

Before: 744.443
After: 3927.669

Introduced with pion/[email protected]

I uploaded the full video analysis here https://gist.github.com/Sean-Der/b162e879101450f4f27db1c95ca0241d

This is a pretty big change. We are moving more towards interceptors. I want to make sure everyone agrees and this has no negative impacts on people that wish to still maintain their own pipelines.

All of the real work here was done by @aler9, thank you so much for this. This is really exciting and a huge step in taking pion/webrtc forward :)

codecov · 2021-02-18T06:02:13Z

Codecov Report

Merging #1684 (174dca3) into master (ae0f74e) will increase coverage by 0.06%.
The diff coverage is 45.45%.

@@            Coverage Diff             @@
##           master    #1684      +/-   ##
==========================================
+ Coverage   75.00%   75.07%   +0.06%     
==========================================
  Files          79       79              
  Lines        5726     5737      +11     
==========================================
+ Hits         4295     4307      +12     
+ Misses       1058     1054       -4     
- Partials      373      376       +3

Flag	Coverage Δ
go	`76.64% <45.45%> (+0.06%)`	⬆️
wasm	`69.90% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
interceptor.go	`61.70% <45.45%> (-4.97%)`	⬇️
datachannel.go	`80.24% <0.00%> (-1.21%)`	⬇️
peerconnection.go	`74.48% <0.00%> (+0.34%)`	⬆️
sctptransport.go	`80.44% <0.00%> (+3.35%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae0f74e...489ba34. Read the comment docs.

jeremija · 2021-02-18T08:08:08Z

That's awesome! I'm looking forward to trying it out once I finally switch to v3. I'm more curious about the high level concept of this feature: does this work by terminating the RTCP packets once they reach the SFU?

For example, if we had a room in with three browser peers (left side) and three pion peers in the SFU (right side) and 6 tracks:

+--------+   SSRC 1 audio           +-------------+
| Peer A |------------------------->| SFU peer A' |>--------------+
|        |                          |             |               |
|        |   SSRC 2 video           |             |               |
|        |------------------------->|             |>--------------|--+
|        |                          |             |               |  |
|        |   SSRC 3 audio           |             |               |  |
|        |<-------------------------|             |<-+            |  |
|        |                          |             |  |            |  |
|        |   SSRC 4 video           |             |  |            |  |
|        |<-------------------------|             |<-|--+         |  |
|        |                          |             |  |  |         |  |
|        |   SSRC 5 video           |             |  |  |         |  |
|        |<-------------------------|             |<-|--|--+      |  |
|        |                          |             |  |  |  |      |  |
|        |   SSRC 6 video           |             |  |  |  |      |  |
|        |<-------------------------|             |<-|--|--|--+   |  |
+--------+                          +-------------+  |  |  |  |   |  |
                                                     |  |  |  |   |  |
+--------+   SSRC 3 audio           +-------------+  |  |  |  |   |  |
| Peer B |------------------------->| SFU peer B' |>-+  |  |  |   |  |
|        |                          |             |     |  |  |   |  |
|        |   SSRC 4 video           |             |     |  |  |   |  |
|        |------------------------->|             |>----+  |  |   |  |
|        |                          |             |     |  |  |   |  |
|        |   SSRC 1 audio           |             |     |  |  |   |  |
|        |<-------------------------|             |<----|--|--|---+  |
|        |                          |             |     |  |  |      |
|        |   SSRC 5 video           |             |     |  |  |      |
|        |<-------------------------|             |<----|--+  |      |
+--------+                          +-------------+     |  |  |      |
                                                        |  |  |      |
+--------+   SSRC 5 audio           +-------------+     |  |  |      |
| Peer C |------------------------->| SFU peer C' |>----|--+  |      |
|        |                          |             |     |     |      |
|        |   SSRC 6 video           |             |     |     |      |
|        |------------------------->|             |>----|-----+      |
|        |                          |             |     |            |
|        |   SSRC 2 video           |             |     |            |
|        |<-------------------------|             |<----|------------+
|        |                          |             |     |
|        |   SSRC 4 video           |             |     |
|        |<-------------------------|             |<----+
+--------+                          +-------------+

(using the same SSRC for negotiated tracks between different peer connections for the sake of clarity)

Would this change enable sender reports and receiver reports between:

Peer A and A' (A' will now send SR for SSRCs 3,4,5,6 and RR for 1, 2)
Peer B and B' (B' will now send SR to B for SSRCs 1 and 5 and RR for 3, 4)
Peer C and C' (C' will now send SR to C for SSRCs 2 and 4 and RR for 5, 6)

Sean-Der · 2021-02-18T23:08:03Z

@jech as well, sorry I just went through the org and tagged people using media API.

Sean-Der · 2021-02-18T23:08:50Z

examples/ice-tcp/main.go

@@ -18,11 +18,6 @@ func doSignaling(w http.ResponseWriter, r *http.Request) {
 	var err error

 	if peerConnection == nil {
-		m := webrtc.MediaEngine{}


This example is only doing DataChannels

jech · 2021-02-19T01:22:45Z

I told you so :-)

On a more serious note, since the report interceptor appears to be generating reasonable loss statistics, it should be safe to deploy in the Internet. Hence, I support this change.

I'm a little confused about what's going on with the NTP time fields, but I'm not too worried — at the worst, lipsynch will be off.

Sean-Der · 2021-02-19T06:02:11Z

I told you so :-)

I never pretended to know what I was doing, people just assumed :p Yea I am glad you brought this up, made it way more urgent from me.

Glad to hear you are feeling good about this change. I would love to move forward with it. Will give it a few more days and then nag people for approvals.

jech · 2021-02-19T12:31:46Z

@jeremija In Galène, we terminate RTCP at the SFU and run a separate congestion controller for every client. Let's take a simpler example than yours: A, B, C are in the same group, A is sending a media stream which is being forwarded to B and C:

A --SFU-> B
     |
     +--> C

For each of the three flows, you terminate RTCP at the SFU. This gives you the ability to serve NACKs and (if you're lucky) PLIs locally at the SFU, thus reducing latency. You run a separate congestion controller on each link, and, in the absence of simulcast, aim for the minimum of the bitrates indicated by the three congestion controllers.

The one tricky bit is getting lipsynch right. For that, you need to parse the sender report from A, work out the offset between A's NTP clock and A's RTP clock(s), and forward the offsets to B and C. When I implemented this in Galène, the effect was dramatic — suddenly the SFU became usable on links with high jitter (bad WiFi).

FWIW, Galène implements all of the above, except that the congestion controller in the A->SFU direction is incomplete (only the loss-based half of GCC is implemented).

jeremija · 2021-02-19T18:11:10Z

@jech wow thanks so much for the explanation. This was super useful, I learned some new things and you confirmed some of the things I already knew about but wasn't sure if my understanding is correct. Thank you!

When you say "congestion controller", are you referring to the component that calculates and reports the maximum bitrate (e.g. REMB), or something else?

jech · 2021-02-19T18:57:11Z

The congestion controller is the algorithm that decides the maximum allowable bitrate for a flow. It usually runs at the sender, with the receiver providing feedback of some sort, but it may run at the receiver, which must then somehow communicate the allowable bitrate to the sender:

in TCP-Reno, the congestion controller runs at the sender, with the receiver providing feedback implicitly by sending ACKs (or failing to send them);
the loss-based congestion controller in GCC (Google-style WebRTC) runs at the sender, with the receiver providing feedback explicitly by sending loss estimates in receiver reports;
the original delay-based congestion controller in GCC runs at the receiver, which then communicates its conclusions to the sender using REMB feedback;
the revised delay-based congestion controller in GCC runs at the sender, based on detailed reports provided by the receiver through TWCC feedback.

I'm being told that the revised delay-based controller is better, but I don't understand why — it has the advantage of being similar to traditional congestion controllers (by virtue of running at the sender), but it is way more chatty than the original delay-based controller.

Sean-Der · 2021-02-19T20:17:59Z

@jeremija @jech knows this stuff better then, super lucky to have him in the community :)

Also check out pion/interceptor#25 it is my plan to get stuff into Pion itself. It will all be opt-in/composable. It will be the second point Google Style WebRTC and will use receiver reports.

I want to do TWCC/Delay-based eventually. I don't feel confident I can do it right in a good amount of time.

adwpc · 2021-02-20T05:10:15Z

@jeremija @jech knows this stuff better then, super lucky to have him in the community :)

Also check out pion/interceptor#25 it is my plan to get stuff into Pion itself. It will all be opt-in/composable. It will be the second point Google Style WebRTC and will use receiver reports.

I want to do TWCC/Delay-based eventually. I don't feel confident I can do it right in a good amount of time.

Yep, TWCC is better for SFU.
Im not sure SR/RR shoud be configuable?
There is also a SR/RR in ion-sfu, what do do you think? @OrlandoCo

jech · 2021-02-20T13:10:48Z

TWCC is better for SFU

Why?

Sean-Der · 2021-02-22T23:24:54Z

@aler9 mind approving this? I haven't heard any push back so I vote to merge and push :)

Users can easily opt out of this if they wish. I believe that most users will want this by default.

aler9

The browser will not send higher quality streams unless it has the available bandwidth. You can look at the bandwidth estimation in chrome://webrtc-internals. It is under VideoBwe when Read Stats From: Legacy non-Standard is selected.

This is partially true; in order to obtain the maximum available bitrate, a webrtc implementation must

implement RR/SR (ok)
implement bandwidth estimation
when it's receiving the stream, edit the SDP and add a TIAS bandwidth that contains the maximum bandwidth available

In this way, bitrates of 20/30 mbps can be achieved.

The play-from-disk examples sees the average bitrate using Chromium 90.0.4412.3 when enabled on loopback for a 3 minute session. Before: 744.443 After: 3927.669 Introduced with pion/[email protected]

Sean-Der requested review from OrlandoCo, jeremija, adwpc, aler9, at-wat, cameronelliott, mission-liao, robin-raymond and tarrencev February 18, 2021 05:58

Sean-Der force-pushed the rtcp-reports branch 2 times, most recently from 3fe6c22 to 583887b Compare February 18, 2021 06:01

Sean-Der commented Feb 18, 2021

View reviewed changes

Sean-Der force-pushed the rtcp-reports branch 2 times, most recently from 48435fd to 7c9f653 Compare February 19, 2021 18:06

aler9 approved these changes Feb 23, 2021

View reviewed changes

Enable Sender and Receiver Reports by default

489ba34

The play-from-disk examples sees the average bitrate using Chromium 90.0.4412.3 when enabled on loopback for a 3 minute session. Before: 744.443 After: 3927.669 Introduced with pion/[email protected]

Sean-Der force-pushed the rtcp-reports branch from 7c9f653 to 489ba34 Compare February 24, 2021 06:31

Sean-Der merged commit 33d953e into master Feb 24, 2021

Sean-Der deleted the rtcp-reports branch February 24, 2021 06:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Sender and Receiver Reports by default #1684

Enable Sender and Receiver Reports by default #1684

Sean-Der commented Feb 18, 2021 •

edited

Loading

codecov bot commented Feb 18, 2021 •

edited

Loading

jeremija commented Feb 18, 2021 •

edited

Loading

Sean-Der commented Feb 18, 2021

Sean-Der Feb 18, 2021

jech commented Feb 19, 2021

Sean-Der commented Feb 19, 2021 •

edited

Loading

jech commented Feb 19, 2021 •

edited

Loading

jeremija commented Feb 19, 2021

jech commented Feb 19, 2021

Sean-Der commented Feb 19, 2021 •

edited

Loading

adwpc commented Feb 20, 2021

jech commented Feb 20, 2021

Sean-Der commented Feb 22, 2021 •

edited

Loading

aler9 left a comment •

edited

Loading

Enable Sender and Receiver Reports by default #1684

Enable Sender and Receiver Reports by default #1684

Conversation

Sean-Der commented Feb 18, 2021 • edited Loading

codecov bot commented Feb 18, 2021 • edited Loading

Codecov Report

jeremija commented Feb 18, 2021 • edited Loading

Sean-Der commented Feb 18, 2021

Sean-Der Feb 18, 2021

Choose a reason for hiding this comment

jech commented Feb 19, 2021

Sean-Der commented Feb 19, 2021 • edited Loading

jech commented Feb 19, 2021 • edited Loading

jeremija commented Feb 19, 2021

jech commented Feb 19, 2021

Sean-Der commented Feb 19, 2021 • edited Loading

adwpc commented Feb 20, 2021

jech commented Feb 20, 2021

Sean-Der commented Feb 22, 2021 • edited Loading

aler9 left a comment • edited Loading

Choose a reason for hiding this comment

Sean-Der commented Feb 18, 2021 •

edited

Loading

codecov bot commented Feb 18, 2021 •

edited

Loading

jeremija commented Feb 18, 2021 •

edited

Loading

Sean-Der commented Feb 19, 2021 •

edited

Loading

jech commented Feb 19, 2021 •

edited

Loading

Sean-Der commented Feb 19, 2021 •

edited

Loading

Sean-Der commented Feb 22, 2021 •

edited

Loading

aler9 left a comment •

edited

Loading