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

Specify the DynamicsCompressor #1278

Merged
merged 15 commits into from
Aug 10, 2017
Merged

Conversation

padenot
Copy link
Member

@padenot padenot commented Jul 26, 2017

Here is it. It should be fairly complete (I addressed the comments we made during the f2f).

@padenot padenot self-assigned this Jul 26, 2017
@padenot
Copy link
Member Author

padenot commented Jul 26, 2017

@svgeesus
Copy link
Contributor

This seems fairly clearly explained, to me. (A couple of typos - treashold -> threshold, Enveloppe -> Envelope).
We should probably log an actual EnvelopeFollower node as a post-v1 feature request, as it is a useful building block.

Copy link
Member

@rtoy rtoy left a comment

Choose a reason for hiding this comment

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

Thanks for doing this hard work! Looks quite good, but I have a few grammatical issues and a few questions on the details.

index.html Outdated
<li>The third part is a linear function of equation \(f(x) =
\frac{1}{ratio} \cdot x \).
</li>
</ul>This curve MUST be continuously differentiable, and
Copy link
Member

Choose a reason for hiding this comment

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

According to Wikipedia continuously differentiable means that the derivative is also continuous. This precludes the hard-knee in the diagram. Should the hard knee in the diagram be removed?

Or maybe you really meant the curve just needs to be differentiable? This would allow the hard knee as a valid curve.

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 we need to write that it is allowed to have a function that is not continuously differentiable iff the knee is 0, which kind of makes sense, because the purpose of a hard knee is to have an hard edge.

Copy link
Contributor

Choose a reason for hiding this comment

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

The exception for knee == 0 seems odd to me. Based on my reading of the code, doesn't the function actually need to be differentiable (not "continuously differentiable") since the derivative of the function is required for some of the calculations done in the compressor?

In which case, we can still have a hard knee, as long as it isn't really a discontinuity in the derivative, but just a much more abrupt change. We could define some absolute minimum for the internal value of the knee parameter, which would keep the function differentiable in all cases.

Copy link
Member

Choose a reason for hiding this comment

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

If you just say continuous and piece-wise differentiable instead of continuously differentiable, everything should work out. The hard knee is continuous and piece-wise differentiable.

index.html Outdated
var compression = new EnveloppeFollower();

input.connect(delay).connect(gain).connect(output);
input.connect(compression).connect(gain.gain);</pre>
Copy link
Member

Choose a reason for hiding this comment

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

A picture instead of code would be nice here, but not necessary.

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 added a picture as well.

index.html Outdated
<code>input</code> and <code>output</code> respectively being the
input and output <a>AudioNode</a>, <code>context</code> the
<a>BaseAudioContext</a> for this <a>DynamicsCompressorNode</a>, and
new class, <code>EnveloppeFollower</code>, that instantiate a
Copy link
Member

Choose a reason for hiding this comment

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

Typo: "EnveloppeFollower", here and elsewhere.

index.html Outdated
linear gain value.
</p>
<ol>
<li>Let <var>input signal</var> be the the block of audio to be
Copy link
Member

Choose a reason for hiding this comment

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

"block" -> "`render quantum"?

index.html Outdated
parameters).
</li>
<li>Let <var>ratio</var> have the value of the <code>ratio</code>
<a>Audioparam</a>, sampled at the time of processing of this block
Copy link
Member

Choose a reason for hiding this comment

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

Typo: "Audioparam" -> "AudioParam". Then the link will show up correctly, I think.

index.html Outdated
respects the following characteristics:
</p>
<ol>
<li>This function is the identify up to the value of the linear
Copy link
Member

Choose a reason for hiding this comment

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

Typo: "identify" -> "identity"?

index.html Outdated
</p>
<ol>
<li>This function is the identify up to the value of the linear
<code>threshold</code> (i.e., \(f(sample) = sample\)).
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just use f(x) = x? Easier to read as a mathematical formula.

index.html Outdated
</li>
<li>From the <code>threshold</code> up to the <code>threshold +
knee</code>, User-Agents can choose the curve shape. The whole
function MUST be continuous (except as noted in the note below) and
Copy link
Member

Choose a reason for hiding this comment

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

"except as noted in the note below" -> "except as noted below"?

index.html Outdated
<div class="note">
As such, if the <code>knee</code> is 0, the
<a>DynamicsCompressorNode</a> is a hard-knee compressor, and
the function has a discontinuity at <code>threshold</code>.
Copy link
Member

Choose a reason for hiding this comment

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

This seems to contradict the requirement that the curve MUST be continuously differentiable.

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 the MUST applies to the whole sentence: it MUST (be continuously differentiable except when the knee is 0, and then that is the case there is a discontinuity at the knee).

I've changed the prose a bit.

