From 840d96f8699f1c19b9478d2bc40067a791e5e21e Mon Sep 17 00:00:00 2001 From: Marcin Wojtczak Date: Tue, 19 Nov 2024 20:48:12 +0000 Subject: [PATCH 1/2] Revert "fix(audio-mute): fix sometimes out-of-sync remote audio mute (#3797)" This reverts commit d354100b91c0f9dbf84b2ba65f8e762c05648b63. --- .../src/locus-info/selfUtils.ts | 5 ++ .../plugin-meetings/src/meeting/muteState.ts | 7 +-- .../test/unit/spec/locus-info/selfUtils.js | 48 +++++++++---------- .../test/unit/spec/meeting/muteState.js | 24 ---------- 4 files changed, 29 insertions(+), 55 deletions(-) diff --git a/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts b/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts index 0ba3e4d8178..27079d8221b 100644 --- a/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts +++ b/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts @@ -428,6 +428,11 @@ SelfUtils.mutedByOthersChanged = (oldSelf, changedSelf) => { return false; } + // there is no need to trigger user update if no one muted user + if (changedSelf.selfIdentity === changedSelf.modifiedBy) { + return false; + } + return ( changedSelf.remoteMuted !== null && (oldSelf.remoteMuted !== changedSelf.remoteMuted || diff --git a/packages/@webex/plugin-meetings/src/meeting/muteState.ts b/packages/@webex/plugin-meetings/src/meeting/muteState.ts index 2adbd562887..43f72fcd501 100644 --- a/packages/@webex/plugin-meetings/src/meeting/muteState.ts +++ b/packages/@webex/plugin-meetings/src/meeting/muteState.ts @@ -379,12 +379,7 @@ export class MuteState { } if (muted !== undefined) { this.state.server.remoteMute = muted; - - // We never want to unmute the local stream from a server remote mute update. - // Moderated unmute is handled by a different function. - if (muted) { - this.muteLocalStream(meeting, muted, 'remotelyMuted'); - } + this.muteLocalStream(meeting, muted, 'remotelyMuted'); } } diff --git a/packages/@webex/plugin-meetings/test/unit/spec/locus-info/selfUtils.js b/packages/@webex/plugin-meetings/test/unit/spec/locus-info/selfUtils.js index 9ad22a69acb..c0656946711 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/locus-info/selfUtils.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/locus-info/selfUtils.js @@ -345,39 +345,37 @@ describe('plugin-meetings', () => { }); describe('mutedByOthersChanged', () => { - it('throws an error if changedSelf is not provided', function () { - assert.throws( - () => SelfUtils.mutedByOthersChanged({}, null), - 'New self must be defined to determine if self was muted by others.' - ); + it('throws an error if changedSelf is not provided', function() { + assert.throws(() => SelfUtils.mutedByOthersChanged({}, null), 'New self must be defined to determine if self was muted by others.'); }); - it('return false when oldSelf is not defined', function () { - assert.equal(SelfUtils.mutedByOthersChanged(null, {remoteMuted: false}), false); + it('return false when oldSelf is not defined', function() { + assert.equal(SelfUtils.mutedByOthersChanged(null, { remoteMuted: false }), false); }); - it('should return true when remoteMuted is true on entry', function () { - assert.equal(SelfUtils.mutedByOthersChanged(null, {remoteMuted: true}), true); + it('should return true when remoteMuted is true on entry', function() { + assert.equal(SelfUtils.mutedByOthersChanged(null, { remoteMuted: true }), true); }); - it('should return true when remoteMuted values are different', function () { - assert.equal( - SelfUtils.mutedByOthersChanged( - {remoteMuted: false}, - {remoteMuted: true, selfIdentity: 'user1', modifiedBy: 'user2'} - ), - true - ); + it('should return false when selfIdentity and modifiedBy are the same', function() { + assert.equal(SelfUtils.mutedByOthersChanged( + { remoteMuted: false }, + { remoteMuted: true, selfIdentity: 'user1', modifiedBy: 'user1' } + ), false); }); - it('should return true when remoteMuted is true and unmuteAllowed has changed', function () { - assert.equal( - SelfUtils.mutedByOthersChanged( - {remoteMuted: true, unmuteAllowed: false}, - {remoteMuted: true, unmuteAllowed: true, selfIdentity: 'user1', modifiedBy: 'user2'} - ), - true - ); + it('should return true when remoteMuted values are different', function() { + assert.equal(SelfUtils.mutedByOthersChanged( + { remoteMuted: false }, + { remoteMuted: true, selfIdentity: 'user1', modifiedBy: 'user2' } + ), true); + }); + + it('should return true when remoteMuted is true and unmuteAllowed has changed', function() { + assert.equal(SelfUtils.mutedByOthersChanged( + { remoteMuted: true, unmuteAllowed: false }, + { remoteMuted: true, unmuteAllowed: true, selfIdentity: 'user1', modifiedBy: 'user2' } + ), true); }); }); diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js index c89e591ff06..976f1eb8b32 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js @@ -113,30 +113,6 @@ describe('plugin-meetings', () => { assert.isTrue(audio.isRemotelyMuted()); }); - it('does not locally unmute on a server unmute', async () => { - const setServerMutedSpy = meeting.mediaProperties.audioStream.setServerMuted; - - // simulate remote mute - audio.handleServerRemoteMuteUpdate(meeting, true, true); - - assert.isTrue(audio.isRemotelyMuted()); - assert.isTrue(audio.isLocallyMuted()); - - // mutes local - assert.calledOnceWithExactly(setServerMutedSpy, true, 'remotelyMuted'); - - setServerMutedSpy.resetHistory(); - - // simulate remote unmute - audio.handleServerRemoteMuteUpdate(meeting, false, true); - - assert.isFalse(audio.isRemotelyMuted()); - assert.isTrue(audio.isLocallyMuted()); - - // does not unmute local - assert.notCalled(setServerMutedSpy); - }); - it('does local audio unmute if localAudioUnmuteRequired is received', async () => { // first we need to have the local stream user muted meeting.mediaProperties.audioStream.userMuted = true; From 0a566614922e887b42efe121acd4cced1c3bbb9b Mon Sep 17 00:00:00 2001 From: Marcin Wojtczak Date: Wed, 20 Nov 2024 16:02:12 +0000 Subject: [PATCH 2/2] fix: moderated unmute when client is remotely muted --- .../plugin-meetings/src/locus-info/index.ts | 3 +- .../src/locus-info/selfUtils.ts | 26 +++++----- .../plugin-meetings/src/meeting/index.ts | 48 +++++++++++-------- .../plugin-meetings/src/meeting/muteState.ts | 28 +++++++++++ 4 files changed, 72 insertions(+), 33 deletions(-) diff --git a/packages/@webex/plugin-meetings/src/locus-info/index.ts b/packages/@webex/plugin-meetings/src/locus-info/index.ts index 489d12fda57..b43c8ddece0 100644 --- a/packages/@webex/plugin-meetings/src/locus-info/index.ts +++ b/packages/@webex/plugin-meetings/src/locus-info/index.ts @@ -1406,7 +1406,7 @@ export default class LocusInfo extends EventsScope { } ); } - if (parsedSelves.updates.isMutedByOthersChanged) { + if (parsedSelves.updates.isMutedByOthersChanged.changed) { this.emitScoped( { file: 'locus-info', @@ -1416,6 +1416,7 @@ export default class LocusInfo extends EventsScope { { muted: parsedSelves.current.remoteMuted, unmuteAllowed: parsedSelves.current.unmuteAllowed, + isModifiedBySelf: parsedSelves.updates.isMutedByOthersChanged.isModifiedBySelf, } ); } diff --git a/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts b/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts index 27079d8221b..a9ce5b8a059 100644 --- a/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts +++ b/packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts @@ -419,25 +419,27 @@ SelfUtils.mutedByOthersChanged = (oldSelf, changedSelf) => { throw new ParameterError('New self must be defined to determine if self was muted by others.'); } + const isModifiedBySelf = changedSelf.selfIdentity === changedSelf.modifiedBy; + if (!oldSelf || oldSelf.remoteMuted === null) { if (changedSelf.remoteMuted) { - return true; // this happens when mute on-entry is enabled + return {changed: false, isModifiedBySelf}; // this happens when mute on-entry is enabled } // we don't want to be sending the 'meeting:self:unmutedByOthers' notification on meeting join - return false; - } - - // there is no need to trigger user update if no one muted user - if (changedSelf.selfIdentity === changedSelf.modifiedBy) { - return false; + return {changed: false, isModifiedBySelf}; } - - return ( - changedSelf.remoteMuted !== null && - (oldSelf.remoteMuted !== changedSelf.remoteMuted || - (changedSelf.remoteMuted && oldSelf.unmuteAllowed !== changedSelf.unmuteAllowed)) + console.log( + `marcin: mutedByOthersChanged: old.remoteMuted=${oldSelf.remoteMuted} new.remoteMuted=${changedSelf.remoteMuted} selfId=${changedSelf.selfIdentity} modifiedBy=${changedSelf.modifiedBy}` ); + + return { + changed: + changedSelf.remoteMuted !== null && + (oldSelf.remoteMuted !== changedSelf.remoteMuted || + (changedSelf.remoteMuted && oldSelf.unmuteAllowed !== changedSelf.unmuteAllowed)), + isModifiedBySelf, + }; }; SelfUtils.localAudioUnmuteRequestedByServer = (oldSelf: any = {}, changedSelf: any) => { diff --git a/packages/@webex/plugin-meetings/src/meeting/index.ts b/packages/@webex/plugin-meetings/src/meeting/index.ts index e7ffb18fd1f..36da4f115c8 100644 --- a/packages/@webex/plugin-meetings/src/meeting/index.ts +++ b/packages/@webex/plugin-meetings/src/meeting/index.ts @@ -3144,26 +3144,34 @@ export default class Meeting extends StatelessWebexPlugin { this.locusInfo.on(LOCUSINFO.EVENTS.SELF_REMOTE_MUTE_STATUS_UPDATED, (payload) => { if (payload) { - if (this.audio) { - this.audio.handleServerRemoteMuteUpdate(this, payload.muted, payload.unmuteAllowed); - } - // with "mute on entry" server will send us remote mute even if we don't have media configured, - // so if being muted by others, always send the notification, - // but if being unmuted, only send it if we are also locally unmuted - if (payload.muted || !this.audio?.isMuted()) { - Trigger.trigger( - this, - { - file: 'meeting/index', - function: 'setUpLocusInfoSelfListener', - }, - payload.muted - ? EVENT_TRIGGERS.MEETING_SELF_MUTED_BY_OTHERS - : EVENT_TRIGGERS.MEETING_SELF_UNMUTED_BY_OTHERS, - { - payload, - } - ); + const ignore = this.audio + ? this.audio.shouldIgnoreRemoteMuteUpdate(payload.muted, payload.isModifiedBySelf) + : false; + + if (!ignore) { + if (this.audio) { + this.audio.handleServerRemoteMuteUpdate(this, payload.muted, payload.unmuteAllowed); + } + // with "mute on entry" server will send us remote mute even if we don't have media configured, + // so if being muted by others, always send the notification, + // but if being unmuted, only send it if we are also locally unmuted + if (payload.muted || !this.audio?.isMuted()) { + Trigger.trigger( + this, + { + file: 'meeting/index', + function: 'setUpLocusInfoSelfListener', + }, + payload.muted + ? EVENT_TRIGGERS.MEETING_SELF_MUTED_BY_OTHERS + : EVENT_TRIGGERS.MEETING_SELF_UNMUTED_BY_OTHERS, + { + payload, + } + ); + } + } else { + console.log(`marcin: ignoring remote mute update payload.muted=${payload.muted}`); } } }); diff --git a/packages/@webex/plugin-meetings/src/meeting/muteState.ts b/packages/@webex/plugin-meetings/src/meeting/muteState.ts index 43f72fcd501..5dc0b7eaaea 100644 --- a/packages/@webex/plugin-meetings/src/meeting/muteState.ts +++ b/packages/@webex/plugin-meetings/src/meeting/muteState.ts @@ -32,6 +32,7 @@ export class MuteState { }; server: {localMute: boolean; remoteMute: boolean; unmuteAllowed: boolean}; syncToServerInProgress: boolean; + isRemoteUnmutePendingLocusDtoUpdate: boolean; // true if we've sent a remote unmute request to Locus and haven't received a Locus DTO confirming it happened, yet }; type: any; @@ -62,6 +63,7 @@ export class MuteState { unmuteAllowed: type === AUDIO ? meeting.unmuteAllowed : meeting.unmuteVideoAllowed ?? true, }, syncToServerInProgress: false, + isRemoteUnmutePendingLocusDtoUpdate: false, }; } @@ -327,6 +329,13 @@ export class MuteState { `Meeting:muteState#sendRemoteMuteRequestToServer --> ${this.type}: sending remote mute:${remoteMute} to server` ); + this.state.isRemoteUnmutePendingLocusDtoUpdate = true; + + setTimeout(() => { + console.log('marcin: resetting isRemoteUnmutePendingLocusDtoUpdate after timeout'); + this.state.isRemoteUnmutePendingLocusDtoUpdate = false; + }, 10 * 1000); + return meeting.members .muteMember(meeting.members.selfId, remoteMute, this.type === AUDIO) .then(() => { @@ -337,6 +346,8 @@ export class MuteState { this.state.server.remoteMute = remoteMute; }) .catch((remoteUpdateError) => { + this.state.isRemoteUnmutePendingLocusDtoUpdate = false; + LoggerProxy.logger.warn( `Meeting:muteState#sendRemoteMuteRequestToServer --> ${this.type}: failed to apply remote mute ${remoteMute} to server: ${remoteUpdateError}` ); @@ -359,6 +370,23 @@ export class MuteState { } } + public shouldIgnoreRemoteMuteUpdate(remoteMute: boolean, isModifiedBySelf: boolean) { + console.log( + `marcin: shouldIgnoreRemoteMuteUpdate: flag=${this.state.isRemoteUnmutePendingLocusDtoUpdate} remoteMute=${remoteMute}, isModifiedBySelf=${isModifiedBySelf}` + ); + if (this.state.isRemoteUnmutePendingLocusDtoUpdate && isModifiedBySelf && !remoteMute) { + console.log('marcin: FIX: ignoring remote mute update'); + + this.state.isRemoteUnmutePendingLocusDtoUpdate = false; + + return true; + } + + console.log('marcin: FIX: NOT ignoring remote mute update'); + + return false; + } + /** * This method should be called whenever the server remote mute state is changed *