Make panner's doppler rate continuous as the source position approaches the listener position.

The issue is summarized here:  https://github.com/WebAudio/web-audio-api/issues/324

This CL can't solve all issues when the source position approaches the listener position because when the locations are equal, we get a singular point in the equations.

This just makes it continuous as the source approaches the listener. If the source then goes past, there will be a unavoidable discontinuity.

TEST=Listen to http://jsfiddle.net/UxwGP/10. When the slider position is 0, there should be no sudden change in pitch.

TEST=Listen to http://jsfiddle.net/qWHAt/34/ using mono source and HRTF panner.  Initially, the pitch is very low. Move the slider just to the left or right of center and the pitch goes up. As you move the slider through center slowly, the pitch drops drastically at center.  With this CL, this the pitch does not start out low, and as you move the slider around, the pitch never drops low at center. There is a discontinuity moving from left to right, but this is because the source was moving away from the listener and is not moving towards the listener.

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

[Raymond Toy, 2014-05-27 21:41:53]

PTAL.

[Ken Russell (switch to Gerrit), 2014-05-27 23:24:43]

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
File Source/modules/webaudio/PannerNode.cpp (right):

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:389: // Degenerate case if source and
listener are at the same point. To preserve continuity,
Comparing against zero seems fragile. You're the expert, but wouldn't this be
more stable if there were an epsilon defined in this file and the comparison
happened against that?

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:465: if (!sourceListenerMagnitude) {
Same comment here.

[KhNo, 2014-05-28 13:57:20]

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
File Source/modules/webaudio/PannerNode.cpp (right):

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:388: if (sourceListener.isZero()) {
How do you think if we remove line from 388 to 394 although
sourceListener.isZero() is true.
It might create meaningful values even if distance is zero. outAzimuth and
outElevation might be meaningful after calculation with zero.

[Raymond Toy, 2014-05-28 17:17:22]

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
File Source/modules/webaudio/PannerNode.cpp (right):

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:389: // Degenerate case if source and
listener are at the same point. To preserve continuity,
On 2014/05/27 23:24:43, Ken Russell wrote:
> Comparing against zero seems fragile. You're the expert, but wouldn't this be
> more stable if there were an epsilon defined in this file and the comparison
> happened against that?

I think this is ok here. If it's not zero, sourceListener.normalize() should
work fine and the rest of the computations should be ok.

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:465: if (!sourceListenerMagnitude) {
On 2014/05/27 23:24:43, Ken Russell wrote:
> Same comment here.

I believe this is ok here too.  It's only a problem when the magnitude is
exactly zero. For any other value, we aren't dividing by zero and causing
problems.

[Raymond Toy, 2014-05-28 17:19:16]

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
File Source/modules/webaudio/PannerNode.cpp (right):

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:388: if (sourceListener.isZero()) {
On 2014/05/28 13:57:20, KhNo wrote:
> How do you think if we remove line from 388 to 394 although
> sourceListener.isZero() is true.
> It might create meaningful values even if distance is zero. outAzimuth and
> outElevation might be meaningful after calculation with zero.

I think sourceListener.normalize() will fail (return NaN) because the length is
0 so you can't normalize it in any sane way.

[Ken Russell (switch to Gerrit), 2014-05-28 17:37:39]

I still think it would be more robust to use the cached values when the source
and listener positions are close together, say 1.0e-5 units, and not just
absolutely identical. You might run into problems with +/-0. Still, you're the
expert, so LGTM.

Would be good to add a layout test for this case.

[Raymond Toy, 2014-05-28 18:25:26]

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
File Source/modules/webaudio/PannerNode.cpp (right):

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:388: if (sourceListener.isZero()) {
On 2014/05/28 17:19:16, Raymond Toy wrote:
> On 2014/05/28 13:57:20, KhNo wrote:
> > How do you think if we remove line from 388 to 394 although
> > sourceListener.isZero() is true.
> > It might create meaningful values even if distance is zero. outAzimuth and
> > outElevation might be meaningful after calculation with zero.
> 
> I think sourceListener.normalize() will fail (return NaN) because the length
is
> 0 so you can't normalize it in any sane way.

Oh, wait.  normalize() does nothing if the length is 0. This might be ok.  Let
me check.

[Raymond Toy, 2014-05-28 18:29:55]

On 2014/05/28 17:37:39, Ken Russell wrote:
> I still think it would be more robust to use the cached values when the source
> and listener positions are close together, say 1.0e-5 units, and not just
> absolutely identical. You might run into problems with +/-0. Still, you're the
> expert, so LGTM.

Let me think about this.
> 
> Would be good to add a layout test for this case.

Yeah, I tried to come up with something, but without any kind of panner
automation on the position, it's really difficult to get something consistent.

[Raymond Toy, 2014-05-28 18:43:22]

On 2014/05/28 18:25:26, Raymond Toy wrote:
>
https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
> File Source/modules/webaudio/PannerNode.cpp (right):
> 
>
https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
> Source/modules/webaudio/PannerNode.cpp:388: if (sourceListener.isZero()) {
> On 2014/05/28 17:19:16, Raymond Toy wrote:
> > On 2014/05/28 13:57:20, KhNo wrote:
> > > How do you think if we remove line from 388 to 394 although
> > > sourceListener.isZero() is true.
> > > It might create meaningful values even if distance is zero. outAzimuth and
> > > outElevation might be meaningful after calculation with zero.
> > 
> > I think sourceListener.normalize() will fail (return NaN) because the length
> is
> > 0 so you can't normalize it in any sane way.
> 
> Oh, wait.  normalize() does nothing if the length is 0. This might be ok.  Let
> me check.

This basically works. There is still a discontinuity (azimuth goes suddenly from
90 to -.250448e-6) which would be ok, but strangely, on the first test, the
sound suddenly changes from mono (right) to stereo. I don't understand that.

[Raymond Toy, 2014-05-28 23:34:31]

On 2014/05/28 18:43:22, Raymond Toy wrote:
> On 2014/05/28 18:25:26, Raymond Toy wrote:
> >
>
https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
> > File Source/modules/webaudio/PannerNode.cpp (right):
> > 
> >
>
https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
> > Source/modules/webaudio/PannerNode.cpp:388: if (sourceListener.isZero()) {
> > On 2014/05/28 17:19:16, Raymond Toy wrote:
> > > On 2014/05/28 13:57:20, KhNo wrote:
> > > > How do you think if we remove line from 388 to 394 although
> > > > sourceListener.isZero() is true.
> > > > It might create meaningful values even if distance is zero. outAzimuth
and
> > > > outElevation might be meaningful after calculation with zero.
> > > 
> > > I think sourceListener.normalize() will fail (return NaN) because the
length
> > is
> > > 0 so you can't normalize it in any sane way.
> > 
> > Oh, wait.  normalize() does nothing if the length is 0. This might be ok. 
Let
> > me check.
> 
> This basically works. There is still a discontinuity (azimuth goes suddenly
from
> 90 to -.250448e-6) which would be ok, but strangely, on the first test, the
> sound suddenly changes from mono (right) to stereo. I don't understand that.

It goes to stereo with the equalpower panner because you are exactly half way
"between" the source so both left and right channels are receiving sound. Once
you move slightly away, the sound is either completely from the left or right.
No audio leaks between the two sides. For HRTF, audio is conducted through your
head so there is audio on both left and right at all times.

[KhNo, 2014-05-29 16:55:44]

On 2014/05/28 23:34:31, Raymond Toy wrote:
> On 2014/05/28 18:43:22, Raymond Toy wrote:
> > On 2014/05/28 18:25:26, Raymond Toy wrote:
> > >
> >
>
https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
> > > File Source/modules/webaudio/PannerNode.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
> > > Source/modules/webaudio/PannerNode.cpp:388: if (sourceListener.isZero()) {
> > > On 2014/05/28 17:19:16, Raymond Toy wrote:
> > > > On 2014/05/28 13:57:20, KhNo wrote:
> > > > > How do you think if we remove line from 388 to 394 although
> > > > > sourceListener.isZero() is true.
> > > > > It might create meaningful values even if distance is zero. outAzimuth
> and
> > > > > outElevation might be meaningful after calculation with zero.
> > > > 
> > > > I think sourceListener.normalize() will fail (return NaN) because the
> length
> > > is
> > > > 0 so you can't normalize it in any sane way.
> > > 
> > > Oh, wait.  normalize() does nothing if the length is 0. This might be ok. 
> Let
> > > me check.
> > 
> > This basically works. There is still a discontinuity (azimuth goes suddenly
> from
> > 90 to -.250448e-6) which would be ok, but strangely, on the first test, the
> > sound suddenly changes from mono (right) to stereo. I don't understand that.
> 
> It goes to stereo with the equalpower panner because you are exactly half way
> "between" the source so both left and right channels are receiving sound. Once
> you move slightly away, the sound is either completely from the left or right.
> No audio leaks between the two sides. For HRTF, audio is conducted through
your
> head so there is audio on both left and right at all times.

If you think it is not ideal behavior, your approaching for using cached values
is enough although that can not fully resolve all as your description.
LGTM.

[Raymond Toy, 2014-05-29 18:22:13]

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
File Source/modules/webaudio/PannerNode.cpp (right):

https://codereview.chromium.org/301733002/diff/1/Source/modules/webaudio/Pann...
Source/modules/webaudio/PannerNode.cpp:388: if (sourceListener.isZero()) {
On 2014/05/28 18:25:26, Raymond Toy wrote:
> On 2014/05/28 17:19:16, Raymond Toy wrote:
> > On 2014/05/28 13:57:20, KhNo wrote:
> > > How do you think if we remove line from 388 to 394 although
> > > sourceListener.isZero() is true.
> > > It might create meaningful values even if distance is zero. outAzimuth and
> > > outElevation might be meaningful after calculation with zero.
> > 
> > I think sourceListener.normalize() will fail (return NaN) because the length
> is
> > 0 so you can't normalize it in any sane way.
> 
> Oh, wait.  normalize() does nothing if the length is 0. This might be ok.  Let
> me check.

This is ok. The special case is removed.

[Raymond Toy, 2014-05-30 16:33:31]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2014-05-30 16:35:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rtoy@chromium.org/301733002/20001

[commit-bot: I haz the power, 2014-05-30 17:46:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6532)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/9962)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-05-30 18:48:34]

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

[commit-bot: I haz the power, 2014-05-30 18:48:34]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[Raymond Toy, 2014-05-30 21:52:11]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2014-05-30 21:53:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rtoy@chromium.org/301733002/20001

[commit-bot: I haz the power, 2014-05-30 23:57:14]

Change committed as 175175