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

Whether process() is called for an AudioWorkletNode should depend on whether the active source flag is true and whether the AudioNodes connected to inputs are actively processing #2574

Open
karlt opened this issue Mar 11, 2024 · 4 comments

Comments

@karlt
Copy link
Contributor

karlt commented Mar 11, 2024

The "rendering a render quantum" steps steps appear to keep calling process() for all AudioWorkletNodes that are "still alive" and have [[callable process]] true.

The only place that I see the active source flag read is in the definition of actively processing.

"still alive" is not defined, but I don't think it is necessary either.

It's tempting to simply replace "still alive" with "actively processing", but this would be problematic due to the dependency of "actively processing" state on the "in a cycle" state, which has not yet been determined at this point in the algorithm.

Perhaps a more carefully placed check on "actively processing" in the "rendering a render quantum" steps would be sufficient to address this.

@hoch
Copy link
Member

hoch commented May 2, 2024

This change is an editorial one with no behavior change.

  1. Remove "still alive" from the step 4.2.2 of the rendering loop algorithm.
  2. Add a check for "actively processing" 4.2.8.1 - "If node is marked or not actively processing, abort these steps."

@karlt Probably I am missing some corner cases, but this can be a starting point of the discussion. Does this makes sense to you?

@karlt
Copy link
Contributor Author

karlt commented May 7, 2024

Thanks for taking a look.
I think your proposed change makes sense and correctly skips processing.
It also correctly implements "Note that once a unhandled exception is thrown, the processor will output silence throughout its lifetime."

The thing I'm not sure is clearly specified is the silent output for nodes that are not actively processing. Would it be worthwhile explicitly "muting" nodes that are not actively processing, to clarify that their last buffer made available for reading is not used? i.e. instead of changing step 1 of "Visiting a node", replace step 5 "Add node to the beginning of ordered node list" with
5. If node is not actively processing, then mute node.
6. Else, add node to the beginning of ordered node list.

@karlt
Copy link
Contributor Author

karlt commented May 7, 2024

While checking the [[callable process]] behavior, I noticed that 4.4.11 "Set [[callable process]] to true" seems out of place. [[callable process]] is initialized true in the constructor, and we don't want it set to true again after "processorerror" has been triggered.
Looks like 4.4.11 can be removed?

@padenot
Copy link
Member

padenot commented May 14, 2024

AudioWG call 2024-05-14:

Let's fix this by doing what @hoch suggests, followed-up by what @karlt says. We can also do what @karlt says in his last comment in the same PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants