Skip to content

Commit

Permalink
Fix race conditions on negotiation retry.
Browse files Browse the repository at this point in the history
Don't start streaming until peerConnection.iceConnectionState is 'connected'. Previously, clients would attempt start streaming whenever a description or candidate was received. There are guards in place e.g. makingOffer and otherClient.streaming and I'd still like to figure out why this race condition still persisted, but this approach caused excessive calls to Client#streamTo.
  • Loading branch information
domchristie committed Jun 25, 2021
1 parent a0423b6 commit 88de78d
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
4 changes: 2 additions & 2 deletions app/assets/javascripts/controllers/room_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export default class RoomController extends Controller {
clientId: this.client.id
})

this.signaller.on('description candidate', ({ detail: { from } }) => {
this.startStreamingTo(this.clients[from])
this.client.on('iceConnection:connected', ({ detail: { otherClient } }) => {
this.startStreamingTo(otherClient)
})
}

Expand Down
10 changes: 6 additions & 4 deletions app/assets/javascripts/models/webrtc_negotiation.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,10 @@ export default class WebrtcNegotiation {
})
}

this._onconnectionstatechange = (event) => {
if (this.peerConnection.connectionState === 'connected') {
this._oniceconnectionstatechange = (event) => {
this.client.broadcast(`iceConnection:${this.peerConnection.iceConnectionState}`, { otherClient: this.otherClient })

if (this.peerConnection.iceConnectionState === 'connected') {
this.retryCount = 0
}
}
Expand All @@ -158,15 +160,15 @@ export default class WebrtcNegotiation {

this.peerConnection.addEventListener('negotiationneeded', this._onnegotiationneeded)
this.peerConnection.addEventListener('icecandidate', this._onicecandidate)
this.peerConnection.addEventListener('connectionstatechange', this._onconnectionstatechange)
this.peerConnection.addEventListener('iceconnectionstatechange', this._oniceconnectionstatechange)
this.peerConnection.addEventListener('track', this._ontrack)
}

teardownPeerConnection () {
this.peerConnection.close()
this.peerConnection.removeEventListener('negotiationneeded', this._onnegotiationneeded)
this.peerConnection.removeEventListener('icecandidate', this._onicecandidate)
this.peerConnection.removeEventListener('connectionstatechange', this._onconnectionstatechange)
this.peerConnection.removeEventListener('iceconnectionstatechange', this._oniceconnectionstatechange)
this.peerConnection.removeEventListener('track', this._ontrack)
this.peerConnection = null
}
Expand Down

0 comments on commit 88de78d

Please sign in to comment.