Bug 69292 – WEB_AUDIO does not compile on Leopard 32-bit.

• Component: Web Audio
• Status: RESOLVED FIXED

[Jer Noble]

WEB_AUDIO fails to compile on the Leopard bots due to many 32<->64 casting warnings.

http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Build%29/builds/40113/steps/compile-webkit/logs/warnings%20%28178%29

[Jer Noble]

Created attachment 109695 [details]
Patch

[Jer Noble]

Created attachment 109696 [details]
Patch

This time, without the changes to project.pbxproj and SVGResourcePattern.cpp

[Chris Rogers]

+kbr

Thanks Jer, I'm looking through it now.  I'm also applying the patch and measuring any performance difference in the DynamicsCompressor.  I don't expect to see too much difference but I just want to do a quick sanity check.

[Chris Rogers]

I've run a few performance tests and it seems like I'm seeing some performance impact due to switching everything to double precision in ZeroPole.cpp and DynamicsCompressorKernel.cpp.  It worries me a bit, so I think it's probably a better idea to uniformly switch both of these over to using "float" everywhere in these places.  Sorry, I know I suggested you go the double route, but after having tested the performance...

As far as the DoublePoint3D - it probably is fine either way (using double as in your patch or switching everything to float).  Performance will not be an issue here.  I would go with DoublePoint3D if you think it will be useful to have this used in other parts of WebCore.

[Jer Noble]

Created attachment 109814 [details]
Patch

Made the changes Chris Rogers suggested, namely operating as much as possible as floats.

[WebKit Review Bot]

Attachment 109814 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/audio/Panner.h:50:  The parameter name "model" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/audio/HRTFKernel.h:80:  The parameter name "channel" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 89 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 109814 [details]
Patch

Attachment 109814 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9958324

[Jer Noble]

Created attachment 109823 [details]
Patch

[WebKit Review Bot]

Comment on attachment 109823 [details]
Patch

Attachment 109823 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9942782

[Chris Rogers]

patch seems pretty good so far -- I noticed it fails the cr-linux bots since some small changes are necessary to AudioDestinationChromium.cpp (due to sampleRate changing to float).
Could you please change AudioDestinationChromium?


In file included from /Volumes/Channel/Source/ChromiumTrunk/src/third_party/WebKit/Source/WebKit/chromium/src/AudioDestinationChromium.cpp:33:
src/AudioDestinationChromium.h:52:12: error: virtual function 'sampleRate' has a different return type ('double') than the function it overrides (which has return type 'float')
    double sampleRate() const { return m_sampleRate; }
           ^
../../WebCore/platform/audio/AudioDestination.h:53:19: note: overridden virtual function is here
    virtual float sampleRate() const = 0;
                  ^
/Volumes/Channel/Source/ChromiumTrunk/src/third_party/WebKit/Source/WebKit/chromium/src/AudioDestinationChromium.cpp:53:48: error: out-of-line definition of 'create' does not match any declaration in 'WebCore::AudioDestination'
PassOwnPtr<AudioDestination> AudioDestination::create(AudioSourceProvider& provider, double sampleRate)
                                               ^~~~~~
/Volumes/Channel/Source/ChromiumTrunk/src/third_party/WebKit/Source/WebKit/chromium/src/AudioDestinationChromium.cpp:104:26: error: out-of-line definition of 'WebCore::AudioDestination::hardwareSampleRate' differs from the declaration in the return type
double AudioDestination::hardwareSampleRate()
                         ^
../../WebCore/platform/audio/AudioDestination.h:54:18: note: previous declaration is here
    static float hardwareSampleRate();
                 ^
3 errors generated.

[Jer Noble]

Created attachment 109839 [details]
Patch

[Chris Rogers]

This looks fine to me.  There's one layout test "webaudio/audiobuffersource-playbackrate.html" which I'll need to rebaseline after this (I'll do it).  The test is audibly identical, but differs in the fringes due to the precision changes introduced in this patch.

All audio samples run fine - performance is fine

[Chris Rogers]

By the way, can you tell me how I can set my Xcode project to see these types of Leopard 32-bit warnings? I'd rather not bother you guys too much as code is modified and new code added...

[Jer Noble]

(In reply to comment #13)
> By the way, can you tell me how I can set my Xcode project to see these types of Leopard 32-bit warnings? I'd rather not bother you guys too much as code is modified and new code added...

You need to set the compiler to GCC and build 32-bit.  In Xcode 4, select the WebCore project, and choose the WebCore Build Settings panel.  The option to choose the compiler is in Build Options / Compiler for C / C++ / Objective-C, and LLVM GCC 4.2 should be an option.

[Simon Fraser (smfr)]

Comment on attachment 109839 [details]
Patch

rs=me

[Jer Noble]

Committed r96745: <http://trac.webkit.org/changeset/96745>