Skip to content

Revise instantiation process of AudioWorkletNode and AudioWorkletProcessor #1265

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

Merged
merged 8 commits into from
Aug 24, 2017
Merged

Revise instantiation process of AudioWorkletNode and AudioWorkletProcessor #1265

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8bf921b
Revise instantiation process of AWN/AWP
hoch Jul 6, 2017
2407420
Fix `processorConstructor` link
hoch Jul 6, 2017
5ffa1a4
Apply tidy
hoch Jul 6, 2017
bb6ee07
Addressing feedback from @joeberkovitz
hoch Jul 12, 2017
f87704c
Fix the event type to `statechange`
hoch Jul 20, 2017
98a9df0
Reordering
hoch Jul 20, 2017
690b9a9
Addresing feedback from @rtoy
hoch Jul 24, 2017
71c1f3a
Addressing feedback from padenot@
hoch Aug 23, 2017
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
206 changes: 141 additions & 65 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6035,6 +6035,19 @@ <h2 id="AudioWorklet-concepts">
"#widl-AudioWorkletGlobalScope-registerProcessor-void-DOMString-name-VoidFunction-processorCtor">
registerProcessor</a> method is called.
</dd>
<dt>
<dfn>node name to parameter descriptor map</dfn>
</dt>
<dd>
Belongs to <a>BaseAudioContext</a>. This map contains an
identical set of string keys from <a>node name to processor
definition map</a> that are associated with the matching
<a>parameterDescriptors</a> values. This internal storage is
populated when a promise from <a href=
"https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule">addModule()</a>
on <a href="#widl-Window-audioWorklet">audioWorklet</a> gets
resolved.
</dd>
</dl>
<pre class="example" title=
"Registering an AudioWorkletProcessor class definition">
Expand Down Expand Up @@ -6453,8 +6466,8 @@ <h2 id="defining-a-valid-audioworkletprocessor">
<a>AudioWorkletProcessor</a>. The subclass MUST define a method
named <code>process()</code> that implements the audio processing
algorithm and have a valid static property named
<code>parameterDescriptors</code> which is an iterable of
<a>AudioParamDescriptor</a> that is looked up by the
<dfn><code>parameterDescriptors</code></dfn> which is an iterable
of <a>AudioParamDescriptor</a> that is looked up by the
<a>AudioWorkletProcessor</a> constructor to create instances of
<a>AudioParam</a> in the <code>parameters</code> maplike storage
in the node. The step 5 and 6 of <a href=
Expand Down Expand Up @@ -6693,92 +6706,147 @@ <h2 id="instantiation-of-AudioWorkletNode-and-AudioWorkletProcessor">
<a>AudioWorkletNode</a> instance is destroyed.
</p>
<p>
When <a>AudioWorkletNode</a>(<var>context</var>, <var>name</var>,
<var>options</var>) constructor is called, the user agent MUST run
the following steps <a data-lt="atomic">atomically</a>.
Note that the instantiation of these two objects spans the control
thread and the rendering thread.
</p>
<p>
When <a>AudioWorkletNode</a>(<var>context</var>,
<var>nodeName</var>, <var>options</var>) constructor is invoked,
the user agent MUST perform the following steps on the control
thread, where the constructor was called.
</p>
<ol>
<li>Let <var>this</var> be the instance being created by
constructor of <a>AudioWorkletNode</a> or its subclass.
</li>
<li>If <var>name</var> does not exists as a key in the
<a>AudioWorkletGlobalScope</a>’s <a>node name to processor
definition map</a>, throw a <code>NotSupportedError</code>
exception and abort these steps.
<li>If <var>nodeName</var> does not exists as a key in the
<a>BaseAudioContext</a>’s <a>node name to parameter descriptor
map</a>, throw a <code>NotSupportedError</code> exception and abort
Copy link
Member

Choose a reason for hiding this comment

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

Need to wrap the "throw" part in a <span synchronous> tag.

Copy link

Choose a reason for hiding this comment

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

What is <span synchronous>?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that it's a way of explicitly saying "this operation is synchronous". However, I am not sure why that is necessary here. This section of algorithm happens in the main thread, so everything is sequential and synchronous.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, and it's actually <span class="synchronous">. And not really sure it's needed, but most of the spec has this wherever we throw an error.

Copy link

Choose a reason for hiding this comment

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

That seems like a really weird convention. Where did it come from? In general I'd recommend following the conventions established by https://infra.spec.whatwg.org/ and HTML.

Copy link

Choose a reason for hiding this comment

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

In any case it seems moot if we are in the main thread and not employing any contexts belonging to other threads. In the past, we had been looking at data belonging to the AWP.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like we digressed. Is there anything to be fixed in this PR? I think the current spec text here has a clear distinction between the main thread and the worklet thread. So adding a synchronous icon seems redundant to me.

