Closed mlabbe closed 1 year ago
Thanks! This looks good from my quick scan just now. I'll give it a more detailed review soon, but I don't expect there will be any issues. The pDevice
thing is fine.
One possible design issue to consider is whether you want to dispatch an unlock notification on all platforms immediately after start. Otherwise, in application code, everyone will have to write a variant of this pseudocode:
int is_audio_unlocked(void) {
return !EMSCRIPTEN_BUILD || has_unlock_notification_occurred;
}
I'm happy with this. Merged. Thanks a lot for the report and PR. Regarding the unlock notification on other platforms, I'm not sure to be honest. I almost want to wait for feedback from the community. Regardless, we'll treat that as a separate task. Is it a practical issue for you?
Per #759 , this adds a new unlocked notification. I am confident that this is the right approach-- the unlock only fires AFTER successful resumption of the audio context. On failure, an error is now logged to the javascript console, instead of silent failure.
Prior to this PR, simply touching the screen would attempt a resume, and a downstream developer of miniaudio would have to try and monitor touches/clicks in their application to mirror the one being done in miniaudio. In some cases, this could lead to subtle errors where the client code would monitor for a click or touch down, which starts audio processing prematurely, when we really want it to start on touch up.
One possible wart in this pull request -- I had to store the pDevice in the
device
javascript object so I could call a C function with the pDevice pointer from Javascript.Tested successfully on MacOS Chrome, iOS 17 Safari and Android 14 Chrome, in addition to natively compiling without warnings on clang 14 on Linux.
Pull request is to split files; sorry. Hopefully the changes are small enough.