w3c / webrtc-nv-use-cases

Use cases for WebRTC NV
https://w3c.github.io/webrtc-nv-use-cases/
Other
32 stars 13 forks source link

Need for Initial Bitrate by the Application/RtpSender? #31

Open vr000m opened 7 years ago

vr000m commented 7 years ago

With the ongoing discussion in the relative bitrate and RTCPriority, I was wondering if providing the initial bitrate to the RTCSender was ever discussed. If it was, what was the outcome of that discussion?

If not, my strawman would be: This would be a local property for the send media side, and this would be input to the congestion control to start sending at and normal congestion control procedures would define what happens next. If the application does not set it, the default of the implementation is used.

A developer may want to use a past value from a previous call that just finished as the initial bitrate to override any default initial bitrate set by the implementor. This could of course be the tail bitrate (the last few seconds of the call), average/median/the maximum sustained in the call -- it would be up to the developer how they come up with the value.

This is especially useful if the current default is higher than what the implementation thinks is the default start bitrate.

taylor-b commented 7 years ago

This is a use case we've run into as well. Also, sometimes the application may have more knowledge about the network than the browser does, and has a better idea of what bandwidth it's capable of supporting.

We added a native-only API for this kind of use case. I think the only question is whether this is in scope for WebRTC 1.0.

This also may be slightly related: https://github.com/w3c/ortc/issues/603

juberti commented 7 years ago

Seems like this should be an initial bandwidth estimate, rather than an initial bitrate (which may overshoot/undershoot the estimate for various reasons).

As such, not sure this belongs on the sender, but rather on the PC, perhaps in the RTCConfig.

taylor-b commented 7 years ago

That is what the native API I linked actually does, though the documentation may not make it obvious enough. And it does go on PeerConnection; I guess it could have gone on RTCConfig, but I think we didn't do that because "current bandwidth estimate" didn't seem fitting there.

vr000m commented 7 years ago

Thanks for the pointer to the native API, that does solve an issue. However, having this baked into the JS API would make the feature available universally. Furthermore, the fact that it is useful for the native bindings further endorses this point.

I could live with this being an initial estimate for all the streams carried over the peerconnection. RTCConfig sounds reasonable. About it being in v1.0, I am not very particular about it, although it seems straightforward to surface an existing feature with simple semantics.

@taylor-b was that a naming thing or did it not make sense to put it in there?

taylor-b commented 7 years ago

It's more that semantically, it didn't make sense. It's not a configuration, it's an active value. We'd have the question of "what does getConfiguration return; the last value passed in, or the actual current bandwidth estimate"? "What happens if you call setConfiguration, changing some other field but leaving currentBitrateEstimate alone? Does it reset the current estimate?" And so on.

vr000m commented 7 years ago

So answering the two questions:

Proposal: initialBandwidthEstimate to be set in setConfiguration. and when someone calls getConfiguration they will see the set value. Stats would report what the availableOutgoingBitrate from the ICE candidate pair will be, which can be verified as soon as the ICE goes in to connected state.

What happens when set is called without initialBandwidthEstimate, I guess that removes the initialBandwidthEstimate and the congestion control can revert to default behaviour.

taylor-b commented 7 years ago

This may be something the application would want to set in the middle of the connection, though. For example, in response to the selected ICE candidate pair changing, indicating the use of a different network path.

vr000m commented 7 years ago

Wouldn't the congestion control reset when the ICE candidates change? again the application could unset the initialBandwidthEstimate when the iceRestart begins and set it when the connection is set up.

taylor-b commented 7 years ago

Wouldn't the congestion control reset when the ICE candidates change?

Right, which would be why, just like for the initial connection, the application may have some knowledge it can use to populate a better initial estimate.

Also: with your suggestion, what happens if I call pc.setConfiguration(pc.getConfiguration())? getConfiguration() returns the previously set value, so what happens when it gets passed back into setConfiguration()? Does it reset the bandwidth estimate? Or would would we have the rule "it's only reset if it's changed to a different value"? Then you have the corner case where you want to reset it to the same value as before, and need to add/remove 1 to make it different.

This is the exact conversation we had during code review, and why we decided it was cleaner to just make it a separate API from setConfiguration. The "get/set" model just didn't really make sense for it, and there wasn't a good reason to not add a new method.

aboba commented 7 years ago

@vr000m Can you produce a PR?

fluffy commented 7 years ago

I think this would need to discussed with transport folks at IETF of exactly what is needed and what the attack model is. The idea that an ad network could set the initial send rate in JS seems a bit scary.

aboba commented 7 years ago

TPAC discussion indicated that this issue should be placed in the Icebox for now: https://www.w3.org/2017/11/06-webrtc-minutes.html