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 clarification that inbound-rtp.bytesReceived includes retransmissions. #755

Closed
henbos opened this issue May 2, 2023 · 6 comments · Fixed by #756
Closed

Add clarification that inbound-rtp.bytesReceived includes retransmissions. #755

henbos opened this issue May 2, 2023 · 6 comments · Fixed by #756

Comments

@henbos
Copy link
Collaborator

henbos commented May 2, 2023

For bytesSent we say:

Total number of bytes sent for this SSRC. This includes retransmissions. Calculated as defined in [RFC3550] section 6.4.1.

For bytesReceived we say:

Total number of bytes received for this SSRC. Calculated as defined in [RFC3550] section 6.4.1.

They both seemingly refer to the same thing, and having symmetry here is good (bytesSent should be roughly equal to bytesReceived). And yes, we should include retransmissions because header bytes + bytes should equal to the total bitrate of that SSRC.

So it seems like a small discrepancy that we forgot to say "This includes retransmissions" for the bytesReceived case as well. Right? @alvestrand @vr000m @fippo

@henbos
Copy link
Collaborator Author

henbos commented May 2, 2023

Related issue: #751

@henbos
Copy link
Collaborator Author

henbos commented May 2, 2023

Not sure if we are consistent with RFC though since we made it so that "bytesSent includes RTX" and the RTX could be sent over a separate SSRC if separate RTX stream is supported.

@henbos
Copy link
Collaborator Author

henbos commented May 2, 2023

There is some confusion here about whether or not probes need/should be included. I suspect they should be included on both send and receive side - but I'm not sure what we do today. @fippo can you help break down the current state of things?

Edit: Especially what the state of the probes are as those seem to account for a large percentage of overall traffic

@fippo
Copy link
Contributor

fippo commented May 2, 2023

The overall definition of "this SSRC" should probably be "this SSRC and all the SSRCs related to it such as retransmissions or FEC" in a lot of places. This relationship is commonly defined via ssrc-group, not sure if JSEP in its "we don't need SSRCs" narrative actually solves that problem or not.

The spec seems fine apart from pointing out that "This includes retransmissions" for inbound-rtp which is easy to fix, I'll do a PR.

RTX probes (for which we don't have an actual spec definition, whoops) are a bit tricky. They look like retransmitted packets on the receiver (one can try to be smart but given the lack of a spec that is a bad idea). Which means they will get accounted for differently, on outbound-rtp they might get accounted for as padding (maybe, like padding probes).
This means that retransmittedPacketsReceived may be nonzero even when there is no packet loss. That is a bit asymmetric but ok and according to the definition.

@henbos
Copy link
Collaborator Author

henbos commented May 2, 2023

If I may quote you @fippo running https://jsfiddle.net/fippo/hvpxf5L0/3/ with and without https://webrtc-review.googlesource.com/c/src/+/295260 which did modify inbound-rtp counters:

with this patch yields (--force-fieldtrials=WebRtcStatsReportIdl/Disabled/ helps)
SENT outbound-rtp p=58066 h=5980 packetsSent=98 retransmittedPacketsSent=0
SENT transport p=66422 h=undefined packetsSent=103 retransmittedPacketsSent=undefined
RECV inbound-rtp p=58066 h=5980 packetsReceived=98 retransmittedPacketsReceived=21
RECV transport p=66422 h=undefined packetsReceived=103 retransmittedPacketsReceived=undefined

while without it the following (from a different connection)
SENT outbound-rtp p=83872 h=3536 packetsSent=108 retransmittedPacketsSent=0
SENT transport p=89814 h=undefined packetsSent=112 retransmittedPacketsSent=undefined
RECV inbound-rtp p=78714 h=2536 packetsReceived=97 retransmittedPacketsReceived=undefined
RECV transport p=89814 h=undefined packetsReceived=112 retransmittedPacketsReceived=undefined

@fippo
Copy link
Contributor

fippo commented May 3, 2023

If I may quote you @fippo running https://jsfiddle.net/fippo/hvpxf5L0/3/

The fiddle demonstrates that after stopping a transceiver and waiting a bit for things to settle we should have roughly the same number of inbound-rtp packetsReceived as we have on outbound-rtp packetsSent.

fippo added a commit to fippo/webrtc-stats that referenced this issue May 3, 2023
henbos pushed a commit that referenced this issue May 11, 2023
* Clarify RTX accounting

fixes #755

* clarify more
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 a pull request may close this issue.

2 participants