WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
47624
Add AudioResamplerKernel files
https://bugs.webkit.org/show_bug.cgi?id=47624
Summary
Add AudioResamplerKernel files
Chris Rogers
Reported
2010-10-13 14:43:42 PDT
Add AudioResamplerKernel files
Attachments
Patch
(9.75 KB, patch)
2010-10-13 14:45 PDT
,
Chris Rogers
no flags
Details
Formatted Diff
Diff
Patch
(9.97 KB, patch)
2010-10-27 12:12 PDT
,
Chris Rogers
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Rogers
Comment 1
2010-10-13 14:45:12 PDT
Created
attachment 70667
[details]
Patch
Kenneth Russell
Comment 2
2010-10-26 18:51:03 PDT
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?
Chris Rogers
Comment 3
2010-10-27 12:07:02 PDT
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.
Chris Rogers
Comment 4
2010-10-27 12:12:07 PDT
Created
attachment 72068
[details]
Patch
Kenneth Russell
Comment 5
2010-10-27 13:44:38 PDT
Comment on
attachment 72068
[details]
Patch Looks good.
WebKit Commit Bot
Comment 6
2010-10-27 15:28:16 PDT
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.
WebKit Commit Bot
Comment 7
2010-10-27 15:29:08 PDT
Comment on
attachment 72068
[details]
Patch Clearing flags on attachment: 72068 Committed
r70719
: <
http://trac.webkit.org/changeset/70719
>
WebKit Commit Bot
Comment 8
2010-10-27 15:29:15 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug