[webkit-reviews] review denied: [Bug 77856] Have the DynamicsCompressorNode support multi-channel data : [Attachment 127876] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 21 13:22:11 PST 2012


Chris Rogers <crogers at google.com> has denied Raymond <rgbbones at gmail.com>'s
request for review:
Bug 77856: Have the DynamicsCompressorNode support multi-channel data
https://bugs.webkit.org/show_bug.cgi?id=77856

Attachment 127876: Patch
https://bugs.webkit.org/attachment.cgi?id=127876&action=review

------- Additional Comments from Chris Rogers <crogers at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=127876&action=review


> Source/WebCore/platform/audio/DynamicsCompressor.cpp:141
> +    float** destinationChannels = new float* [numberOfChannels];

Unfortunately it's very much against WebKit style to use raw "new" and "delete"
like this.  Here are two solutions I can think of:

1.  Revert back to the old way using simple local arrays, but define a
"maxNumberOfChannels" static constant for this class (and add ASSERTs for this
constant here and in setNumberOfChannels()
2.  Use WTF::OwnArrayPtr, then the code would be something like this

OwnArrayPtr<float*> destinationChannels = adoptArrayPtr(new float*
[numberOfChannels]);

Using (2) then you don't have to worry about calling delete [] as you have
below, since it automatically does this.  The reason WebKit prefers this is to
protect against possible leaks which can happen when using raw "new" and
"delete" calls.

If using (2) then it probably would be even better to allocate the
"sourceChannels" and "destinationChannels" variables as member variables which
are initialized in setNumberOfChannels() to avoid the constant
allocs/deallocs...


More information about the webkit-reviews mailing list