From 0f687fb8b8d9fdb1efc731985d365fa2be251671 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 13 May 2022 17:58:59 +0100 Subject: [PATCH 1/3] Fix races on mute / unmute By serialising everything on a promise chain --- src/room/usePTT.ts | 135 ++++++++++++++++++++++++++++++--------------- 1 file changed, 91 insertions(+), 44 deletions(-) diff --git a/src/room/usePTT.ts b/src/room/usePTT.ts index d84907b..9c4bac3 100644 --- a/src/room/usePTT.ts +++ b/src/room/usePTT.ts @@ -22,6 +22,30 @@ import { logger } from "matrix-js-sdk/src/logger"; import { PlayClipFunction, PTTClipID } from "../sound/usePttSounds"; +// Works out who the active speaker should be given what feeds are active and +// the power level of each user. +function getActiveSpeakerFeed( + feeds: CallFeed[], + groupCall: GroupCall +): CallFeed { + const activeSpeakerFeeds = feeds.filter((f) => !f.isAudioMuted()); + + let activeSpeakerFeed; + let highestPowerLevel; + for (const feed of activeSpeakerFeeds) { + const member = groupCall.room.getMember(feed.userId); + if ( + highestPowerLevel === undefined || + member.powerLevel > highestPowerLevel + ) { + highestPowerLevel = member.powerLevel; + activeSpeakerFeed = feed; + } + } + + return activeSpeakerFeed; +} + export interface PTTState { pttButtonHeld: boolean; isAdmin: boolean; @@ -39,6 +63,24 @@ export const usePTT = ( userMediaFeeds: CallFeed[], playClip: PlayClipFunction ): PTTState => { + // Used to serialise all the mute calls so they don't race. It has + // its own state as its always set separately from anything else. + const [mutePromise, setMutePromise] = useState(Promise.resolve()); + + // Wrapper to serialise all the mute operations on the promise + const setMicMuteWrapper = useCallback( + (muted) => { + setMutePromise( + mutePromise.then(() => { + groupCall.setMicrophoneMuted(muted).catch((e) => { + logger.error("Failed to unmute microphone", e); + }); + }) + ); + }, + [groupCall, mutePromise] + ); + const [ { pttButtonHeld, @@ -51,7 +93,7 @@ export const usePTT = ( ] = useState(() => { const roomMember = groupCall.room.getMember(client.getUserId()); - const activeSpeakerFeed = userMediaFeeds.find((f) => !f.isAudioMuted()); + const activeSpeakerFeed = getActiveSpeakerFeed(userMediaFeeds, groupCall); return { isAdmin: roomMember.powerLevel >= 100, @@ -62,38 +104,52 @@ export const usePTT = ( }; }); - useEffect(() => { - function onMuteStateChanged(...args): void { - const activeSpeakerFeed = userMediaFeeds.find((f) => !f.isAudioMuted()); + const onMuteStateChanged = useCallback(() => { + const activeSpeakerFeed = getActiveSpeakerFeed(userMediaFeeds, groupCall); - if (activeSpeakerUserId === null && activeSpeakerFeed.userId !== null) { - if (activeSpeakerFeed.userId === client.getUserId()) { - playClip(PTTClipID.START_TALKING_LOCAL); - } else { - playClip(PTTClipID.START_TALKING_REMOTE); - } - } else if ( - pttButtonHeld && - activeSpeakerUserId === client.getUserId() && - activeSpeakerFeed?.userId !== client.getUserId() - ) { - // We were talking but we've been cut off - playClip(PTTClipID.BLOCKED); + let blocked = false; + if (activeSpeakerUserId === null && activeSpeakerFeed.userId !== null) { + if (activeSpeakerFeed.userId === client.getUserId()) { + playClip(PTTClipID.START_TALKING_LOCAL); + } else { + playClip(PTTClipID.START_TALKING_REMOTE); } + } else if ( + pttButtonHeld && + activeSpeakerUserId === client.getUserId() && + activeSpeakerFeed?.userId !== client.getUserId() + ) { + // We were talking but we've been cut off + setMicMuteWrapper(true); + blocked = true; + playClip(PTTClipID.BLOCKED); + } - setState((prevState) => ({ + setState((prevState) => { + return { ...prevState, activeSpeakerUserId: activeSpeakerFeed ? activeSpeakerFeed.userId : null, - })); - } + transmitBlocked: blocked, + }; + }); + }, [ + playClip, + groupCall, + pttButtonHeld, + activeSpeakerUserId, + client, + userMediaFeeds, + setMicMuteWrapper, + ]); + useEffect(() => { for (const callFeed of userMediaFeeds) { callFeed.addListener(CallFeedEvent.MuteStateChanged, onMuteStateChanged); } - const activeSpeakerFeed = userMediaFeeds.find((f) => !f.isAudioMuted()); + const activeSpeakerFeed = getActiveSpeakerFeed(userMediaFeeds, groupCall); setState((prevState) => ({ ...prevState, @@ -108,29 +164,26 @@ export const usePTT = ( ); } }; - }, [userMediaFeeds, activeSpeakerUserId, client, playClip, pttButtonHeld]); + }, [userMediaFeeds, onMuteStateChanged, groupCall]); const startTalking = useCallback(async () => { if (pttButtonHeld) return; let blocked = false; - if (!activeSpeakerUserId || (isAdmin && talkOverEnabled)) { - if (groupCall.isMicrophoneMuted()) { - try { - await groupCall.setMicrophoneMuted(false); - } catch (e) { - logger.error("Failed to unmute microphone", e); - } - } - } else { + if (activeSpeakerUserId && !(isAdmin && talkOverEnabled)) { playClip(PTTClipID.BLOCKED); blocked = true; } + // setstate before doing the async call to mute / unmute the mic setState((prevState) => ({ ...prevState, pttButtonHeld: true, transmitBlocked: blocked, })); + + if (!blocked && groupCall.isMicrophoneMuted()) { + setMicMuteWrapper(false); + } }, [ pttButtonHeld, groupCall, @@ -139,25 +192,18 @@ export const usePTT = ( talkOverEnabled, setState, playClip, + setMicMuteWrapper, ]); - const stopTalking = useCallback(() => { - setState((prevState) => ({ - ...prevState, - pttButtonHeld: false, - unmuteError: null, - })); - - if (!groupCall.isMicrophoneMuted()) { - groupCall.setMicrophoneMuted(true); - } - + const stopTalking = useCallback(async () => { setState((prevState) => ({ ...prevState, pttButtonHeld: false, transmitBlocked: false, })); - }, [groupCall]); + + setMicMuteWrapper(true); + }, [setMicMuteWrapper]); useEffect(() => { function onKeyDown(event: KeyboardEvent): void { @@ -181,7 +227,7 @@ export const usePTT = ( function onBlur(): void { // TODO: We will need to disable this for a global PTT hotkey to work if (!groupCall.isMicrophoneMuted()) { - groupCall.setMicrophoneMuted(true); + setMicMuteWrapper(true); } setState((prevState) => ({ ...prevState, pttButtonHeld: false })); @@ -204,6 +250,7 @@ export const usePTT = ( isAdmin, talkOverEnabled, pttButtonHeld, + setMicMuteWrapper, ]); const setTalkOverEnabled = useCallback((talkOverEnabled) => { From d34c8d08a4e2d0497439fb6c47b6849e437bfe37 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 13 May 2022 18:09:45 +0100 Subject: [PATCH 2/3] Add comment --- src/room/usePTT.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/room/usePTT.ts b/src/room/usePTT.ts index 9c4bac3..5d4682e 100644 --- a/src/room/usePTT.ts +++ b/src/room/usePTT.ts @@ -119,7 +119,11 @@ export const usePTT = ( activeSpeakerUserId === client.getUserId() && activeSpeakerFeed?.userId !== client.getUserId() ) { - // We were talking but we've been cut off + // We were talking but we've been cut off: mute our own mic + // (this is the easier way of cutting other speakers off if an + // admin barges in: we could also mute the non-admin speaker + // on all receivers, but we'd have to make sure we unmuted them + // correctly.) setMicMuteWrapper(true); blocked = true; playClip(PTTClipID.BLOCKED); From f6f0c20b08f434d88a39bb75efc5a6dfc02f530b Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 13 May 2022 20:39:21 +0100 Subject: [PATCH 3/3] Chain promises correctly --- src/room/usePTT.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/room/usePTT.ts b/src/room/usePTT.ts index 5d4682e..3b80f8c 100644 --- a/src/room/usePTT.ts +++ b/src/room/usePTT.ts @@ -65,14 +65,16 @@ export const usePTT = ( ): PTTState => { // Used to serialise all the mute calls so they don't race. It has // its own state as its always set separately from anything else. - const [mutePromise, setMutePromise] = useState(Promise.resolve()); + const [mutePromise, setMutePromise] = useState( + Promise.resolve(false) + ); // Wrapper to serialise all the mute operations on the promise const setMicMuteWrapper = useCallback( - (muted) => { + (muted: boolean) => { setMutePromise( mutePromise.then(() => { - groupCall.setMicrophoneMuted(muted).catch((e) => { + return groupCall.setMicrophoneMuted(muted).catch((e) => { logger.error("Failed to unmute microphone", e); }); })