-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[CI] Enable audio tests in Chrome #23665
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
Changes from 10 commits
42e032e
c858dea
29451a4
b7fb25a
a9132c8
92c8b5c
a42c6d4
4b80dd2
9bcf181
f9a781b
e8ca2c4
9fd83f7
9e63502
e9f37db
5d27dc8
9008192
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -22,16 +22,17 @@ | |||
begin to fire. | ||||
*/ | ||||
|
||||
// REPORT_RESULT is defined when running in Emscripten test harness. You can | ||||
// strip these out in your own project. | ||||
#ifdef REPORT_RESULT | ||||
// TEST_AND_EXIT is defined when running in Emscripten test harness. You can | ||||
// strip these out in your own project (otherwise playback will end quickly). | ||||
#ifdef TEST_AND_EXIT | ||||
_Thread_local int testTlsVariable = 1; | ||||
int lastTlsVariableValueInAudioThread = 1; | ||||
#endif | ||||
|
||||
// This function will be called for every fixed-size buffer of audio samples to be processed. | ||||
bool ProcessAudio(int numInputs, const AudioSampleFrame *inputs, int numOutputs, AudioSampleFrame *outputs, int numParams, const AudioParamFrame *params, void *userData) { | ||||
#ifdef REPORT_RESULT | ||||
#ifdef TEST_AND_EXIT | ||||
// Only running in the test harness, see main_thread_tls_access() | ||||
assert(testTlsVariable == lastTlsVariableValueInAudioThread); | ||||
++testTlsVariable; | ||||
lastTlsVariableValueInAudioThread = testTlsVariable; | ||||
|
@@ -63,14 +64,15 @@ EM_JS(void, InitHtmlUi, (EMSCRIPTEN_WEBAUDIO_T audioContext), { | |||
}; | ||||
}); | ||||
|
||||
#ifdef REPORT_RESULT | ||||
#ifdef TEST_AND_EXIT | ||||
bool main_thread_tls_access(double time, void *userData) { | ||||
// Try to mess the TLS variable on the main thread, with the expectation that | ||||
// it should not change the TLS value on the AudioWorklet thread. | ||||
// it should not change the TLS value on the AudioWorklet thread, asserted in | ||||
// ProcessAudio(). | ||||
testTlsVariable = (int)time; | ||||
// Exit to the test harness after enough calls to ProcessAudio() | ||||
if (lastTlsVariableValueInAudioThread >= 100) { | ||||
REPORT_RESULT(0); | ||||
return false; | ||||
emscripten_force_exit(EXIT_SUCCESS); | ||||
} | ||||
return true; | ||||
} | ||||
|
@@ -79,7 +81,11 @@ bool main_thread_tls_access(double time, void *userData) { | |||
// This callback will fire after the Audio Worklet Processor has finished being | ||||
// added to the Worklet global scope. | ||||
void AudioWorkletProcessorCreated(EMSCRIPTEN_WEBAUDIO_T audioContext, bool success, void *userData) { | ||||
if (!success) return; | ||||
if (!success) { | ||||
emscripten_out("Stopped in AudioWorkletProcessorCreated"); | ||||
assert(0); | ||||
return; | ||||
} | ||||
|
||||
// Specify the input and output node configurations for the Wasm Audio | ||||
// Worklet. A simple setup with single mono output channel here, and no | ||||
|
@@ -97,7 +103,8 @@ void AudioWorkletProcessorCreated(EMSCRIPTEN_WEBAUDIO_T audioContext, bool succe | |||
// Connect the audio worklet node to the graph. | ||||
emscripten_audio_node_connect(wasmAudioWorklet, audioContext, 0, 0); | ||||
|
||||
#ifdef REPORT_RESULT | ||||
#ifdef TEST_AND_EXIT | ||||
// Schedule this to exit after ProcessAudio() has been called 100 times | ||||
emscripten_set_timeout_loop(main_thread_tls_access, 10, 0); | ||||
#endif | ||||
|
||||
|
@@ -108,7 +115,11 @@ void AudioWorkletProcessorCreated(EMSCRIPTEN_WEBAUDIO_T audioContext, bool succe | |||
// AudioWorklet global scope, and is now ready to begin adding Audio Worklet | ||||
// Processors. | ||||
void WebAudioWorkletThreadInitialized(EMSCRIPTEN_WEBAUDIO_T audioContext, bool success, void *userData) { | ||||
if (!success) return; | ||||
if (!success) { | ||||
emscripten_out("Stopped in WebAudioWorkletThreadInitialized"); | ||||
assert(0); | ||||
return; | ||||
} | ||||
|
||||
WebAudioWorkletProcessorCreateOptions opts = { | ||||
.name = "noise-generator", | ||||
|
@@ -132,4 +143,9 @@ int main() { | |||
// and kick off Audio Worklet scope initialization, which shares the Wasm | ||||
// Module and Memory to the AudioWorklet scope and initializes its stack. | ||||
emscripten_start_wasm_audio_worklet_thread_async(context, wasmAudioWorkletStack, sizeof(wasmAudioWorkletStack), WebAudioWorkletThreadInitialized, 0); | ||||
|
||||
#ifdef TEST_AND_EXIT | ||||
// We're in the test harness and exiting is via main_thread_tls_access() | ||||
emscripten_exit_with_live_runtime(); | ||||
#endif | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking or a separate PR to enable audio testing CI. PR1: enable audio testing in CI There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, NP. I’ll see how I get on with FF on Monday but I might split it for Chrome first too (I set up a VM with the same Debian and FF and so far, even with no audio device, it doesn’t fail; left to try is headless). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After getting an audio-less Debian VM set-up with all the fallback sound devices removed, I finally managed to recreate what Firefox does. It fails here playing manually, or the test times out otherwise: emscripten/test/webaudio/audioworklet.c Line 60 in f9a781b
The promise calls neither the success nor failure route, it just silently fails and the audio remains permanently suspended. I'll call this a Firefox bug and keep the I'll just enable audio testing in CI for Chrome for now. |
||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we make this change should we also remove the
EMTEST_LACKS_SOUND_HARDWARE
setting above?I'm kind of inclined to make this change as its one separate PR, e.g.
[CI] Enable audio testing on chrome
?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't have much time today so it was mostly experimenting and coming back to it, but:
EMTEST_LACKS_SOUND_HARDWARE
✅Is it fine to always enable the dummy sound hardware and bypass the media interaction? I can't think what this would break (and where real sound hardware is present its used as a preference, at least from my manual testing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine enabling dummy sounds hardware and bypass the interaction check should be fine for all out tests..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I limited removing
EMTEST_LACKS_SOUND_HARDWARE
to Chrome (FF I can't get to work, documented in the comments).