Intent to Implement and Ship: RTCPeerConnection.setConfiguration

70 views
Skip to first unread message

Taylor Brandstetter

unread,
Dec 5, 2016, 6:28:18 PM12/5/16
to blin...@chromium.org

Contact emails

dead...@chromium.org


Spec

https://www.w3.org/TR/webrtc

https://datatracker.ietf.org/doc/draft-ietf-rtcweb-jsep/?include_text=1

Tag review: https://github.com/w3ctag/spec-reviews/issues/14


Summary

The setConfiguration method allows an application to modify the RTCConfiguration an RTCPeerConnection was constructed with. Specifically, it allows changes to the ICE transport policy, ICE servers and ICE candidate pool size.


Motivation

This is highly desired by web developers in order to specify new TURN credentials when the existing credentials expire. Currently, there's no workaround for this scenario aside from a full teardown of the connection.


Another desired use case is changing the ICE transport policy depending on the phase of a call. For example, a call may begin with only relay connections (either to speed up call setup or to protect the user's privacy), then later switch to the "all" policy.


Interoperability and Compatibility Risk

Low. There are no existing implementations of this method to my knowledge, but the specification for it has been stable for some time.


Ongoing technical constraints

  • The iceCandidatePoolSize property is not yet supported in RTCConfiguration; this includes the existing RTCPeerConnection constructor.
  • The "needs-ice-restart" bit described by JSEP is not implemented. This means applications will need to explicitly request an ICE restart after calling setConfiguration.
  • The current implementation of setConfiguration does not throw the correct DOMExceptions when invalid parameters are supplied.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.


OWP launch tracking bug

crbug.com/587453


Link to entry on the feature dashboard

https://www.chromestatus.com/feature/5596193748942848


Requesting approval to ship?

Yes.

Chris Harrelson

unread,
Dec 6, 2016, 8:45:49 PM12/6/16
to Taylor Brandstetter, blink-dev
Hi,

On Mon, Dec 5, 2016 at 6:28 PM, 'Taylor Brandstetter' via blink-dev <blin...@chromium.org> wrote:
  • The current implementation of setConfiguration does not throw the correct DOMExceptions when invalid parameters are supplied.
Is this one hard to fix before shipping?
 

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.


OWP launch tracking bug

crbug.com/587453


Link to entry on the feature dashboard

https://www.chromestatus.com/feature/5596193748942848


Requesting approval to ship?

Yes.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

Taylor Brandstetter

unread,
Dec 6, 2016, 9:58:51 PM12/6/16
to Chris Harrelson, blink-dev
Not very. We just need to add a way to propagate errors from the WebRTC library.

Harald Alvestrand

unread,
Dec 7, 2016, 2:37:53 AM12/7/16
to Taylor Brandstetter, Chris Harrelson, blink-dev
Is there a standards-compliant way of requesting ICE restart without calling setConfiguration?

If not, this is also strongly desirable to fix before shipping (and shouldn't be too hard, as long as there exists a non-standard method).

PhistucK

unread,
Dec 7, 2016, 3:06:36 AM12/7/16
to Taylor Brandstetter, blink-dev

On Tue, Dec 6, 2016 at 1:28 AM, 'Taylor Brandstetter' via blink-dev <blin...@chromium.org> wrote:
Low. There are no existing implementations of this method to my knowledge, but the specification for it has been stable for some time.

​Unless other vendors have indicated that they will implement it, the interoperability risk is high.​



PhistucK

Harald Alvestrand

unread,
Dec 7, 2016, 4:34:52 AM12/7/16
to PhistucK, Taylor Brandstetter, blink-dev
The Mozilla bug for implementing it is here:


Microsoft has been actively discussing the feature, their WebRTC 1.0 support is "in development", and so far they've shown strong willingness to implement the API as specified.

So I think there's good signs that there will be implementations.

--

PhistucK

unread,
Dec 7, 2016, 4:55:46 AM12/7/16
to Harald Alvestrand, Taylor Brandstetter, blink-dev
The Mozilla bug says nothing. The willingness from Microsoft is indeed something, but nothing huge. I would say medium risk at least.


PhistucK

Taylor Brandstetter

unread,
Dec 7, 2016, 2:11:34 PM12/7/16
to Harald Alvestrand, Chris Harrelson, blink-dev
Yes; you can call createOffer({"iceRestart":true}), which is standards compliant.

I agree that this wouldn't be hard to fix. Any of the caveats listed could be fixed before shipping if needed. Though iceCandidatePoolSize is a bit different, since that could be considered a new feature by itself.

Philip Jägenstedt

unread,
Jan 4, 2017, 7:27:16 AM1/4/17
to Taylor Brandstetter, Harald Alvestrand, Chris Harrelson, blink-dev
In https://bit.ly/blinkintents it is marked as having 3xLGTM, but that looks like a mistake, this still needs to be resolved. On the technical constraints mentioned:

  • The iceCandidatePoolSize property is not yet supported in RTCConfiguration; this includes the existing RTCPeerConnection constructor.
It's not in Gecko's RTCConfiguration.webidl either, so that's fine I think.
  • The "needs-ice-restart" bit described by JSEP is not implemented. This means applications will need to explicitly request an ICE restart after calling setConfiguration.
I take it this would have no effect in the constructor, and only be useful with setConfiguration? If there's no way to feature detect this, it would be a shame if it's not supported from day one. Any idea of the effort required?
  • The current implementation of setConfiguration does not throw the correct DOMExceptions when invalid parameters are supplied.


Certain cases do throw in parseConfiguration in RTCPeerConnection.cpp. Of the exceptions that are spelled out in the spec, which are never thrown? The validation-style ones should be trivial to add while implementing, so if what's left is of corner case nature, adding tests for that in web-platform-tests (that would fail in Chromium) and filing bugs to fix them would be OK I think.

Thanks for taking on this work!

On Wed, Dec 7, 2016 at 8:11 PM 'Taylor Brandstetter' via blink-dev <blin...@chromium.org> wrote:
Yes; you can call createOffer({"iceRestart":true}), which is standards compliant.

I agree that this wouldn't be hard to fix. Any of the caveats listed could be fixed before shipping if needed. Though iceCandidatePoolSize is a bit different, since that could be considered a new feature by itself.
On Tue, Dec 6, 2016 at 11:37 PM, Harald Alvestrand <h...@google.com> wrote:
Is there a standards-compliant way of requesting ICE restart without calling setConfiguration?

If not, this is also strongly desirable to fix before shipping (and shouldn't be too hard, as long as there exists a non-standard method).

On Wed, Dec 7, 2016 at 3:58 AM, 'Taylor Brandstetter' via blink-dev <blin...@chromium.org> wrote:
Not very. We just need to add a way to propagate errors from the WebRTC library.
On Tue, Dec 6, 2016 at 5:45 PM, Chris Harrelson <chri...@chromium.org> wrote:
Hi,

On Mon, Dec 5, 2016 at 6:28 PM, 'Taylor Brandstetter' via blink-dev <blin...@chromium.org> wrote:
  • The current implementation of setConfiguration does not throw the correct DOMExceptions when invalid parameters are supplied.
Is this one hard to fix before shipping?
 

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.


OWP launch tracking bug

crbug.com/587453


Link to entry on the feature dashboard

https://www.chromestatus.com/feature/5596193748942848


Requesting approval to ship?

Yes.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.


--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.


Taylor Brandstetter

unread,
Jan 4, 2017, 1:23:06 PM1/4/17
to Philip Jägenstedt, Harald Alvestrand, Chris Harrelson, blink-dev
  • The "needs-ice-restart" bit described by JSEP is not implemented. This means applications will need to explicitly request an ICE restart after calling setConfiguration.
I take it this would have no effect in the constructor, and only be useful with setConfiguration? If there's no way to feature detect this, it would be a shame if it's not supported from day one. Any idea of the effort required?

I actually implemented this recently, so that constraint is no longer applicable: https://codereview.webrtc.org/2563153002
I was going to send an email after I also got the exceptions wired up.

Certain cases do throw in parseConfiguration in RTCPeerConnection.cpp. Of the exceptions that are spelled out in the spec, which are never thrown?

Ah, I didn't notice that. In that case, it may just be the InvalidModificationError. This could probably be handled relatively easily at the Blink level, but I was working on adding error information at the webrtc level: https://codereview.webrtc.org/2587133004/

I think this approach will reduce some code duplication, and is needed for some of the more low-level errors.


Hi,

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.


--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.



Taylor Brandstetter

unread,
Jan 11, 2017, 9:25:57 PM1/11/17
to blink-dev
Update: the technical constraints listed above have been fixed (aside from iceCandidatePoolSize being unimplemented; I started a separate intent thread for that).

Still looking for review from API owners. This was marked as having 3 LGTMs, but it actually has none I can see.

foo...@google.com

unread,
Jan 17, 2017, 4:21:05 AM1/17/17
to blink-dev
Sorry about that Taylor, lgtm1.

BTW, do you plan on shipping setConfiguration and getConfiguration in the same milestone? They presumably make the most sense together.

Regarding the error handling, I think that is perhaps best handled in code review.

Chris Harrelson

unread,
Jan 17, 2017, 1:07:48 PM1/17/17
to Philip Jägenstedt, blink-dev
LGTM2

Taylor Brandstetter

unread,
Jan 17, 2017, 2:01:52 PM1/17/17
to foo...@google.com, blink-dev
BTW, do you plan on shipping setConfiguration and getConfiguration in the same milestone? They presumably make the most sense together.

Yes; it's getting late for M57 so these will probably be shipped with M58.

On Tue, Jan 17, 2017 at 1:21 AM, foolip via blink-dev <blin...@chromium.org> wrote:

Taylor Brandstetter

unread,
Jan 24, 2017, 4:19:33 PM1/24/17
to blink-dev
Bumping; still need one more LGTM. As mentioned above, the technical constraints originally listed have been fixed.

TAMURA, Kent

unread,
Jan 25, 2017, 3:08:00 AM1/25/17
to Taylor Brandstetter, blink-dev
LGTM3.

--
TAMURA Kent
Software Engineer, Google


Reply all
Reply to author
Forward
0 new messages