Skip to content

Commit 53ad9f3

Browse files
JamesPHoughtonclaude
JamesPHoughton
and
claude
authored
fix(audio): pause and release audio elements on cleanup to prevent buffer loop (#1205)
* fix(audio): pause and release audio elements on cleanup to prevent buffer loop Fixes a bug where audio would stutter in a loop after the call ended (or a component unmounted) because the browser held the last buffered chunk from a MediaStreamTrack/Audio object that was never explicitly stopped. Changes: - VideoCall.jsx: pause + srcObject=null on all <audio> elements on unmount, breaking the DailyAudio buffer loop that occurred when callObject.leave() completed after React removed DailyAudio from the DOM - AudioElement.jsx: rewrite from render-body side effect (no cleanup possible) to a proper useEffect with removeEventListener/pause/src="" cleanup - IdleProvider.jsx: store chime Audio in a ref so it can be paused/released on cleanup when idle state changes or component unmounts - Countdown.jsx: pause and clear chime Audio src in effect cleanup alongside the existing clearInterval call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(elements): add Playwright component tests for AudioElement (AE-001 to AE-003) Adds the first component tests outside the call/ subsystem, establishing a new playwright/component-tests/elements/ directory. Tests verify: - AE-001: audio plays when CDN URL resolves via useFileURL - AE-002: Audio object is not recreated on re-render (regression guard for the old render-body instantiation pattern) - AE-003: audio is paused and src cleared on unmount Infrastructure added: - playwright/mocks/empirica-global-hooks.js — mock for @empirica/core/player/react; exports useGlobal that reads from window.__mockGlobal, enabling useFileURL to resolve URLs in tests - playwright.config.mjs — new Vite alias for @empirica/core/player/react Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(tests): reorganize mocks into service subdirectories Groups mock infrastructure by the service being mocked, making it easier to find the right file when working with a specific integration: mocks/empirica/ MockPlayer, MockGame, MockStage, MockEmpiricaProvider, hooks.js, global-hooks.js mocks/daily/ MockDailyProvider, hooks.jsx mocks/sentry/ mock.js mocks/utils/ console-capture.js Old flat files are converted to thin re-exports so all existing explicit import paths in test files continue to work without changes. Vite config aliases updated to point directly to the canonical new locations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address Copilot review comments on PR #1205 - AudioElement: add return null (components must not return undefined) - VideoCall: guard audio cleanup with srcObject check to avoid pausing unrelated page audio on unmount (only Daily MediaStream tracks) - MockPlayer: remove verbose console.log from set() hot path to reduce noise in console-assertion tests (keep console.warn for missing _onChange) - MockEmpiricaProvider: add eslint-disable comments for the intentional handleChange omission from useMemo deps (stable callback, documented reason) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b729b3c commit 53ad9f3

26 files changed

Lines changed: 1601 additions & 1876 deletions

‎client/src/call/VideoCall.jsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,23 @@ export function VideoCall({
116116

117117
useDailyEventLogger();
118118

119+
// Pause all <audio> elements on unmount to prevent the browser's audio
120+
// pipeline from looping the last buffered chunk after the call ends.
121+
// callObject.leave() is fire-and-forget in useCallLifecycle's cleanup, so
122+
// Daily may not have torn down tracks before React removes DailyAudio from
123+
// the DOM. Explicitly pausing first breaks the loop.
124+
React.useEffect(() => () => {
125+
// Only target audio elements backed by a MediaStream (Daily tracks).
126+
// Checking srcObject avoids accidentally pausing unrelated page audio.
127+
document.querySelectorAll("audio").forEach((el) => {
128+
if (el.srcObject) {
129+
el.pause();
130+
// eslint-disable-next-line no-param-reassign
131+
el.srcObject = null;
132+
}
133+
});
134+
}, []);
135+
119136
// ------------------- monitor AudioContext state for autoplay debugging ---------------------
120137
// Browsers (especially Safari) may suspend AudioContext due to autoplay policies.
121138
// This hook monitors AudioContext state and provides controls to resume it.

‎client/src/components/IdleProvider.jsx‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,16 @@ export function IdleProvider({
6868
const [modalVisible, setModalVisible] = useState(false); // Controls modal visibility
6969
const isIdle = useIdle(timeout);
7070
const chimeTimerRef = useRef(null);
71+
const chimeAudioRef = useRef(null);
7172

7273
const playChime = () => {
74+
if (chimeAudioRef.current) {
75+
chimeAudioRef.current.pause();
76+
// eslint-disable-next-line no-param-reassign
77+
chimeAudioRef.current.src = "";
78+
}
7379
const audio = new Audio("/counter_bell.mp3");
80+
chimeAudioRef.current = audio;
7481
audio.play().catch((error) => {
7582
console.error("Error playing chime:", error);
7683
});
@@ -90,12 +97,18 @@ export function IdleProvider({
9097
chimeTimerRef.current = null;
9198
}
9299

93-
// Cleanup the timer when dependencies change or component unmounts
100+
// Cleanup the timer and any playing audio when dependencies change or component unmounts
94101
return () => {
95102
if (chimeTimerRef.current) {
96103
clearInterval(chimeTimerRef.current);
97104
chimeTimerRef.current = null;
98105
}
106+
if (chimeAudioRef.current) {
107+
chimeAudioRef.current.pause();
108+
// eslint-disable-next-line no-param-reassign
109+
chimeAudioRef.current.src = "";
110+
chimeAudioRef.current = null;
111+
}
99112
};
100113
}, [isIdle, allowIdle, chimeInterval]);
101114

‎client/src/elements/AudioElement.jsx‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,30 @@
1-
import { useState } from "react";
1+
import { useEffect } from "react";
22
import { useFileURL } from "../components/hooks";
33

44
export function AudioElement({ file }) {
55
const fileURL = useFileURL({ file });
6-
const [hasPlayed, setHasPlayed] = useState(false);
76

8-
if (!hasPlayed && fileURL) {
7+
useEffect(() => {
8+
if (!fileURL) return undefined;
9+
910
const sound = new Audio(fileURL);
10-
// sound.play();
11-
// todo: catch "NotAllowedError: play() failed because the user didn't interact with the document first. https://goo.gl/xX8pDD"
12-
sound.addEventListener("canplaythrough", () => {
13-
sound.play();
14-
console.log(`Playing Audio`);
15-
});
16-
setHasPlayed(true);
17-
}
11+
12+
const handleCanPlay = () => {
13+
sound.play().catch((err) => {
14+
// NotAllowedError fires when autoplay is blocked by the browser
15+
console.warn("[AudioElement] Play failed:", err);
16+
});
17+
console.log("Playing Audio");
18+
};
19+
20+
sound.addEventListener("canplaythrough", handleCanPlay);
21+
22+
return () => {
23+
sound.removeEventListener("canplaythrough", handleCanPlay);
24+
sound.pause();
25+
sound.src = "";
26+
};
27+
}, [fileURL]);
28+
29+
return null;
1830
}

‎client/src/intro-exit/Countdown.jsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ export function Countdown({ launchDate, next }) {
5454
playChime(); // Play chime immediately
5555
const chimeTime = !window.Cypress ? 1000 * 90 : 1000 * 6; // Play chime every 90 seconds, or every 6 seconds in Cypress
5656
const chimeInterval = setInterval(playChime, chimeTime);
57-
return () => clearInterval(chimeInterval);
57+
return () => {
58+
clearInterval(chimeInterval);
59+
chime.pause();
60+
chime.src = "";
61+
};
5862
}, [launched]);
5963

6064
useEffect(() => {

‎playwright/component-tests/elements/AudioElement.ct.jsx‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import React from 'react';
2+
import { test, expect } from '@playwright/experimental-ct-react';
3+
import { AudioElement } from '../../../client/src/elements/AudioElement';
4+
5+
/**
6+
* Component Tests for AudioElement
7+
*
8+
* AudioElement plays a CDN-hosted audio file when a stage element has type
9+
* "audio". The file URL is resolved via useFileURL (which reads Empirica
10+
* globals for the CDN base URL) and played via the Web Audio API.
11+
*
12+
* These tests verify:
13+
* AE-001 Audio plays when the file URL resolves from CDN config
14+
* AE-002 Audio is not recreated on re-render with the same file (regression
15+
* guard: old code called `new Audio()` in the render body, creating
16+
* a new instance—and triggering setHasPlayed—on every render)
17+
* AE-003 Audio is paused and src cleared on unmount (prevents the same
18+
* buffer-loop bug seen with DailyAudio on call end)
19+
*
20+
* Mock setup:
21+
* window.__mockGlobal – read by the useGlobal mock alias for
22+
* @empirica/core/player/react; provides cdnList and
23+
* recruitingBatchConfig so useFileURL can resolve URLs
24+
* window.Audio – replaced with a spy that records instantiations,
25+
* play/pause calls, and canplaythrough listeners
26+
*/
27+
28+
// ---------------------------------------------------------------------------
29+
// Helpers
30+
// ---------------------------------------------------------------------------
31+
32+
async function setupAudioMock(page) {
33+
await page.evaluate(() => {
34+
window.mockAudioInstances = [];
35+
window.Audio = function AudioMock(url) {
36+
const instance = {
37+
src: url,
38+
_played: false,
39+
_paused: false,
40+
_listeners: {},
41+
play() {
42+
instance._played = true;
43+
return Promise.resolve();
44+
},
45+
pause() {
46+
instance._paused = true;
47+
},
48+
addEventListener(evt, fn) {
49+
instance._listeners[evt] = fn;
50+
},
51+
removeEventListener(evt, fn) {
52+
if (instance._listeners[evt] === fn) {
53+
delete instance._listeners[evt];
54+
}
55+
},
56+
};
57+
window.mockAudioInstances.push(instance);
58+
return instance;
59+
};
60+
});
61+
}
62+
63+
async function setupGlobalsMock(page) {
64+
await page.evaluate(() => {
65+
window.__mockGlobal = {
66+
get(key) {
67+
if (key === 'cdnList') return { prod: 'http://test-cdn.example.com' };
68+
if (key === 'recruitingBatchConfig') return { cdn: 'prod' };
69+
return null;
70+
},
71+
};
72+
});
73+
}
74+
75+
// ---------------------------------------------------------------------------
76+
// Tests
77+
// ---------------------------------------------------------------------------
78+
79+
/** AE-001: Audio plays when file URL resolves from CDN config */
80+
test('AE-001: audio plays when file URL resolves', async ({ mount, page }) => {
81+
await setupAudioMock(page);
82+
await setupGlobalsMock(page);
83+
84+
await mount(<AudioElement file="test-audio.mp3" />);
85+
86+
// useFileURL resolves asynchronously via useEffect; wait for Audio instantiation
87+
await expect.poll(() => page.evaluate(() => window.mockAudioInstances.length))
88+
.toBeGreaterThan(0);
89+
90+
// Correct CDN URL was used
91+
const audioSrc = await page.evaluate(() => window.mockAudioInstances[0]?.src);
92+
expect(audioSrc).toBe('http://test-cdn.example.com/test-audio.mp3');
93+
94+
// Simulate browser signalling audio is ready to play
95+
await page.evaluate(() => {
96+
window.mockAudioInstances[0]?._listeners.canplaythrough?.();
97+
});
98+
99+
// play() was called
100+
const played = await page.evaluate(() => window.mockAudioInstances[0]?._played);
101+
expect(played).toBe(true);
102+
});
103+
104+
/** AE-002: Audio element is not recreated on re-render with the same file */
105+
test('AE-002: audio is not recreated on re-render', async ({ mount, page }) => {
106+
await setupAudioMock(page);
107+
await setupGlobalsMock(page);
108+
109+
const component = await mount(<AudioElement file="test-audio.mp3" />);
110+
111+
// Wait for initial Audio creation
112+
await expect.poll(() => page.evaluate(() => window.mockAudioInstances.length))
113+
.toBe(1);
114+
115+
// Re-render with the same file prop — fileURL dep is unchanged, effect should not re-run
116+
await component.update(<AudioElement file="test-audio.mp3" />);
117+
118+
const count = await page.evaluate(() => window.mockAudioInstances.length);
119+
expect(count).toBe(1);
120+
});
121+
122+
/** AE-003: Audio is paused and src cleared on unmount */
123+
test('AE-003: audio is paused and src cleared on unmount', async ({ mount, page }) => {
124+
await setupAudioMock(page);
125+
await setupGlobalsMock(page);
126+
127+
const component = await mount(<AudioElement file="test-audio.mp3" />);
128+
129+
// Wait for Audio to be instantiated
130+
await expect.poll(() => page.evaluate(() => window.mockAudioInstances.length))
131+
.toBeGreaterThan(0);
132+
133+
await component.unmount();
134+
135+
const paused = await page.evaluate(() => window.mockAudioInstances[0]?._paused);
136+
const src = await page.evaluate(() => window.mockAudioInstances[0]?.src);
137+
expect(paused).toBe(true);
138+
expect(src).toBe('');
139+
});

0 commit comments

Comments
 (0)