Skip to content
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

Feature/audio overhaul #191

Merged
merged 27 commits into from
Mar 12, 2022
Merged

Feature/audio overhaul #191

merged 27 commits into from
Mar 12, 2022

Conversation

200sc
Copy link
Contributor

@200sc 200sc commented Feb 14, 2022

See #189, and also #131, although the latter is not done yet

Before this is merged, the new audio system should have implementations for (or spiked and failed for):

  • windows
  • darwin
  • linux
  • js
  • android

@200sc
Copy link
Contributor Author

200sc commented Feb 19, 2022

Linux is working with two needed things

  • go mod tidy
  • investigate this:
    image
    The top left pixel there indicates the audio we're writing for triangle waves (and I believe sin waves) (and maybe just for 32 bit) is problematic. On linux this appears via lower pitched sin and triangle waves being quieter than upper pitches, where pulse and sin do not have this condition.

@200sc
Copy link
Contributor Author

200sc commented Feb 19, 2022

JS has been spiked and it looks like one needs to use AudioWorklets (https://developer.mozilla.org/en-US/docs/Web/API/AudioWorkletNode) to handle audio like we want to, with a streaming interface. The instructions for doing this revolve around having multiple js files that load each other as classes, which I don't know how to do with syscall/js, if it's even possible.

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2022

Codecov Report

Merging #191 (499ae48) into master (ebf404b) will decrease coverage by 0.41%.
The diff coverage is 18.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #191      +/-   ##
==========================================
- Coverage   92.67%   92.26%   -0.42%     
==========================================
  Files         141      141              
  Lines        6406     6438      +32     
==========================================
+ Hits         5937     5940       +3     
- Misses        407      435      +28     
- Partials       62       63       +1     
Impacted Files Coverage Δ
dlog/strings.go 0.00% <ø> (ø)
drawLoop.go 44.57% <0.00%> (-17.69%) ⬇️
config.go 98.23% <100.00%> (+0.01%) ⬆️
dlog/default.go 100.00% <100.00%> (ø)
event/trigger.go 81.81% <100.00%> (-0.33%) ⬇️
sceneLoop.go 91.01% <0.00%> (-3.38%) ⬇️
event/handler.go 96.66% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebf404b...499ae48. Read the comment docs.

@200sc
Copy link
Contributor Author

200sc commented Feb 19, 2022

Darwin can also use our pulse implementation (or our jfreymuth/pulse wrapper, to be less generous)-- and personally I prefer that requirement to the other roads one would need to go down, i.e. compiling more C / C variants to interact with osx specific audio libraries.

@200sc
Copy link
Contributor Author

200sc commented Feb 27, 2022

Current thought is to do:

  1. Install pulse on the linux arm box and adjust the non-arm test suite to skip examples (they are more effectively tested by graphical runners)
  2. Expose a choice of which audio backing you want to use (with the current options being pulse for unix and direct sound for
  3. Change the old and new direct sound audio systems to share a device so they can both be compiled in
  4. Maybe try to expose to the api a choice of output sound device? Might be too early for that.

Then merge this in and cut a minor release bump for it.

@200sc
Copy link
Contributor Author

200sc commented Feb 27, 2022

(JS audio can't follow this API as syscall js currently works and android audio basically only works with OpenAL by my understanding, both of which are worth working on but neither worth holding back this branch).

@Implausiblyfun
Copy link
Contributor

Other than the todo with the early return the only other thing of note that I see is the unlimited draw rate change.
Namely can we just add it to the description so we dont miss it during changelog creation?

@200sc
Copy link
Contributor Author

200sc commented Mar 6, 2022

Changelog:

  • Adds a streaming audio API under audio/pcm, with direct sound support on windows and pulse audio support on osx/linux.
  • Adds a piano example to test and demonstrate the pcm interface.
  • Adds additional key handling to JS (Shift and Delete)
  • Adds a UnlimitedDrawRate configuration option which will cause the draw loop to not wait at all between draw frames. This setting is, experientially, particularly useful on WIndows, where a frame rate of 60fps is high enough that it exceeds Window's default timer precision and will result in inconsistent frame times.

@200sc
Copy link
Contributor Author

200sc commented Mar 6, 2022

Issues to open after this is merged and released:

  • Support listing and selecting a specific audio device to play audio on
  • Support consuming audio input (e.g. midi controllers or microphones)
  • Support more audio APIs on windows (XAudio2)
  • Support more audio APIs on Linux (ALSA)
  • Support more audio APIs on OSX (IOKit)
  • Support OpenAL (for Android specifically but for all OSes)
  • Support explicitly non-streaming audio for JS in the new PCM package
  • OakV4: combine audio packages

Or maybe this is just a "big audio overhaul" issue with these points as sub goals?

@200sc
Copy link
Contributor Author

200sc commented Mar 6, 2022

@Implausiblyfun Objection to merging this and cutting oak 3.4.0?

Or should we toss in #170 and #179 to 3.4.0 quick?

@200sc 200sc marked this pull request as ready for review March 6, 2022 17:48
@200sc 200sc merged commit ed3d9b5 into master Mar 12, 2022
@200sc 200sc deleted the feature/audio-overhaul branch March 12, 2022 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants