| Summary: | Fix BiquadFilterNode's lowpass filter | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
| Component: | Web Audio | Assignee: | Chris Dumez <cdumez> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | clark_wang, darin, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=181191 | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 212611 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Chris Dumez
2020-08-11 10:16:14 PDT
Created attachment 406389 [details]
Patch
Created attachment 406406 [details]
Patch
Comment on attachment 406406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406406&action=review > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:41 > + , m_parameter2(AudioParam::create(context, "Q", 1, -FLT_MAX, FLT_MAX)) > + , m_parameter3(AudioParam::create(context, "gain", 0.0, -FLT_MAX, 1541)) > + , m_parameter4(AudioParam::create(context, "detune", 0.0, -153600, 153600)) What tests these? > LayoutTests/webaudio/biquad-lowpass.html:18 > +window.OfflineAudioContext = window.OfflineAudioContext || window.webkitOfflineAudioContext; Not sure why we need this. What happens if we leave this line of code out? (In reply to Darin Adler from comment #3) > Comment on attachment 406406 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406406&action=review > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:41 > > + , m_parameter2(AudioParam::create(context, "Q", 1, -FLT_MAX, FLT_MAX)) > > + , m_parameter3(AudioParam::create(context, "gain", 0.0, -FLT_MAX, 1541)) > > + , m_parameter4(AudioParam::create(context, "detune", 0.0, -153600, 153600)) > > What tests these? I will check. I may need to add coverage. > > > LayoutTests/webaudio/biquad-lowpass.html:18 > > +window.OfflineAudioContext = window.OfflineAudioContext || window.webkitOfflineAudioContext; > > Not sure why we need this. What happens if we leave this line of code out? Without this change we are unable to run the test in other browsers. I wanted to compare the behavior in major browsers and had to make this change locally. I figured we may as well land it. Created attachment 406412 [details]
Patch
(In reply to Darin Adler from comment #3) > Comment on attachment 406406 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406406&action=review > > > Source/WebCore/Modules/webaudio/BiquadProcessor.cpp:41 > > + , m_parameter2(AudioParam::create(context, "Q", 1, -FLT_MAX, FLT_MAX)) > > + , m_parameter3(AudioParam::create(context, "gain", 0.0, -FLT_MAX, 1541)) > > + , m_parameter4(AudioParam::create(context, "detune", 0.0, -153600, 153600)) > > What tests these? I added a test. > > > LayoutTests/webaudio/biquad-lowpass.html:18 > > +window.OfflineAudioContext = window.OfflineAudioContext || window.webkitOfflineAudioContext; > > Not sure why we need this. What happens if we leave this line of code out? Nothing, in WebKit. This is only for other browsers. Committed r265517: <https://trac.webkit.org/changeset/265517> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406412 [details]. |