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.

Sign up for GitHub

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

Add limiter to Playground runtime #11

Closed
wants to merge 3 commits into from
Closed

Add limiter to Playground runtime #11

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e7f50b
Add limiter to Playground runtime
mark-mxwl Apr 30, 2024
640695a
Merge branch 'elemaudio:main' into mark-mxwl-playground-patch-1
mark-mxwl Jul 26, 2024
9700955
matched import version for core lib.; limiter tweaks
mark-mxwl Jul 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"lint": "next lint"
},
"dependencies": {
"@elemaudio/core": "^2.1.0",
"@elemaudio/web-renderer": "^2.1.0",
"@elemaudio/core": "^3.2.1",
"@elemaudio/web-renderer": "^3.2.3",
"@heroicons/react": "^2.0.18",
"@monaco-editor/react": "^4.5.2",
"lz-string": "^1.5.0",
Expand Down
8 changes: 6 additions & 2 deletions src/components/playground/runtime.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { el } from '@elemaudio/core';

export function getImportMapScript(version) {
return `{ "imports": { "@elemaudio/core": "https://cdn.skypack.dev/@elemaudio/core@^${version}" } }`;
}
Expand Down Expand Up @@ -44,9 +46,11 @@ export class Runtime {
throw new Error('Missing render function on default export');

const userOutput = render();
// This limiter setting preserves approx. 1 dB of headroom.
const limit = (input) => el.compress(1, 10, -4, 20, input, input);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compress function that we're using right here comes from the el library imported from @elemaudio/core version at the top of the file. But the userOutput signal that we get from invoking the user module is built using the el export of whatever version @elemaudio/core is loaded in the user module.

It's really quite rare that there would be a conflict between these versions, but these versions do sometimes have requirements onto the renderer. For example, the core library might export a utility that uses a native node that only a recent version of the renderer knows about.

I do anticipate the next major version bringing some breaking changes, so we could arrive in a situation here where the user code is running a v4 web-renderer instance and v4 elemaudio/core but the limiter here is written against v3 elemaudio/core, and we could be getting into tricky territory there.

Even if this wouldn't be a problem, I think it'd be safer to make sure that we try to use the el.compress function from the same version that the user is running. Luckily, we know exactly what version that is because we force it using the importMap script above. Then on line 16 we fetch a matching web-renderer version. I think the fix here would be to add another line right below 16 which fetches the appropriate elemaudio/core version and grabs the el library out of that, then uses that compress function here to apply the limiting

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting back, @nick-thompson. I see your concern, and I totally agree that keeping the versions aligned on import is a much better solution. I'll try to knock out the changes this weekend.

const stats = Array.isArray(userOutput)
? await this.core.render(...userOutput)
: await this.core.render(userOutput, userOutput);
? await this.core.render(...userOutput.map(limit))
: await this.core.render(limit(userOutput), limit(userOutput));

return {
ok: true,
Expand Down