media: add flag to enable ICE renomination

[nandito]

Description

Summary:

Enable renomination if the iceRenominationOn feature is enabled.

Testing

1. Enable iceRenominationOn
2. Join the room 2x
3. Go to chrome://webrtc-internals and inspect the local and remote descriptions: the renomination keyword should appear in a a=ice-options line.

Screenshots/GIFs (if applicable)

Checklist

• My code follows the project's coding standards.
• Prefixed the PR title and commit messages with the service or package name
• I have written unit tests (if applicable).
• [] I have updated the documentation (if applicable).
• By submitting this pull request, I confirm that my contribution is made
  under the terms of the MIT license.

Additional Information

Read more about the renomination draft here: https://datatracker.ietf.org/doc/html/draft-thatcher-ice-renomination-01

[changeset-bot]

🦋 Changeset detected

Latest commit: 4c8b80d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@whereby.com/media	Minor
@whereby.com/core	Patch
@whereby.com/browser-sdk	Patch
@whereby.com/react-native-sdk	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[misi-whereby]

We may need to white list the browsers that we know that supports this feature, or
somehow fallback to original SDP if setLocalDescription gives back an error.
My guess that if the browser does not supports this ice option, then probably it will give back an error when we try to setLocalDescription /setRemoteDescription the sdp.

[nandito]

/canary

[github-actions]

🚀 The canary releases have been published to npm.

You can test the releases by installing the newly published versions:

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

[kevinwhereby]

I see all the relevant things when chrome <=> chrome, however joining with 1 chrome and 1 firefox I get this in chrome logs:

And my setRemoteDescription has renomination:

a=ice-options:trickle renomination

in Session.handleOffer we modify the incoming offer

 handleOffer(message: any) {
        // ...
        let sdp = message.sdp;
        if (this._iceRenominationOn) sdp = sdpModifier.enableIceRenomination(sdp);

Should check the offer for renomination before setting? Or maybe rely on the other client to send an offer with it enabled?

[nandito]

I see all the relevant things when chrome <=> chrome, however joining with 1 chrome and 1 firefox I get this in chrome logs:

And my setRemoteDescription has renomination:

a=ice-options:trickle renomination

in Session.handleOffer we modify the incoming offer

 handleOffer(message: any) {
        // ...
        let sdp = message.sdp;
        if (this._iceRenominationOn) sdp = sdpModifier.enableIceRenomination(sdp);

Should check the offer for renomination before setting? Or maybe rely on the other client to send an offer with it enabled?

We don't set the renomination in the offers/answers that we signal to the other participants.
We only set it on the local side when the local participant receives or creates an offer or an answer, we set the renomination in the SDP before storing it via setRemoteDescription or setLocalDescription.
The SDP we signal to the remote participant does not have this keyword as FF did not work with a=ice-options:trickle renomination

[nandito]

These browserName === "firefox" conditions can be problematic in FF nightly with media.navigator.mozgetusermedia.enabled = false. The webrtc-adapter returns "Not a supported browser." in this case in firefox, therefore these conditions are going to be falsy.

The related issue: webrtcHacks/adapter#1160 could fix that, but maybe it would be safer to parse the useragent on our own instead. (?)

[nandito]

/canary

[github-actions]

🚀 The canary releases have been published to npm.

You can test the releases by installing the newly published versions:

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

[jan-ivar]

Have you confirmed any difference in ICE behavior from this?

Read more about the renomination draft here: https://datatracker.ietf.org/doc/html/draft-thatcher-ice-renomination-01

That draft expired in 2017. From my testing with https://jsfiddle.net/jib1/6h75teaj/ no browser negotiates (responds in kind in an answer to an offer containing) a=ice-options:trickle renomination, including Chrome or Canary.

We may need to white list the browsers that we know that supports this feature, or somehow fallback to original SDP if setLocalDescription gives back an error. My guess that if the browser does not supports this ice option, then probably it will give back an error when we try to setLocalDescription /setRemoteDescription the sdp.

I don't think so. Even if I munge both ends https://jsfiddle.net/jib1/6h75teaj/34/ none of the browsers fail sLD/sRD. This matches my understanding of SDP which lets end-points advertise and negotiate a shared subset of support, without failure. Hopefully there's no need for UA-sniff here.

If there's some feature enabled by this, we'd love to learn what it is so it can be standardized!

[nandito]

Thank you for your input! 🙏

Have you confirmed any difference in ICE behavior from this?

Read more about the renomination draft here: https://datatracker.ietf.org/doc/html/draft-thatcher-ice-renomination-01

That draft expired in 2017. From my testing with https://jsfiddle.net/jib1/6h75teaj/ no browser negotiates (responds in kind in an answer to an offer containing) a=ice-options:trickle renomination, including Chrome or Canary.

We did a few measurements by running the example scenario from the linked draft. In most tests, we experienced slightly faster reconnects when the renomination keyword was added to the SDP, but this improvement and ICE behavior change is not fully confirmed yet.

Starting Chrome with --enable-logging=stderr --v=2, we can see the renomination enabled or disabled in the logs; this indicates that the keyword is not completely ignored. We haven't spent too much time trying to understand what happens under the hood in the browser by examining the Chromium codebase, though.

We're investigating how to measure the impact of this flag.

Hopefully there's no need for UA-sniff here.

There's no need to add any UA-sniff here. I thought first Firefox ignores the SDP if it has this renomination keyword, but it turned out it was because a bug I added in an earlier version of this PR. 🙈 But there's no UA-sniffing since I fixed that.

[nandito]

/canary

[github-actions]

🚀 The canary releases have been published to npm.

You can test the releases by installing the newly published versions:

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]

yarn add @whereby.com/[email protected]