Make ConvolverNode conform to spec

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

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 30 matching lines @@
 // a good value.  But, the Reverb object is multi-threaded, so we want this as
 // high as possible without losing too much accuracy.  Very large FFTs will have
 // worse phase errors. Given these constraints 32768 is a good compromise.
 const size_t MaxFFTSize = 32768;
 
 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;
   setInternalChannelCountMode(ClampedMax);
   setInternalChannelInterpretation(AudioBus::Speakers);
 
   initialize();
 }
 
 PassRefPtr<ConvolverHandler> ConvolverHandler::create(AudioNode& node,
@@ skipping 67 matching lines @@
   // reference to it is kept for later use in that class.
   RefPtr<AudioBus> bufferBus =
       AudioBus::create(numberOfChannels, bufferLength, false);
   for (unsigned i = 0; i < numberOfChannels; ++i)
     bufferBus->setChannelMemory(i, buffer->getChannelData(i)->data(),
                                 bufferLength);
 
   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().
     MutexLocker locker(m_processLock);
     m_reverb = std::move(reverb);
     m_buffer = buffer;
   }
 }
 
 AudioBuffer* ConvolverHandler::buffer() {
@@ skipping 18 matching lines @@
   if (tryLocker.locked())
     return m_reverb
                ? m_reverb->latencyFrames() /
                      static_cast<double>(context()->sampleRate())
                : 0;
   // Since we don't want to block the Audio Device thread, we return a large
   // value instead of trying to acquire the lock.
   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) {
   setHandler(ConvolverHandler::create(*this, context.sampleRate()));
 }
 
 ConvolverNode* ConvolverNode::create(BaseAudioContext& context,
                                      ExceptionState& exceptionState) {
   DCHECK(isMainThread());
 
@@ skipping 38 matching lines @@
 
 bool ConvolverNode::normalize() const {
   return convolverHandler().normalize();
 }
 
 void ConvolverNode::setNormalize(bool normalize) {
   convolverHandler().setNormalize(normalize);
 }
 
 }  // namespace blink