Make ConvolverNode conform to spec

third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp

Index: third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp b/third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp
index ca23bcfe9784dbffa7c003c9944e82e448bfab8c..85780cfe002aea6eb73a7a00d33cd9144250f2b9 100644
--- a/third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp
@@ -48,7 +48,7 @@ namespace blink {
 ConvolverHandler::ConvolverHandler(AudioNode& node, float sampleRate)
     : AudioHandler(NodeTypeConvolver, node, sampleRate), m_normalize(true) {
   addInput();
-  addOutput(2);
+  addOutput(1);

[hongchan, 2017/03/28 16:44:35]

Why 1? The default channel value is 1 and then can be changed later? Can we have
a comment why this starts from 1? The line below it says the channel count is 2.

[Raymond Toy, 2017/03/28 17:34:49]

We start the convolver with 1 output channel.  m_channelCount is really
unrelated because it's the channelCount that goes into the mixing rules for the
input to the convolver.

[hongchan, 2017/03/28 19:01:43]

Acknowledged.

 
   // Node-specific default mixing rules.
   m_channelCount = 2;
@@ -136,9 +136,10 @@ void ConvolverHandler::setBuffer(AudioBuffer* buffer,
   bufferBus->setSampleRate(buffer->sampleRate());
 
   // Create the reverb with the given impulse response.
-  std::unique_ptr<Reverb> reverb = WTF::wrapUnique(new Reverb(
-      bufferBus.get(), AudioUtilities::kRenderQuantumFrames, MaxFFTSize, 2,
-      context() && context()->hasRealtimeConstraint(), m_normalize));
+  std::unique_ptr<Reverb> reverb = WTF::wrapUnique(
+      new Reverb(bufferBus.get(), AudioUtilities::kRenderQuantumFrames,
+                 MaxFFTSize, numberOfChannels,
+                 context() && context()->hasRealtimeConstraint(), m_normalize));
 
   {
     // Synchronize with process().
@@ -177,6 +178,65 @@ double ConvolverHandler::latencyTime() const {
   return std::numeric_limits<double>::infinity();
 }
 
+void ConvolverHandler::setChannelCount(unsigned long channelCount,
+                                       ExceptionState& exceptionState) {
+  DCHECK(isMainThread());
+  BaseAudioContext::AutoLocker locker(context());
+
+  // channelCount must be 2.
+  if (channelCount != 2) {

[hongchan, 2017/03/28 16:44:35]

If this cannot be changed, why did we start from 1?

[Raymond Toy, 2017/03/28 17:34:49]

m_channelCount is initialized to 2.

[hongchan, 2017/03/28 19:01:43]

Acknowledged.

+    exceptionState.throwDOMException(
+        NotSupportedError,
+        "ConvolverNode: channelCount cannot be changed from 2");
+  }
+}
+
+void ConvolverHandler::setChannelCountMode(const String& mode,
+                                           ExceptionState& exceptionState) {
+  DCHECK(isMainThread());
+  BaseAudioContext::AutoLocker locker(context());
+
+  // channcelCountMode must be 'clamped-max'.
+  if (mode != "clamped-max") {
+    exceptionState.throwDOMException(
+        NotSupportedError,
+        "ConvolverNode: channelCountMode cannot be changed from 'clamped-max'");
+  }
+}
+
+void ConvolverHandler::checkNumberOfChannelsForInput(AudioNodeInput* input) {
+  DCHECK(context()->isAudioThread());
+#if DCHECK_IS_ON()
+  DCHECK(context()->isGraphOwner());
+#endif
+
+  DCHECK(input);
+  DCHECK_EQ(input, &this->input(0));
+  if (input != &this->input(0))
+    return;
+
+  if (m_buffer) {
+    unsigned numberOfChannels = input->numberOfChannels();
+    unsigned numReverbChannels = m_buffer->numberOfChannels();

[hongchan, 2017/03/28 16:44:35]

numReverbChannels => numberOfReverbChannels

[Raymond Toy, 2017/03/28 17:34:49]

I was following the style in other parts of this file, but
numberOfReverbChannels is better.

+    unsigned numOutputsDesired =

[hongchan, 2017/03/28 16:44:35]

Ditto. I think it's better to spell out as much as possible.

+        std::min(2u, std::max(numberOfChannels, numReverbChannels));
+
+    if (isInitialized() && numOutputsDesired != output(0).numberOfChannels()) {
+      // We're already initialized but the channel count has changed.
+      uninitialize();
+    }
+
+    if (!isInitialized()) {
+      // This will propagate the channel count to any nodes connected further
+      // downstream in the graph.
+      output(0).setNumberOfChannels(numOutputsDesired);
+      initialize();
+    }
+  }
+
+  // Update the input's internal bus if needed.
+  AudioHandler::checkNumberOfChannelsForInput(input);
+}
 // ----------------------------------------------------------------
 
 ConvolverNode::ConvolverNode(BaseAudioContext& context) : AudioNode(context) {