Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

fix: voice chat undefined setremotedescription #1887

Closed
wants to merge 7 commits into from
Closed

fix: voice chat undefined setremotedescription #1887

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8725a67
added RTCLoopbackConnection retry when dst is undefined
pravusjif Dec 28, 2020
48b0b9a
Add debugging info
agusaldasoro Dec 28, 2020
422e742
Remove duplicated logging
agusaldasoro Dec 28, 2020
583356c
Fix lint
agusaldasoro Dec 28, 2020
ae6464e
Use correct answer
agusaldasoro Dec 28, 2020
d8c73bc
Remove semicolon
agusaldasoro Dec 28, 2020
85d7949
Merge branch 'master' into fix/VoiceChatUndefinedSetRemoteDescription
agusaldasoro Dec 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions kernel/packages/voice-chat-codec/VoiceCommunicator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,28 +283,27 @@ export class VoiceCommunicator {
this.input?.workletNode.port.postMessage({ topic: topic })
}

private createRTCLoopbackConnection(
currentRetryNumber: number = 0
): { src: RTCPeerConnection; dst: RTCPeerConnection } {
private createRTCLoopbackConnection(retryNumber: number = 0): { src: RTCPeerConnection; dst: RTCPeerConnection } {
const src = new RTCPeerConnection()
const dst = new RTCPeerConnection()

let retryNumber = currentRetryNumber
// Apparently the RTCPeerConnection can be 'undefined' until the user accepts/blocks the microphone usage in the browser (https://github.com/webRTC-io/webrtc.io-client/issues/30#issuecomment-15991572)
// @ts-ignore
if (!dst || !src) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition will always return false.

There is NO WAY in that calling new X returns other than an object. It was a misinterpretation of the comment in the link.

That if should be removed

return this.createRTCLoopbackConnection(retryNumber + 1)
}

;(async () => {
(async () => {
// When having an error, we retry in a couple of seconds. Up to 10 retries.
src.onconnectionstatechange = (e) => {
if (
src.connectionState === 'closed' ||
src.connectionState === 'disconnected' ||
(src.connectionState === 'failed' && currentRetryNumber < 10)
(src.connectionState === 'failed' && retryNumber < 10)
) {
// Just in case, we close connections to free resources
this.closeLoopbackConnections()
this.loopbackConnections = this.createRTCLoopbackConnection(retryNumber)
} else if (src.connectionState === 'connected') {
// We reset retry number when the connection succeeds
retryNumber = 0
this.loopbackConnections = this.createRTCLoopbackConnection(retryNumber + 1)
}
}

Expand All @@ -317,20 +316,26 @@ export class VoiceCommunicator {

const offer = await src.createOffer()

await src.setLocalDescription(offer)
await src.setLocalDescription(offer).catch((err) => {
return Promise.reject(err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following two snippets are exactly the same:

await src.setLocalDescription(offer)
await src.setLocalDescription(offer).catch((err) => {
  return Promise.reject(err)
})

We should remove the .catch to avoid confussions

})

await dst.setRemoteDescription(offer)
await dst.setRemoteDescription(offer).catch((err) => {
return Promise.reject(err)
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

const answer = await dst.createAnswer()

const answerSdp = parse(answer.sdp!)

answerSdp.media[0].fmtp[0].config = 'ptime=5;stereo=1;sprop-stereo=1;maxaveragebitrate=256000'

answer.sdp = write(answerSdp)

await dst.setLocalDescription(answer)
await dst.setLocalDescription(answer).catch((err) => {
return Promise.reject(err)
})

await src.setRemoteDescription(answer)
await src.setRemoteDescription(answer).catch((err) => {
return Promise.reject(err)
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here too

})().catch((e) => {
defaultLogger.error('Error creating loopback connection', e)
})
Expand Down