Support multichannel audio stream more than 2 in MediaStreamDestinationNode

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

Index: third_party/WebKit/Source/modules/webaudio/MediaStreamAudioDestinationNode.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/MediaStreamAudioDestinationNode.cpp b/third_party/WebKit/Source/modules/webaudio/MediaStreamAudioDestinationNode.cpp
index be69b5c8d497a621efd9ceb3b3876068927a129c..0ec17fa2be37a1bd2d921c36aca023a59fe5f67a 100644
--- a/third_party/WebKit/Source/modules/webaudio/MediaStreamAudioDestinationNode.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/MediaStreamAudioDestinationNode.cpp
@@ -26,6 +26,9 @@
 #if ENABLE(WEB_AUDIO)
 #include "modules/webaudio/MediaStreamAudioDestinationNode.h"
 
+#include "bindings/core/v8/ExceptionMessages.h"
+#include "bindings/core/v8/ExceptionState.h"
+#include "core/dom/ExceptionCode.h"
 #include "modules/webaudio/AbstractAudioContext.h"
 #include "modules/webaudio/AudioNodeInput.h"
 #include "platform/UUID.h"
@@ -35,6 +38,10 @@
 
 namespace blink {
 
+// WebAudioCapturerSource ignores the channel count beyond 8, so we set the
+// block here to avoid anything can cause the crash.
+static unsigned long kMaxChannelCount = 8;
+
 MediaStreamAudioDestinationHandler::MediaStreamAudioDestinationHandler(AudioNode& node, size_t numberOfChannels)
     : AudioBasicInspectorHandler(NodeTypeMediaStreamAudioDestination, node, node.context()->sampleRate(), numberOfChannels)
     , m_mixBus(AudioBus::create(numberOfChannels, ProcessingSizeInFrames))
@@ -63,10 +70,52 @@ MediaStreamAudioDestinationHandler::~MediaStreamAudioDestinationHandler()
 
 void MediaStreamAudioDestinationHandler::process(size_t numberOfFrames)
 {
+    // Conform the input bus into the internal mix bus, which represents
+    // MediaStreamDestination's channel count.
     m_mixBus->copyFrom(*input(0).bus());
+

[Raymond Toy, 2015/11/20 14:47:18]

Why a new blank line here?

[hongchan, 2015/11/20 18:02:59]

Because the comment above only applies to the line 75.

[Raymond Toy, 2015/11/30 18:06:09]

Acknowledged.

     m_source->consumeAudio(m_mixBus.get(), numberOfFrames);
 }
 
+void MediaStreamAudioDestinationHandler::setChannelCount(unsigned long channelCount, ExceptionState& exceptionState)
+{
+    ASSERT(isMainThread());
+
+    // Currently the maximum channel count supported for this node is 8,
+    // which is constrained by m_source (WebAudioCapturereSource). Although
+    // it has its own safety check for the excessive channels, throwing an
+    // exception here is useful to developers.
+    if (channelCount > maxChannelCount()) {
+        exceptionState.throwDOMException(
+            IndexSizeError,
+            ExceptionMessages::indexOutsideRange<unsigned>("channel count",
+                channelCount,
+                1,
+                ExceptionMessages::InclusiveBound,
+                maxChannelCount(),
+                ExceptionMessages::InclusiveBound));
+        return;
+    }
+
+    unsigned long oldChannelCount = this->channelCount();
+    AudioHandler::setChannelCount(channelCount, exceptionState);
+
+    // Update the pipeline wit the new channel count only if absolutely

[Raymond Toy, 2015/11/20 14:47:18]

Typo: "wit" -> "with"

[hongchan, 2015/11/20 18:02:59]

Done.

+    // necessary.
+    //
+    // TODO(hongchan): There might be a data race here since both threads
+    // have access to m_mixBus.

[Raymond Toy, 2015/11/20 14:47:18]

I think you should lock the context here. Or maybe a process lock should be
added so that you don't change the m_mixBus out from under the process()
function above.

[hongchan, 2015/11/20 18:02:59]

Yes I agree. I will add the lock here. However, note that
DefaultAudioDestinatioNode::setChannelCount also does not have a lock in the
same scenario.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Perhaps it is safe because of stopping/recreating the destination?

+    if (!exceptionState.hadException() && this->channelCount() != oldChannelCount && isInitialized()) {
+        m_mixBus = AudioBus::create(channelCount, ProcessingSizeInFrames);
+        m_source->setAudioFormat(channelCount, context()->sampleRate());
+    }
+}
+
+unsigned long MediaStreamAudioDestinationHandler::maxChannelCount() const

[Raymond Toy, 2015/11/20 14:47:18]

Is this used anywhere else?  If not, is there a reason not to use
kMaxChannelCount directly?

[hongchan, 2015/11/20 18:02:59]

I don't have a strong opinion on this and I am simply following conventions. You
prefer to have kMaxChannelCount rather than a getter?

[Raymond Toy, 2015/11/30 18:06:09]

Up to you.  This isn't speed-critical, so I'm fine with it.  Getters are
probably better in general.

+{
+    return kMaxChannelCount;
+}
+
 // ----------------------------------------------------------------
 
 MediaStreamAudioDestinationNode::MediaStreamAudioDestinationNode(AbstractAudioContext& context, size_t numberOfChannels)