Don't expose so many RTCCodecStats!

[henbos]

We create an awful lot of codec stats objects. Every PT is listed for every m= section. And they are listed for both inbound and outbound direction, so you get all of them twice usually.

In a 50 participant Google Meet call I observed 872 codec stats objects, several KB worth of data.

Edit: Turns out most of them are due to a bug, after the fix we're down to 43 codec stats objects in this case.

Performance / bloat issues

This is bad for getStats() performance due so many objects being created on the both C++ heap and the V8 heap and all the string copies this entails. Every DOMString in the dictionary is copied from an RTCStatsMember<std::string> which is copied from lower layers. Copies and object creation does not just happen initially when the report is created, but also every time you iterate the RTCStatsReport (which an app likely does a few times). Because of WebIDL dictionaries being passed-by-value, copies and object creation happens every time you iterate, even if you've seen that codec before.

The legacy getStats() is more performant than the modern getStats(), and this may help explain why.

This also bloats the report so you typically have to filter them out on manual inspection. chrome://webrtc-internals/ chooses not to list them because they occlude the page and the rendering is unable to keep up if we include them all.

This is not even useful

While useful to know what codec you are currently using, it is NOT useful to know every possible combination of PT on every m= section.

• If you wanted to know about codec capabilities, you'd use RTCRtpSender/RTCRtpReceiver.getCapabilties().codecs, not getStats().
• If you wanted to inspect the SDP, you'd use pc.localDescription or pc.remoteDescription, which contains the same information already.

Proposal

Let's only create RTCCodecStats objects representing what is currently in use by RTCOutboundRtpStreamStats/RTCInboundRtpStreamStats. If it's not referenced by a codecId, it doesn't exist.

[henbos]

Thoughts? @alvestrand @jan-ivar @vr000m @fippo

[dontcallmedom]

somewhat related: (part of) the codec stats bloat was discussed in #614 and deemed as a Chrome implementation bug IIUC:

From testing I see Chrome creating a full set of "codec" entries (46) for each transceiver (10 transceivers = 460 "codec" entries), even though there's only 1 "transport" (because BUNDLE). I read the spec as sayng that 10 transceivers should produce 46 "codec" instances (in Chrome), not 460.

[henbos]

Interesting. I think we should fix the Chrome bug and re-assess. Part of me wants to go even further and have the codec information just be members of inbound-rtp/outbound-rtp and not even have codec objects, but that may be less necessary if we can avoid most bloat with a bug fix.

Do we want codec stats objects for codecs not referenced by any codecId?

[henbos]

I filed https://crbug.com/webrtc/14414

[fippo]

do we agree that those stats should only show up after negotiation, i.e. when both local and remote description have been set? This would reduce the number of codecs by discarding all unused payload types (e.g. when talking to a server that only does VPx)

Chrome already behaves that way which is 👍

[henbos]

I have a fix in Chrome, this brings the 872 codec objects down to 43 instead so even without any spec changes we're in a much better state.

If we only expose codecs in use, this could bring it down even further, e.g. 4 codecs if all audio/video/send/recv streams use the same codec per kind and direction.

[alvestrand]

When I wrote this spec, I thought the platform would create one codec object per configuration it supported, and let all transceivers refer to the same object. When we added transportId, that meant the objects were transport dependent; internally they should be representable as a (static object representing a supported configuration) + (dynamic object with transport ID and PT). But I don't see any reason to have multiple codec objects for the same transport.

[henbos]

Thanks for the input. With the bugfix that landed today, the "hundreds of objects" have been brought down to ~43 so this issue is less alarming, but before we close this spec issue we should decide on:

• Proposal A: 43 objects is good enough, close this issue.
• Proposal B: 43 objects is unnecessary when the information is available in pc.local/remoteDescription already. Only expose the codecs that are currently in use (have a codecId reference), in most cases this would be one per kind (or maybe direction and kind) if all streams used the same codec.

I'd be happy to reduce the codec count further to simplify code and reduce bloat, but I also don't care very strongly.

[dontcallmedom]

I'm +1 to Proposal B

[vr000m]

1B seems the right thing.

[henbos]

I'm happy with B :)

[henbos]

Any objections, @alvestrand?

[henbos]

And @jan-ivar

[henbos]

PR exists: #669

[henbos]

TPAC: This is already available as https://w3c.github.io/webrtc-pc/#dom-rtcrtpcodecparameters, no objection to remove codecs not currently in use from getStats.