Changeset 202845 in webkit

Timestamp:

Jul 5, 2016 9:11:44 PM (8 years ago)

Author:

Brent Fulgham

Message:

Throw exceptions for invalid number of channels for ConvolverNode
<​https://webkit.org/b/159238>

Patch by David Kilzer <​ddkilzer@apple.com> on 2016-07-05
Reviewed by Brent Fulgham.

Source/WebCore:

Fix based on a Blink change (patch by <​rtoy@chromium.org>):
<​https://chromium.googlesource.com/chromium/src.git/+/0cc26bbb7175aec77910d0b47faf9f8c8a640fe5>

Also includes a related fix for ReverbConvolverStage (patch by <​rtoy@chromium.org>):
<​https://src.chromium.org/viewvc/blink?revision=157832&view=revision>

Test: webaudio/convolver-channels.html

• Modules/webaudio/ConvolverNode.cpp:

(WebCore::ConvolverNode::setBuffer): Throw an exception for
anything but 1, 2 or 4 channels.

• platform/audio/ReverbConvolverStage.cpp:

(WebCore::ReverbConvolverStage::ReverbConvolverStage): Don't read past the end of
the impulseResponse array.

LayoutTests:

Test based on a Blink change (patch by <​rtoy@chromium.org>):
<​https://chromium.googlesource.com/chromium/src.git/+/0cc26bbb7175aec77910d0b47faf9f8c8a640fe5>

compatibility.js based on a Blink change (patch by <Raymond Toy>):
<​https://chromium.googlesource.com/chromium/src.git/+/f846f5a461d1fcdbe5152898576c125058079ed1>

• webaudio/convolver-channels-expected.txt: Added.
• webaudio/convolver-channels.html: Added.
• webaudio/resources/compatibility.js: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/webaudio/convolver-channels-expected.txt (added)
• LayoutTests/webaudio/convolver-channels.html (added)
• LayoutTests/webaudio/resources/compatibility.js (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/webaudio/ConvolverNode.cpp (modified) (2 diffs)
• Source/WebCore/platform/audio/ReverbConvolverStage.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-07-05  David Kilzer  <ddkilzer@apple.com>
+
+        Throw exceptions for invalid number of channels for ConvolverNode
+        <https://webkit.org/b/159238>
+
+        Reviewed by Brent Fulgham.
+
+        Test based on a Blink change (patch by <rtoy@chromium.org>):
+        <https://chromium.googlesource.com/chromium/src.git/+/0cc26bbb7175aec77910d0b47faf9f8c8a640fe5>
+
+        compatibility.js based on a Blink change (patch by <rtoy@google.com>):
+        <https://chromium.googlesource.com/chromium/src.git/+/f846f5a461d1fcdbe5152898576c125058079ed1>
+
+        * webaudio/convolver-channels-expected.txt: Added.
+        * webaudio/convolver-channels.html: Added.
+        * webaudio/resources/compatibility.js: Added.
+
 2016-07-05  Johan K. Jensen  <jj@johanjensen.dk>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-07-05  David Kilzer  <ddkilzer@apple.com>
+
+        Throw exceptions for invalid number of channels for ConvolverNode
+        <https://webkit.org/b/159238>
+
+        Reviewed by Brent Fulgham.
+
+        Fix based on a Blink change (patch by <rtoy@chromium.org>):
+        <https://chromium.googlesource.com/chromium/src.git/+/0cc26bbb7175aec77910d0b47faf9f8c8a640fe5>
+
+        Also includes a related fix for ReverbConvolverStage (patch by <rtoy@chromium.org>):
+        <https://src.chromium.org/viewvc/blink?revision=157832&view=revision>
+
+        Test: webaudio/convolver-channels.html
+
+        * Modules/webaudio/ConvolverNode.cpp:
+        (WebCore::ConvolverNode::setBuffer): Throw an exception for
+        anything but 1, 2 or 4 channels.
+        * platform/audio/ReverbConvolverStage.cpp:
+        (WebCore::ReverbConvolverStage::ReverbConvolverStage): Don't read past the end of
+        the impulseResponse array.
+
 2016-07-05  Johan K. Jensen  <jj@johanjensen.dk>
 

trunk/Source/WebCore/Modules/webaudio/ConvolverNode.cpp

@@ -1 +1 @@
 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
+ * Copyright (C) 2016, Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -132 +133 @@
     size_t bufferLength = buffer->length();
 
-    // The current implementation supports up to four channel impulse responses, which are interpreted as true-stereo (see Reverb class).
-    bool isBufferGood = numberOfChannels > 0 && numberOfChannels <= 4 && bufferLength;
-    ASSERT(isBufferGood);
-    if (!isBufferGood)
+    // The current implementation supports only 1-, 2-, or 4-channel impulse responses, with the
+    // 4-channel response being interpreted as true-stereo (see Reverb class).
+    bool isChannelCountGood = (numberOfChannels == 1 || numberOfChannels == 2 || numberOfChannels == 4) && bufferLength;
+
+    if (!isChannelCountGood) {
+        ec = NOT_SUPPORTED_ERR;
         return;
+    }
 
     // Wrap the AudioBuffer by an AudioBus. It's an efficient pointer set and not a memcpy().

trunk/Source/WebCore/platform/audio/ReverbConvolverStage.cpp

@@ -58 +58 @@
         m_fftConvolver = std::make_unique<FFTConvolver>(fftSize);
     } else {
+        ASSERT(!stageOffset);
+        ASSERT(stageLength <= fftSize / 2);
+
         m_directKernel = std::make_unique<AudioFloatArray>(fftSize / 2);
-        m_directKernel->copyToRange(impulseResponse + stageOffset, 0, fftSize / 2);
+        m_directKernel->copyToRange(impulseResponse, 0, stageLength);
         m_directConvolver = std::make_unique<DirectConvolver>(renderSliceSize);
     }