Bug 56980 - web audio: Properly sample-rate convert audio assets in chromium port
Summary: web audio: Properly sample-rate convert audio assets in chromium port
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-23 17:44 PDT by Chris Rogers
Modified: 2011-03-24 19:13 PDT (History)
2 users (show)

See Also:

Attachments
Patch (9.49 KB, patch)
2011-03-23 17:54 PDT, Chris Rogers 		no flags Details | Formatted Diff | Diff
Patch (10.74 KB, patch)
2011-03-24 14:13 PDT, Chris Rogers 		no flags Details | Formatted Diff | Diff
Patch (10.76 KB, patch)
2011-03-24 16:19 PDT, Chris Rogers 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Rogers 2011-03-23 17:44:01 PDT 
web audio: Properly sample-rate convert audio assets in chromium port
Comment 1 Chris Rogers 2011-03-23 17:54:59 PDT 
Created attachment 86727 [details]
Patch
Comment 2 Chris Rogers 2011-03-23 18:12:19 PDT 
In looking over this again, I see that it would be more efficient to mix-to-mono first, before sample-rate converting.  I'll upload a new patch changing that part, but the other parts can still be reviewed...
Comment 3 Chris Rogers 2011-03-23 18:20:46 PDT 
I also need to conditionally compile AudioBus::createBySampleRateConverting for chromium-only, or provide an implementation for the mac-port as well.

It's not actually needed for the mac port, so will probably conditionally compile.
Comment 4 Kenneth Russell 2011-03-23 19:43:22 PDT 
Comment on attachment 86727 [details]
Patch

Looks fine to me. Why don't you upload the final version of the patch and I'll r+ that.
Comment 5 Chris Rogers 2011-03-24 14:13:14 PDT 
Created attachment 86835 [details]
Patch
Comment 6 Chris Rogers 2011-03-24 14:15:06 PDT 
AudioBus::createBySampleRateConverting() is now not compiled in for the mac port, since it's not needed there.

I added #if ENABLE(WEB_AUDIO)
to SincResampler which was missing it before.

For efficiency, the resampling now first mixes down to mono (if necessary)
Comment 7 Kenneth Russell 2011-03-24 16:15:34 PDT 
Looks good. r=me
Comment 8 Chris Rogers 2011-03-24 16:19:08 PDT 
Created attachment 86853 [details]
Patch
Comment 9 Chris Rogers 2011-03-24 16:20:54 PDT 
Sorry, Ken I just uploaded another patch with a very minor change to conditionally include:


#if !PLATFORM(MAC)
#include "SincResampler.h"
#endif
Comment 10 Kenneth Russell 2011-03-24 16:50:18 PDT 
Comment on attachment 86853 [details]
Patch

OK.
Comment 11 WebKit Commit Bot 2011-03-24 19:13:52 PDT 
Comment on attachment 86853 [details]
Patch

Clearing flags on attachment: 86853

Committed r81931: <http://trac.webkit.org/changeset/81931>
Comment 12 WebKit Commit Bot 2011-03-24 19:13:57 PDT 
All reviewed patches have been landed.  Closing bug.