Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTCRtpSender.get/setParameters is underspecified #308

Closed
docfaraday opened this issue Sep 21, 2015 · 17 comments
Closed

RTCRtpSender.get/setParameters is underspecified #308

docfaraday opened this issue Sep 21, 2015 · 17 comments

Comments

@docfaraday
Copy link
Contributor

There are a number of questions that occur to me:

  • Are changes to the parameters signaled to the other side? If so, which ones, and does onnegotiationneeded fire?
  • How does content know when its preferences have been applied (or have failed)?
  • It is unclear whether getParameters() is intended to get the current preferences expressed by content, or the current state of the encoder. If it is the latter, there are more questions:
    • After a call to setParameters, it is unclear when the new value should be obtainable by a call to getParameters; does the operation need to actually complete? If renegotiation is required, does this need to finish?
    • If an attempted change to parameters fails (because the remote side prevented it, or some other reason), will the PC fire onnegotiationneeded if it thinks that would allow the preferences to be applied? Will the PC continue trying to apply the preferences on each subsequent renegotiation (ie; are these settings "sticky")?
@adam-be
Copy link
Member

adam-be commented Sep 24, 2015

PR #298 is partly taking a stab at this.

@alvestrand
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Member

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

@docfaraday
Copy link
Contributor Author

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
Copy link
Contributor

fluffy commented Oct 30, 2015

So seems decision was setParamters would be async with promise.

@martinthomson
Copy link
Member

As discussed at TPAC:

  • maxBitrate is just another constraint on the sending behaviour, there will be other values that the browser is expected to comply with
  • setParameters() returns a promise. The values provided by getParameters are updated when setParameters resolved.

@martinthomson
Copy link
Member

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

@alvestrand alvestrand assigned fluffy and unassigned adam-be Dec 7, 2015
@fluffy
Copy link
Contributor

fluffy commented Dec 9, 2015

Work on this is started on branch asyncSetParamters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants