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
Labels
category: editorial Editorial changes that do not affect interpretation. https://www.w3.org/policies/process/#class-2 Needs Edits Decision has been made, the issue can be fixed. https://speced.github.io/spec-maintenance/about/ Priority: Eventually "nice to have". https://speced.github.io/spec-maintenance/about/ size: S Small amount of work expected to resolve.

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 hoch added the Needs Discussion The issue needs more discussion before it can be fixed. label Apr 4, 2024
@hoch hoch added category: editorial Editorial changes that do not affect interpretation. https://www.w3.org/policies/process/#class-2 Priority: Eventually "nice to have". https://speced.github.io/spec-maintenance/about/ labels May 2, 2024
@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.

@padenot padenot added Needs Edits Decision has been made, the issue can be fixed. https://speced.github.io/spec-maintenance/about/ and removed Needs Discussion The issue needs more discussion before it can be fixed. labels May 14, 2024
@mjwilson-google mjwilson-google added the size: S Small amount of work expected to resolve. label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: editorial Editorial changes that do not affect interpretation. https://www.w3.org/policies/process/#class-2 Needs Edits Decision has been made, the issue can be fixed. https://speced.github.io/spec-maintenance/about/ Priority: Eventually "nice to have". https://speced.github.io/spec-maintenance/about/ size: S Small amount of work expected to resolve.
Projects
None yet
Development

No branches or pull requests

4 participants