Two peers sending video/audio leads to immediate renegotiation

[feross]

Using simple-peer in a relatively simple setup. Seeing the non-initiator peer immediately request renegotiation even though everything is specified upfront in the constructor. I see two offer/answers sent from the initiator peer.

Here's my setup: Two peers, each initialized with the stream option, so addTrack() is called twice (once for video, once for audio) internally in the constructor. For the non-initiator peer, this._isNegotiating is already true because of this line in the constructor:

simple-peer/index.js

Line 89 in 90c3dfd

this._isNegotiating = this.negotiated ? false : !this.initiator // is this peer waiting for negotiation to complete?

Therefore, for the non-initiator peer, the calls to this._needsNegotiation() in response to addTrack() cause a negotiation to be immediately queued. Once the connection is stable it triggers the re-negotiation.

Is this intentional?

[feross]

I have a draft PR that fixes the problem, but in a hacky way: #671

Thoughts?

[nazar-pc]

I agree, it seems to be a bit wrong. I'd expect renegotiation to not be triggered before the first offer was received and answer for it was generated. It is just pointless in this case.

[t-mullen]

I've implemented a fix for this in #722, with tests and more debugging logs.

The causes were multiple:

• Add support for negotiated channels. #570 changed the initial value of _isNegotiating to use the channelConfig.negotiated option (whether the datachannel is negotiated isn't relevant to the negotiation of the peer).
• _firstStable was set in the wrong place.
• The entire approach of dropping the non-initiator's 1st queued renegotiation request on first stable is wrong - this may be a valid request if we add a track right after creating an answer. What we actually want to do is start negotiation on both peers and immediately discard the non-initiator's 1st batched negotiation (since we know the initiator will negotiate). This was an unlikely race condition, but I've fixed it anyways.

[feross]

Released as 9.8.0