fix(audio-mute): fix sometimes out-of-sync remote audio mute #3797

chburket · 2024-08-28T13:35:49Z

COMPLETES

https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-555818

This pull request addresses

Fixes a BEMS case defect where remote audio mute can get out of sync with local audio mute.

BEMS space: webexteams://im?space=c4706ce0-56e0-11ef-81df-a39e6dd40a42

Steps to recreate bug:

start a meeting as host, unmuted
uncheck "allow participants to unmute"
mute
unmute
mute (problem: sdk's representation of server remote audio is still unmuted)
unmute (because sdk thinks remote is already unmuted, we do not try to change it)

The server remote audio is still muted (notice the muted icon in the participant list)
With the fix, the server remote audio is unmuted as expected.

by making the following changes

First, I needed to revert a fix from months ago. This fix was causing the locus diff update from step 5 to be skipped because the modifiedBy value was the same as the local user. We did not skip the update in step 3 because modifiedBy was undefined. I met with Rajesh Kadikar about this and he indicated that the modifiedBy should contain the current user in this scenario, and that we should not be skipping locus updates, even it they were modified by the current user.

Second, I needed to find a new way to fix what was reverted. I added console.logs in various places in the flow and found that if you unmute and mute quickly, _LocalMicrophoneStream.setUserMuted(true) happens before we receive the locus diff from the unmute. When the locus diff eventually comes in, handleServerRemoteMuteUpdate() unmutes the local stream to match the remote mute state, which should not be done. So I added an if around the local stream update to only execute it when muting, not unmuting.

While testing this PR, I found a few problems around host mute/unmute (unrelated to this change) and opened these Jiras:

https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-559101
https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-559095

Change Type

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Tooling change
Internal code refactor

The following scenarios where tested

Manually tested the scenario in the BEMS Jira (SPARK-555818).
Manually tested the scenario in a related BEMS Jira.
Manually tested the scenario from the reverted PR/Jira.
Manually tested host unmuting a user when in a moderated meeting.

Updated unit tests.

I certified that

I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly

aws-amplify-us-east-2 · 2024-08-28T13:47:11Z

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3797.d3m3l2kee0btzx.amplifyapp.com

chburket · 2024-09-03T19:52:47Z

packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts

-  // there is no need to trigger user update if no one muted user
-  if (changedSelf.selfIdentity === changedSelf.modifiedBy) {
-    return false;
-  }
-


reverted because this was causing the BEMS bug

note to self, ask about this if not resolved lower. edit: resolved in next file

chburket · 2024-09-03T19:53:17Z

packages/@webex/plugin-meetings/src/meeting/muteState.ts

+
+      // never want to unmute the local stream from a server remote mute update
+      if (muted) {
+        this.muteLocalStream(meeting, muted, 'remotelyMuted');
+      }


New way to fix the bug that was fixed by the code I reverted. This is not called when the the meeting is in moderated mode and the host unmutes a user, so it did not affect that.

chburket · 2024-09-03T20:39:32Z

packages/@webex/plugin-meetings/test/unit/spec/locus-info/selfUtils.js

-    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);


removed this

chrisadubois · 2024-09-03T21:24:56Z

packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts

-  // there is no need to trigger user update if no one muted user
-  if (changedSelf.selfIdentity === changedSelf.modifiedBy) {
-    return false;
-  }
-


note to self, ask about this if not resolved lower. edit: resolved in next file

chrisadubois · 2024-09-03T21:25:23Z

packages/@webex/plugin-meetings/src/meeting/muteState.ts

