webrtc-stats
webrtc-stats copied to clipboard
Do we agree removing "sender", "receiver" and "transceiver" stats is a good idea?
Getting rid of "sender", "receiver" and "transceiver" stats may be a good idea, but deserves discussion. Unfortunately https://github.com/w3c/webrtc-stats/pull/628 was merged before this discussion could happen, so I'm opening this issue to discuss it now. @alvestrand @aboba @youennf
A benefit of the "sender", "receiver" and "transceiver" stats were their lifetimes matched the sender, receiver and transceiver API objects. They supplanted "track" as a more stable starting point for (cross) navigating stats.
An argument for their removal seems to be that most metrics they contained have since been moved to RTP stats over the last few years. But RTP stats lifetimes are tied to RTP: "The lifetime of all RTP monitored objects starts when the RTP stream is first used: When the first RTP packet is sent or received on the SSRC it represents, or when the first RTCP packet is sent or received that refers to the SSRC of the RTP stream."
To make things worse, some implementations got the RTP lifetimes wrong #619, which brings into question whether metrics were moved under faulty assumptions and need to be put back if people expect to look them up sooner.
This is putting pressure on the model. There are rumblings in #619 of people now wanting RTP stats lifetimes tied to the API objects instead somehow, but so far no-one has opened an issue to propose details of how this would work.
We need to figure out the model we want and make sure we all see it the same way this time.
Thanks for filing this issue, the lifetime issue deserves more attention. See https://github.com/w3c/webrtc-stats/pull/628#issuecomment-1159405398 regarding what happened and me filing an issue to make sure sender/receiver/transceiver gets its spot in the provisional spec.
Where we are now: After a) the obsoletion of track stats, and b) the addition of mid and trackIdentifier to rtp stats, the sender/receiver/transceiver do not contain any new information. But they do have the useful property of having different lifetimes than rtp stats. As we see in #619, some people think the supposed lifetime of the rtp objects is a problem.
In this issue we should decide if sender/receiver/transceivers dictionaries are needed or if we should change the lifetime of the rtp stats to avoid having more dictionaries that don’t otherwise add new information.
Did Firefox implement ”RTP stats lifetimes are tied to RTP” already? I know Chromium browsers have not.
Update: sender/receiver/transceiver stats got added back to the provisional spec, see https://github.com/w3c/webrtc-provisional-stats/issues/32#issuecomment-1164198553
Regarding this issue...
Topic A: When should RTP stream stats objects be created?
Arguments in favor of adding sender/receiver/transceiver instead of creating RTP stream stats objects early:
- It matches the RTP Media API perfectly.
- While not implemented, sender/receiver/transceiver would be trivial to implement.
Arguments in favor of creating RTP stream stats as soon as the senders and receivers have been negotiated (not delaying this until you get the first packet):
- It's what most implementations do already and possibly what people have come to expect.
- You are already able to identify the RTP stream using mid (and rid in the case of simulcast).
- You are already able to tell whether or not a packet has been sent or received by looking at the packet counters.
- Less bloated stats reports: no need for sender/receiver/transceiver stats objects.
- Not having to know the SSRC in order to create these objects is liberating considering the SDP may or may not have signalled the SSRCs (we still put SSRCs in the SDP for backwards compatibility reasons but one could imagine we stop doing that, it would be nice not to have RTP stream stats be conditioned on the SDP implementation status).
Topic B: When should RTP stream stats objects be destroyed?
The spec says they should never be deleted. Chromium never implemented this, what about Firefox?
Arguments in favor:
- Not deleting them allows inspecting RTP metrics at the end of the call.
Arguments against:
- AFAIK never implemented and "at risk".
- Arguably not that useful, in practice most apps poll getStats every 1-10 seconds.
- This is a memory leak.
Here's another mind bender: what if you start out sending simulcast on ssrc 1 and 2 and then you change to send SVC on ssrc 1... what happens to ssrc 1? what happens to ssrc 2?
I don't think sender/receiver/transceiver objects add enough information to justify adding more objects to the stats report, so I propose we don't add them back to webrtc-stats 1.0. If people are OK with that then the only remaining questions is when to create and destroy the RTP stats objects.
Regarding creation:
- Proposal 1.A: Delay creation of inbound-rtp until after the SSRC is known. In the case of unsignalled SSRC this means delaying it until after the first packet is received, which means some WPTs need to be updated to stop assuming SSRC is signalled.
- Proposal 1.B: Having inbound-rtp conditioned on whether or not the SSRC is known adds an unnecessary dependency on SSRC signalling, let's instead create it when the receiver is configured to receive a stream. The mid is known even if the ssrc isn't. WPTs don't need to be updated, but the ssrc need to be made optional.
Regarding destruction:
- Proposal 2.A: Never delete RTP stats objects ensuring the sum of packet counters always go up. Each new SSRC used is a memory leak and browsers need to implement caching on removal.
- Proposal 2.B: When a configuration changes which RTP streams exist, delete old RTP stats objects. This means getStats only provides a snapshot containing current RTP streams. Easy to implement and no leak, but the app does not have the full picture when old streams go away.
I'm leaning towards Proposal 1.B and Proposal 2.B. Any thoughts or objections, @alvestrand @vr000m @jan-ivar ?
In 1B -- What does SSRC being optional mean? Is it because no packet is received on the mid and therefore the SSRC is unknown? What value would an empty inbound-RTP json have if no packet has been sent or received?
Isn't 1A implemented? Or are the objects created by the receiver ahead of time?
Isn't 1A implemented? Or are the objects created by the receiver ahead of time?
On second thought, yes 1.A is already implemented (link). I revise my vote to 1.A to match implementation and to avoid headache on what to do when ssrc goes from unknown to known on the same stats object.
In 1B -- What does SSRC being optional mean? Is it because no packet is received on the mid and therefore the SSRC is unknown? What value would an empty inbound-RTP json have if no packet has been sent or received?
Yeah in the event of not having received packets, the 1.B proposal would still create the inbound-rtp with undefined or zero values. It's a rather pointless object, it just lets you know which streams have been configured (exposing mids) regardless if packets have been received yet. Not worth the additional complexity I guess.
What about when a receiver is reconfigured for a different ssrc? Spec says to never delete the ssrc stats object, but the implementation does not cache data from old streams, so implementation-wise they are gone.
Fix implementation or change spec to match implementation?
Let's follow up in #667 and #668
@jan-ivar are you okay with closing this issue in lieu of the new issues?
Did Firefox implement ”RTP stats lifetimes are tied to RTP” already?
Yes for outbound-rtp (use release), while for "inbound-rtp" we have a bug and a patch awaiting resolution of this issue.
The spec says they should never be deleted. Chromium never implemented this, what about Firefox?
Pretty sure Firefox doesn't keep them forever. Accumulating rtp stats as participants arrive and drop seems like a bad idea over time.
@jan-ivar are you okay with closing this issue in lieu of the new issues?
Is there WG consensus to remove "sender", "receiver" and "transceiver" stats? We should get that before closing it.
It would be nice to simplify stats, as long as people don't use their absence as an argument in deciding https://github.com/w3c/webrtc-stats/issues/667...
Is it too late to add slides for this to the TPAC agenda? These proposals feel like they deserve more attention.
RESOLUTION: close issue 643 as "yes, it was a good idea to remove them for WebRTC stats"