webrtc-stats
webrtc-stats copied to clipboard
codec.sdpFmtpLine isn't clear about which description to use
webrtc-stats sdpFmtpLine says: "The a=fmtp line in the SDP corresponding to the codec, i.e., after the colon following the PT. This defined by [JSEP] in Section 5.7."
This isn't very clear about which description this SDP is from.
webrtc-pc sdpFmtpLine parameter says: "The "format specific parameters" field from the a=fmtp line in the SDP corresponding to the codec, if one exists, as defined by [RFC8829] (section 5.8.). For an RTCRtpSender, these parameters come from the remote description, and for an RTCRtpReceiver, they come from the local description."
The last one is newer and seems more helpful as it clarifies what description the SDP comes from, but see https://github.com/w3c/webrtc-pc/issues/2710 for clarifying this is sometimes the pending local description.
Should we update this spec to use that clarification as well? Or is stats supposed to return different codec information than getParameters when there is a pending local description?
A separate question is what "the SDP corresponding to the codec" actually means, since a=fmtp is per m-line. But that's https://github.com/w3c/webrtc-stats/issues/614.
Stats should be consistent with getParameters(). Since they may be different, we may need to add a field for distinguishing "receiver" codecs from "sender" codecs.
we may need to add a field for distinguishing "receiver" codecs from "sender" codecs.
We have codecType for that ("decode" and "encode" respectively).
It sounds like we're agreeing that when sdpFmtpLines are the same in both directions we should output:
{
id: "5ec992bb",
timestamp: 1642781368658.561,
type: "codec",
channels: 2,
clockRate: 48000,
mimeType: "audio/opus",
payloadType: 111,
sdpFmtpLine: "useinbandfec=1",
transportId: "8f4d6007"
}
...but if they differ, we should instead output (note codecTypes):
{
id: "5ec992bb",
timestamp: 1642781368658.561,
type: "codec",
channels: 2,
clockRate: 48000,
codecType: "encode",
mimeType: "audio/opus",
payloadType: 111,
sdpFmtpLine: "useinbandfec=1",
transportId: "8f4d6007"
},
{
id: "acc568dd",
timestamp: 1642781368658.561,
type: "codec",
channels: 2,
clockRate: 48000,
codecType: "decode",
mimeType: "audio/opus",
payloadType: 111,
sdpFmtpLine: "minptime=20;useinbandfec=1",
transportId: "8f4d6007"
}
Update: I've added a clarifying note on this in the PR on https://github.com/w3c/webrtc-stats/issues/614.
Stats should be consistent with getParameters().
I'm unsure. After pondering https://github.com/w3c/webrtc-pc/pull/2711#issuecomment-1018857181 I think we had reasons for why we decided receiver.getParameters().codecs should reflect pendingLocalDescription in "have-local-offer" and "have-remote-offer", but I forget what they were.
But regardless, I don't think the same reasons apply to "codec" stats, which instead should always reflect what's "currently being used".
...what's "currently being used".
Unless... is early media still a thing? Is a receiver in "have-local-offer" or "have-remote-offer" prepared to receive any payload types defined in the union of currentLocalDescription and pendingLocalDescription? cc @docfaraday
...what's "currently being used".
Unless... is early media still a thing? Is a receiver in "have-local-offer" or "have-remote-offer" prepared to receive any payload types defined in the union of currentLocalDescription and pendingLocalDescription? cc @docfaraday
In the pre-webrtc days, it was a general rule that you never created an SDP that you were not prepared to honor immediately. However, this is often not possible due to ICE and BUNDLE, because those things rely on having both an offer and an answer to establish network connectivity. You could in theory do this on a renegotiation that reused already established transports, though, and webrtc-pc seems to hint that this is something we're expected to do (grep for "currently prepared to receive").
Of course, there's the whole pranswer business too.
WRT the original problem - I think we should add a pointer to the webrtc spec's definition, and not try to repeat (and maintain) it here.
the note added in #617 did also resolve this, no?
I think this is resolved, yes.