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

Are "codec" stats per transceiver or per transport? #614

Closed
jan-ivar opened this issue Jan 20, 2022 · 9 comments · Fixed by #617
Closed

Are "codec" stats per transceiver or per transport? #614

jan-ivar opened this issue Jan 20, 2022 · 9 comments · Fixed by #617
Assignees

Comments

@jan-ivar
Copy link
Member

TL;DR: The spec states codec stats are per transport, not per m-line, which doesn't match Chrome. But whaddabout sdpFmtpLine?

"codec" "is created when a codec type is registered for an RTP transport. It may be referenced by multiple RTP streams in media sections using that transport, but similar codecs in different transports have different RTCCodecStats objects."

While "RTP transport" isn't clearly defined, there's a transportId member, so I'd assume this refers to that "transport" which says: "The unique identifier of the transport on which this codec is being used, which can be used to look up the corresponding RTCTransportStats object."

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.

But I'm not sure what this means for the sdpFmtpLine, since I believe a=fmtp may vary per m-line (but how often does it)?

One one hand this looks like a spec bug. On the other, returning 460+ JS objects seems like a waste and potential scaling issue.

cc @docfaraday @Pehrsons

@Pehrsons
Copy link

Another corner case is when an fmtp line differs in the local and remote sdps, so the encoder and decoder would be configured differently. If this difference in sdpFmtpLine should be surfaced through stats, the codec stats in this case end up being per rtp-stream rather than per transceiver. Related to #616.

Should codec stats be coalesced up to transport-level while all parameters match? That's certainly an option that avoids lots of waste in most cases.

@alvestrand
Copy link
Contributor

460 codec entries sounds like a bug to me.
The intent when writing the "codec" stats was that one would be able to know the precise configuration for each payload type; the way RTP works, there can't be two different codec configurations for the same payload type on the same transport in the same direction, so it only makes sense to have max 1 codec entry for each PT.

@jan-ivar
Copy link
Member Author

so it only makes sense to have max 1 codec entry for each PT.

...per direction in some cases, but yes this makes sense to me.

@henbos
Copy link
Collaborator

henbos commented Jan 26, 2022

460??? Yeahhhh, I'd say that's a bug

@vr000m
Copy link
Contributor

vr000m commented Feb 17, 2022

In callstats.js, we just filtered out the duplicates, hence, coalescing does make sense.

@fippo
Copy link
Contributor

fippo commented Sep 6, 2022

Given the recent motion to reduce codec stats greatly can we revisit this one? With @henbos latest proposal to only produce codecs that are in use this brings down the number from 4 to 2 but deduping seems more effort than these two objects are worth

@henbos
Copy link
Collaborator

henbos commented Sep 6, 2022

You're referring to the "per direction" note specifically, can you file a separate issue for that? The issue that codecs are per-transport and not per-transceiver still holds.

@fippo
Copy link
Contributor

fippo commented Sep 6, 2022

I am refering to the coalescing note that was added in #617 which is what completed this issue. But yes, specifically the second part of the note

@alvestrand
Copy link
Contributor

sadly, the PT can refer to different configs in the two directions of a transport.
I think it makes sense to think of the fmtp as "this is what I have promised to handle if you send it to me" - thus, the two ends can give different promises, but making two different promises at the same end (such as would happen if fmtp differed between two PT descriptions in the same bundle) makes absolutely no sense to me.

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.

6 participants