From e5ed31fb47c4a6eab2effb760fb5ae34aeb00c2f Mon Sep 17 00:00:00 2001 From: WolverinDEV Date: Mon, 15 Feb 2021 15:53:01 +0100 Subject: [PATCH] Fixed some bugs and fixed a critical bug within the event registry --- shared/js/ConnectionManager.ts | 1 + shared/js/connection/rtc/Connection.ts | 36 ++++++++++++------------- shared/js/connection/rtc/RemoteTrack.ts | 36 ++++++++++++++++++------- shared/js/connection/rtc/SdpUtils.ts | 4 +-- shared/js/events.ts | 3 ++- shared/js/file/FileManager.tsx | 29 +++++++++++--------- shared/js/file/Transfer.ts | 28 +++++++++++-------- shared/js/proto.ts | 2 +- shared/js/settings.ts | 22 ++++++++++++++- web/app/voice/Connection.ts | 9 ++++--- web/app/voice/VoiceClient.ts | 3 +++ web/app/voice/VoicePlayer.ts | 10 +++---- 12 files changed, 116 insertions(+), 67 deletions(-) diff --git a/shared/js/ConnectionManager.ts b/shared/js/ConnectionManager.ts index 5ef4f6a9..2bcb79cb 100644 --- a/shared/js/ConnectionManager.ts +++ b/shared/js/ConnectionManager.ts @@ -138,6 +138,7 @@ loader.register_task(Stage.JAVASCRIPT_INITIALIZING, { name: "server manager init", function: async () => { server_connections = new ConnectionManager(); + (window as any).server_connections = server_connections; }, priority: 80 }); diff --git a/shared/js/connection/rtc/Connection.ts b/shared/js/connection/rtc/Connection.ts index 53fc1179..21d9b42f 100644 --- a/shared/js/connection/rtc/Connection.ts +++ b/shared/js/connection/rtc/Connection.ts @@ -11,6 +11,7 @@ import {ErrorCode} from "tc-shared/connection/ErrorCode"; import {WhisperTarget} from "tc-shared/voice/VoiceWhisper"; import {globalAudioContext} from "tc-backend/audio/player"; import {VideoBroadcastConfig, VideoBroadcastType} from "tc-shared/connection/VideoConnection"; +import {Settings, settings} from "tc-shared/settings"; const kSdpCompressionMode = 1; @@ -372,9 +373,7 @@ class InternalRemoteRTPAudioTrack extends RemoteRTPAudioTrack { if(state === 1) { validateInfo(); this.shouldReplay = true; - if(this.gainNode) { - this.gainNode.gain.value = this.gain; - } + this.updateGainNode(); this.setState(RemoteRTPTrackState.Started); } else { /* There wil be no info present */ @@ -383,9 +382,7 @@ class InternalRemoteRTPAudioTrack extends RemoteRTPAudioTrack { /* since we're might still having some jitter stuff */ this.muteTimeout = setTimeout(() => { this.shouldReplay = false; - if(this.gainNode) { - this.gainNode.gain.value = 0; - } + this.updateGainNode(); }, 1000); } } @@ -882,18 +879,23 @@ export class RTCConnection { iceServers: [{ urls: ["stun:stun.l.google.com:19302", "stun:stun1.l.google.com:19302"] }] }); - /* If set to false FF failed: FIXME! */ - const kAddGenericTransceiver = true; - if(this.audioSupport) { this.currentTransceiver["audio"] = this.peer.addTransceiver("audio"); this.currentTransceiver["audio-whisper"] = this.peer.addTransceiver("audio"); - /* add some other transceivers for later use */ - for(let i = 0; i < 8 && kAddGenericTransceiver; i++) { - const transceiver = this.peer.addTransceiver("audio"); - /* we only want to received on that and don't share any bandwidth limits */ - transceiver.direction = "recvonly"; + if(window.detectedBrowser.name === "firefox") { + /* + * For some reason FF (<= 85.0) does not replay any audio from extra added transceivers. + * On the other hand, if the server is creating that track or we're using it for sending audio as well + * it works. So we just wait for the server to come up with new streams (even though we need to renegotiate...). + * For Chrome we only need to negotiate once in most cases. + * Side note: This does not apply to video channels! + */ + } else { + /* add some other transceivers for later use */ + for(let i = 0; i < settings.getValue(Settings.KEY_RTC_EXTRA_AUDIO_CHANNELS); i++) { + this.peer.addTransceiver("audio", { direction: "recvonly" }); + } } } @@ -901,10 +903,8 @@ export class RTCConnection { this.currentTransceiver["video-screen"] = this.peer.addTransceiver("video"); /* add some other transceivers for later use */ - for(let i = 0; i < 4 && kAddGenericTransceiver; i++) { - const transceiver = this.peer.addTransceiver("video"); - /* we only want to received on that and don't share any bandwidth limits */ - transceiver.direction = "recvonly"; + for(let i = 0; i < settings.getValue(Settings.KEY_RTC_EXTRA_VIDEO_CHANNELS); i++) { + this.peer.addTransceiver("video", { direction: "recvonly" }); } this.peer.onicecandidate = event => this.handleLocalIceCandidate(event.candidate); diff --git a/shared/js/connection/rtc/RemoteTrack.ts b/shared/js/connection/rtc/RemoteTrack.ts index 3d29c3b6..009844b6 100644 --- a/shared/js/connection/rtc/RemoteTrack.ts +++ b/shared/js/connection/rtc/RemoteTrack.ts @@ -65,7 +65,7 @@ export class RemoteRTPTrack { } getSsrc() : number { - return this.ssrc; + return this.ssrc >>> 0; } getTrack() : MediaStreamTrack { @@ -144,7 +144,20 @@ export class RemoteRTPAudioTrack extends RemoteRTPTrack { this.htmlAudioNode.msRealTime = true; /* - TODO: ontimeupdate may gives us a hint whatever we're still replaying audio or not + { + const track = transceiver.receiver.track; + for(let key in track) { + if(!key.startsWith("on")) { + continue; + } + + track[key] = () => console.log("Track %d: %s", this.getSsrc(), key); + } + } + */ + + /* + //TODO: ontimeupdate may gives us a hint whatever we're still replaying audio or not for(let key in this.htmlAudioNode) { if(!key.startsWith("on")) { continue; @@ -153,7 +166,7 @@ export class RemoteRTPAudioTrack extends RemoteRTPTrack { this.htmlAudioNode[key] = () => console.log("AudioElement %d: %s", this.getSsrc(), key); this.htmlAudioNode.ontimeupdate = () => { console.log("AudioElement %d: Time update. Current time: %d", this.getSsrc(), this.htmlAudioNode.currentTime, this.htmlAudioNode.buffered) - } + }; } */ @@ -166,8 +179,7 @@ export class RemoteRTPAudioTrack extends RemoteRTPTrack { const audioContext = globalAudioContext(); this.audioNode = audioContext.createMediaStreamSource(this.mediaStream); this.gainNode = audioContext.createGain(); - - this.gainNode.gain.value = this.shouldReplay ? this.gain : 0; + this.updateGainNode(); this.audioNode.connect(this.gainNode); this.gainNode.connect(audioContext.destination); @@ -195,10 +207,7 @@ export class RemoteRTPAudioTrack extends RemoteRTPTrack { setGain(value: number) { this.gain = value; - - if(this.gainNode) { - this.gainNode.gain.value = this.shouldReplay ? this.gain : 0; - } + this.updateGainNode(); } /** @@ -209,4 +218,13 @@ export class RemoteRTPAudioTrack extends RemoteRTPTrack { this.gainNode.gain.value = 0; } } + + protected updateGainNode() { + if(!this.gainNode) { + return; + } + + this.gainNode.gain.value = this.shouldReplay ? this.gain : 0; + //console.error("Change gain for %d to %f (%o)", this.getSsrc(), this.gainNode.gain.value, this.shouldReplay); + } } \ No newline at end of file diff --git a/shared/js/connection/rtc/SdpUtils.ts b/shared/js/connection/rtc/SdpUtils.ts index 0787d249..25fb7811 100644 --- a/shared/js/connection/rtc/SdpUtils.ts +++ b/shared/js/connection/rtc/SdpUtils.ts @@ -28,7 +28,7 @@ export class SdpProcessor { rate: 48000, encoding: 2, fmtp: { minptime: 1, maxptime: 20, useinbandfec: 1, usedtx: 1, stereo: 0, "sprop-stereo": 0 }, - rtcpFb: [ "transport-cc" ] + rtcpFb: [ "transport-cc", "nack", "goog-remb" ] }, { // Opus Stereo/Opus Music @@ -37,7 +37,7 @@ export class SdpProcessor { rate: 48000, encoding: 2, fmtp: { minptime: 1, maxptime: 20, useinbandfec: 1, usedtx: 1, stereo: 1, "sprop-stereo": 1 }, - rtcpFb: [ "transport-cc" ] + rtcpFb: [ "transport-cc", "nack", "goog-remb" ] }, ]; diff --git a/shared/js/events.ts b/shared/js/events.ts index 8a9002bb..cc0cf62e 100644 --- a/shared/js/events.ts +++ b/shared/js/events.ts @@ -298,7 +298,8 @@ export class Registry = EventMap> implement } } - for(const handler of this.persistentEventHandler[event.type] || []) { + const handlers = [...(this.persistentEventHandler[event.type] || [])]; + for(const handler of handlers) { handler(event); } diff --git a/shared/js/file/FileManager.tsx b/shared/js/file/FileManager.tsx index b5d6ce91..14a92574 100644 --- a/shared/js/file/FileManager.tsx +++ b/shared/js/file/FileManager.tsx @@ -558,8 +558,9 @@ export class FileManager { "proto": 1 }, {process_result: false}); - if(transfer.transferState() === FileTransferState.INITIALIZING) + if(transfer.transferState() === FileTransferState.INITIALIZING) { throw tr("missing transfer start notify"); + } } catch (error) { transfer.setFailed({ @@ -620,8 +621,9 @@ export class FileManager { transfer: transfer, executeCallback: async () => { await callbackInitialize(transfer); /* noexcept */ - if(transfer.transferState() !== FileTransferState.CONNECTING) + if(transfer.transferState() !== FileTransferState.CONNECTING) { return; + } try { const provider = TransferProvider.provider(); @@ -633,12 +635,13 @@ export class FileManager { return; } - if(transfer instanceof FileDownloadTransfer) + if(transfer instanceof FileDownloadTransfer) { provider.executeFileDownload(transfer); - else if(transfer instanceof FileUploadTransfer) + } else if(transfer instanceof FileUploadTransfer) { provider.executeFileUpload(transfer); - else + } else { throw tr("unknown transfer type"); + } } catch (error) { const message = typeof error === "string" ? error : error instanceof Error ? error.message : tr("Unknown error"); transfer.setFailed({ @@ -651,7 +654,7 @@ export class FileManager { finishPromise: new Promise(resolve => { const unregisterTransfer = () => { transfer.events.off("notify_state_updated", stateListener); - transfer.events.off("action_request_cancel", cancelListener); + transfer.events.off("notify_transfer_canceled", unregisterTransfer); const index = this.registeredTransfers_.findIndex(e => e.transfer === transfer); if(index === -1) { @@ -681,6 +684,9 @@ export class FileManager { } else { logWarn(LogCategory.FILE_TRANSFER, tra("File transfer finished callback called with invalid transfer state ({0})", FileTransferState[state])); } + + /* destroy the transfer after all events have been fired */ + setTimeout(() => transfer.destroy(), 250); }; const stateListener = () => { @@ -690,13 +696,9 @@ export class FileManager { } }; - const cancelListener = () => { - unregisterTransfer(); - transfer.events.fire_later("notify_transfer_canceled", {}, resolve); - }; - transfer.events.on("notify_state_updated", stateListener); - transfer.events.on("action_request_cancel", cancelListener); + transfer.events.on("notify_transfer_canceled", unregisterTransfer); + stateListener(); }) }); @@ -705,8 +707,9 @@ export class FileManager { } private scheduleTransferUpdate() { - if(this.scheduledTransferUpdate) + if(this.scheduledTransferUpdate) { return; + } this.scheduledTransferUpdate = setTimeout(() => { this.scheduledTransferUpdate = undefined; diff --git a/shared/js/file/Transfer.ts b/shared/js/file/Transfer.ts index d23e7749..5e482dc0 100644 --- a/shared/js/file/Transfer.ts +++ b/shared/js/file/Transfer.ts @@ -113,8 +113,6 @@ export enum FileTransferDirection { export interface FileTransferEvents { "notify_state_updated": { oldState: FileTransferState, newState: FileTransferState }, "notify_progress": { progress: TransferProgress }, - - "action_request_cancel": { reason: CancelReason }, "notify_transfer_canceled": {} } @@ -239,9 +237,14 @@ export class FileTransfer { this.setTransferState(FileTransferState.PENDING); this.events = new Registry(); - this.events.on("notify_transfer_canceled", () => { + } + + destroy() { + if(!this.isFinished()) { this.setTransferState(FileTransferState.CANCELED); - }); + } + + this.events.destroy(); } isRunning() { @@ -253,7 +256,7 @@ export class FileTransfer { } isFinished() { - return this.transferState() === FileTransferState.FINISHED || this.transferState() === FileTransferState.ERRORED || this.transferState() === FileTransferState.CANCELED; + return this.transferState_ === FileTransferState.FINISHED || this.transferState_ === FileTransferState.ERRORED || this.transferState_ === FileTransferState.CANCELED; } transferState() { @@ -297,16 +300,19 @@ export class FileTransfer { } requestCancel(reason: CancelReason) { - if(this.isFinished()) + if(this.isFinished()) { throw tr("invalid transfer state"); + } this.cancelReason = reason; - this.events.fire("action_request_cancel"); + this.events.fire("notify_transfer_canceled"); + this.setTransferState(FileTransferState.CANCELED); } setTransferState(newState: FileTransferState) { - if(this.transferState_ === newState) + if(this.transferState_ === newState) { return; + } const newIsFinishedState = newState === FileTransferState.CANCELED || newState === FileTransferState.ERRORED || newState === FileTransferState.FINISHED; try { @@ -335,8 +341,9 @@ export class FileTransfer { case FileTransferState.FINISHED: case FileTransferState.CANCELED: case FileTransferState.ERRORED: - if(this.isFinished()) + if(this.isFinished()) { throw void 0; + } this.timings.timestampEnd = Date.now(); break; } @@ -358,7 +365,6 @@ export class FileTransfer { } } catch (e) { throw "invalid transfer state transform from " + this.transferState_ + " to " + newState; - return; } const oldState = this.transferState_; @@ -368,7 +374,7 @@ export class FileTransfer { updateProgress(progress: TransferProgress) { this.progress_ = progress; - this.events.fire_later("notify_progress", { progress: progress }); + this.events.fire("notify_progress", { progress: progress }); } awaitFinished() : Promise { diff --git a/shared/js/proto.ts b/shared/js/proto.ts index f604f7c3..e64e2df4 100644 --- a/shared/js/proto.ts +++ b/shared/js/proto.ts @@ -156,7 +156,7 @@ if(!JSON.map_field_to) { if (!Array.prototype.remove) { Array.prototype.remove = function(elem?: T): boolean { - const index = this.indexOf(elem, 0); + const index = this.indexOf(elem); if (index > -1) { this.splice(index, 1); return true; diff --git a/shared/js/settings.ts b/shared/js/settings.ts index 8b29bbb9..df25b3ce 100644 --- a/shared/js/settings.ts +++ b/shared/js/settings.ts @@ -591,10 +591,30 @@ export class Settings { valueType: "boolean", }; + static readonly KEY_RTC_EXTRA_VIDEO_CHANNELS: ValuedRegistryKey = { + key: "rtc_extra_video_channels", + defaultValue: 0, + requireRestart: true, + valueType: "number", + description: "Extra video channels within the initial WebRTC sdp offer.\n" + + "Note: By default the screen/camera share channels are already present" + }; + + static readonly KEY_RTC_EXTRA_AUDIO_CHANNELS: ValuedRegistryKey = { + key: "rtc_extra_audio_channels", + defaultValue: 6, + requireRestart: true, + valueType: "number", + description: "Extra audio channels within the initial WebRTC sdp offer.\n" + + "Note:\n" + + "1. By default the voice/whisper channels are already present.\n" + + "2. This setting does not work for Firefox." + }; + static readonly KEY_RNNOISE_FILTER: ValuedRegistryKey = { key: "rnnoise_filter", defaultValue: true, - description: "Enable the rnnoise filter for supressing background noise", + description: "Enable the rnnoise filter for suppressing background noise", valueType: "boolean", }; diff --git a/web/app/voice/Connection.ts b/web/app/voice/Connection.ts index f0118e20..d5441137 100644 --- a/web/app/voice/Connection.ts +++ b/web/app/voice/Connection.ts @@ -1,3 +1,4 @@ +import * as aplayer from "../audio/player"; import { AbstractVoiceConnection, VoiceConnectionStatus, @@ -15,7 +16,6 @@ import {RTCConnection, RTCConnectionEvents, RTPConnectionState} from "tc-shared/ import {AbstractServerConnection, ConnectionStatistics} from "tc-shared/connection/ConnectionBase"; import {VoicePlayerState} from "tc-shared/voice/VoicePlayer"; import {LogCategory, logDebug, logError, logInfo, logTrace, logWarn} from "tc-shared/log"; -import * as aplayer from "../audio/player"; import {tr} from "tc-shared/i18n/localize"; import {RtpVoiceClient} from "tc-backend/web/voice/VoiceClient"; import {InputConsumerType} from "tc-shared/voice/RecorderBase"; @@ -279,9 +279,9 @@ export class RtpVoiceConnection extends AbstractVoiceConnection { } const client = new RtpVoiceClient(clientId); - this.voiceClients[clientId] = client; - this.voiceClients[clientId].setGloballyMuted(this.speakerMuted); + client.setGloballyMuted(this.speakerMuted); client.events.on("notify_state_changed", this.voiceClientStateChangedEventListener); + this.voiceClients[clientId] = client; return client; } @@ -414,8 +414,9 @@ export class RtpVoiceConnection extends AbstractVoiceConnection { } private setConnectionState(state: VoiceConnectionStatus) { - if(this.connectionState === state) + if(this.connectionState === state) { return; + } const oldState = this.connectionState; this.connectionState = state; diff --git a/web/app/voice/VoiceClient.ts b/web/app/voice/VoiceClient.ts index 45a27fa1..86328f8e 100644 --- a/web/app/voice/VoiceClient.ts +++ b/web/app/voice/VoiceClient.ts @@ -1,5 +1,8 @@ import {VoiceClient} from "tc-shared/voice/VoiceClient"; import {VoicePlayer} from "./VoicePlayer"; +import {LogCategory, logTrace} from "tc-shared/log"; +import {tr} from "tc-shared/i18n/localize"; +import {RemoteRTPAudioTrack} from "tc-shared/connection/rtc/RemoteTrack"; export class RtpVoiceClient extends VoicePlayer implements VoiceClient { private readonly clientId: number; diff --git a/web/app/voice/VoicePlayer.ts b/web/app/voice/VoicePlayer.ts index 2f1fec1d..2f233f73 100644 --- a/web/app/voice/VoicePlayer.ts +++ b/web/app/voice/VoicePlayer.ts @@ -1,12 +1,8 @@ -import { - VoicePlayerEvents, - VoicePlayerLatencySettings, - VoicePlayerState -} from "tc-shared/voice/VoicePlayer"; +import {VoicePlayerEvents, VoicePlayerLatencySettings, VoicePlayerState} from "tc-shared/voice/VoicePlayer"; import {Registry} from "tc-shared/events"; -import {LogCategory, logWarn} from "tc-shared/log"; +import {LogCategory, logTrace, logWarn} from "tc-shared/log"; import {RemoteRTPAudioTrack, RemoteRTPTrackState} from "tc-shared/connection/rtc/RemoteTrack"; -import { tr } from "tc-shared/i18n/localize"; +import {tr} from "tc-shared/i18n/localize"; export interface RtpVoicePlayerEvents { notify_state_changed: { oldState: VoicePlayerState, newState: VoicePlayerState }