Skip to content

Commit

Permalink
Cleanups for RadioTimeSignal and timesignal.c
Browse files Browse the repository at this point in the history 
emscripten is no longer generating broken JS glue for Audio
Worklets.

On the C side, put debugging printfs behind an #ifdef,
update comments, and rework the build script to remove
now-unnecessary awk trickery and build with -O3.

On the JS side, put debugging console.log()s behind
import.meta.env. Also remove the test we had to indirectly
check that the Audio Worklet loaded, since that doesn't
seem to play nice with even the browser test runner.

cf. emscripten-core/emscripten#21192
  • Loading branch information
@kangtastic
kangtastic committed Mar 10, 2024
1 parent 2679b70 commit 2c248dc
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 114 deletions.
18 changes: 12 additions & 6 deletions src/shared/radiotimesignal.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable no-console */

import EventBus from "@shared/eventbus";
import {
TimeSignalReadyEvent,
Expand Down Expand Up @@ -116,12 +118,14 @@ class RadioTimeSignal {
};

#communicate = (state: number) => {
console.log(`RadioTimeSignal.#communicate(${state});`);
if (import.meta.env.DEV)
console.log(`RadioTimeSignal.#communicate(${state});`);
this.state = state;

if (this.state === "idle") {
this.audioContext.suspend().then(() => {
console.log(`Suspended playback at ${Date.now()}`);
if (import.meta.env.DEV)
console.log(`Suspended playback at ${Date.now()}`);
});
} else if (this.state === "reqparams") {
this.#sendParams();
Expand All @@ -142,9 +146,10 @@ class RadioTimeSignal {
noclip,
);

console.log(
`Sent params at ${Date.now()}, output latency ${outputLatencyMs}`,
);
if (import.meta.env.DEV)
console.log(
`Sent params at ${Date.now()}, output latency ${outputLatencyMs}`,
);
};

start(params: TimeSignalModuleParams) {
Expand All @@ -155,7 +160,8 @@ class RadioTimeSignal {
* silence for some time and signals an appropriate state change to
* request params.
*/
console.log(`RadioTimeSignal.start() at ${Date.now()}`);
if (import.meta.env.DEV)
console.log(`RadioTimeSignal.start() at ${Date.now()}`);
this.#params = params;
if (this.audioContext.state === "suspended")
this.audioContext.resume().then(this.#module._tsig_start);
Expand Down
88 changes: 7 additions & 81 deletions src/wasm/build_timesignal.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
set -ue

EMCC_PARAMS=(
"-sEXPORTED_RUNTIME_METHODS="addFunction,emscriptenRegisterAudioObject,emscriptenGetAudioObject,wasmTable""
"-sEXPORTED_RUNTIME_METHODS="addFunction,emscriptenRegisterAudioObject,emscriptenGetAudioObject""
'-sEXPORT_NAME=createTimeSignalModule'
'-sINITIAL_MEMORY=65536'
'-sALLOW_TABLE_GROWTH'
Expand All @@ -13,90 +13,16 @@ EMCC_PARAMS=(
'-sMALLOC=none'
'-sEXPORT_ES6'
'-sJS_MATH'
'-O3'
)

AWK_PROG='
BEGIN{
line = -1
ok = 1
}
{
if ($0 ~ /timesignal.aw.js/) {
sub("timesignal.aw.js", "wasm/timesignal.aw.js")
ok = 1
} else {
if (line < 0 && $0 == "var wasmBinaryFile;")
line = NR
ok = line < 0 || ((NR != line + 1 && NR < line + 6) || NR > line + 9)
}
if (ok)
print $0
}'

AWK_PROG_O3='
{
sub("timesignal.aw.js", "wasm/timesignal.aw.js")
sub(/var wasmBinaryFile;if\(Module\["locateFile"\]\){wasmBinaryFile="timesignal.wasm";if\(!isDataURI\(wasmBinaryFile\)\){wasmBinaryFile=locateFile\(wasmBinaryFile\)}}else{wasmBinaryFile=new URL\("timesignal.wasm",import.meta.url\).href}/,
"var wasmBinaryFile=\"timesignal.wasm\";if(!isDataURI(wasmBinaryFile)){wasmBinaryFile=locateFile(wasmBinaryFile)}")
print $0
}'

# Two problems prevent us from using timesignal.js as is.
#
# First, as of emsdk 3.1.51 (year end 2023), Emscripten generates broken
# JavaScript glue code for a module using the Wasm Audio Worklets API if
# compiled with -sEXPORT_ES6. The problem seems to be that URL() doesn't exist
# in AudioWorkletGlobalScope, and is fixed by turning this:
#
# var wasmBinaryFile;
# if (Module['locateFile']) {
# wasmBinaryFile = 'timesignal.wasm';
# if (!isDataURI(wasmBinaryFile)) {
# wasmBinaryFile = locateFile(wasmBinaryFile);
# }
# } else {
# // Use bundler-friendly `new URL(..., import.meta.url)` pattern; works in browsers too.
# wasmBinaryFile = new URL('timesignal.wasm', import.meta.url).href;
# }
#
# into this:
#
# var wasmBinaryFile = 'timesignal.wasm';
# if (!isDataURI(wasmBinaryFile)) {
# wasmBinaryFile = locateFile(wasmBinaryFile);
# }
#
# Second, Emscripten assumes that timesignal.aw.js will be moved to the server
# root, but we'll be moving it elsewhere. So we need to turn this:
#
# audioWorklet.addModule('timesignal.aw.js').then(() => {
#
# into this:
#
# audioWorklet.addModule('wasm/timesignal.aw.js').then(() => {
#
# We also have to account for the effects of -O3 on the generated .js files.
postprocess_timesignal_js() {
local awk_prog=${AWK_PROG}

for param in "${EMCC_PARAMS[@]}"; do
if [ "$param" == "-O3" ]; then
awk_prog=${AWK_PROG_O3}
break
fi
done

awk "${awk_prog}" timesignal.js > timesignal.js.out
mv timesignal.js.out timesignal.js
}

# Append any user parameters.
for param in "$@"; do
EMCC_PARAMS+=("${param}")
done

emcc timesignal.c -o timesignal.js "${EMCC_PARAMS[@]}"
postprocess_timesignal_js
mkdir -p ../../wasm
cp timesignal.aw.js timesignal.js timesignal.wasm ../../wasm
rm -f timesignal.aw.js timesignal.js timesignal.js.out timesignal.wasm timesignal.ww.js
emcc timesignal.c -o timesignal.js "${EMCC_PARAMS[@]}" &&
sed -i 's|timesignal.aw.js|wasm/timesignal.aw.js|' timesignal.js &&
mkdir -p ../../wasm &&
cp timesignal.aw.js timesignal.js timesignal.wasm ../../wasm &&
rm -f timesignal.aw.js timesignal.js timesignal.wasm timesignal.ww.js
3 changes: 2 additions & 1 deletion src/wasm/datetime.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ typedef struct tsig_datetime_t {
uint16_t msec; /** Millisecond (0-999). */
} tsig_datetime_t;

/* TODO: Remove. */
#ifdef TSIG_DEBUG
static void datetime_print(tsig_datetime_t datetime) {
printf("datetime = {\n");
printf(" .year = %u\n", datetime.year);
Expand All @@ -39,6 +39,7 @@ static void datetime_print(tsig_datetime_t datetime) {
printf(" .msec = %u\n", datetime.msec);
printf("};\n");
}
#endif /* TSIG_DEBUG */

/**
* Determine whether a year is a leap year.
Expand Down
33 changes: 27 additions & 6 deletions src/wasm/timesignal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@
* created. Only the first 3 are necessary; the .ww.js file can be deleted.
* However, timesignal.js needs to be modified before it can be used.
*
* As of emsdk 3.1.51 (year end 2023), Emscripten generates broken JS glue code
* for a module using the Wasm Audio Worklets API if compiled with -sEXPORT_ES6.
* The Wasm* binary is fetched in AudioWorkletGlobalScope instead of the main
* thread, so URL() is undefined. A fix is to turn this:
* Prior to v3.1.54 (cf. https://github.com/emscripten-core/emscripten/pull/21192),
* emscripten generates broken JS glue code for a module using the Wasm Audio
* Worklets API if compiled with -sEXPORT_ES6. The Wasm binary is fetched in
* AudioWorkletGlobalScope instead of the main thread, so URL() is undefined.
* A fix is to turn this:
*
* var wasmBinaryFile;
* if (Module['locateFile']) {
Expand Down Expand Up @@ -194,9 +195,11 @@ EM_BOOL tsig_awp_process_cb(int n_inputs, const AudioSampleFrame *inputs,

tsig_waveform_init(&tsig_ctx.waveform_ctx, &tsig_ctx.params);

#ifdef TSIG_DEBUG
printf("Wasm loaded params at %f, phase delta is %u / %u\n",
tsig_ctx.waveform_ctx.timestamp, tsig_ctx.waveform_ctx.phase_delta,
tsig_ctx.waveform_ctx.phase_base);
#endif /* TSIG_DEBUG */

next_state = TSIG_STATE_FADE_IN;
break;
Expand Down Expand Up @@ -248,6 +251,11 @@ EM_BOOL tsig_awp_process_cb(int n_inputs, const AudioSampleFrame *inputs,
*/
void tsig_awp_create_cb(EMSCRIPTEN_WEBAUDIO_T audio_ctx, EM_BOOL success,
void *init_js_cb) {
#ifdef TSIG_DEBUG
printf("tsig_awp_create_cb(audio_ctx=%d, success=%d, init_js_cb=%p)\n",
audio_ctx, success, init_js_cb);
#endif /* TSIG_DEBUG */

if (!success)
return;

Expand Down Expand Up @@ -278,6 +286,11 @@ void tsig_awp_create_cb(EMSCRIPTEN_WEBAUDIO_T audio_ctx, EM_BOOL success,
*/
void tsig_aw_thread_init_cb(EMSCRIPTEN_WEBAUDIO_T audio_ctx, EM_BOOL success,
void *init_js_cb) {
#ifdef TSIG_DEBUG
printf("tsig_aw_thread_init_cb(audio_ctx=%d, success=%d, init_js_cb=%p)\n",
audio_ctx, success, init_js_cb);
#endif /* TSIG_DEBUG */

if (!success)
return;

Expand All @@ -299,6 +312,11 @@ EMSCRIPTEN_KEEPALIVE void tsig_init(EMSCRIPTEN_WEBAUDIO_T audio_ctx,
uint32_t sample_rate,
tsig_js_cb_func init_js_cb,
tsig_js_cb_func js_cb) {
#ifdef TSIG_DEBUG
printf("tsig_init(audio_ctx=%d, sample_rate=%u, init_js_cb=%p, js_cb=%p)\n",
audio_ctx, sample_rate, init_js_cb, js_cb);
#endif /* TSIG_DEBUG */

atomic_store(&tsig_ctx.state, TSIG_STATE_IDLE);
tsig_ctx.waveform_ctx.sample_rate = sample_rate;
rearm_state_transition_delay();
Expand Down Expand Up @@ -327,10 +345,12 @@ EMSCRIPTEN_KEEPALIVE void tsig_start() {
EMSCRIPTEN_KEEPALIVE void tsig_load_params(double offset, uint8_t station,
uint8_t jjy_khz, int16_t dut1,
uint8_t noclip) {
#ifdef TSIG_DEBUG
printf(
"tsig_load_params(offset=%f, station=%u, jjy_khz=%u, dut1=%d, "
"noclip=%d);\n",
offset, station, jjy_khz, dut1, noclip);
#endif /* TSIG_DEBUG */

tsig_params.offset = offset;
tsig_params.station = station;
Expand All @@ -347,7 +367,7 @@ EMSCRIPTEN_KEEPALIVE void tsig_stop() {
int state = atomic_load(&tsig_ctx.state);
int next_state = TSIG_STATE_FADE_OUT;

/* No need to fade out if Audio Worklet thread never started. */
/* No need to fade out if playback never started. */
if (state < TSIG_STATE_FADE_IN) {
rearm_state_transition_delay();
next_state = TSIG_STATE_IDLE;
Expand All @@ -357,7 +377,7 @@ EMSCRIPTEN_KEEPALIVE void tsig_stop() {
tsig_js_cb(next_state);
}

/* TODO: Remove. */
#ifdef TSIG_DEBUG
EMSCRIPTEN_KEEPALIVE uint32_t tsig_print_timestamp(double timestamp, int n) {
uint32_t ret = 0;

Expand All @@ -378,3 +398,4 @@ EMSCRIPTEN_KEEPALIVE uint32_t tsig_print_timestamp(double timestamp, int n) {

return ret;
}
#endif /* TSIG_DEBUG */
25 changes: 5 additions & 20 deletions test/shared/radiotimesignal.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,13 @@
import { afterAll, describe, expect, it, vi } from "vitest";
import { describe, expect, it } from "vitest";

import EventBus from "@shared/eventbus";
import { TimeSignalReadyEvent } from "@shared/events";
import RadioTimeSignal from "@shared/radiotimesignal";

import { delay } from "@test/utils";

const publishSpy = vi.spyOn(EventBus, "publish");
/*
* We cannot rely upon creating an Audio Worklet thread from WASM in browser
* tests. Without this, very little can be tested.
*/

describe("RadioTimeSignal", () => {
afterAll(() => {
publishSpy.mockRestore();
});

it("publishes TimeSignalReadyEvent", async () => {
/* May be flaky due to rapid WASM compilation. */
for (let retries = 3; retries > 0; retries--) {
/* eslint-disable no-await-in-loop */
if (publishSpy.mock.calls.length > 0) break;
await delay(100);
}
expect(publishSpy).toHaveBeenCalledWith(TimeSignalReadyEvent);
});

it("is a singleton", () => {
const Class = RadioTimeSignal.constructor as any;
expect(() => new Class()).toThrow("RadioTimeSignal is a singleton class.");
Expand Down

0 comments on commit 2c248dc

Please sign in to comment.