Use interpolation for setValueCurveAtTime

Use interpolation for setValueCurveAtTime

In issue https://github.com/WebAudio/web-audio-api/issues/131, the
computation of setValueCurveAtTime is clarified to use interpolation
(https://github.com/WebAudio/web-audio-api/issues/131#issuecomment-113935918).

This is not backward compatible.  Consider a curve with two points
over a long duration.  Previously, the output would be a step.  Now
it's line between the two points.

BUG=514452
TEST=audioparam-setValueCurveAtTime-interpolation.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200349

[Raymond Toy, 2015-08-06 21:50:32]

rtoy@chromium.org changed reviewers:
+ hongchan@chromium.org

[Raymond Toy, 2015-08-06 21:50:33]

PTAL.  I'm still waiting for the try jobs to finish so I can get the appropriate
threshold values for the tests, but that should be all that's needed.

[hongchan, 2015-08-07 17:32:52]

I think we need a better pattern when we use Audit for the same task with
different parameters.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
File LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html
(right):

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:7:
<script src="../resources/js-test.js"></script>
js-test.js should be the first.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:17: //
Chose a sample rate that is a multiple of 128, the rendering quantum size.  This
makes the
Chose -> Choose

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:45:
window.jsTestIsAsync = true;
This can be done right after description() method. We don't need to define a
task for this.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:52: var
testConfigs = [
This can be re-written:

var testConfig = [{
    // The main test
    key1: value1,
    key2: value2
  },{
    // Increase the curve length
    key1: value1
    ...


It is completely optional, but this is convention of multiline JSON key-value
pair.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:128:
function createTaskFunction (config) {
[Optional] Usually the named function does not have a space between the name and
the parentheses.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:129:
return function (done) {
The anonymous function can have a space between the function keyword and the
parentheses.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:147:
function runTest (config) {
No space between the name in the parentheses. The position of curly brace is
correct.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:157: //
TODO: Maybe allow more complicated curves?
TODO(rtoy)

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:185:
function checkResult(config)
Let's move the curly brace one line up.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:245: for
(k = 0; k < curveStartFrame; ++k) {
One-line loop block does not curly embraces around.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:252: for
(; k < curveEndFrame; ++k) {
ditto.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:257: for
(; k < fullGainFrame; ++k) {
ditto.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:262: for
(; k < testDurationFrames; ++k) {
ditto.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:269:
//runTest();
Why is this commented out? Let's remove if this is not intentional.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
File LayoutTests/webaudio/audioparam-setValueCurveAtTime.html (right):

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:1: <!DOCTYPE HTML
PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<!DOCTYPE html>

Let's make it all the header consistent. Please find this file as an example.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

1) There is no indentation in this file.
2) The inclusion order.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:7: <script
src="../resources/js-test.js"></script>
Technically it does not matter, but js-test.js should be the first one.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:12: <div
id="console"></div>
These two elements are programmatically injected by js-test.js Should be
removed.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:34: var freqHz = 440;
Frequency uses Hz as unit. So I don't think we need Hz in the variable name.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:40: function
automation(value, startTime, endTime)
Let's have the curly brace after the function definition. It's Google's JS
coding style. :)

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:42:
gainNode.gain.setValueCurveAtTime(curve, startTime, endTime - startTime);
Personally I am not a big fan of this pattern. |gainNode| must be defined in
|audioparam-testing.js| as a global, but this file does not have any indication
of |gainNode|.

I am aware that we already have this pattern in some places, but I would like to
avoid it as we can in the future.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:47: {
ditto. Let's move one line up the curly brace.

https://codereview.chromium.org/1277443005/diff/60001/LayoutTests/webaudio/re...
File LayoutTests/webaudio/resources/audio-testing.js (right):

https://codereview.chromium.org/1277443005/diff/60001/LayoutTests/webaudio/re...
LayoutTests/webaudio/resources/audio-testing.js:394: this.success = false;
This one does not have a underscore.

https://codereview.chromium.org/1277443005/diff/60001/LayoutTests/webaudio/re...
LayoutTests/webaudio/resources/audio-testing.js:409: this._success = true;
But here we have a underscore.

[Raymond Toy, 2015-08-07 18:52:27]

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
File LayoutTests/webaudio/audioparam-setValueCurveAtTime.html (right):

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:1: <!DOCTYPE HTML
PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
On 2015/08/07 17:32:51, hoch wrote:
> <!DOCTYPE html>
> 
> Let's make it all the header consistent. Please find this file as an example.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> 1) There is no indentation in this file.
> 2) The inclusion order.

Not relevant to this CL. Let's do that in a separate CL and fix all of them at
once.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:7: <script
src="../resources/js-test.js"></script>
On 2015/08/07 17:32:51, hoch wrote:
> Technically it does not matter, but js-test.js should be the first one.

Same comment as above.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:12: <div
id="console"></div>
On 2015/08/07 17:32:51, hoch wrote:
> These two elements are programmatically injected by js-test.js Should be
> removed.

Same comment as above.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:34: var freqHz = 440;
On 2015/08/07 17:32:51, hoch wrote:
> Frequency uses Hz as unit. So I don't think we need Hz in the variable name.

Same comment as above.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:40: function
automation(value, startTime, endTime)
On 2015/08/07 17:32:51, hoch wrote:
> Let's have the curly brace after the function definition. It's Google's JS
> coding style. :)

Same comment as above.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:42:
gainNode.gain.setValueCurveAtTime(curve, startTime, endTime - startTime);
On 2015/08/07 17:32:51, hoch wrote:
> Personally I am not a big fan of this pattern. |gainNode| must be defined in
> |audioparam-testing.js| as a global, but this file does not have any
indication
> of |gainNode|.
> 
> I am aware that we already have this pattern in some places, but I would like
to
> avoid it as we can in the future.

Same comment as above.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime.html:47: {
On 2015/08/07 17:32:51, hoch wrote:
> ditto. Let's move one line up the curly brace.

Same comment as above.

[Raymond Toy, 2015-08-07 18:58:33]

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
File LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html
(right):

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:7:
<script src="../resources/js-test.js"></script>
On 2015/08/07 17:32:51, hoch wrote:
> js-test.js should be the first.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:17: //
Chose a sample rate that is a multiple of 128, the rendering quantum size.  This
makes the
On 2015/08/07 17:32:51, hoch wrote:
> Chose -> Choose

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:45:
window.jsTestIsAsync = true;
On 2015/08/07 17:32:51, hoch wrote:
> This can be done right after description() method. We don't need to define a
> task for this.

Done.  And initialize task deleted.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:52: var
testConfigs = [
On 2015/08/07 17:32:51, hoch wrote:
> This can be re-written:
> 
> var testConfig = [{
>     // The main test
>     key1: value1,
>     key2: value2
>   },{
>     // Increase the curve length
>     key1: value1
>     ...
> 
> 
> It is completely optional, but this is convention of multiline JSON key-value
> pair.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:128:
function createTaskFunction (config) {
On 2015/08/07 17:32:51, hoch wrote:
> [Optional] Usually the named function does not have a space between the name
and
> the parentheses.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:129:
return function (done) {
On 2015/08/07 17:32:51, hoch wrote:
> The anonymous function can have a space between the function keyword and the
> parentheses.

Acknowledged.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:147:
function runTest (config) {
On 2015/08/07 17:32:51, hoch wrote:
> No space between the name in the parentheses. The position of curly brace is
> correct.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:157: //
TODO: Maybe allow more complicated curves?
On 2015/08/07 17:32:51, hoch wrote:
> TODO(rtoy)

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:185:
function checkResult(config)
On 2015/08/07 17:32:51, hoch wrote:
> Let's move the curly brace one line up.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:245: for
(k = 0; k < curveStartFrame; ++k) {
On 2015/08/07 17:32:51, hoch wrote:
> One-line loop block does not curly embraces around.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:252: for
(; k < curveEndFrame; ++k) {
On 2015/08/07 17:32:51, hoch wrote:
> ditto.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:257: for
(; k < fullGainFrame; ++k) {
On 2015/08/07 17:32:51, hoch wrote:
> ditto.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:262: for
(; k < testDurationFrames; ++k) {
On 2015/08/07 17:32:51, hoch wrote:
> ditto.

Done.

https://codereview.chromium.org/1277443005/diff/40001/LayoutTests/webaudio/au...
LayoutTests/webaudio/audioparam-setValueCurveAtTime-interpolation.html:269:
//runTest();
On 2015/08/07 17:32:51, hoch wrote:
> Why is this commented out? Let's remove if this is not intentional.

Done.

[Raymond Toy, 2015-08-07 23:21:31]

PTAL.  Patch 7 has the corrected thresholds and expected results now.

[hongchan, 2015-08-11 15:15:00]

lgtm

[Raymond Toy, 2015-08-11 15:26:58]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2015-08-11 15:27:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1277443005/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1277443005/120001

[commit-bot: I haz the power, 2015-08-11 20:13:15]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200349