Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toggling audio output too early leaves LocalAudioOutputProvider's state out-of-sync #964

Open
5 tasks done
tr00st opened this issue Sep 16, 2024 · 0 comments
Open
5 tasks done

Comments

@tr00st
Copy link

tr00st commented Sep 16, 2024

What happened and what did you expect to happen?

From useLocalAudioOutput(), after using toggleAudio(), isAudioOn should correctly reflect the current state of audio output.

If toggleAudio() is called too early in the meeting lifecycle (eg: in our case, just after the audioVideoDidStart event is fired from the underlying meeting), isAudioOn gets toggled to false, but when the meeting finishes initializing, the underlying audio feed is set up as if isAudioOn was set to true as that's the default.

Have you reviewed our existing documentation?

Reproduction steps

Create a component that uses the following

const Component = () => {
    // We have this hooked up based on a user's role/setting
    const [audioOutputShouldBeEnabled, setAudioOutputShouldBeEnabled] = useState(false);

    // We had chimeWaitingForAvStart hooked up as a flag externally to the audioVideoDidStart event - observation
    // implies this might be hooked up earlier in the event stack than the one used for useAudioVideo, so
    // that might be important
    const chimeWaitingForAvStart = false;

    const { isAudioOn, toggleAudio } = useLocalAudioOutput();
    useEffect(() => {
        if (!chimeWaitingForAvStart) {
            if (audioOutputShouldBeEnabled !== isAudioOn) {
                toggleAudio();
            }
        }
    }, [audioOutputShouldBeEnabled, isAudioOn, toggleAudio, chimeWaitingForAvStart]);

    // Button for illustrative purposes
    return <button onClick={() => setAudioOutputShouldBeEnabled(!audioOutputShouldBeEnabled)}>
        Toggle Audio Output
    </button>;
};

Triggering the bug involves firing toggleAudio as early as possible (ie: before LocalAudioOutputProvider gets a reference to audioVideo).

In the above example, toggling audioOutputShouldBeEnabled to true is then a NOP, and toggling it back to false successfully unbinds the audio output.

Amazon Chime SDK React Components Library version

3.8.0

What browsers are you seeing the problem on?

Chrome (n/a)

Browser version

Latest (n/a)

Device Information

Desktop (n/a)

Meeting and Attendee ID Information.

No response

Browser console logs

None relevant.

Add any other context about the problem here.

On initial meeting load, toggleAudio getting fired early sets LocalAudioOutputProvider's isAudioOn to false. This initial call doesn't unbind the audio directly, as audioVideo is still unset. Shortly thereafter, audioVideo gets hooked up in LocalAudioOutputProvider, and the first useEffect in LocalAudioOutputProvider fires. This effect doesn't check for the state of isAudioOn, so it binds the audio element, but doesn't reset isAudioOn, so the state is now mismatched.

The fix here would presumably involve either:

  • Setting isAudioOn to true when initially binding the audio element
  • Or not binding the audio element when audioVideo is initially bound

I'd guess the latter would be a better option here, to avoid an unnecessary bind-unbind cycle (and possibly a corresponding short burst of unwanted audio).

A workaround is to hook onto audioVideo in the application and wait for it to be populated, so we don't send the update too early, as below:

    const audioVideo = useAudioVideo();
    const { isAudioOn, toggleAudio } = useLocalAudioOutput();
    useEffect(() => {
        if (!chimeWaitingForAvStart && audioVideo) {
            if (audioOutputShouldBeEnabled !== isAudioOn) {
                toggleAudio();
            }
        }
    }, [audioOutputShouldBeEnabled, isAudioOn, toggleAudio, chimeWaitingForAvStart, audioVideo]);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant