mirrors/signal-desktop
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

Don't use useMemo() for AudioContext/Audio/Cache

Browse source 
`useMemo()` doesn't guarantee that the value won't be recomputed during
re-renders. Unfortunately, every time `AudioContext` is instantiated -
there is an audible click. This click happens during the change between
conversations and is very annoying.

Move both `AudioContext` instance to the
GlobalAudioContext's top-level declarations, and `Audio`/`WaveformCache`
to `useRef()`s.
This commit is contained in:
Fedor Indutny 2021-03-11 10:35:39 -08:00 • committed by Josh Perez
parent 0bd3c78187
commit 8c9e556a22
2 changed files with 45 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
ts/components/GlobalAudioContext.tsx
View file

@ -21,6 +21,8 @@ export type GlobalAudioProps = {
children?: React.ReactNode | React.ReactChildren;
};
const audioContext = new AudioContext();
/**
* A global context that holds Audio, AudioContext, LRU instances that are used
* inside the conversation by ts/components/conversation/MessageAudio.tsx
@ -29,45 +31,33 @@ export const GlobalAudioProvider: React.FC<GlobalAudioProps> = ({
conversationId,
children,
}) => {
const audio = React.useMemo(() => {
window.log.info(
'GlobalAudioProvider: re-generating audio for',
conversationId
);
return new Audio();
}, [conversationId]);
const audio = React.useRef<HTMLAudioElement | null>(null);
const waveformCache = React.useRef<WaveformCache | null>(null);
// NOTE: the number of active audio contexts is limited per tab/window
// See: https://developer.mozilla.org/en-US/docs/Web/API/AudioContext/AudioContext#google_chrome
const audioContext = React.useMemo(() => {
window.log.info('Instantiating new audio context');
return new AudioContext();
}, []);
const waveformCache: WaveformCache = React.useMemo(() => {
return new LRU({
// NOTE: We don't want to construct these values on every re-render hence
// the constructor calls have to be guarded by `if`s.
if (!audio.current) {
audio.current = new Audio();
}
if (!waveformCache.current) {
waveformCache.current = new LRU({
max: MAX_WAVEFORM_COUNT,
});
}, []);
}
// When moving between conversations - stop audio
React.useEffect(() => {
return () => {
audio.pause();
if (audio.current) {
audio.current.pause();
}
};
}, [audio, conversationId]);
React.useEffect(() => {
return () => {
window.log.info('Closing old audio context');
audioContext.close();
};
}, [audioContext]);
}, [conversationId]);
const value = {
audio,
audio: audio.current,
audioContext,
waveformCache,
waveformCache: waveformCache.current,
};
return (

34
ts/util/lint/exceptions.json
View file

@ -11903,6 +11903,14 @@
"reasonCategory": "falseMatch",
"updated": "2018-09-19T18:06:35.446Z"
},
{
"rule": "jQuery-wrap(",
"path": "node_modules/pino/browser.js",
"line": " wrap(opts, logger, level)",
"lineNumber": 181,
"reasonCategory": "falseMatch",
"updated": "2021-03-09T20:56:35.403Z"
},
{
"rule": "jQuery-$(",
"path": "node_modules/popper.js/dist/esm/popper.min.js",
@ -14696,6 +14704,24 @@
"updated": "2020-11-11T21:56:04.179Z",
"reasonDetail": "Needed to render the remote video element."
},
{
"rule": "React-useRef",
"path": "ts/components/GlobalAudioContext.js",
"line": " const audio = React.useRef(null);",
"lineNumber": 38,
"reasonCategory": "usageTrusted",
"updated": "2021-03-11T17:20:05.355Z",
"reasonDetail": "Need this to avoid re-creating Audio on every re-render"
},
{
"rule": "React-useRef",
"path": "ts/components/GlobalAudioContext.js",
"line": " const waveformCache = React.useRef(null);",
"lineNumber": 39,
"reasonCategory": "usageTrusted",
"updated": "2021-03-11T17:20:05.355Z",
"reasonDetail": "Need this to avoid re-creating WaveformCache on every re-render"
},
{
"rule": "React-useRef",
"path": "ts/components/GroupCallOverflowArea.js",
@ -15444,13 +15470,5 @@
"reasonCategory": "usageTrusted",
"updated": "2021-01-08T15:46:32.143Z",
"reasonDetail": "Doesn't manipulate the DOM. This is just a function."
},
{
"rule": "jQuery-wrap(",
"path": "node_modules/pino/browser.js",
"line": " wrap(opts, logger, level)",
"lineNumber": 181,
"reasonCategory": "falseMatch",
"updated": "2021-03-09T20:56:35.403Z"
}
]