1456259 - AnalyserNode constructor issues

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P2)

Description

[toy.raymond]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.117 Safari/537.36

Steps to reproduce:

Run the following in a console:

c = new AudioContext();
n0 = new AnalyserNode(c);
n0.smoothingTimeConstant
n1 = new AnalyserNode(c, {minDecibels: -13, maxDecibels: -1});

According to the spec, n0.smoothingTimeConstant should be 0.8, not 0.800000011920929. (I think that value is what you get when convert the double-float 0.8 to a single-float and then back to double-float).  Perhaps the definition of the dictionary for smoothingTimeConstant is incorrect?

For n1, minDecibels is less than maxDecibels, so this should be allowed.  If you change the value of minDecibels to be less than -30 (the default value for maxDecibels), then the construction works.  I'm guessing the code compares minDecibels with the default maxDecibels value before assigning maxDecibels option.

Comment 1

[Alex Chronopoulos [:achronop]]

For the first issue we need to update the webidl [1] according to the spec and make `maxDecibels`, `minDecibels` and `smoothingTimeConstant` double instead of floats.

[1] https://searchfox.org/mozilla-central/source/dom/webidl/AnalyserNode.webidl#13
[2] https://webaudio.github.io/web-audio-api/#dictdef-analyseroptions

Comment 2

[Alex Chronopoulos [:achronop]]

The second issue is happening because, in constructor, we update minDecibels  before maxDecibels [1]. The setter compares the given value with maxDecibels but at the time of compare it contains the previous value (which is the default).

[1] https://searchfox.org/mozilla-central/source/dom/media/webaudio/AnalyserNode.cpp#123

Comment 3

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1456259 - Correct AnalyserNode construction issues.

Review commit: https://reviewboard.mozilla.org/r/240032/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240032/

Comment 4

[Alex Chronopoulos [:achronop]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=500cbd125e15ff0c847d4059675d23a16a640c22

Comment 5

[Paul Adenot (:padenot)]

Comment on attachment 8971267 [details]
Bug 1456259 - Correct AnalyserNode construction issues.

https://reviewboard.mozilla.org/r/240032/#review246440

Comment 6

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 3 changes to 3 files
remote: 
remote: WebIDL file dom/webidl/AnalyserNode.webidl altered in changeset c6b578743f52 without DOM peer review
remote: 
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: 
remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote: 
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.d_webidl hook failed
abort: push failed on remote

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8971267 [details]
Bug 1456259 - Correct AnalyserNode construction issues.

https://reviewboard.mozilla.org/r/240032/#review246510

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8480289cd439
Correct AnalyserNode construction issues. r=padenot,smaug

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/8480289cd439