Bug 80254 – Add a gyp option 'use_webaudio_ipp' to support webaudio optimization with IPP.

• Status: RESOLVED
• Resolution: INVALID
• Component: Web Audio

[Gao Chun]

Add a gyp option "use_webaudio_ipp" for building chromium/WebKit.

To enable all macro #if USE(WEBAUDIO_IPP), use:
   build/gyp_chromium -Duse_webaudio_ipp=1

To disable:
   build/gyp_chromium -Duse_webaudio_ipp=0
or
   build/gyp_chromium

[Gao Chun]

Created attachment 130081 [details]
Patch

[Raymond Toy]

Comment on attachment 130081 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130081&action=review

> Source/WebKit/chromium/features.gypi:158
> +      # Mac OS X uses Accelerate.framework FFT by default instead of FFmpeg.

nit: Maybe add additional comment here that Mac will not use IPP functions either?  It's a little confusing because IPP includes more than just the FFT.

[Gao Chun]

Created attachment 130225 [details]
Patch

[Gao Chun]

FIXED. Thanks!

(In reply to comment #2)
> (From update of attachment 130081 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130081&action=review
> 
> > Source/WebKit/chromium/features.gypi:158
> > +      # Mac OS X uses Accelerate.framework FFT by default instead of FFmpeg.
> 
> nit: Maybe add additional comment here that Mac will not use IPP functions either?  It's a little confusing because IPP includes more than just the FFT.

[Gao Chun]

Comment on attachment 130081 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130081&action=review

>>> Source/WebKit/chromium/features.gypi:158
>>> +      # Mac OS X uses Accelerate.framework FFT by default instead of FFmpeg.
>> 
>> nit: Maybe add additional comment here that Mac will not use IPP functions either?  It's a little confusing because IPP includes more than just the FFT.
> 
> 

Improved, Please review again.

[Raymond Toy]

Comment on attachment 130081 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130081&action=review

>>>> Source/WebKit/chromium/features.gypi:158
>>>> +      # Mac OS X uses Accelerate.framework FFT by default instead of FFmpeg.
>>> 
>>> nit: Maybe add additional comment here that Mac will not use IPP functions either?  It's a little confusing because IPP includes more than just the FFT.
>> 
>> 
> 
> Improved, Please review again.

I'm sorry, but I don't see that anything changed.  The comment is still the same as the original.  Did you forgot to upload the change or do you want to keep the original comment?

[Gao Chun]

Created attachment 131575 [details]
Patch

[Gao Chun]

(In reply to comment #6)
> (From update of attachment 130081 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130081&action=review
> 
> >>>> Source/WebKit/chromium/features.gypi:158
> >>>> +      # Mac OS X uses Accelerate.framework FFT by default instead of FFmpeg.
> >>> 
> >>> nit: Maybe add additional comment here that Mac will not use IPP functions either?  It's a little confusing because IPP includes more than just the FFT.
> >> 
> >> 
> > 
> > Improved, Please review again.
> 
> I'm sorry, but I don't see that anything changed.  The comment is still the same as the original.  Did you forgot to upload the change or do you want to keep the original comment?

New patch has been committed, pls review again, thanks!

[Raymond Toy]

Comment on attachment 131575 [details]
Patch

This looks good, but I'm not really familiar with gyp files.

[Chris Rogers]

Comment on attachment 131575 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=131575&action=review

> Source/WebKit/chromium/features.gypi:155
> +        'use_skia%': 1,

I assume this use-skia change was not supposed to be part of this patch?

[Gao Chun]

Created attachment 134730 [details]
Patch

[Gao Chun]

(In reply to comment #10)
> (From update of attachment 131575 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131575&action=review
> 
> > Source/WebKit/chromium/features.gypi:155
> > +        'use_skia%': 1,
> 
> I assume this use-skia change was not supposed to be part of this patch?

I have removed this and uploaded a new patch. Thanks.

[Gao Chun]

Any feedback?

[Darin Adler]

Chromium-specific, it seems.