Check if transport wide CC is supported

[IvanMorozoff]

I'm testing janus/videocalltest.html with my local build.

commit 175eea78b2592382261190569ca3ff4d9b1608c1
Date:   Tue Nov 27 11:26:41 2018 +0100

I've discovered that videocalltest.htm and videoroomtest.html behave different.
In case of videoroomtest.htm I get less number of NACKs in my test environment.

In case of videocalltest.html
handle->stream->do_transport_wide_cc are different (0 and 1) for the connected peers,
but both local and remote SDP contain extmap:5 and "transport-cc" lines.

Remote SDP:

a=extmap:5 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=rtcp-fb:101 transport-cc

Local SDP:

a=extmap:5 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
a=rtcp-fb:107 transport-cc

I suspect do_transport_wide_cc is not set properly in case of answer. See janus.c:

				if(!offer) {
...
				} else {
					/* Check if transport wide CC is supported */
					int transport_wide_cc_ext_id = janus_rtp_header_extension_get_id(jsep_sdp, JANUS_RTP_EXTMAP_TRANSPORT_WIDE_CC);
					handle->stream->do_transport_wide_cc = transport_wide_cc_ext_id > 0 ? TRUE : FALSE;
					handle->stream->transport_wide_cc_ext_id = transport_wide_cc_ext_id;
				}

[lminiero]

Yes, initially it was done on purpose, as the main target was the VideoRoom, so publishers only (who always offer). We loosened that constraint to allow other plugins to use it, but I never thought about scenarios with equal peers like the VideoCall.

Honestly I'll have to think about that change: we only support the receiving part of this mechanism (we receive the sequence numbers in the RTP extension, and send RTCP feedback), not the other way around, and I don't know if this will mean the browser will start assuming we do, and possible make performance worse. Have you tried forcing it for answers as well to see if it fixes your use case?

[IvanMorozoff]

Please clarify the following question:
Does this mean that in the case of VideoCall and if both peers support transport-cc
we always have connections as below:

caller<->janus: "transport_wide_cc"
answerer<->janus: "remb" 

?

[lminiero]

@IvanMorozoff just a quick note to explain I have not forgotten about this. I'm keeping the issue open as a memo, as it will be part of the effort needed whenever we'll start working on bridging extensions.

Let me clarify why it isn't simply a matter of negotiating the extension also on the callee's side, that is where we offer. If we did that, then the callee would add the RTP extension with their own global sequence numbers as well, which would indeed trigger the TWCC mechanism on that side too. Anyway, as it works right now, the same extension would be added to all outgoing packets reaching the caller as well: this would result in the caller to start sending us RTCP feedback related to TWCC, thinking we're implementing our side of things too, which we don't. And while this is only a relative issue for the VideoCall plugin, it would be a much more problematic issue in other plugins that envisage source switching of some sort (e.g., VideoRoom), as in that case the recipient would receive completely bogus info in those extensions (inconsistent numbering any time the source changes).

As such, before adding that support, we should first work on the rewriting/removal/addition of RTP extensions in the core depending on what's being negotiated, which is something we don't have yet. This will be needed for more than just this (for mid and rid, mostly), and won't be easy to implement. We'll need some form of info that provides the RTP extension IDs mappings on both sides, and something that "survives" going through plugins.

So, to answer (VERY late, I know) your question: yes, this is how it works right now in the VideoCall, and how it will keep on working until we change that:

caller<->janus: "transport_wide_cc"
answerer<->janus: "remb" 

[lminiero]

@IvanMorozoff not sure you still care about this issue or not, but I did keep the issue open for a reason, that is act as a reminder on some more complex things we needed to work on to get this working. We finally started the engines on that, so the PR you see referenced above does implement that part: it now allows transport-wide CC to be negotiated on both sides of a VideoCall, which should solve the issue you had. In the future, we'll implement actual BWE on top of that.

Please let me know if you're still interested in this, and want to test if it works for you, or if I can close this issue as I don't need the reminder anymore.

[lminiero]

Closing.