| Summary: | Improve interpolation algorithm in OscillatorNode | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
| Component: | Web Audio | Assignee: | Chris Dumez <cdumez> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, eric.carlson, ews-watchlist, ggaren, glenn, jer.noble, philipj, sergio, webkit-bug-importer, youennf | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 212611 | ||||||||
| Attachments: |
|
||||||||
|
Description
Chris Dumez
2020-09-04 09:35:44 PDT
Created attachment 407985 [details]
Patch
Created attachment 407989 [details]
Patch
Comment on attachment 407989 [details]
Patch
r=me
Committed r266627: <https://trac.webkit.org/changeset/266627> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407989 [details]. Comment on attachment 407989 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407989&action=review > Source/WebCore/Modules/webaudio/OscillatorNode.cpp:210 > + // Consider a typical sample rate of 44100 Hz and max periodic wave > + // size of 4096. The relationship between |incr| and the frequency > + // of the oscillator is |incr| = freq * 4096/44100. Or freq = > + // |incr|*44100/4096 = 10.8*|incr|. > + // > + // For the |incr| thresholds below, this means that we use linear > + // interpolation for all freq >= 3.2 Hz, 3-point Lagrange > + // for freq >= 1.7 Hz and 5-point Lagrange for every thing else. > + // > + // We use Lagrange interpolation because it's relatively simple to > + // implement and fairly inexpensive, and the interpolator always > + // passes through known points. You should note somewhere that this is based on code in Chrome's oscillator_node.cc Comment on attachment 407989 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407989&action=review >> Source/WebCore/Modules/webaudio/OscillatorNode.cpp:210 >> + // passes through known points. > > You should note somewhere that this is based on code in Chrome's oscillator_node.cc I said in the Changelog: "Align our OscillatorNode implementation with Chromium". How to you suggest I do better? Comment on attachment 407989 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407989&action=review >>> Source/WebCore/Modules/webaudio/OscillatorNode.cpp:210 >>> + // passes through known points. >> >> You should note somewhere that this is based on code in Chrome's oscillator_node.cc > > I said in the Changelog: "Align our OscillatorNode implementation with Chromium". How to you suggest I do better? Some of you other patches have said something like "cherry picked from Chrome ...", but the ChangeLog comment is fine too. (In reply to Eric Carlson from comment #8) > Comment on attachment 407989 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=407989&action=review > > >>> Source/WebCore/Modules/webaudio/OscillatorNode.cpp:210 > >>> + // passes through known points. > >> > >> You should note somewhere that this is based on code in Chrome's oscillator_node.cc > > > > I said in the Changelog: "Align our OscillatorNode implementation with Chromium". How to you suggest I do better? > > Some of you other patches have said something like "cherry picked from > Chrome ...", but the ChangeLog comment is fine too. Ok, I will try and be clearer. |