-
Notifications
You must be signed in to change notification settings - Fork 47
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
RTCOutboundRtpStreamStats.headerBytesSent/totalPacketSendDelay and RTX seems underspecified #747
Comments
retransmittedPacketsSent and retransmittedBytesSent are already included in packetsSent and bytesSent. So they should be include retransmissions. Unless I am missing something, the definitions are correct, however, based on the ticket, a clarification would help the implementor. Would the following improvements clarify the definitions:
For totalPacketSendDelay says the measurement is added each time the packetSent is incremented. And the packetSent explicitly states that it includes retransmissions. Would the following clarification help?
|
Yeah, I think being explicit here is necessary. |
How is this kind of stuff testable? |
I've checked what libWebRTC does as part of implementing RTX stats. It does not include the payload of RTX packets, so the accurate thing to say would be:
|
Agree that the clarification is appropriate. Header bytes of retransmissions. Can you confirm if the retransmission bytes are added to bytesSent? |
Spec explicitly says that we should be totaling up packetsSent, bytesSent, retransmittedPacketsSent, and retransmittedBytesSent. But what about headerBytesSent/totalPacketSendDelay?
headerBytesSent
It seems pretty clear that this should include RTX, since spec says "headerBytesSent + bytesSent equals the number of bytes sent as payload over the transport.", and bytesSent accounts for RTX.
totalPacketSendDelay
This probably ought to account for RTX as well. The only way I can see this metric being useful is by comparing it to packetsSent (over some time window), and packetsSent includes RTX packets.
The text was updated successfully, but these errors were encountered: