webrtc-stats icon indicating copy to clipboard operation
webrtc-stats copied to clipboard

codec.sdpFmtpLine isn't clear about which description to use

Open jan-ivar opened this issue 3 years ago • 6 comments

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.

jan-ivar avatar Jan 20 '22 04:01 jan-ivar

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.

alvestrand avatar Jan 20 '22 21:01 alvestrand

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.

jan-ivar avatar Jan 21 '22 16:01 jan-ivar

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".

jan-ivar avatar Jan 21 '22 21:01 jan-ivar

...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

jan-ivar avatar Jan 21 '22 21:01 jan-ivar

...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.

docfaraday avatar Jan 21 '22 22:01 docfaraday

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.

alvestrand avatar Feb 22 '22 14:02 alvestrand

the note added in #617 did also resolve this, no?

fippo avatar Sep 06 '22 15:09 fippo

I think this is resolved, yes.

alvestrand avatar Oct 04 '22 12:10 alvestrand