@@ -379,7 +379,11 @@ export class MuteState {
    }
    if (muted !== undefined) {
      this.state.server.remoteMute = muted;
-      this.muteLocalStream(meeting, muted, 'remotelyMuted');
+
+      // never want to unmute the local stream from a server remote mute update


chrisadubois · 2024-09-03T21:27:07Z

packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js

@@ -113,6 +113,30 @@ describe('plugin-meetings', () => {
      assert.isTrue(audio.isRemotelyMuted());
    });

+    it('does not locally unmute on a server unmute', async () => {


chrisadubois · 2024-09-03T21:27:52Z

packages/@webex/plugin-meetings/src/meeting/muteState.ts

+
+      // never want to unmute the local stream from a server remote mute update
+      if (muted) {
+        this.muteLocalStream(meeting, muted, 'remotelyMuted');


this logic makes sense and is a lot cleaner

…8-out-of-sync-remote-audio-mute

)

…ebex#3797)" This reverts commit d354100.

chburket marked this pull request as ready for review August 28, 2024 13:37

chburket requested review from a team as code owners August 28, 2024 13:37

chburket added the validated If the pull request is validated for automation. label Aug 28, 2024

chburket marked this pull request as draft August 28, 2024 14:15

fix(audio-mute): fix for remote audio mute out-of-sync

7eac0be

chburket force-pushed the SPARK-555818-out-of-sync-remote-audio-mute branch from 7929174 to 7eac0be Compare September 3, 2024 19:24

chburket commented Sep 3, 2024

View reviewed changes

chburket added 2 commits September 3, 2024 20:36

fix(audio-mute): updated tests

1ccc7e0

fix(audio-mute): updated tests

dc5148e

chburket commented Sep 3, 2024

View reviewed changes

chburket marked this pull request as ready for review September 3, 2024 20:40

chrisadubois approved these changes Sep 3, 2024

View reviewed changes

chburket added 2 commits September 4, 2024 16:47

fix(audio-mute): better comment

b5608e6

Merge branch 'next' of github.com:webex/webex-js-sdk into SPARK-55581…

89089bd

…8-out-of-sync-remote-audio-mute

marcin-bazyl approved these changes Sep 4, 2024

View reviewed changes

chburket enabled auto-merge (squash) September 4, 2024 16:57

chburket merged commit d354100 into webex:next Sep 4, 2024
11 checks passed

chburket deleted the SPARK-555818-out-of-sync-remote-audio-mute branch September 4, 2024 17:15

pagour98 pushed a commit to pagour98/webex-js-sdk that referenced this pull request Sep 27, 2024

fix(audio-mute): fix sometimes out-of-sync remote audio mute (webex#3797

968fb1d

)

pagour98 pushed a commit to pagour98/webex-js-sdk that referenced this pull request Sep 27, 2024

fix(audio-mute): fix sometimes out-of-sync remote audio mute (webex#3797

451f646

)

marcin-bazyl added a commit to marcin-bazyl/webex-js-sdk that referenced this pull request Nov 20, 2024

Revert "fix(audio-mute): fix sometimes out-of-sync remote audio mute (w…

aaa7b13

…ebex#3797)" This reverts commit d354100.

marcin-bazyl added a commit to marcin-bazyl/webex-js-sdk that referenced this pull request Nov 20, 2024

Revert "fix(audio-mute): fix sometimes out-of-sync remote audio mute (w…

840d96f

…ebex#3797)" This reverts commit d354100.

marcin-bazyl mentioned this pull request Nov 20, 2024

fix(plugin-meetings): moderated unmute when client is remotely muted #3995

Draft

marcin-bazyl added a commit to marcin-bazyl/webex-js-sdk that referenced this pull request Dec 20, 2024

Revert "fix(audio-mute): fix sometimes out-of-sync remote audio mute (w…

101fa7d

…ebex#3797)" This reverts commit d354100.

marcin-bazyl added a commit to marcin-bazyl/webex-js-sdk that referenced this pull request Dec 20, 2024

Revert "fix(audio-mute): fix sometimes out-of-sync remote audio mute (w…

bb50482

…ebex#3797)" This reverts commit d354100.

marcin-bazyl mentioned this pull request Dec 20, 2024

fix: Moderated unmute mode not working when web client is remotely muted #4039

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(audio-mute): fix sometimes out-of-sync remote audio mute #3797

fix(audio-mute): fix sometimes out-of-sync remote audio mute #3797

chburket commented Aug 28, 2024 •

edited

Loading

aws-amplify-us-east-2 bot commented Aug 28, 2024

chburket Sep 3, 2024

chrisadubois Sep 3, 2024

chburket Sep 3, 2024 •

edited

Loading

chburket Sep 3, 2024

chrisadubois Sep 3, 2024

chrisadubois Sep 3, 2024

chrisadubois Sep 3, 2024

chrisadubois Sep 3, 2024

fix(audio-mute): fix sometimes out-of-sync remote audio mute #3797

fix(audio-mute): fix sometimes out-of-sync remote audio mute #3797

Conversation

chburket commented Aug 28, 2024 • edited Loading

COMPLETES

This pull request addresses

by making the following changes

Change Type

The following scenarios where tested

I certified that

aws-amplify-us-east-2 bot commented Aug 28, 2024

chburket Sep 3, 2024

Choose a reason for hiding this comment

chrisadubois Sep 3, 2024

Choose a reason for hiding this comment

chburket Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

chburket Sep 3, 2024

Choose a reason for hiding this comment

chrisadubois Sep 3, 2024

Choose a reason for hiding this comment

chrisadubois Sep 3, 2024

Choose a reason for hiding this comment

chrisadubois Sep 3, 2024

Choose a reason for hiding this comment

chrisadubois Sep 3, 2024

Choose a reason for hiding this comment

chburket commented Aug 28, 2024 •

edited

Loading

chburket Sep 3, 2024 •

edited

Loading