Implement Automations for PannerNode and AutioListener

Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of
the position and orientation of the Panner node are added
as AudioParams.  Likewise, the components of the position,
up, and forward vectors for the AudioListener are exposed
as AudioParams.

Use counters are added for setPosition and setOrientation
to keep track of these since the spec has deprecated these
methods.  Also changed the use counter name for
createPanner() so we can keep track of the usage of this
new Panner node with automations.

The automations are a-rate, and the a-rate update is
implemented only for the "equalpower" panner.  The "HRTF"
panning model is still k-rate.  (We expect the inherent
smoothing in HRTF is sufficient and the huge increase in
complexity is not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent: https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjDBgAJ

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html

Committed: https://crrev.com/2f2105fe40e9097470fe79d9d7b6fe2d88e73c38
Cr-Commit-Position: refs/heads/master@{#393983}

[Raymond Toy, 2016-04-20 21:05:02]

Description was changed from

==========
Implement Automations for PannerNode

BUG=590000
TEST=
==========

to

==========
Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of the position and
orientation of the Panner node are added as AudioParams.  Likewise, the
components of the position, up, and forward vectors for the AudioListener are
exposed as AudioParams.

Use counters are added for setPosition and setOrientation to keep track of these
since the spec has deprecated these methods.  Also changed the use counter name
for createPanner() so we can keep track of the usage of this new Panner node
with automations.

The automations are a-rate, and the a-rate update is implemented only for the
"equalpower" panner.  The "HRTF" panning model is still k-rate.  (We expect the
inherent smoothing in HRTF is sufficient and the huge increase in complexity is
not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjD...

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html
==========

[Raymond Toy, 2016-04-20 21:06:21]

Description was changed from

==========
Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of the position and
orientation of the Panner node are added as AudioParams.  Likewise, the
components of the position, up, and forward vectors for the AudioListener are
exposed as AudioParams.

Use counters are added for setPosition and setOrientation to keep track of these
since the spec has deprecated these methods.  Also changed the use counter name
for createPanner() so we can keep track of the usage of this new Panner node
with automations.

The automations are a-rate, and the a-rate update is implemented only for the
"equalpower" panner.  The "HRTF" panning model is still k-rate.  (We expect the
inherent smoothing in HRTF is sufficient and the huge increase in complexity is
not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjD...

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html
==========

to

==========
Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of
the position and orientation of the Panner node are added
as AudioParams.  Likewise, the components of the position,
up, and forward vectors for the AudioListener are exposed
as AudioParams.

Use counters are added for setPosition and setOrientation
to keep track of these since the spec has deprecated these
methods.  Also changed the use counter name for
createPanner() so we can keep track of the usage of this
new Panner node with automations.

The automations are a-rate, and the a-rate update is
implemented only for the "equalpower" panner.  The "HRTF"
panning model is still k-rate.  (We expect the inherent
smoothing in HRTF is sufficient and the huge increase in
complexity is not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjD...

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html
==========

[Raymond Toy, 2016-04-25 16:18:41]

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

[Raymond Toy, 2016-04-25 16:18:42]

PTAL.

[Raymond Toy, 2016-04-26 20:36:06]

The CQ bit was checked by rtoy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-26 20:36:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1820403002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1820403002/180001

[commit-bot: I haz the power, 2016-04-26 20:45:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-26 20:45:26]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hongchan, 2016-05-03 18:15:01]

Due to the size of CL, I will break the review into several pieces - this is the
first batch.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html
(right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:29: } =
nodes;
How about:

var nodes = { context, source, panner } = createGraph();

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:42: } =
nodes;
ditto.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:55: } =
nodes;
ditto.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:208:
//"finish"
I can accept this as a pattern. However, is this intentional?

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:222: return
{ context: context,
Let's add a line break after {.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:231: } =
options;
ditto here - let's put them in a single line.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:235: } =
nodes;
ditto.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/resources/panner-model-testing.js
(right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/resources/panner-model-testing.js:54:
//panner[k].setPosition(position[k].x, 0, position[k].z);
Are we keeping this commented line as well?

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:119:
RELEASE_ASSERT(framesToProcess <= m_positionXValues.size());
Note: If this is true for any instance of AudioParamHandler, perhaps we should
have this check inside of calculateSampleAccurateValues() method.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/audio/EqualPowerPanner.h (right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/EqualPowerPanner.h:41: void reset()
override { }
If this doesn't do anything, why it's here?

[Raymond Toy, 2016-05-03 20:32:21]

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html
(right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:29: } =
nodes;
On 2016/05/03 18:15:01, hoch wrote:
> How about:
> 
> var nodes = { context, source, panner } = createGraph();

No problems with this, but js-beautify wants to indent it wrapped as shown.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:208:
//"finish"
On 2016/05/03 18:15:01, hoch wrote:
> I can accept this as a pattern. However, is this intentional?

No. A bit of debugging left over.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/resources/panner-model-testing.js
(right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/resources/panner-model-testing.js:54:
//panner[k].setPosition(position[k].x, 0, position[k].z);
On 2016/05/03 18:15:01, hoch wrote:
> Are we keeping this commented line as well?

Should be deleted.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:119:
RELEASE_ASSERT(framesToProcess <= m_positionXValues.size());
On 2016/05/03 18:15:01, hoch wrote:
> Note: If this is true for any instance of AudioParamHandler, perhaps we should
> have this check inside of calculateSampleAccurateValues() method.

That would require refactoring calculateSampleAccurateValues which only takes a
float*, not Vector (or other object with a size).

We can do that, but I'd rather not do that here.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/audio/EqualPowerPanner.h (right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/EqualPowerPanner.h:41: void reset()
override { }
On 2016/05/03 18:15:01, hoch wrote:
> If this doesn't do anything, why it's here?

reset() is abstract virtual function defined in Panner.h.

[hongchan, 2016-05-03 20:38:58]

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html
(right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:29: } =
nodes;
On 2016/05/03 20:32:21, Raymond Toy wrote:
> On 2016/05/03 18:15:01, hoch wrote:
> > How about:
> > 
> > var nodes = { context, source, panner } = createGraph();
> 
> No problems with this, but js-beautify wants to indent it wrapped as shown.

That's perhaps because js-beautify doesn't know about the destructuring pattern.
The curly brace does not represent a JS object here.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/audio/EqualPowerPanner.h (right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/EqualPowerPanner.h:41: void reset()
override { }
On 2016/05/03 20:32:21, Raymond Toy wrote:
> On 2016/05/03 18:15:01, hoch wrote:
> > If this doesn't do anything, why it's here?
> 
> reset() is abstract virtual function defined in Panner.h.

Got it. Confirmed this is a 'thing' in our chromium code base.

[Raymond Toy, 2016-05-03 20:46:48]

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html
(right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:29: } =
nodes;
On 2016/05/03 20:38:58, hoch wrote:
> On 2016/05/03 20:32:21, Raymond Toy wrote:
> > On 2016/05/03 18:15:01, hoch wrote:
> > > How about:
> > > 
> > > var nodes = { context, source, panner } = createGraph();
> > 
> > No problems with this, but js-beautify wants to indent it wrapped as shown.
> 
> That's perhaps because js-beautify doesn't know about the destructuring
pattern.
> The curly brace does not represent a JS object here.

Yes, js-beautify probably doesn't know.  Isn't multiple assignment like this bad
style in javascript?  C/C++ styles don't usually allow it.

[hongchan, 2016-05-03 20:54:48]

On 2016/05/03 20:46:48, Raymond Toy wrote:
>
https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
> File third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html
> (right):
> 
>
https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:29: } =
> nodes;
> On 2016/05/03 20:38:58, hoch wrote:
> > On 2016/05/03 20:32:21, Raymond Toy wrote:
> > > On 2016/05/03 18:15:01, hoch wrote:
> > > > How about:
> > > > 
> > > > var nodes = { context, source, panner } = createGraph();
> > > 
> > > No problems with this, but js-beautify wants to indent it wrapped as
shown.
> > 
> > That's perhaps because js-beautify doesn't know about the destructuring
> pattern.
> > The curly brace does not represent a JS object here.
> 
> Yes, js-beautify probably doesn't know.  Isn't multiple assignment like this
bad
> style in javascript?  C/C++ styles don't usually allow it.

Since Google JS style guide doesn't say anything about the multiple assignment,
so I would just leave this up to you.

[Raymond Toy, 2016-05-03 21:03:46]

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html
(right):

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:29: } =
nodes;
On 2016/05/03 18:15:01, hoch wrote:
> How about:
> 
> var nodes = { context, source, panner } = createGraph();

Done, except no multiple assignment.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:42: } =
nodes;
On 2016/05/03 18:15:01, hoch wrote:
> ditto.

Done.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:55: } =
nodes;
On 2016/05/03 18:15:01, hoch wrote:
> ditto.

Done.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:222: return
{ context: context,
On 2016/05/03 18:15:01, hoch wrote:
> Let's add a line break after {.

Done.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:231: } =
options;
On 2016/05/03 18:15:01, hoch wrote:
> ditto here - let's put them in a single line.

Done.

https://codereview.chromium.org/1820403002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-basic.html:235: } =
nodes;
On 2016/05/03 18:15:01, hoch wrote:
> ditto.

Done.

[hongchan, 2016-05-06 18:41:27]

Mostly styles and nits.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webaudio/audiolistener-automation-position.html
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audiolistener-automation-position.html:18:
var renderFrames = 1024;
The comment and the actual number do not match.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audiolistener-automation-position.html:21:
var endTime = 0.75 * renderDuration;
Why 0.75?

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audiolistener-automation-position.html:105:
return {context: context, source: source, panner: panner};
This needs to be separated into multiple lines.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audiolistener-automation-position.html:112:
return runPannerTest(options).then(function (renderedBuffer) {
I believe this can be rewrite something like this. (options.endPosition should
be taken care of too, but...)

Promise.all([
  runPannerTest(options),
  runListenerTest(options)
]).then(function (refBuffer, renderedBuffer) {
  compareResults(refBuffer, renderedBuffer, options);
});

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:114:
gainX.gain.linearRampToValueAtTime(options.endPosition[0], 0.75 *
renderDuration);
Let's set a variable for 0.75 * renderDuration.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:163:
//console.log(data0);
Remove console.log()

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:169:
//console.log(zcoord);
Ditto.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:189:
//console.log(distanceGain);
Ditto.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:208: ]);
The convention is:

calculateAzimuth([
  item1,
  item2,
  item3
], [
  item1,
  item2,
  item3
], [
  item1,
  item2,
  item3
]);

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:227:
prefix += options.endPosition[2] + "]: ";
var prefix = "string"
  + "string"
  + "string"
  + "string"

I find this much easier to read.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/resources/panner-formulas.js
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/resources/panner-formulas.js:150:
function applyPanner(azimuth, srcL, srcR, numberOfChannels)
Let's bring up the curly brace.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/resources/panner-formulas.js:156: if
(numberOfChannels == 1) {
Not sure if I am understanding this - there is nothing in the if clause.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:127:
RELEASE_ASSERT(framesToProcess <= m_upZValues.size());
Do we really need all these? Can you describe when framesToProcess <= foo.size()
is possible?

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:143: const float*
AudioListener::getPositionXValues(int framesToProcess)
int? or unsigned? ditto from now on.

[Raymond Toy, 2016-05-06 21:20:50]

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/webaudio/audiolistener-automation-position.html
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/audiolistener-automation-position.html:112:
return runPannerTest(options).then(function (renderedBuffer) {
On 2016/05/06 18:41:26, hoch wrote:
> I believe this can be rewrite something like this. (options.endPosition should
> be taken care of too, but...)
> 
> Promise.all([
>   runPannerTest(options),
>   runListenerTest(options)
> ]).then(function (refBuffer, renderedBuffer) {
>   compareResults(refBuffer, renderedBuffer, options);
> });

As we discussed, this doesn't quite work out as we expected.  So no Promise.all.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:208: ]);
On 2016/05/06 18:41:26, hoch wrote:
> The convention is:
> 
> calculateAzimuth([
>   item1,
>   item2,
>   item3
> ], [
>   item1,
>   item2,
>   item3
> ], [
>   item1,
>   item2,
>   item3
> ]);

This is what js-beautify returns; I'm ok with changing it; I didn't like what
js-beautify did here anyway.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/resources/panner-formulas.js
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/resources/panner-formulas.js:156: if
(numberOfChannels == 1) {
On 2016/05/06 18:41:26, hoch wrote:
> Not sure if I am understanding this - there is nothing in the if clause.

I think because I didn't actually implement or test the mono case.  Do you want
a mono test as well?

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:127:
RELEASE_ASSERT(framesToProcess <= m_upZValues.size());
On 2016/05/06 18:41:26, hoch wrote:
> Do we really need all these? Can you describe when framesToProcess <=
foo.size()
> is possible?

I don't think it's possible, but if for some reason it fails,
calculateSampleAccurateValues will write past the end of the array.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:143: const float*
AudioListener::getPositionXValues(int framesToProcess)
On 2016/05/06 18:41:27, hoch wrote:
> int? or unsigned? ditto from now on.

Because I hate unsigned if it's not truly necessary.  But we use unsigned (or
size_t) everywhere for framesToProcess, so ok.

[Raymond Toy, 2016-05-11 19:28:30]

PTAL again.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:114:
gainX.gain.linearRampToValueAtTime(options.endPosition[0], 0.75 *
renderDuration);
On 2016/05/06 18:41:26, hoch wrote:
> Let's set a variable for 0.75 * renderDuration.

Done.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:163:
//console.log(data0);
On 2016/05/06 18:41:26, hoch wrote:
> Remove console.log()

Done.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:169:
//console.log(zcoord);
On 2016/05/06 18:41:26, hoch wrote:
> Ditto.

Done.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:189:
//console.log(distanceGain);
On 2016/05/06 18:41:26, hoch wrote:
> Ditto.

Done.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:208: ]);
On 2016/05/06 18:41:26, hoch wrote:
> The convention is:
> 
> calculateAzimuth([
>   item1,
>   item2,
>   item3
> ], [
>   item1,
>   item2,
>   item3
> ], [
>   item1,
>   item2,
>   item3
> ]);

Done.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/panner-automation-position.html:227:
prefix += options.endPosition[2] + "]: ";
On 2016/05/06 18:41:26, hoch wrote:
> var prefix = "string"
>   + "string"
>   + "string"
>   + "string"
> 
> I find this much easier to read.

Done.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/webaudio/resources/panner-formulas.js
(right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/webaudio/resources/panner-formulas.js:156: if
(numberOfChannels == 1) {
On 2016/05/06 21:20:50, Raymond Toy wrote:
> On 2016/05/06 18:41:26, hoch wrote:
> > Not sure if I am understanding this - there is nothing in the if clause.
> 
> I think because I didn't actually implement or test the mono case.  Do you
want
> a mono test as well?

Implemented.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:127:
RELEASE_ASSERT(framesToProcess <= m_upZValues.size());
On 2016/05/06 21:20:50, Raymond Toy wrote:
> On 2016/05/06 18:41:26, hoch wrote:
> > Do we really need all these? Can you describe when framesToProcess <=
> foo.size()
> > is possible?
> 
> I don't think it's possible, but if for some reason it fails,
> calculateSampleAccurateValues will write past the end of the array.

Different check added, and not a release assert.

https://codereview.chromium.org/1820403002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:143: const float*
AudioListener::getPositionXValues(int framesToProcess)
On 2016/05/06 21:20:50, Raymond Toy wrote:
> On 2016/05/06 18:41:27, hoch wrote:
> > int? or unsigned? ditto from now on.
> 
> Because I hate unsigned if it's not truly necessary.  But we use unsigned (or
> size_t) everywhere for framesToProcess, so ok.

Using size_t.

[hongchan, 2016-05-12 16:04:08]

The CQ bit was checked by hongchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-12 16:04:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1820403002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1820403002/260001

[commit-bot: I haz the power, 2016-05-12 16:06:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-12 16:06:25]

Dry run: Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[hongchan, 2016-05-13 00:33:54]

lgtm. I think the CL is ready for review of the non-webaudio part.

[Raymond Toy, 2016-05-13 15:27:18]

rtoy@chromium.org changed reviewers:
+ mpearson@chromium.org, tkent@chromium.org

[Raymond Toy, 2016-05-13 15:27:19]

mpearson@ PTAL at histograms.xml (and UseCounter.h)

tkent@ PTAL as API owner

[Mark P, 2016-05-13 17:16:21]

histograms.xml lgtm

[tkent, 2016-05-15 23:45:09]

lgtm

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:129:
ASSERT(sizesAreGood);
ASSERT -> DCHECK

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/audio/EqualPowerPanner.cpp (right):

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/EqualPowerPanner.cpp:159:
ASSERT(isInputSafe);
ASSERT -> DCHECK

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/EqualPowerPanner.cpp:166:
ASSERT(isOutputSafe);
ASSERT -> DCHECK

[Raymond Toy, 2016-05-16 15:49:31]

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:129:
ASSERT(sizesAreGood);
On 2016/05/15 23:45:09, tkent wrote:
> ASSERT -> DCHECK

Done.

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/audio/EqualPowerPanner.cpp (right):

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/EqualPowerPanner.cpp:159:
ASSERT(isInputSafe);
On 2016/05/15 23:45:09, tkent wrote:
> ASSERT -> DCHECK

Done.

https://codereview.chromium.org/1820403002/diff/300001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/EqualPowerPanner.cpp:166:
ASSERT(isOutputSafe);
On 2016/05/15 23:45:09, tkent wrote:
> ASSERT -> DCHECK

Done.

[Raymond Toy, 2016-05-16 15:49:45]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-05-16 15:49:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
hongchan@chromium.org, tkent@chromium.org, mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/1820403002/#ps320001
(title: "Address comments and rebase")

[commit-bot: I haz the power, 2016-05-16 15:49:55]

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

[commit-bot: I haz the power, 2016-05-16 20:21:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-16 20:21:03]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hongchan, 2016-05-16 21:49:58]

lgtm on new changes in audioparam-nominal-range.

[Raymond Toy, 2016-05-16 21:50:33]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-05-16 21:50:33]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/1820403002/#ps340001
(title: "Update test because CL for min/maxValue AudioParam landed")

[commit-bot: I haz the power, 2016-05-16 21:51:04]

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

[commit-bot: I haz the power, 2016-05-16 23:49:10]

Description was changed from

==========
Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of
the position and orientation of the Panner node are added
as AudioParams.  Likewise, the components of the position,
up, and forward vectors for the AudioListener are exposed
as AudioParams.

Use counters are added for setPosition and setOrientation
to keep track of these since the spec has deprecated these
methods.  Also changed the use counter name for
createPanner() so we can keep track of the usage of this
new Panner node with automations.

The automations are a-rate, and the a-rate update is
implemented only for the "equalpower" panner.  The "HRTF"
panning model is still k-rate.  (We expect the inherent
smoothing in HRTF is sufficient and the huge increase in
complexity is not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjD...

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html
==========

to

==========
Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of
the position and orientation of the Panner node are added
as AudioParams.  Likewise, the components of the position,
up, and forward vectors for the AudioListener are exposed
as AudioParams.

Use counters are added for setPosition and setOrientation
to keep track of these since the spec has deprecated these
methods.  Also changed the use counter name for
createPanner() so we can keep track of the usage of this
new Panner node with automations.

The automations are a-rate, and the a-rate update is
implemented only for the "equalpower" panner.  The "HRTF"
panning model is still k-rate.  (We expect the inherent
smoothing in HRTF is sufficient and the huge increase in
complexity is not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjD...

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html
==========

[commit-bot: I haz the power, 2016-05-16 23:49:11]

Committed patchset #18 (id:340001)

[commit-bot: I haz the power, 2016-05-16 23:50:43]

Description was changed from

==========
Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of
the position and orientation of the Panner node are added
as AudioParams.  Likewise, the components of the position,
up, and forward vectors for the AudioListener are exposed
as AudioParams.

Use counters are added for setPosition and setOrientation
to keep track of these since the spec has deprecated these
methods.  Also changed the use counter name for
createPanner() so we can keep track of the usage of this
new Panner node with automations.

The automations are a-rate, and the a-rate update is
implemented only for the "equalpower" panner.  The "HRTF"
panning model is still k-rate.  (We expect the inherent
smoothing in HRTF is sufficient and the huge increase in
complexity is not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjD...

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html
==========

to

==========
Implement Automations for PannerNode and AudioListener

To support smoother Panner effects, the 3D components of
the position and orientation of the Panner node are added
as AudioParams.  Likewise, the components of the position,
up, and forward vectors for the AudioListener are exposed
as AudioParams.

Use counters are added for setPosition and setOrientation
to keep track of these since the spec has deprecated these
methods.  Also changed the use counter name for
createPanner() so we can keep track of the usage of this
new Panner node with automations.

The automations are a-rate, and the a-rate update is
implemented only for the "equalpower" panner.  The "HRTF"
panning model is still k-rate.  (We expect the inherent
smoothing in HRTF is sufficient and the huge increase in
complexity is not worth the effort.)

WebAudio issue: https://github.com/WebAudio/web-audio-api/issues/749
Spec: http://webaudio.github.io/web-audio-api/#the-pannernode-interface

Intent:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/zH8QYCU0Y2I/qgsvKYjD...

BUG=590000
TEST=panner-automation*.html, audiolistener-automation*.html

Committed: https://crrev.com/2f2105fe40e9097470fe79d9d7b6fe2d88e73c38
Cr-Commit-Position: refs/heads/master@{#393983}
==========

[commit-bot: I haz the power, 2016-05-16 23:50:45]

Patchset 18 (id:??) landed as
https://crrev.com/2f2105fe40e9097470fe79d9d7b6fe2d88e73c38
Cr-Commit-Position: refs/heads/master@{#393983}