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

Globals specified with [Exposed=*] not present in AudioWorkletGlobalScope #2499

Open
Liamolucko opened this issue Aug 3, 2022 · 20 comments
Open
Labels
Needs External Dependency Resolution An external dependency is blocking progress on this issue.

Comments

@Liamolucko
Copy link

Describe the issue

There are several globals such as TextEncoder, TextDecoder and URL which seem to have been intentionally left out of AudioWorkletGlobalScope. However, these are specified with [Exposed=*] in WebIDL, which is defined as:

The definition of AudioWorkletGlobalScope uses [Global], so should technically contain these:

[Global=(Worklet, AudioWorklet), Exposed=AudioWorklet]
interface AudioWorkletGlobalScope : WorkletGlobalScope {
  undefined registerProcessor (DOMString name,
                               AudioWorkletProcessorConstructor processorCtor);
  readonly attribute unsigned long long currentFrame;
  readonly attribute double currentTime;
  readonly attribute float sampleRate;
};

To be clear, I'm not saying they should be added to AudioWorkletGlobalScope; I'm saying that it needs to be specified somewhere that they aren't there.

It's possible I may have just missed a note about this somewhere in the spec, but I couldn't find anything.

Where Is It

I'm not entirely sure whether this should be fixed in the Web Audio spec itself or the specs which define these globals with [Exposed=*].

Some possibilities, though:

@Liamolucko Liamolucko added Needs Discussion The issue needs more discussion before it can be fixed. Untriaged labels Aug 3, 2022
@Liamolucko Liamolucko changed the title Globals specified with [Exposed=*] not present in `AudioWorkletGlobalScope Globals specified with [Exposed=*] not present in AudioWorkletGlobalScope Aug 3, 2022
@hoch
Copy link
Member

hoch commented Aug 3, 2022

As you mentioned, the WG's intention is to limit the exposure of AudioWorkletGlobalScope only for the worklet-based audio processing. I am not sure if there's a way to notate the exception of specific Globals using WebIDL.

It also seems like there's inconsistency between the spec and the actual implementation:
Chrome's IDL
FireFox's IDL

We'll triage this issue in the next teleconference and think about solution.

@guest271314

This comment was marked as off-topic.

@guest271314

This comment was marked as off-topic.

@hoch
Copy link
Member

hoch commented Sep 14, 2022

@Liamolucko It looks like we need to open issues against TextEncoder and URL specs. Could you do that?

@hoch hoch removed the Needs Discussion The issue needs more discussion before it can be fixed. label Sep 14, 2022
@inexorabletash
Copy link

FYI, TextEncoder/TextDecoder were made [Exposed=*] here whatwg/encoding@f09648a with discussion at tc39/proposal-shadowrealm#331 - TL;DR there are a set of APIs specified as Web APIs that could just as easily have been specified as parts of ECMAScript (streams, encoding, URL, etc) and it was felt that these should be present in every context.

So... might require discussion w/ TC39 folks etc. if you want to revisit that.

@inexorabletash
Copy link

More specifically: if you have a reason to exclude these APIs, it would follow there are parts of ECMAScript that perhaps you'd also want to exclude (e.g. parts of ECMA-402 Internationalization?), now or in the future.

@hoch
Copy link
Member

hoch commented Sep 12, 2023

That's a good point. Thanks @inexorabletash!

@hoch
Copy link
Member

hoch commented Sep 13, 2023

2023 TPAC Audio WG Discussion:
The WG decided to remove some globally exposed interfaces, but #2499 (comment) suggested that a wider discussion might be needed to exclude them from the AudioWorkletGlobalScope.

Pinging implementors to get more thoughts: @padenot @jyavenard

@guest271314

This comment was marked as off-topic.

@guest271314

This comment was marked as off-topic.

@padenot
Copy link
Member

padenot commented Sep 20, 2023

The problem with excluding methods from an API is users will nonetheless, inevitably, want to use those methods in the API.

This is not the case. It is a mistake to use those APIs in a real-time context such as the AudioWorkletGlobalScope, because those APIs aren't real-time safe.

@guest271314

This comment was marked as off-topic.

@guest271314

This comment was marked as off-topic.

@guest271314

This comment was marked as off-topic.

@padenot
Copy link
Member

padenot commented Sep 21, 2023

Based on what tests and empirical evidence that I can vet myself?

Real-time safety can be assessed theoretically in the cases we care about here, by simply thinking about the constructs in use in the AudioWorkletGlobalScope. https://en.wikipedia.org/wiki/Real-time_computing explain what real-time safety means. Any code in AudioWorkletGlobalScope should be real-time-safe. An exception is when the AudioContext is suspended, in which case some often preparatory and otherwise non-real-time code can run.

It follows that anything but the simplest amount of necessary computations is to be done on a different thread than the thread the AudioWorkletGlobaleScope uses. Generally, it's reading and writing from memory, and performing computations, often digital signal processing. Any allocations, lock usage, system calls (including any for of network or file IO) is proscribed. Anything that can be done in a different scope is better done there. Text manipulation or anything of that sort have no place in this scope. On the web platform, we need to also add anything that can create JavaScript objects, because this causes garbage-collection to happen, and garbage-collection in JavaScript engines are very optimized, but not real-time safe.

https://paul.cx/public/karplus-stress-tester/ is an empirical test you can run, source is available. https://blog.paul.cx/post/a-wait-free-spsc-ringbuffer-for-the-web/ is a discussion on this. You're welcome to write your own tests, you'll find the same result. This article shows how to precisely assess the performance you're getting.

As said in the article above, communication to and from this scope is better done using wait or lock-free data structure, that are real-time-safe, as widely documented in the literature, but this classic article is a good introduction to the entire field.

And we've tested asynchronous code extensively while implementing, it's a disaster for performance and causes glitches in the simplest case. It's always possible to write bad code on any platform, but if we can, it's better to not allow it.

@guest271314

This comment was marked as off-topic.

@daxpedda
Copy link

2023 TPAC Audio WG Discussion: The WG decided to remove some globally exposed interfaces, but #2499 (comment) suggested that a wider discussion might be needed to exclude them from the AudioWorkletGlobalScope.

Pinging implementors to get more thoughts: @padenot @jyavenard

Was there any further discussion or conclusion on which interfaces exactly should not be exposed?

Coming from Wasm here it would be good to know if TextDecoder and TextEncoder will be supported or not.

@hoch
Copy link
Member

hoch commented Feb 23, 2024

We do not have a comprehensive list of blocked APIs yet, but these are good questions to ask:

  1. Does it do any significant memory allocation within its operation?
  2. Does it have any unbound thread blocking operation?

If the answer to them is yes, then it is likely to be excluded from the AudioWorkletGlobalScope.

@daxpedda
Copy link

@padenot has pointed out in Mozilla's Bugzilla, that any allocation is bad and considering there is already a non-allocating API for TextEncoder (TextEncoder.encodeInto()), that one could be exposed instead.

So the proposal is to expose TextDecoder fully and TextEncoder partly (exclude TextEncoder.encode()).

It was suggested to ask for clarification in WHATWG Encoding, which I would like to do after getting some official statement here. Does that sound like an appropriate plan?

@hoch hoch added the Needs External Dependency Resolution An external dependency is blocking progress on this issue. label Apr 16, 2024
@padenot
Copy link
Member

padenot commented Apr 16, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs External Dependency Resolution An external dependency is blocking progress on this issue.
Projects
None yet
Development

No branches or pull requests

6 participants