Implement Transport::setMinOutgoingBitrate

[jcague]

Implements this new API to set the minimum outgoing bitrate for WebRTC transports. It is similar to the existing setMaxIncomingBitrate and setMaxOutgoingBitrate.

The main use-case would be to enforce a bitrate between client and server when the network is reliable.

[ibc]

@jmillan how much putada is merging this in v3 (it will cause conflicts and will require migration to flatbuffers).

[jmillan]

It is some putada, but I don't think we can stop development due to it. I'll resume flatbuffers work ASAP.

[ibc]

The main use-case would be to enforce a bitrate between client and server when the network is reliable.

Is this the real use-case? Or is this a workaround to deal with the bad bandwidth estimations of the integrated libwebrtc stuff? I don't really understand the purpose of this PR. Imagine that there are perfect network conditions and the Transport just handles a single Consumer which is sending 1000bps (which is the bitrate of the original sender) to the receiver device. Now you call `transport.setMinOutgoingBitrate(2000). What does it mean exactly?

[jcague]

Yes, that's the use case we have in mind. Some clients are running in the same network as mediasoup, and we don't want the BWE to mess things up in some cases. We have yet to see those issues in production, but this new API is just a way to ensure we have the max possible video quality for those cases.

In your example, if the producer sends 1000bps and you call transport.setMinOutgoingBitrate(2000) to the consumer, it will receive 1000bps, or what the BWE estimates. In other pseudo-code, the result of calling setMinOutgoingBitrate would be something like:

reportedEstimatedBW = max(minOutgoingBitrate, estimatedBW)

[ibc]

I'm afraid this is not the way to go. Basically you want to disable BWE in certain scenarios (such as when clients are in same network than mediasoup) because current BWE doesn't behave correctly (super well known old issue). The thing is:

• If BWE system reports 1000bps, how do you know that it's wrong? Just because this is local network and available bandwidth should be way higher?
• Of course that's true, but what it proves is that current BWE in mediasoup doesn't work properly, so that's the thing to fix, We should have a BWE system that properly estimates available bandwidth rather than assuming that it works wrong and imposing artificial bottom limits via API to workaround it.

Look, there is a huge effort here #922 to make BWE in mediasoup work properly. It comes with an updated version of libwebrtc code plus many custom changes to make it work as expected in mediasoup. There is still work to do, but results are promising. We don't want to provide an API to "mitigate current BWE flaws". People will start using such a new setMinOutgoingBitrate() API and it will look as if it works "better", but it's not true. Whenever real network issues happen it will behave terribly by congesting users' network due to the artificial BWE bottom limit.

So we are not gonna merge this featue. This is the anti way to go. What we need is that people interested in this stuff contribute to the huge ongoing work of @sarumjanuch in #922 (try it, test it, contribute to it). We don't want to hide current flaws and we don't want that this outstanding long-term real issue gets even less attraction from mediasoup community "because there is a workaround".

BTW, if you just want to disable BWE in certain cases you can do it by removing transport-cc feedback (and optionally goog-remb as well) from rtpCapabilities given to transport.consume() in mediasoup side. Just that. Having an artificial no-sense API for that is not the way to go.

[jcague]

Removing transport-cc and goog-remb is a good alternative, and we can use it.

I'm sorry I was not explicit enough. I don't want to work around or mitigate any possible issue. To clarify, I think this functionality would make sense even if the BWE has 100% accuracy. The goal is not to run a complex system (BWE) when we don't need it (dedicated local networks). I would do the same between Chromes if I needed it (using x-google-min-bitrate?).

A solution using minOutgoingBitrate sounds less hacky to me than removing transport-cc/remb, but I might be wrong, and I understand that you prefer not to add a new API.

[ibc]

I would do the same between Chromes if I needed it (using x-google-min-bitrate?).

I guess that such a value is not intended to override current estimated uplink bandwidth. Anyway it's a "x-google" flag for good reasons.

A solution using minOutgoingBitrate sounds less hacky to me than removing transport-cc/remb

I don't agree. Clients are supposed to tell api-media what they support and what they want or what they do not want. The way to communicate it is by properly filling rtpCapabilities given to transport.consume(). There could be a fancy 3rd party util library that modifies those rtpCapabilities but that's a high level thing we don't want to include as part of mediasoup.

[jcague]

I guess that such a value is not intended to override current estimated uplink bandwidth. Anyway it's a "x-google" flag for good reasons.

Agree, I was not saying that it is intended for overriding the BWE, but that's maybe the way to achieve the goal I described. Not sure anyway, as I haven't tested it.

Clients are supposed to tell api-media what they support and what they want or what they do not want.

Fair enough, that's the way to go, then.

Thanks for taking the time to review the PR and discuss this topic!

[ibc]

After talking to @ggarber I'll be nicer and accept this functionality regardless I'm not super fan of it.

@jmillan I'll take care of adding it into flatbuffers branch.

[ibc]

@jcague I've basically copied this PR into a new one #1038 because I needed to test some missing things. All credits to you. closing this one.