aws / aws/amazon-chime-sdk-component-library-react

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

Open
#964 0 comments 0 reactions 0 assignees View on GitHub
Bug
Dominant language
TypeScript
Stars
281
Forks
166
PR merge metrics
No merged PRs in 30d

Description

### 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?

- [X] Amazon Chime SDK for JavaScript GitHub [issues](https://github.com/aws/amazon-chime-sdk-js/issues)
- [X] Amazon Chime SDK React Components Library GitHub [issues](https://github.com/aws/amazon-chime-sdk-component-library-react/issues)
- [X] Storybook [documentation](https://aws.github.io/amazon-chime-sdk-component-library-react/?path=/story/introduction--page)
- [X] README [page](https://github.com/aws/amazon-chime-sdk-component-library-react#readme)
- [X] React SDK meeting [demo](https://github.com/aws-samples/amazon-chime-sdk/tree/main/apps/meeting#readme---react-meeting-demo)

### Reproduction steps

Create a component that uses the following
```javascript
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 setAudioOutputShouldBeEnabled(!audioOutputShouldBeEnabled)}>
Toggle Audio Output
;
};
```

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:

```javascript
const audioVideo = useAudioVideo();
const { isAudioOn, toggleAudio } = useLocalAudioOutput();
useEffect(() => {
if (!chimeWaitingForAvStart && audioVideo) {
if (audioOutputShouldBeEnabled !== isAudioOn) {
toggleAudio();
}
}
}, [audioOutputShouldBeEnabled, isAudioOn, toggleAudio, chimeWaitingForAvStart, audioVideo]);
```

Contributor guide

Open the contributing guide

Research direction

Start at LocalAudioOutputProvider and the useLocalAudioOutput hook, focusing on the effect that runs when audioVideo becomes available. Reproduce a toggle before the provider has an audioVideo reference, then verify that the provider's state matches whether audio output is bound after initialization. Done means early toggling no longer leaves isAudioOn out of sync with the underlying audio output.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
audio-video-rtc, frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.