Closed Bug 957024 Opened 6 years ago Closed 3 years ago

Web Audio sources don't play when connected to PannerNodes with position 0,0,0

Categories

(Core :: Web Audio, defect, P1)

26 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: robman, Assigned: dminor)

References

Details

(Whiteboard: [bugday-20140113])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131205075310

Steps to reproduce:

Created an audio context, a buffer source and a panner node and connected them together (e.g. source -> panner -> context.destination). Then loaded a sound file into a buffer via xhr2 and then onload poured the loaded data into the source's buffer, set it to loop and called start(0.0).

Tested on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0

See attached example files.
wa2.html shows two separate source/panner graphs. To test changing the position enter "panner1.setPosition(1,0,0)" and "panner2.setPosition(1,0,0)" or similar into the javascript console - wav files not included.


Actual results:

Silence.

But if you call panner.setPosition() with anything but 0 for any of the x,y,z params (within a reasonable distance range) then the sound starts playing - beware low values blowing your ears out if you have headphones in 8)

Even more frustratingly, if you have more than one panner node, then if ANY of them are set to position 0,0,0 then all sounds connected to panner nodes stop playing. The panner positions can be updated independently and work correctly as long as they all have non 0,0,0 values.



Expected results:

The sounds connected to panners at position 0,0,0 should play and sound like they are at the same position the context.listener is at (default 0,0,0). And if any of them go to position 0,0,0 then the others should still keep playing like normal and not go silent.

This behaviour works as expected in Chrome.
Component: Untriaged → Web Audio
Product: Firefox → Core
Whiteboard: [bugday-20140113]
The default PannerNode distanceModel and rolloffFactor treats sources as point sources.  In order for point sources to be heard at a distance > 0, the volume at the point must be ∞.  That means the expected result for a panner at the listener point would be sound with infinite volume.  In floating point arithmetic, that would be a signal with +inf, -inf, and NaNs at the zero crossings.

The silence implies that a conversion from infs and NaNs to zero is happening, which is probably better than a full volume square wave from clipping.

The influence of a single panner node at zero distance on the whole graph implies that the conversion is happening after the mixing.  I guess it could be silenced before mixing, but I don't see much point as there is still going to be a full volume clipped signal if any panner node has distance close to zero.
Priority: -- → P3
Blocks: 1077368
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 1077368
The spec has changed so that distances less than refDistance should be treated as refDistance when calculating distanceGain.
Priority: P3 → P2
This can cause significant problems with clipping, and shouldn't be too hard to fix.
OS: Mac OS X → All
Priority: P2 → P1
Hardware: x86 → All
Duplicate of this bug: 1012610
Rank: 25 → 17
Assignee: nobody → dminor
Status: NEW → ASSIGNED
Strangely enough, I wasn't able to reproduce this failure. The test case seems simple enough but maybe I'm missing something. Regardless, we should update the gain equations to match the spec. I've also added a test to catch future regressions.
Comment on attachment 8781580 [details]
Bug 957024 - Update PannerNode distance calculations to match the web audio spec;

https://reviewboard.mozilla.org/r/71978/#review84958

::: dom/media/webaudio/PannerNode.cpp:374
(Diff revision 1)
>  
>  // Those three functions are described in the spec.
>  float
>  PannerNodeEngine::LinearGainFunction(float aDistance)
>  {
> -  return 1 - mRolloffFactor * (aDistance - mRefDistance) / (mMaxDistance - mRefDistance);
> +  return 1 - mRolloffFactor * (std::max(std::min((double)aDistance, mMaxDistance), mRefDistance) - mRefDistance) / (mMaxDistance - mRefDistance);

PannerNodeEngine::ComputeDistanceGain() converts a double sqrt() to a float
|distance| before passing to one of these functions.  Can you change
|distance| and |aDistance| to double type in order to remove that implicit
cast, please, and remove the explicit C-style (double) casts introduced in
this patch?

::: dom/media/webaudio/test/mochitest.ini:88
(Diff revision 1)
>  [test_bug867203.html]
>  [test_bug875221.html]
>  [test_bug875402.html]
>  [test_bug894150.html]
>  [test_bug956489.html]
> +[test_bug957024.html]

If you can call this something like test_pannerNodeAtZeroDistance.html, then what is wrong in a test failure would be obvious from the output.

::: dom/media/webaudio/test/test_bug957024.html:4
(Diff revision 1)
> +<!DOCTYPE HTML>
> +<html>
> +<head>
> +  <title>Test PannerNode produces output even when the position is 0, 0, 0</title>

"... even when the distance is from the listener is zero" describes the relevant situation more precisely.

::: dom/media/webaudio/test/test_bug957024.html:71
(Diff revision 1)
> +    var silence = true;
> +    var array = buffer.getChannelData(0);
> +    for (var i = 0; i < buffer.length; i++) {
> +      if (array[i] != 0) {
> +        silence = false;
> +      }
> +    }
> +    ok(!silence, "The buffer is not silent.");

This won't detect a revert to previous behavior because currently output is infinite, which is not = 0.  Better to compare against the precise expected output.  Is that monoBuffer?
Attachment #8781580 - Flags: review?(karlt) → review+
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3687c50bc52e
Update PannerNode distance calculations to match the web audio spec; r=karlt
https://hg.mozilla.org/mozilla-central/rev/3687c50bc52e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.