RESOLVED FIXED 47623
Add AudioResampler files 
https://bugs.webkit.org/show_bug.cgi?id=47623
Summary Add AudioResampler files
Chris Rogers
Reported 2010-10-13 14:41:44 PDT
Add AudioResampler files
Attachments
Patch (8.36 KB, patch)
2010-10-13 14:43 PDT, Chris Rogers 		no flags
DetailsFormatted DiffDiff
Patch (8.45 KB, patch)
2010-10-18 12:46 PDT, Chris Rogers 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Rogers
Comment 1 2010-10-13 14:43:01 PDT
Created attachment 70665 [details] Patch
chris fleizach
Comment 2 2010-10-18 00:43:58 PDT
Comment on attachment 70665 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70665&action=review > WebCore/platform/audio/AudioResampler.cpp:105 > + for (unsigned i = 0; i < m_kernels.size(); ++i) { dont' call a method within a loop. initialize the length outside the loop > WebCore/platform/audio/AudioResampler.cpp:121 > + for (unsigned i = 0; i < m_kernels.size(); ++i) same here as above > WebCore/platform/audio/AudioResampler.h:41 > + AudioResampler(); // default to mono This comment seems like it should be in the class description above, not inline. > WebCore/platform/audio/AudioResampler.h:53 > + // 0 < rate <= MaxRate seems like MaxRate should be in the header so that clients could use the max rate if they so desired. > WebCore/platform/audio/AudioResampler.h:59 > +protected: should probably be private
Chris Rogers
Comment 3 2010-10-18 12:46:24 PDT
Created attachment 71073 [details] Patch
Chris Rogers
Comment 4 2010-10-18 12:49:10 PDT
Comment on attachment 70665 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70665&action=review >> WebCore/platform/audio/AudioResampler.cpp:105 >> + for (unsigned i = 0; i < m_kernels.size(); ++i) { > > dont' call a method within a loop. initialize the length outside the loop FIXED >> WebCore/platform/audio/AudioResampler.cpp:121 >> + for (unsigned i = 0; i < m_kernels.size(); ++i) > > same here as above FIXED >> WebCore/platform/audio/AudioResampler.h:41 >> + AudioResampler(); // default to mono > > This comment seems like it should be in the class description above, not inline. FIXED: moved comment >> WebCore/platform/audio/AudioResampler.h:53 >> + // 0 < rate <= MaxRate > > seems like MaxRate should be in the header so that clients could use the max rate if they so desired. It *is* declared a few lines after this >> WebCore/platform/audio/AudioResampler.h:59 >> +protected: > > should probably be private FIXED
chris fleizach
Comment 5 2010-10-18 17:45:14 PDT
Comment on attachment 71073 [details] Patch r=me
WebKit Commit Bot
Comment 6 2010-10-18 18:59:29 PDT
Comment on attachment 71073 [details] Patch Clearing flags on attachment: 71073 Committed r70015: <http://trac.webkit.org/changeset/70015>
WebKit Commit Bot
Comment 7 2010-10-18 18:59:35 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.