binji / binjgb

Gameboy emulator implemented in C, that also runs in the browser
https://binji.github.io/binjgb/
MIT License
534 stars 61 forks source link

Skip audio frames while audio context is suspended #44

Closed eliot-akira closed 2 years ago

eliot-akira commented 2 years ago

I was seeing an audio latency issue on a page with the emulator embedded, in cases when the audio context starts out in "suspended" state.

As you know, Web Audio API has an autoplay policy which can require a user gesture, such as click or keypress event, and calling resume() on the audio context, before it's in a "running" state.

The issue was that, because I'm starting the emulator immediately upon page load, audio buffers were being created while the audio context was still suspended. This happens in the Audio class, the method pushBuffer(). Every time Audio.ctx.createBuffer is called, it produces a console warning:

An AudioContext was prevented from starting automatically. It must be created or resumed after a user gesture on the page.

Then, when the audio context is "resumed" after a user gesture, it apparently starts processing all the buffered audio, causing significant audio latency in the emulator.

The solution was to avoid creating an audio buffer when the audio context is suspended.

Before this condition:

if (this.startSec >= nowSec) {
  const buffer = Audio.ctx.createBuffer(2, AUDIO_FRAMES, this.sampleRate)
  ...
}

I added a line:

if (Audio.ctx.state !== 'running') return

This is probably too small for a pull request, but I thought it could be a possible improvement for the emulator.


Just wanted to add, I'm thoroughly enjoying the emulator and its code base. Such wonderful work!

binji commented 2 years ago

Thanks for the thoughtful comment! It's funny, I noticed this issue and thought I fixed it, but it seems I only did for simple.js (see https://github.com/binji/binjgb/commit/8f6a681eb4b7eff879750c1ddbf1d5c3c64ca552). I guess I should port that fix over to demo.js too!

eliot-akira commented 2 years ago

Oh, I see, the Audio class in simple.js does this:

pushBuffer() {
  if (!this.started) { return; }
  ...
}

Using a boolean is more efficient than comparing a string every time too. OK, I'll study how that's done and copy over the difference. I'll go ahead and close the issue. Much appreciated!