-
Notifications
You must be signed in to change notification settings - Fork 196
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
Rainfly - An AudioWorklet DSP Playground [MVP] #395
base: main
Are you sure you want to change the base?
Conversation
@hoch @mjwilson-google I've created the Rainfly directory just directly under |
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.
The directory location looks good to me.
But don't we need to exclude this directory from Eleventy's scanning? Can we include that change in this PR as well?
That sounds good. Eleventy doesn't build this directory yet but I'll make sure that it's all working. I'll copy the rainfly repo into this directory for this PR. |
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.
LGTM with nits
It's okay to land this as is, but I am asking to work on nits from the review as a follow-up.
@@ -19,7 +19,7 @@ jobs: | |||
- name: Install Node.js | |||
uses: actions/setup-node@v3 | |||
with: | |||
node-version: 16 | |||
node-version: 18 |
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.
Just curious - is 18 required?
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.
Yes, 18 is required for Svelte.
* an object with the name and data of the file | ||
*/ | ||
export async function fetchTextFile(url) { | ||
let filename = url.split('/').pop(); |
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 don't see any error handling here. Is jszip
handling errors somehow?
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.
@hoch Do you mean error handling for the filename handling or the fetch requests (next few lines)? jszip
isn't involved in this part of the code. I've gone ahead and wrapped the fetch requests in a try catch.
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.
Thanks for continuing to work on this!
I didn't try running it yet, but I have some small comments. Since Hongchan has already approved, please feel free to resolve my comments and land if necessary.
@@ -0,0 +1,9 @@ | |||
// Use `context` as the AudioContext | |||
// @sampleRate = 48000 |
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.
Is this a parameter? Generally the output device sample rate is used if nothing is specified (https://webaudio.github.io/web-audio-api/#dictionary-audiocontextoptions-members).
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.
Yes this is a parameter that we preprocess. If not specified, default is also 48000.
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.
Would the Canopy parameter syntax of @sampleRate 48000
be preferred?
@@ -0,0 +1,7 @@ | |||
// Use `context` as the AudioContext | |||
// @sampleRate = 48000 |
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.
Same comment as for the other sampleRate location.
This PR is to introduce Rainfly: An AudioWorklet DSP Playground for Web Audio. This online AudioWorklet IDE streamlines writing custom JavaScript AudioWorklet code, building Web Audio DSP graphs, and recording output for easy visualization, analysis and validation.
Demo: https://googlechromelabs.github.io/web-audio-samples/rainfly/
Rainfly MVP Features:
AudioWorkletProcessor
andAudioContext
graphAudioContext
, can pause and playAuthors:
GSoC 2024