Skip to content

Commit b8242bd

Browse files
committed
Destroy the VoiceFocus transformer on unmount of VoiceFocusProvider
1 parent a52adba commit b8242bd

File tree

9 files changed

+1029
-842
lines changed

9 files changed

+1029
-842
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616
### Removed
1717

1818
### Changed
19+
- Updated `VoiceFocusProvider` to destroy the Voice Focus worker thread on unmount.
1920

2021
### Fixed
2122

package-lock.json

Lines changed: 975 additions & 806 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
"@typescript-eslint/eslint-plugin": "^4.33.0",
8585
"@typescript-eslint/parser": "^4.33.0",
8686
"add": "^2.0.6",
87-
"amazon-chime-sdk-js": "^3.8.0",
87+
"amazon-chime-sdk-js": "^3.10.0",
8888
"babel-loader": "^8.1.0",
8989
"css-loader": "^6.7.1",
9090
"eslint": "^7.32.0",
@@ -123,7 +123,7 @@
123123
"webpack-cli": "^4.9.2"
124124
},
125125
"peerDependencies": {
126-
"amazon-chime-sdk-js": "^3.8.0",
126+
"amazon-chime-sdk-js": "^3.10.0",
127127
"react": "^17.0.1",
128128
"react-dom": "^17.0.1",
129129
"styled-components": "^5.0.0",
@@ -132,4 +132,4 @@
132132
"prettier": {
133133
"singleQuote": true
134134
}
135-
}
135+
}

src/components/sdk/MeetingControls/AudioInputVFControl.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ const AudioInputVFControl: React.FC<Props> = ({
8787
} else {
8888
setIsVoiceFocusEnabled(false);
8989
}
90+
return () => {
91+
if (selectedDevice instanceof VoiceFocusTransformDevice) {
92+
selectedDevice.stop();
93+
}
94+
};
9095
}, [selectedDevice]);
9196

9297
useEffect(() => {

src/providers/BackgroundBlurProvider/index.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,9 @@ const BackgroundBlurProvider: FC<Props> = ({ spec, options, children }) => {
8888
);
8989

9090
useEffect(() => {
91-
initializeBackgroundBlur();
9291
return () => {
9392
logger.info(
94-
'Specs or options were changed. Destroying and re-initializing background blur processor.'
93+
'Specs or options were changed. Destroying background blur processor.'
9594
);
9695
backgroundBlurProcessor?.destroy();
9796
};
@@ -148,6 +147,9 @@ const BackgroundBlurProvider: FC<Props> = ({ spec, options, children }) => {
148147
selectedDevice
149148
)}`
150149
);
150+
// TODO: We don't need to intialize a new processor every time we create a background blur device
151+
// We could potentially check for if a processor exists already AND that the processor isn't destroyed.
152+
// If both of those statements are true, then chooseNewInnerDevice instead of creating a new processor
151153
const currentProcessor = await initializeBackgroundBlur();
152154
try {
153155
const logger =

src/providers/BackgroundReplacementProvider/index.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,9 @@ const BackgroundReplacementProvider: FC<Props> = ({
9595
);
9696

9797
useEffect(() => {
98-
initializeBackgroundReplacement();
9998
return () => {
10099
logger.info(
101-
'Specs or options were changed. Destroying and re-initializing background replacement processor.'
100+
'Specs or options were changed. Destroying background replacement processor.'
102101
);
103102
backgroundReplacementProcessor?.destroy();
104103
};
@@ -156,6 +155,9 @@ const BackgroundReplacementProvider: FC<Props> = ({
156155
selectedDevice
157156
)}`
158157
);
158+
// TODO: We don't need to intialize a new processor every time we create a background replacement device
159+
// We could potentially check for if a processor exists already AND that the processor isn't destroyed.
160+
// If both of those statements are true, then chooseNewInnerDevice instead of creating a new processor
159161
const currentProcessor = await initializeBackgroundReplacement();
160162
try {
161163
const logger =

src/providers/VoiceFocusProvider/docs/VoiceFocusProvider.stories.mdx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import { VoiceFocusProvider } from '../';
88
The `VoiceFocusProvider` provides a function transforming a normal audio device to an Amazon Voice Focus device, and whether Amazon Voice Focus is supported.
99
Amazon Voice Focus related logs can be found in the browser developer tools when Amazon Voice Focus is enabled.
1010

11+
The Voice Focus Provider manages the state of a `VoiceFocusDeviceTransformer` behind the scenes. When you call `addVoiceFocus` the provider will use the current `VoiceFocusDeviceTransformer`
12+
to create a new `VoiceFocusTransformerDevice` and provide it to the consumer. The `VoiceFocusDeviceTransformer` that is used to create a `VoiceFocusTransformDevice` will be destroyed when
13+
the provider is unmounted.
14+
1115
This provider is independent from `MeetingProvider`. You can put `VoiceFocusProvider` wherever you want in the component tree to control the workflow of Voice Focus, so long
1216
as any component which relies on `VoiceFocusProvider` is nested within `VoiceFocusProvider`.
1317

src/providers/VoiceFocusProvider/index.tsx

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,9 @@ const VoiceFocusProvider: React.FC<Props> = ({
9090
logger.info(
9191
`Add Amazon Voice Focus to the following audio input device ${device}`
9292
);
93-
94-
if (voiceFocusDevice) {
95-
const vf = await voiceFocusDevice.chooseNewInnerDevice(device);
96-
logger.info(
97-
'Re-used the same internal state to create an Amazon Voice Focus transform device.'
98-
);
99-
setVoiceFocusDevice(vf);
100-
return vf;
101-
}
93+
// TODO: We don't need to intialize a new transformer every time we create a voice focus transformer device
94+
// We could potentially check for if a transformer exists already AND that the voiceFocusDevice exists and hasnt been stopped.
95+
// If both of those statements are true, then chooseNewInnerDevice instead of creating a new processor
10296

10397
if (!isVoiceFocusSupported) {
10498
logger.debug('Not supported, not creating device.');
@@ -188,6 +182,10 @@ const VoiceFocusProvider: React.FC<Props> = ({
188182
createMeetingResponse: JoinMeetingInfo | undefined
189183
) {
190184
// Throw away the old one and reinitialize.
185+
voiceFocusDevice?.stop();
186+
if (voiceFocusTransformer) {
187+
VoiceFocusDeviceTransformer.destroyVoiceFocus(voiceFocusTransformer);
188+
}
191189
setVoiceFocusTransformer(null);
192190
setVoiceFocusDevice(null);
193191
createVoiceFocusDeviceTransformer(
@@ -226,8 +224,32 @@ const VoiceFocusProvider: React.FC<Props> = ({
226224
`Current Amazon Voice Focus transform device: ${voiceFocusDevice}`
227225
);
228226
}
227+
return () => {
228+
if (voiceFocusDevice) {
229+
logger.info(
230+
'Destroying voice focus device : ' + JSON.stringify(voiceFocusDevice)
231+
);
232+
voiceFocusDevice?.stop();
233+
} else {
234+
logger.info("Voice focus device doesn't exist");
235+
}
236+
};
229237
}, [voiceFocusDevice]);
230238

239+
useEffect(() => {
240+
return () => {
241+
if (voiceFocusTransformer) {
242+
VoiceFocusDeviceTransformer.destroyVoiceFocus(voiceFocusTransformer);
243+
logger.info(
244+
'Destroying voice focus transformer : ' +
245+
JSON.stringify(voiceFocusTransformer)
246+
);
247+
} else {
248+
logger.info("VoiceFocusTransformer doesn't exist");
249+
}
250+
};
251+
}, [voiceFocusTransformer]);
252+
231253
const value: VoiceFocusState = {
232254
isVoiceFocusSupported,
233255
addVoiceFocus,

tst/providers/BackgroundBlurProvider/index.test.tsx

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -59,27 +59,9 @@ describe.only('BackgroundBlurProvider', () => {
5959
// happens when the component remounts. If it is happening more than
6060
// once, that means some dependency or parent is changing the parameters
6161
// erroneously.
62-
expect(loggerInfoMock).toHaveBeenCalledTimes(4);
62+
expect(loggerInfoMock).toHaveBeenCalledTimes(1);
6363
expect(loggerInfoMock).toHaveBeenCalledWith(
64-
`Initializing background blur processor with, spec: ${JSON.stringify(
65-
undefined
66-
)}, options: ${JSON.stringify(blurOptions)}`
67-
);
68-
expect(loggerInfoMock).toHaveBeenCalledWith(
69-
'Specs or options were changed. Destroying and re-initializing background blur processor.'
70-
);
71-
72-
// Even though we are using a NoOpVideoFrameProcessor, the input specs
73-
// and options that are passed to the amazon-chime-sdk-js are still
74-
// run through `resolveOptions` and `resolveSpec` in the
75-
// `BackgroundBlurVideoFrameProcessor` constructor so any invalid API
76-
// boundary changes to those are still validated by this test. The
77-
// first warning call is output by `BackgroundBlurVideoFrameProcessor`
78-
// and we don't validate it strictly because there's a non-deterministic
79-
// timestamp. Verifying the call count should be good enough.
80-
expect(loggerWarnMock).toHaveBeenCalledTimes(2);
81-
expect(loggerWarnMock).toHaveBeenLastCalledWith(
82-
expect.stringContaining('Initialized NoOpVideoFrameProcessor')
64+
'Specs or options were changed. Destroying background blur processor.'
8365
);
8466

8567
// No errors should happen.

0 commit comments

Comments
 (0)