From da1d068ade96aa5974c07b630fe1c716ea31ec65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= <s@saghul.net> Date: Thu, 8 Aug 2024 10:42:06 +0200 Subject: [PATCH] feat(statistics) remove AudioOutputProblemDetector It has been broken for over 3 years now, since https://github.com/jitsi/lib-jitsi-meet/commit/ca325f5ef93f853d12ad39ceb40b9bac6b05a56c#diff-9e19da30f465ca5665ac3d7ca1aa03d0498aed1be0cb2d7eeb27684a2636da77 Ever since that change, the "audioLevelReportHistory" property is not populated, so it justs acts on nothing an generates bogus log lines such as: ``` [modules/statistics/AudioOutputProblemDetector.js] A potential problem is detected with the audio output for participant b5fb30bc, local audio levels: [null,null], remote audio levels: undefined ``` Since nobody seems to have noticed in 3 years it's safe to assume we don't need this at all, so it gets the axe treatment. --- JitsiConference.js | 14 -- modules/connectivity/ConnectionQuality.js | 6 +- .../statistics/AudioOutputProblemDetector.js | 160 ------------------ modules/statistics/RTPStatsCollector.js | 28 +-- .../AudioOutputProblemDetector.d.ts | 6 - 5 files changed, 3 insertions(+), 211 deletions(-) delete mode 100644 modules/statistics/AudioOutputProblemDetector.js delete mode 100644 types/hand-crafted/modules/statistics/AudioOutputProblemDetector.d.ts diff --git a/JitsiConference.js b/JitsiConference.js index 07f5675a2d..d4ada0be6a 100644 --- a/JitsiConference.js +++ b/JitsiConference.js @@ -30,7 +30,6 @@ import { LiteModeContext } from './modules/litemode/LiteModeContext'; import { QualityController } from './modules/qualitycontrol/QualityController'; import RecordingManager from './modules/recording/RecordingManager'; import Settings from './modules/settings/Settings'; -import AudioOutputProblemDetector from './modules/statistics/AudioOutputProblemDetector'; import AvgRTPStatsReporter from './modules/statistics/AvgRTPStatsReporter'; import LocalStatsCollector from './modules/statistics/LocalStatsCollector'; import SpeakerStatsCollector from './modules/statistics/SpeakerStatsCollector'; @@ -221,14 +220,6 @@ export default function JitsiConference(options) { this.avgRtpStatsReporter = new AvgRTPStatsReporter(this, options.config.avgRtpStatsN || 15); - /** - * Detects issues with the audio of remote participants. - * @type {AudioOutputProblemDetector} - */ - if (!options.config.disableAudioLevels) { - this._audioOutputProblemDetector = new AudioOutputProblemDetector(this); - } - /** * Indicates whether the connection is interrupted or not. */ @@ -649,11 +640,6 @@ JitsiConference.prototype.leave = async function(reason) { this.avgRtpStatsReporter = null; } - if (this._audioOutputProblemDetector) { - this._audioOutputProblemDetector.dispose(); - this._audioOutputProblemDetector = null; - } - if (this.e2eping) { this.e2eping.stop(); this.e2eping = null; diff --git a/modules/connectivity/ConnectionQuality.js b/modules/connectivity/ConnectionQuality.js index 5ed2c075fe..3aef1e9292 100644 --- a/modules/connectivity/ConnectionQuality.js +++ b/modules/connectivity/ConnectionQuality.js @@ -366,8 +366,7 @@ export default class ConnectionQuality { connectionQuality: this._localStats.connectionQuality, jvbRTT: this._localStats.jvbRTT, serverRegion: this._localStats.serverRegion, - maxEnabledResolution: this._localStats.maxEnabledResolution, - avgAudioLevels: this._localStats.localAvgAudioLevels + maxEnabledResolution: this._localStats.maxEnabledResolution }; try { @@ -450,8 +449,7 @@ export default class ConnectionQuality { connectionQuality: data.connectionQuality, jvbRTT: data.jvbRTT, serverRegion: data.serverRegion, - maxEnabledResolution: data.maxEnabledResolution, - avgAudioLevels: data.avgAudioLevels + maxEnabledResolution: data.maxEnabledResolution }; this.eventEmitter.emit( diff --git a/modules/statistics/AudioOutputProblemDetector.js b/modules/statistics/AudioOutputProblemDetector.js deleted file mode 100644 index 8ace9356bd..0000000000 --- a/modules/statistics/AudioOutputProblemDetector.js +++ /dev/null @@ -1,160 +0,0 @@ -import { getLogger } from '@jitsi/logger'; - -import * as ConferenceEvents from '../../JitsiConferenceEvents'; -import { MediaType } from '../../service/RTC/MediaType'; -import * as ConnectionQualityEvents from '../../service/connectivity/ConnectionQualityEvents'; -import { createAudioOutputProblemEvent } from '../../service/statistics/AnalyticsEvents'; - -import Statistics from './statistics'; - -const logger = getLogger(__filename); - -/** - * Number of local samples that will be used for comparison before and after the remote sample is received. - */ -const NUMBER_OF_LOCAL_SAMPLES = 2; - -/** - * Collects the average audio levels per participant from the local stats and the stats received by every remote - * participant and compares them to detect potential audio problem for a participant. - */ -export default class AudioOutputProblemDetector { - - /** - * Creates new <tt>AudioOutputProblemDetector</tt> instance. - * - * @param {JitsiConference} conference - The conference instance to be monitored. - */ - constructor(conference) { - this._conference = conference; - this._localAudioLevelCache = {}; - this._reportedParticipants = []; - this._audioProblemCandidates = {}; - this._numberOfRemoteAudioLevelsReceived = {}; - this._onLocalAudioLevelsReport = this._onLocalAudioLevelsReport.bind(this); - this._onRemoteAudioLevelReceived = this._onRemoteAudioLevelReceived.bind(this); - this._clearUserData = this._clearUserData.bind(this); - this._conference.on(ConnectionQualityEvents.REMOTE_STATS_UPDATED, this._onRemoteAudioLevelReceived); - this._conference.statistics.addConnectionStatsListener(this._onLocalAudioLevelsReport); - this._conference.on(ConferenceEvents.USER_LEFT, this._clearUserData); - } - - /** - * A listener for audio level data received by a remote participant. - * - * @param {string} userID - The user id of the participant that sent the data. - * @param {Object} result - The result object. - * @param {number} [result.avgAudioLevels] - The average audio level value. - * @returns {void} - */ - _onRemoteAudioLevelReceived(userID, { avgAudioLevels }) { - const numberOfReports = (this._numberOfRemoteAudioLevelsReceived[userID] + 1) || 0; - - this._numberOfRemoteAudioLevelsReceived[userID] = numberOfReports; - - if (this._reportedParticipants.indexOf(userID) !== -1 || (userID in this._audioProblemCandidates) - || avgAudioLevels <= 0 || numberOfReports < 3) { - return; - } - - const participant = this._conference.getParticipantById(userID); - - if (participant) { - const tracks = participant.getTracksByMediaType(MediaType.AUDIO); - - if (tracks.length > 0 && participant.isAudioMuted()) { - // We don't need to report an error if everything seems fine with the participant and its tracks but - // the participant is audio muted. Since those are average audio levels we potentially can receive non - // zero values for muted track. - return; - } - } - - const localAudioLevels = this._localAudioLevelCache[userID]; - - if (!Array.isArray(localAudioLevels) || localAudioLevels.every(audioLevel => audioLevel === 0)) { - this._audioProblemCandidates[userID] = { - remoteAudioLevels: avgAudioLevels, - localAudioLevels: [] - }; - } - } - - /** - * A listener for audio level data retrieved by the local stats. - * - * @param {TraceablePeerConnection} tpc - The <tt>TraceablePeerConnection</tt> instance used to gather the data. - * @param {Object} avgAudioLevels - The average audio levels per participant. - * @returns {void} - */ - _onLocalAudioLevelsReport(tpc, { avgAudioLevels }) { - if (tpc !== this._conference.getActivePeerConnection()) { - return; - } - - Object.keys(avgAudioLevels).forEach(userID => { - if (this._reportedParticipants.indexOf(userID) !== -1) { - return; - } - - const localAudioLevels = this._localAudioLevelCache[userID]; - - if (!Array.isArray(localAudioLevels)) { - this._localAudioLevelCache[userID] = [ ]; - } else if (localAudioLevels.length >= NUMBER_OF_LOCAL_SAMPLES) { - localAudioLevels.shift(); - } - - this._localAudioLevelCache[userID].push(avgAudioLevels[userID]); - }); - - - Object.keys(this._audioProblemCandidates).forEach(userID => { - const { localAudioLevels, remoteAudioLevels } = this._audioProblemCandidates[userID]; - - localAudioLevels.push(avgAudioLevels[userID]); - - if (localAudioLevels.length === NUMBER_OF_LOCAL_SAMPLES) { - if (localAudioLevels.every(audioLevel => typeof audioLevel === 'undefined' || audioLevel === 0)) { - const localAudioLevelsString = JSON.stringify(localAudioLevels); - - Statistics.sendAnalytics( - createAudioOutputProblemEvent(userID, localAudioLevelsString, remoteAudioLevels)); - logger.warn(`A potential problem is detected with the audio output for participant ${ - userID}, local audio levels: ${localAudioLevelsString}, remote audio levels: ${ - remoteAudioLevels}`); - this._reportedParticipants.push(userID); - this._clearUserData(userID); - } - - delete this._audioProblemCandidates[userID]; - } - }); - } - - /** - * Clears the data stored for a participant. - * - * @param {string} userID - The id of the participant. - * @returns {void} - */ - _clearUserData(userID) { - delete this._localAudioLevelCache[userID]; - } - - /** - * Disposes the allocated resources. - * - * @returns {void} - */ - dispose() { - this._conference.off(ConnectionQualityEvents.REMOTE_STATS_UPDATED, this._onRemoteAudioLevelReceived); - this._conference.off(ConferenceEvents.USER_LEFT, this._clearUserData); - this._conference.statistics.removeConnectionStatsListener(this._onLocalAudioLevelsReport); - this._localAudioLevelCache = undefined; - this._audioProblemCandidates = undefined; - this._reportedParticipants = undefined; - this._numberOfRemoteAudioLevelsReceived = undefined; - this._conference = undefined; - } -} diff --git a/modules/statistics/RTPStatsCollector.js b/modules/statistics/RTPStatsCollector.js index ef278afd4b..fbdffab1a2 100644 --- a/modules/statistics/RTPStatsCollector.js +++ b/modules/statistics/RTPStatsCollector.js @@ -137,7 +137,6 @@ export default function StatsCollector(peerconnection, audioLevelsInterval, stat this.peerconnection = peerconnection; this.currentStatsReport = null; this.previousStatsReport = null; - this.audioLevelReportHistory = {}; this.audioLevelsIntervalId = null; this.eventEmitter = eventEmitter; this.conferenceStats = new ConferenceStats(); @@ -385,29 +384,6 @@ StatsCollector.prototype._processAndEmitReport = function() { calculatePacketLoss(lostPackets.upload, totalPackets.upload) }; - const avgAudioLevels = {}; - let localAvgAudioLevels; - - Object.keys(this.audioLevelReportHistory).forEach(ssrc => { - const { data, isLocal } = this.audioLevelReportHistory[ssrc]; - const avgAudioLevel = data.reduce((sum, currentValue) => sum + currentValue) / data.length; - - if (isLocal) { - localAvgAudioLevels = avgAudioLevel; - } else { - const track = this.peerconnection.getTrackBySSRC(Number(ssrc)); - - if (track) { - const participantId = track.getParticipantId(); - - if (participantId) { - avgAudioLevels[participantId] = avgAudioLevel; - } - } - } - }); - this.audioLevelReportHistory = {}; - this.eventEmitter.emit( StatisticsEvents.CONNECTION_STATS, this.peerconnection, @@ -418,9 +394,7 @@ StatsCollector.prototype._processAndEmitReport = function() { resolution: resolutions, framerate: framerates, codec: codecs, - transport: this.conferenceStats.transport, - localAvgAudioLevels, - avgAudioLevels + transport: this.conferenceStats.transport }); this.conferenceStats.transport = []; }; diff --git a/types/hand-crafted/modules/statistics/AudioOutputProblemDetector.d.ts b/types/hand-crafted/modules/statistics/AudioOutputProblemDetector.d.ts deleted file mode 100644 index 59e4ebcd85..0000000000 --- a/types/hand-crafted/modules/statistics/AudioOutputProblemDetector.d.ts +++ /dev/null @@ -1,6 +0,0 @@ -import JitsiConference from '../../JitsiConference'; - -export default class AudioOutputProblemDetector { - constructor( conference: JitsiConference ); - dispose: () => void; -}