index.html Outdated
</div>
</li>
<li>This function returned is linear, based on the ratio, after the
<code>threshold</code> and the soft knee (i.e., \(f(sample) =
Copy link
Member

Choose a reason for hiding this comment

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

Copy the formula from earlier so that it has the same form (1/ratio * x).

@joeberkovitz
Copy link
Contributor

index.html Outdated
@@ -9085,7 +9085,9 @@ <h3 id="example-2">
<p>
Let <var>node</var> be a new <a>DynamicsCompressorNode</a>
object. <a href="#audionode-constructor-init">Initialize</a>
<var>node</var>, and return <var>node</var>.
<var>node</var>. Let [[\reduction]] be a private slote on this
Copy link
Contributor

Choose a reason for hiding this comment

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

slote -> slot

index.html Outdated
@@ -9144,7 +9146,8 @@ <h3 id="example-2">
A read-only decibel value for metering purposes, representing the
current amount of gain reduction that the compressor is applying
to the signal. If fed no signal the value will be 0 (no gain
reduction).
reduction). When this attribute is read, return the value of the
private slot [[\reduction]].
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be simpler to force [[reduction]] to assume a zero value when no signal is present, as part of the compression algorithm?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it not what I've done?

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, what you've done is to say [[reduction]] is ignored when the signal is present, and zero is returned instead.

index.html Outdated
</li>
<li>The compression curve is has three parts:
<ul>
<li>The first part is the identify: \(f(x) = x\).
Copy link
Contributor

Choose a reason for hiding this comment

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

identify -> identity

index.html Outdated
<code>reduction</code> property on the
<a>DynamicsCompressorNode</a>
</li>
<li>The compression curve is has three parts:
Copy link
Contributor

Choose a reason for hiding this comment

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

delete "is"

index.html Outdated
<li>The third part is a linear function of equation \(f(x) =
\frac{1}{ratio} \cdot x \).
</li>
</ul>This curve MUST be continuously differentiable, and
Copy link
Contributor

Choose a reason for hiding this comment

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

The exception for knee == 0 seems odd to me. Based on my reading of the code, doesn't the function actually need to be differentiable (not "continuously differentiable") since the derivative of the function is required for some of the calculations done in the compressor?

In which case, we can still have a hard knee, as long as it isn't really a discontinuity in the derivative, but just a much more abrupt change. We could define some absolute minimum for the internal value of the knee parameter, which would keep the function differentiable in all cases.

index.html Outdated
<dfn id="#envelope-rate">Computing the envelope rate</dfn> is done
by applying a function to the ratio of the <var>compressor
gain</var> and the <var>detector average</var>. User-agents are
allowed to choose the shape of the attack and release curve.
Copy link
Contributor

Choose a reason for hiding this comment

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

The sudden introduction of "curves" here is confusing. I think we need a sentence or two explaining that the envelope rate is a scale factor by which some gain quantity is multiplied, in each sample frame. These successive multiplications, in turn, generate the envelope curve.

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 now well defined in the algorithm above.

index.html Outdated
</ul>
<div class="note">
It is allowed, for example, to have a compressor that performs an
<em>adaptative release</em>, that is, releasing faster the harder
Copy link
Contributor

Choose a reason for hiding this comment

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

adaptative -> adaptive

index.html Outdated
function in the range \([0, 1]\).
</li>
<li>The release curve MUST be a continuous, monotonically
decreasing function that is always greater to 1.
Copy link
Contributor

Choose a reason for hiding this comment

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

greater than 1

index.html Outdated
</div>
</li>
<li>The attack curve MUST be a continuous, monotonically increasing
function in the range \([0, 1]\).
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the above eplanation that I am suggesting, these requirements follow directly from the constraints on the value of the envelope rate.

index.html Outdated
However, they MUST respect the following constraints:
</p>
<ul>
<li>The input to the curve MUST be the ratio of the <var>compressor
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than the "input to the curve", this is more clearly described as the envelope rate itself (which in turn determines the curve). So... "the envelope rate MUST be calculated from the ratio of..."

index.html Outdated
@@ -9085,7 +9085,9 @@ <h3 id="example-2">
<p>
Let <var>node</var> be a new <a>DynamicsCompressorNode</a>
object. <a href="#audionode-constructor-init">Initialize</a>
<var>node</var>, and return <var>node</var>.
<var>node</var>. Let [[\reduction]] be a private slote on this
Copy link
Member

Choose a reason for hiding this comment

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

There's already an attribute named reduction. Naming the private slot [[reduction]] is confusing.

index.html Outdated
<li>The third part is a linear function of equation \(f(x) =
\frac{1}{ratio} \cdot x \).
</li>
</ul>This curve MUST be continuously differentiable, and
Copy link
Member

Choose a reason for hiding this comment

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

If you just say continuous and piece-wise differentiable instead of continuously differentiable, everything should work out. The hard knee is continuous and piece-wise differentiable.

</p>
<figure>
<img alt="Graphical representation of a compression curve" src=
"images/compression-curve.svg" style="width: 50%">
Copy link
Member

Choose a reason for hiding this comment

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

In the diagram, you draw ratio as if it's the angle of the line after the knee. It's not; it's the reciprocal of the slope. Not exactly sure how to show that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is somewhat of a language abuse. In music software, it's often represented like that, but we need to be more precise here.

Copy link
Member

Choose a reason for hiding this comment

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

I'll leave it up to you to come up with something. (Or leave the diagram as is and add some text.)

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've tried something else, have a look.

<img src="images/dynamicscompressor-internal-graph.svg" alt=
"Schema of
the internal graph used by the DynamicCompressorNode">
<figcaption>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the diagram! Looks really nice. One nit: Kind of hard to tell that the envelope follower output connects to the gain attribute of the gain node. Not exactly sure how to best illustrate this, though.

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 tried to put a little "Gain" label, maybe we can do something better.

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok, if you leave it as is. The code above clearly describes what's happening.

index.html Outdated
gain</var> and the <var>detector average</var>.
<div class="note">
When attacking, this number less than or equal to 1, when
releasing, this number is strictly greater to 1.
Copy link
Member

Choose a reason for hiding this comment

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

"greater to" -> "greater than"

index.html Outdated
<p>
Applying a <dfn>compression curve</dfn> to a value means computing
the value of this sample when passed to a function, and returning
the computed value. This function MUST respects the following
Copy link
Member

Choose a reason for hiding this comment

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

"respects" -> "respect"

index.html Outdated
knee</code>, User-Agents can choose the curve shape. The whole
function MUST be monotonically increasing and continuous except
when the <code>knee</code> parameter is 0, in which case a
discontinuity happens at the treshold.
Copy link
Member

Choose a reason for hiding this comment

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

In the diagram with a hard knee, the curve is still continuous, but not differentiable; there is no discontinuity. I think it's enough here to say that the curve in monotonic increasing and continuous. No exception needs to be made when the knee is 0.

index.html Outdated
<ol>
<li>If <var>v</var> is equal to zero, return -1000.
</li>
<li>Else, return \( 20 \times \log_{10}{v} \)
Copy link
Member

Choose a reason for hiding this comment

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

\times not really needed, but ok. (Might need \, or something if you do remove the times, so that there's a tiny space between 20 and log.)

index.html Outdated
<ul>
<li>The first part is the identify: \(f(x) = x\).
<li>The first part is the identity: \(f(x) = x\).
</li>
<li>The second part is the soft-knee portion, which MUST be a
monotonically increasing function.
</li>
<li>The third part is a linear function of equation \(f(x) =
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "linear function of equation" -> "the linear function:"

The ":" to make to match what you say in line 9253 above.

index.html Outdated
Now, the actual algorithm to determine the reduction gain, in
linear gain value.
Now, the actual algorithm to determine <var>reduction gain</var>, in
linear gain value, for each sample of input.
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a complete sentence. Maybe just replace the final period with a colon?

index.html Outdated
<a href="compression-curve">compression curve</a> to the
absolute value of <var>input</var> .
<li>If the absolute value of <var>input</var> is less than
0.0001, let <var>attenuation</var> be 1.0. Else, let
Copy link
Member

Choose a reason for hiding this comment

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

Is this what Chrome and Firefox do today?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

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 it's good enough to avoid the division by zero.

index.html Outdated
@@ -9411,34 +9421,44 @@ <h3 id="example-2">
<dfn id="#envelope-rate">Computing the envelope rate</dfn> is done
by applying a function to the ratio of the <var>compressor
gain</var> and the <var>detector average</var>. User-agents are
allowed to choose the shape of the attack and release curve.
allowed to choose the shape the enveloppe function.
Copy link
Member

Choose a reason for hiding this comment

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

"enveloppe" -> "envelope", here and elsewhere. (Is this the French spelling?)

Copy link

Choose a reason for hiding this comment

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

Exactly 🇫🇷

index.html Outdated
@@ -9085,7 +9085,9 @@ <h3 id="example-2">
<p>
Let <var>node</var> be a new <a>DynamicsCompressorNode</a>
object. <a href="#audionode-constructor-init">Initialize</a>
<var>node</var>, and return <var>node</var>.
<var>node</var>. Let [[\reduction]] be a private slot on this
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks as though you changed the name of the slot everywhere except here.

index.html Outdated
<var>input</var>.
</li>
<li>Let <var>decibels per frame</var> be the result of dividing
<var>attenuationDB</var> by <var>releaseRate</var>.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

attenuationDB does not appear to be defined anywhere

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you meant to divide attenuation by the quantity known in the C++ code as sat_release_frames, which is the number of frames in a release time interval. Note that releaseRate is not computed until the following step (and it's computed from the work done in this step!)

index.html Outdated
</li>
<li>Let <var>decibels per frame</var> be the result of dividing
<var>attenuationDB</var> by <var>releaseRate</var>.</li>
<li>Set <var>relaseRate</var> be the result of
Copy link
Contributor

Choose a reason for hiding this comment

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

relaseRate -> releaseRate

index.html Outdated
<a href="#db-to-linear">converting <var>decibels per frames
</var>to linear gain</a>, and substract one.</li>
<li>Set <var>change-rate</var> to the difference between <var>
attenuation</var> and <var>detector average</var>.
Copy link
Contributor

Choose a reason for hiding this comment

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

This where releaseRate is supposed to be multiplied by the difference: the new change rate is (attenuation - detector average) * release rate.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is, unless we're attacking.

index.html Outdated
<li>If attacking, set <var>attack</var> to <var>detector
average</var> minus <var>compressor gain</var>. Set
<var>compressor gain</var> to the multiplication of
<var>attack</var> by <var>envelope rate</var>. Clamp
Copy link
Contributor

Choose a reason for hiding this comment

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

The sentence "Set compressor gain to the multiplication of..." appears to be left over from before. It should be replaced by the new language in line 9382 about multiplying attack by envelope rate and adding attack to compressor gain.

Copy link
Member

@rtoy rtoy left a comment

Choose a reason for hiding this comment

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

Just a bunch of nits and typos and minor clarifications.

index.html Outdated
<li>Configurable attack speed, release speed, threshold, knee
hardness and ratio.
</li>
<li>Side-chaining is not supported
Copy link
Member

Choose a reason for hiding this comment

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

Nit. End sentence with a period, like you do elsewhere.

index.html Outdated
</li>
<li>The gain reduction is reported <em>via</em> the
<code>reduction</code> property on the
<a>DynamicsCompressorNode</a>
Copy link
Member

Choose a reason for hiding this comment

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

Nit. End sentence with a period.

</p>
<figure>
<img alt="Graphical representation of a compression curve" src=
"images/compression-curve.svg" style="width: 50%">
Copy link
Member

Choose a reason for hiding this comment

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

I'll leave it up to you to come up with something. (Or leave the diagram as is and add some text.)

<img src="images/dynamicscompressor-internal-graph.svg" alt=
"Schema of
the internal graph used by the DynamicCompressorNode">
<figcaption>
Copy link
Member

Choose a reason for hiding this comment

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

I'm ok, if you leave it as is. The code above clearly describes what's happening.

index.html Outdated
<a>DynamicsCompressorNode</a> is <a href=
"#associated">associated</a> with.
</li>
<li>For each samples <var>input</var> of <var>input signal</var>,
Copy link
Member

Choose a reason for hiding this comment

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

Nit: "each samples" -> "each sample"

index.html Outdated
execute the following steps:
<ol>
<li>Let <var>envelope rate</var> be the result of <a href=
"#envelope-rate">computing the envelope rate</a>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: End with a period, like you do elsewhere.

index.html Outdated
0.0001, let <var>attenuation</var> be 1.0. Else, let
<var>attenuation</var> be the value of applying the <a
href="#compression-curve">compression curve</a> to the absolute
value of <var>input</var>, divided by the absolute value of
Copy link
Member

Choose a reason for hiding this comment

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

This phrasing is a bit confusing because I interpreted this as dividing the absolute value of the input by the absolute value of the input. A closer reading shows that you really mean compression curve value divided by input absolute value. Maybe rephrase or use two sentences?

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've used two sentences and an intermediate variable so make this extra-clear.

index.html Outdated
<dfn id="#envelope-rate">Computing the envelope rate</dfn> is done
by applying a function to the ratio of the <var>compressor
gain</var> and the <var>detector average</var>. User-agents are
allowed to choose the shape the enveloppe function.
Copy link
Member

Choose a reason for hiding this comment

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

"envelope"

index.html Outdated
However, they MUST respect the following constraints:
</p>
<ul>
<li>The enveloppe rate MUST be the calculated from the ratio of the
Copy link
Member

Choose a reason for hiding this comment

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

"envelope"

Also are you referring to the "envelope rate" variable?

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've clarified this.

index.html Outdated
<a>DynamicsCompressorNode</a> is called a hard-knee compressor.
</div>
</li>
<li>This function returned is linear, based on the ratio, after the
Copy link
Member

Choose a reason for hiding this comment

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

"This function returned is linear" -> "This function is linear"

@padenot
Copy link
Member Author

padenot commented Aug 10, 2017

The last few commits refactor the algorithm so that things are in the right order, change the diagram, address the other comments, and tidy.

@padenot padenot merged commit 0c80e57 into WebAudio:gh-pages Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants