Closed Bug 1456259 Opened 2 years ago Closed 2 years ago

AnalyserNode constructor issues

Categories

(Core :: Web Audio, defect, P2)

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: toy.raymond, Assigned: achronop)

Details

Attachments

(1 file)

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.
Component: Untriaged → Web Audio
Product: Firefox → Core
Rank: 15
Priority: -- → P2
Status: UNCONFIRMED → NEW
Ever confirmed: true
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
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 on attachment 8971267 [details]
Bug 1456259 - Correct AnalyserNode construction issues.

https://reviewboard.mozilla.org/r/240032/#review246440
Attachment #8971267 - Flags: review?(padenot) → review+
Assignee: nobody → achronop
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
Attachment #8971267 - Flags: review?(bugs)
Comment on attachment 8971267 [details]
Bug 1456259 - Correct AnalyserNode construction issues.

https://reviewboard.mozilla.org/r/240032/#review246510
Attachment #8971267 - Flags: review?(bugs) → review+
Pushed by achronop@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/8480289cd439
Correct AnalyserNode construction issues. r=padenot,smaug
https://hg.mozilla.org/mozilla-central/rev/8480289cd439
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.