| Summary: | Add AudioResamplerKernel files | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Rogers <crogers> | ||||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cmarrin, commit-queue, dglazkov, eric.carlson, jamesr, jer.noble, kbr, simon.fraser | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | OS X 10.5 | ||||||||
| Attachments: |
|
||||||||
|
Description
Chris Rogers
2010-10-13 14:43:42 PDT
Created attachment 70667 [details]
Patch
Comment on attachment 70667 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70667&action=review A few comments and questions. > WebCore/platform/audio/AudioResamplerKernel.cpp:59 > + int endIndex = static_cast<int>(nextFractionalIndex + 1.0); // round up to next integer index Should this just use ceil()? > WebCore/platform/audio/AudioResamplerKernel.cpp:102 > + double sample2 = source[readIndex + 1]; It would be really nice to have some assertions (perhaps hoisted out of the loop) that m_virtualReadIndex + (framesToProcess * rate) + 1 is not going to walk off the end of the source buffer. > WebCore/platform/audio/AudioResamplerKernel.h:57 > +protected: Can these members be private rather than protected? Comment on attachment 70667 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70667&action=review >> WebCore/platform/audio/AudioResamplerKernel.cpp:59 >> + int endIndex = static_cast<int>(nextFractionalIndex + 1.0); // round up to next integer index > > Should this just use ceil()? ceil() doesn't have quite the same behavior. For example ceil(1.0) == 1.0, but we need the value 2 since we need to have one sample *past* the rounded-down version. >> WebCore/platform/audio/AudioResamplerKernel.cpp:102 >> + double sample2 = source[readIndex + 1]; > > It would be really nice to have some assertions (perhaps hoisted out of the loop) that m_virtualReadIndex + (framesToProcess * rate) + 1 is not going to walk off the end of the source buffer. FIXED: added some assertions before entering the loop. Created attachment 72068 [details]
Patch
Comment on attachment 72068 [details]
Patch
Looks good.
The commit-queue encountered the following flaky tests while processing attachment 72068 [details]: fast/workers/storage/use-same-database-in-page-and-workers.html Please file bugs against the tests. These tests were authored by dumi@chromium.org. The commit-queue is continuing to process your patch. Comment on attachment 72068 [details] Patch Clearing flags on attachment: 72068 Committed r70719: <http://trac.webkit.org/changeset/70719> All reviewed patches have been landed. Closing bug. |