[Webkit-unassigned] [Bug 69703] HRTF Database consolidation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69703





--- Comment #5 from Chris Rogers <crogers at google.com>  2011-10-27 17:16:28 PST ---
(From update of attachment 110272)
View in context: https://bugs.webkit.org/attachment.cgi?id=110272&action=review

> Source/WebCore/platform/audio/HRTFElevation.cpp:55
> +static AudioBusMap audioBusMap;

I think it's against WebKit style to allow statically initialized C++ objects.  I think we'll need to make audioBusMap be a pointer to AudioBusMap, and lazily initialize it at first use.

> Source/WebCore/platform/audio/HRTFElevation.cpp:108
> +    AudioBus* bus;

I would suggest breaking out the code from lines 108 - 125 into a separate function/method which returns the AudioBus*

> Source/WebCore/platform/audio/HRTFElevation.cpp:119
> +    size_t expectedLength = static_cast<size_t>(240 * 256 * (sampleRate / 44100.0));

Please don't use magic constant 240. instead define a constant value at the top of the file, such as:

const size_t totalNumberOfResponses = 240;

We should define similar constants for 256, and 44100.0   --  also need to get rid of .0 so   44100.0 -> 44100

> Source/WebCore/platform/audio/HRTFElevation.cpp:127
> +    int elevationsPerAzimuth = 10;

Can we use the already defined constant HRTFDatabase::NumberOfRawElevations instead of defining a new one?

> Source/WebCore/platform/audio/HRTFElevation.cpp:132
> +    int index = ((azimuth / AzimuthSpacing) * elevationsPerAzimuth) + elevationIndex;

Can we add a sanity check on the "index" value?

> Source/WebCore/platform/audio/HRTFElevation.cpp:133
> +    int length = 256;

Use same constant for 256 that we define as on line 119.

> Source/WebCore/platform/audio/HRTFElevation.cpp:139
> +    AudioChannel* rightEarImpulseResponse = new AudioChannel(length);

To avoid a memory leak and to follow WebKit style, I believe you'll need to use OwnPtr<AudioChannel> here, something like:

OwnPtr<AudioChannel> leftEarImpulseResponse = adoptPtr(new AudioChannel(length));
OwnPtr<AudioChannel> rightEarImpulseResponse = adoptPtr(new AudioChannel(length));

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.