these steps.
</li>
<li>Let <var>processorConstructor</var> be the result of looking up
<var>name</var> on the <a>AudioWorkletGlobalScope</a>’s <a>node
name to processor definition map</a>.
<li>Let <var>node</var> be a new <a>AudioWorkletNode</a> object.
</li>
<li>Let <var>messageChannel</var> be a new <a href=
"https://html.spec.whatwg.org/multipage/#message-channels">MessageChannel</a>.
</li>
<li>Let <var>nodePort</var> be the value of
<var>messageChannel</var>'s <code>port1</code> attribute.
</li>
<li>Let <var>processorInstance</var> be the result of <a href=
"http://www.ecma-international.org/ecma-262/6.0/#sec-construct">Construct</a>(<var>processorConstructor</var>,
<var>« options »</var>). The argument list <var>options</var> is
serialized using the <a href=
"http://w3c.github.io/html/infrastructure.html#safe-passing-of-structured-data">
structured clone</a> algorithm.
<li>Let <var>processorPortOnThisSide</var> be the value of
<var>messageChannel</var>'s <code>port2</code> attribute.
</li>
<li>Let <var>parameterDescriptors</var> be the result of <a href=
"https://tc39.github.io/ecma262/#sec-get-o-p">Get</a>(O=<var>processorConstructor</var>,
P="parameterDescriptors"). If <var>parameterDescriptors</var> is
<code>undefined</code>, skip to the step 8.
<li>Let <var>processorPortSerialization</var> be <a href=
"https://html.spec.whatwg.org/multipage/infrastructure.html#structuredserializewithtransfer">
StructuredSerializeWithTransfer</a>(<var>processorPortOnThisSide</var>,
« <var>processorPortOnThisSide</var> »).
</li>
<li>Let <var>parameterMap</var> be the result of <a href=
"https://tc39.github.io/ecma262/#sec-map-iterable">Map</a>().
<li>Set <var>node</var>'s <a href=
"#widl-AudioWorkletNode-port">port</a> to <var>nodePort</var>.
</li>
<li>For each <var>descriptor</var> in
<var>parameterDescriptors</var> perform the following substeps:
<li>Let <var>parameterDescriptors</var> be the result of retrieval
of <var>nodeName</var> from <a>node name to parameter descriptor
map</a>:
<ol>
<li>Let <var>audioParam</var> be a new <a>AudioParam</a>
instance with:
<ul>
<li>
<var>paramName</var> being the result <a href=
"https://tc39.github.io/ecma262/#sec-get-o-p">Get</a>(O=<var>descriptor</var>,
P="name").
<li>Let <var>audioParamMap</var> be a new <a>AudioParamMap</a>
object.
</li>
<li>For each <var>descriptor</var> of
<var>parameterDescriptors</var>:
<ol>
<li>Let <var>paramName</var> be the value of
<var>descriptor</var>'s <a href=
"#widl-AudioParamDescriptor-name">name</a>.
</li>
<li>
<var>defaultValue</var> being the result <a href=
"https://tc39.github.io/ecma262/#sec-get-o-p">Get</a>(O=<var>descriptor</var>,
P="defaultValue").
<li>Let <var>audioParam</var> be a new <a>AudioParam</a>
instance.
</li>
<li>
<var>minValue</var> being the result <a href=
"https://tc39.github.io/ecma262/#sec-get-o-p">Get</a>(O=<var>descriptor</var>,
P="minValue").
<li>Append (<var>paramName</var>, <var>audioParam</var>) to
<var>audioParamMap</var>'s entries.
</li>
</ol>
</li>
<li>For each <var>paramNameInOption</var> → <var>value</var> of
<var>options</var>:
<ol>
<li>If there exists an entry with name member equal to
<var>paramNameInOption</var> inside
<var>audioParamMap</var>, set that <a>AudioParam</a>'s
value to <var>value</var>.
</li>
<li>
<var>maxValue</var> being the result <a href=
"https://tc39.github.io/ecma262/#sec-get-o-p">Get</a>(O=<var>descriptor</var>,
P="maxValue").
<var>paramNameInOption</var> will be ignored when:
<ul>
<li>
<var>audioParamMap</var> does not have any entry with
the same name member.
</li>
<li>
<var>value</var> is not a type of <code>Number</code>
or out of the range specified in
<a>AudioParamDescriptor</a>.
</li>
</ul>
</li>
</ul>
</li>
<li>If <var>options</var> contains a dictionary member with
name <var>paramName</var> whose value is of type
<code>Number</code>, perform <a href=
"https://tc39.github.io/ecma262/#sec-set-o-p-v-throw">Set</a>(<var>audioParam</var>,
"value", <a href="https://tc39.github.io/ecma262/#sec-get-o-p">
Get</a>(O=<var>options</var>, P=<var>paramName</var>),
false).
</ol>
</li>
<li>Perform <a href=
"https://tc39.github.io/ecma262/#https://tc39.github.io/ecma262/#sec-set-o-p-v-throw">
Set</a>(O=<var>parameterMap</var>, P=<var>paramName</var>,
V=<var>audioParam</var>).
<li>Set <var>node</var>'s <a href=
"#widl-AudioWorkletNode-parameters">parameters</a> to
<var>audioParamMap</var>.
</li>
</ol>
</li>
<li>Perform <a href=
"https://tc39.github.io/ecma262/#sec-createmethodproperty">CreateDataProperty</a>(O=<var>this</var>,
P="parameters", V=<var>parameterMap</var>).
<li>
<a href="#queuing">Queue a control message</a> to create an
<a>AudioWorkletProcessor</a>, given <var>nodeName</var>,
<var>processorPortSerialization</var>, and <var>node</var>.
</li>
<li>Return <var>node</var>.
</li>
</ol>
<p>
In order to process a control message for the construction of an
<a>AudioWorkletProcessor</a>, given a string <var>nodeName</var>, a
serialization record <var>processorPortSerialization</var>, and an
<a>AudioWorkletNode</a> <var>node</var>, perform the following
steps on the <a>rendering thread</a>. If any of these steps throws
Copy link
Member

Choose a reason for hiding this comment

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

Can the rendering thread throw an exception? And if it can, where does it go? I think what you're saying is if something bad happens on the rendering thread, the rendering thread stops processing and queues a task to the control thread. We're not throwing an exception in the JS sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

@bfgeek What should happen when JS code throws in WorkletGlobalScope?

Copy link
Member Author

Choose a reason for hiding this comment

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

According to @bfgeek, WorkletGlobalScope does not have any error handling. So any exception that is being thrown here will be silently ignored. I think this issue should be handled by the worklet infra.

Copy link

Choose a reason for hiding this comment

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

Don't audio worklets introduce a subclass of that class? Couldn't you add something there? Since you have message passing it seems good to be able to catch uncaught exceptions as well. (Maybe best discussed as a new issue?)

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO adding its own error handling whereas the parent class does not one is likely to cause the layering issue. What if the error handling will be added into WorkletGlobalScope in the future and it is different from what we do in AudioWorkletGlobalScope?

By the way, I filed an issue to Worklet folks.

an exception (either explicitly or implicitly), abort the rest of
steps and queue a task on the <a>control thread</a> to fire
<a href="#widl-AudioWorkletNode-onprocessorstatechange"><code>processorstatechange</code></a>
event to <var>node</var> with <code>error</code> state.
</p>
<ol>
<li>Let <var>processorPort</var> be <a href=
"https://html.spec.whatwg.org/multipage/infrastructure.html#structureddeserializewithtransfer">
StructuredDeserializeWithTransfer</a>(<var>processorPortSerialization</var>,
the current Realm).
</li>
<li>Let <var>processorConstructor</var> be the result of looking up
<var>nodeName</var> on the <a>AudioWorkletGlobalScope</a>'s <a>node
name to processor definition map</a>.
</li>
<li>If <var>processorConstructor</var> is <code>undefined</code>,
throw a <code>NotSupportedError</code> DOMException.
Copy link
Member

Choose a reason for hiding this comment

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

NotSupportedError or InvalidStateError?

Copy link
Member Author

Choose a reason for hiding this comment

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

PaintWorklet throws TypeError. According to ES spec, it "Indicates the actual type of an operand is different than the expected type."

Makes sense to me. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

In some Chrome CLs, haraken@ said that TypeError should be handled at the JS binding layer. If the binding handles this (which is seems it could?), then TypeError is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is actually a part of Worklet infra, so we should follow what it does.

</li>
<li>Let <var>processor</var> be the result of
Construct(<var>processorConstructor</var>).
</li>
<li>If <var>processor</var> does not implement the
<a>AudioWorkletProcessor</a> interface, throw an
<code>"InvalidStateError"</code> DOMException.
</li>
<li>Set <var>processor</var>'s <a href=
"#widl-AudioWorkletProcessor-port">port</a> to
<var>processorPort</var>.
</li>
<li>Set <a>node reference</a> in <var>processorInstance</var> to
<var>this</var>.
<li>Set <var>processor</var>'s <a>node reference</a> to
<var>node</var>.
</li>
<li>Set <a>processor reference</a> in <var>this</var> to
<var>processorInstance</var>.
<li>Set <var>node</var>'s <a>processor reference</a> to
<var>processor</var>.
</li>
<li>Return <var>this</var> as a result of the construction.
<li>Queue a task the <a>control thread</a> to set the associated
<a>AudioWorkletNode</a>'s state to <code>running</code>, then fire
a <code>statechange</code> event.
</li>
</ol>
</section>
Expand Down Expand Up @@ -6927,6 +6995,14 @@ <h3>

});
</pre>
<div class="note">
<p>
In the definition of <a>AudioWorkletProcessor</a> class, an
<code>InvalidStateError</code> will be thrown if the
author-supplied constructor uses JavaScript's return-override
feature, or does not properly call <code>super()</code>.
</p>
</div>
</section>
<section>
<h3>
Expand Down