w3c / webrtc-pc

WebRTC 1.0 API
https://w3c.github.io/webrtc-pc/
Other
437 stars 115 forks source link

RTCRtpSender.get/setParameters is underspecified #308

Closed docfaraday closed 8 years ago

docfaraday commented 9 years ago

There are a number of questions that occur to me:

adam-be commented 9 years ago

PR #298 is partly taking a stab at this.

alvestrand commented 9 years ago

I think we have consensus that RTPSender.setParameters() never causes signalling to happen. Either it succeeds or it fails. It's not a promise-returning function, so it has to raise an exception when it fails.

docfaraday commented 9 years ago

I think we have consensus that RTPSender.setParameters() never causes signalling to happen

I think this implies one of three things:

  1. A WebRTC endpoint must be prepared for the received bitrate to exceed the maximum bitrate negotiated previously (similar story for other constraints, when they are defined). This does not mesh well with the work being done over in mmusic on simulcast. If this is what you want, there needs to be some discussion over there.
  2. We expect setParameters to fail if a parameter is inconsistent with what was negotiated previously. This would be pretty bad.
  3. We expect setParameters to wait until renegotiation happens to apply the new parameters, if a parameter is inconsistent with what was negotiated previously.

It's not a promise-returning function, so it has to raise an exception when it fails

This implies that setParameters does not queue a task, nor can it be asynchronous, which probably means that the most it can do in practice is remember the parameters, but not actually apply them. This is probably ok with me, but I'll need to think it over.

jan-ivar commented 9 years ago

I think it's 2. A synchronous control surface to a background process I think can be fine, provided the API only exposes previously set settings, or things that don't change often. The lack of a promise then merely means the JS has no access to the delay inherent in changing things in the background, plus the UA has no means of communicating failures beyond input validation errors unrelated to live state. Whether the latter is a problem, I don't know.

My only concern would be any readonly runtime "parameters", since getParameters is synchronous as well. I don't know at last count if we still have any of those, but we might want to be careful to avoid any API that would force a background thread to continuously have to update the main JS thread just on the chance that JS might want to query a runtime value synchronously.

docfaraday commented 9 years ago

Option 2 seems really awful to me. How would you increase the max bitrate for example? The only way I could see is to remove the track, renegotiate, and re-add it with the new max bitrate value. Just terrible.

martinthomson commented 9 years ago

I believe that if we had a bitrate setting somewhere, you could renegotiate without changing the bitrate value. But if setParameters() is expected to not affect negotiation, then it would be surprising to have it affect negotiation.

I think that we could consider bitrate control at the browser to be solely the domain of local control. And that negotiation is only to set the hard upper bound. If you take that position, then you would not be able to increase bitrate via negotiation.

docfaraday commented 9 years ago

We do have a maxBitrate setting in RTCRtpEncodingParameters, and my understanding is that this is the knob you turn to change the max send bitrate on a given simulcast stream, which must then be sent to the other end in SDP (otherwise, how does the other end know which stream is going to be the higher bandwidth one).

martinthomson commented 9 years ago

I really don't know what the simulcast solution looks like here. It could be that the first one is the biggest one. Or that this is signaled out of band.

alvestrand commented 9 years ago

My understanding is that: 1) we don't have a simulcast solution yet, and may still abandon hope of one for 1.0 2) most simulcast solutions I've seen in other contexts add some kind of sequence (lowest-highest) so that one doesn't have to guess.

docfaraday commented 9 years ago

Ok, this is at a pretty fundamental impedance mismatch with the ongoing work over in mmusic; this work relies on explicit indication of maximums of things like send bitrate, resolution, fps, etc.

https://mailarchive.ietf.org/arch/msg/mmusic/W90OvjhOh-eGtInYWZz-JEoaJr4

alvestrand commented 9 years ago

Yes. The MMUSIC hints (I wouldn't call it ongoing work as long as the proposals are not brought to the list) are all abot negotiating limits in SDP. The WebRTC RTPSender.setParamters thing is designed to be SDP-independent, and to be capable of being expanded to taking the result of SDP negotiation and applying it to a sender/receiver pair. This is part of supporting the charter decision that the WebRTC "next version" be capable of operating without SDP.

jan-ivar commented 9 years ago

Odd then that we've decided to limit these APIs to the negotiated envelope. The WG had that choice as recently as replaceTrack.

docfaraday commented 9 years ago

When talking about simulcast, keep in mind that SDP interop is going to be a fact of life for a very long time. This is what conferencing servers use, and that's the primary use-case for simulcast. It would be a very good idea to ensure that the work in mmusic is compatible with the model over here in W3C.

One way or another, the other end is going to need to be able to figure out which stream is which. If we say that maxBitrate is not signaled, that requires one of two things:

  1. Some sort of implicit ordering.
    • This relies on content not doing something silly like setting a higher maxBitrate on the second stream than the first. Maybe not a huge deal.
    • Implicit ordering only helps so much. If you know a priori how many streams your conferencing server wants, and what the relative bitrates are, you're ok, but if you're not sure whether you need to offer two or three streams, it becomes very muddy very quickly. Let's say you offer three, and the conferencing server wants only two; which two should it pick? If we're making a baseline assumption that content JS already knows what the other end wants, we should explicitly say so.
  2. The other end must inspect the RTP and determine which is "small" and "large".
    • This is pretty clunky, and still makes it hard for a conferencing server to select which streams it wants.
fluffy commented 8 years ago

So seems decision was setParamters would be async with promise.

martinthomson commented 8 years ago

As discussed at TPAC:

martinthomson commented 8 years ago

@fluffy will coordinate on generating a PR for the above.

fluffy commented 8 years ago

Work on this is started on branch asyncSetParamters