New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement BiquadFilterNode #21750

Merged
merged 4 commits into from Sep 20, 2018

Conversation

Projects
4 participants
@Manishearth
Member

Manishearth commented Sep 19, 2018

A bunch of tests still fail but some of it may be a timing issue, looking at it the tests are at least affected by #21659 (changing how they work to avoid problems from that does not make them pass but does change the exact value of the error), so I feel like I should fix that first before investigating these.

r? @ferjm


This change is Reviewable

@highfive

This comment has been minimized.

highfive commented Sep 19, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/biquadfilternode.rs, components/script/dom/webidls/BaseAudioContext.webidl, components/script/dom/mod.rs, components/script/dom/baseaudiocontext.rs, components/script/dom/webidls/BiquadFilterNode.webidl
  • @KiChjang: components/script/dom/biquadfilternode.rs, components/script/dom/webidls/BaseAudioContext.webidl, components/script/dom/mod.rs, components/script/dom/baseaudiocontext.rs, components/script/dom/webidls/BiquadFilterNode.webidl

@Manishearth Manishearth force-pushed the Manishearth:biquad branch from 0b3eb84 to 2f8bd54 Sep 19, 2018

@ferjm

ferjm approved these changes Sep 20, 2018

Looks good.

Remember to update tests/wpt/mozilla/tests/mozilla/interfaces.html, please.

DomRoot::from_ref(&self.gain)
}
// https://webaudio.github.io/web-audio-api/#dom-biquadfilternode-gain

This comment has been minimized.

@ferjm

ferjm Sep 20, 2018

Member

This comment should be https://webaudio.github.io/web-audio-api/#dom-biquadfilternode-q

DomRoot::from_ref(&self.gain)
}
// https://webaudio.github.io/web-audio-api/#dom-biquadfilternode-gain

This comment has been minimized.

@ferjm

ferjm Sep 20, 2018

Member

https://webaudio.github.io/web-audio-api/#dom-biquadfilternode-detune

@Manishearth Manishearth force-pushed the Manishearth:biquad branch from 2f8bd54 to daf8591 Sep 20, 2018

@Manishearth

This comment has been minimized.

Member

Manishearth commented Sep 20, 2018

@bors-servo r=ferjm

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 20, 2018

📌 Commit daf8591 has been approved by ferjm

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 20, 2018

⌛️ Testing commit daf8591 with merge ac331c6...

bors-servo added a commit that referenced this pull request Sep 20, 2018

Auto merge of #21750 - Manishearth:biquad, r=ferjm
Implement BiquadFilterNode

A bunch of tests still fail but some of it may be a timing issue, looking at it the tests are *at least* affected by #21659 (changing how they work to avoid problems from that does not make them pass but does change the exact value of the error), so I feel like I should fix that first before investigating these.

r? @ferjm

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21750)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 20, 2018

💔 Test failed - linux-rel-wpt

@Manishearth

This comment has been minimized.

Member

Manishearth commented Sep 20, 2018

These seem to be more timing-related intermittents. Filing.

@Manishearth

This comment has been minimized.

Member

Manishearth commented Sep 20, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 20, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 20, 2018

@bors-servo bors-servo merged commit daf8591 into servo:master Sep 20, 2018

2 of 4 checks passed

Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@ferjm ferjm added this to Done in WebAudio Nov 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment