Complete the implementation of up/down-mixing rules for AudioNode

third_party/WebKit/Source/platform/audio/AudioBus.cpp

Index: third_party/WebKit/Source/platform/audio/AudioBus.cpp
diff --git a/third_party/WebKit/Source/platform/audio/AudioBus.cpp b/third_party/WebKit/Source/platform/audio/AudioBus.cpp
index 2b29e70916b8184cddcae076ec3fde3595e0316c..7cccb70431b58f0c477d26915932e2217e62ddb6 100644
--- a/third_party/WebKit/Source/platform/audio/AudioBus.cpp
+++ b/third_party/WebKit/Source/platform/audio/AudioBus.cpp
@@ -219,20 +219,31 @@ void AudioBus::copyFrom(const AudioBus& sourceBus, ChannelInterpretation channel
     unsigned numberOfSourceChannels = sourceBus.numberOfChannels();
     unsigned numberOfDestinationChannels = numberOfChannels();
 
-    if (numberOfDestinationChannels == numberOfSourceChannels) {
+    // If the channel numbers are equal, perform channels-wise copy.
+    if (numberOfSourceChannels == numberOfDestinationChannels) {
         for (unsigned i = 0; i < numberOfSourceChannels; ++i)
             channel(i)->copyFrom(sourceBus.channel(i));
-    } else {
-        switch (channelInterpretation) {
-        case Speakers:
-            speakersCopyFrom(sourceBus);
-            break;
-        case Discrete:
-            discreteCopyFrom(sourceBus);
-            break;
-        default:
-            ASSERT_NOT_REACHED();
-        }
+
+        return;
+    }
+
+    // Otherwise perform up/down-mix or the discrete transfer based on the
+    // number of channels and the channel interpretation.
+    switch (channelInterpretation) {
+    case Speakers:
+        // Copying effectively replaces the current bus content.
+        zero();

[Raymond Toy, 2016/03/08 18:04:07]

Don't quite understand why you need to zero the bus here.

[hongchan, 2016/03/08 23:53:35]

Because copying is basically zeroing-out and then summing into.

+
+        if (numberOfSourceChannels < numberOfDestinationChannels)
+            processUpMix(sourceBus);
+        else
+            processDownMix(sourceBus);
+        break;
+    case Discrete:
+        discreteCopyFrom(sourceBus);
+        break;
+    default:

[Raymond Toy, 2016/03/08 18:04:07]

This shouldn't be needed since channelInterpretation is a ChannelInterpretation
object.

I was expecting the compiler to warn that the default case doesn't happen.

[hongchan, 2016/03/08 23:53:35]

Then I will remove this.

+        ASSERT_NOT_REACHED();
     }
 }
 
@@ -244,131 +255,231 @@ void AudioBus::sumFrom(const AudioBus& sourceBus, ChannelInterpretation channelI
     unsigned numberOfSourceChannels = sourceBus.numberOfChannels();
     unsigned numberOfDestinationChannels = numberOfChannels();
 
-    if (numberOfDestinationChannels == numberOfSourceChannels) {
+    // If the channel numbers are equal, perform channels-wise summing.
+    if (numberOfSourceChannels == numberOfDestinationChannels) {
         for (unsigned i = 0; i < numberOfSourceChannels; ++i)
             channel(i)->sumFrom(sourceBus.channel(i));
-    } else {
-        switch (channelInterpretation) {
-        case Speakers:
-            speakersSumFrom(sourceBus);
-            break;
-        case Discrete:
-            discreteSumFrom(sourceBus);
-            break;
-        default:
-            ASSERT_NOT_REACHED();
-        }
+
+        return;
+    }
+
+    // Otherwise perform up/down-mix or the discrete transfer based on the
+    // number of channels and the channel interpretation.
+    switch (channelInterpretation) {
+    case Speakers:
+        if (numberOfSourceChannels < numberOfDestinationChannels)
+            processUpMix(sourceBus);
+        else
+            processDownMix(sourceBus);
+        break;
+    case Discrete:
+        discreteSumFrom(sourceBus);
+        break;
+    default:
+        ASSERT_NOT_REACHED();

[Raymond Toy, 2016/03/08 18:04:07]

This switch statement is almost identical to the switch above.  Can they be
refactored into one function?

[hongchan, 2016/03/08 23:53:35]

We discussed this offline and decided to keep them as they are. Will revisit
later when the optimization is needed.

     }
 }
 
-void AudioBus::speakersCopyFrom(const AudioBus& sourceBus)
+void AudioBus::processUpMix(const AudioBus& sourceBus)
 {
-    // FIXME: Implement down mixing 5.1 to stereo.
-    // https://bugs.webkit.org/show_bug.cgi?id=79192
-
     unsigned numberOfSourceChannels = sourceBus.numberOfChannels();
     unsigned numberOfDestinationChannels = numberOfChannels();
 
-    if (numberOfDestinationChannels == 2 && numberOfSourceChannels == 1) {
-        // Handle mono -> stereo case (for now simply copy mono channel into both left and right)
-        // FIXME: Really we should apply an equal-power scaling factor here, since we're effectively panning center...
+    // Up-mixing: 1 -> 2, 1 -> 4
+    //   output.L = input
+    //   output.R = input
+    //   output.SL = 0 (in the case of 1 -> 4)
+    //   output.SR = 0 (in the case of 1 -> 4)
+    if ((numberOfSourceChannels == 1 && numberOfDestinationChannels == 2) || (numberOfSourceChannels == 1 && numberOfDestinationChannels == 4)) {
         const AudioChannel* sourceChannel = sourceBus.channel(0);
-        channel(0)->copyFrom(sourceChannel);
-        channel(1)->copyFrom(sourceChannel);
-    } else if (numberOfDestinationChannels == 1 && numberOfSourceChannels == 2) {
-        // Handle stereo -> mono case. output = 0.5 * (input.L + input.R).
-        AudioBus& sourceBusSafe = const_cast<AudioBus&>(sourceBus);
+        channel(0)->sumFrom(sourceChannel);
+        channel(1)->sumFrom(sourceChannel);
 
-        const float* sourceL = sourceBusSafe.channelByType(ChannelLeft)->data();
-        const float* sourceR = sourceBusSafe.channelByType(ChannelRight)->data();
+        return;

[Raymond Toy, 2016/03/08 18:04:07]

Is it obvious that channels 2 and 3 are always zero in this case?  What if the
input node suddenly changed channel counts?

[hongchan, 2016/03/08 23:53:35]

All up/down-mixing is in-place summing. The destination will be zero when 1)
this gets called from 'copyFrom' or 2) the first connection is processed.

By the way, I believe all AudioBus operation happens in the audio thread and
will be safe from of 'audiograph change', which only happens in
DeferredTaskHandler running on the main thread. Right?

+    }
 
-        float* destination = channelByType(ChannelLeft)->mutableData();
-        vadd(sourceL, 1, sourceR, 1, destination, 1, length());
-        float scale = 0.5;
-        vsmul(destination, 1, &scale, destination, 1, length());
-    } else if (numberOfDestinationChannels == 6 && numberOfSourceChannels == 1) {
-        // Handle mono -> 5.1 case, copy mono channel to center.
-        channel(2)->copyFrom(sourceBus.channel(0));
-        channel(0)->zero();
-        channel(1)->zero();
-        channel(3)->zero();
-        channel(4)->zero();
-        channel(5)->zero();
-    } else if (numberOfDestinationChannels == 1 && numberOfSourceChannels == 6) {
-        // Handle 5.1 -> mono case.
-        zero();
-        speakersSumFrom5_1_ToMono(sourceBus);
-    } else {
-        // Fallback for unknown combinations.
-        discreteCopyFrom(sourceBus);
+    // Up-mixing: 1 -> 5.1
+    //   output.L = 0
+    //   output.R = 0
+    //   output.C = input (put in center channel)
+    //   output.LFE = 0
+    //   output.SL = 0
+    //   output.SR = 0
+    if (numberOfSourceChannels == 1 && numberOfDestinationChannels == 6) {
+        channel(2)->sumFrom(sourceBus.channel(0));
+
+        return;

[Raymond Toy, 2016/03/08 18:04:08]

Are the other channels always zero?  Same question for the cases below.

[hongchan, 2016/03/08 23:53:35]

Unlike the comment, all up/down-mixing is in-place summing. So leaving unused
channels is okay.

+    }
+
+    // Up-mixing: 2 -> 4, 2 -> 5.1
+    //   output.L = input.L
+    //   output.R = input.R
+    //   output.C = 0 (in the case of 2 -> 5.1)
+    //   output.LFE = 0 (in the case of 2 -> 5.1)
+    //   output.SL = 0
+    //   output.SR = 0
+    if ((numberOfSourceChannels == 2 && numberOfDestinationChannels == 4) || (numberOfSourceChannels == 2 && numberOfDestinationChannels == 6)) {
+        channel(0)->sumFrom(sourceBus.channel(0));
+        channel(1)->sumFrom(sourceBus.channel(1));
+
+        return;
     }
+
+    // Up-mixing: 4 -> 5.1
+    //   output.L = input.L
+    //   output.R = input.R
+    //   output.C = 0
+    //   output.LFE = 0
+    //   output.SL = input.SL
+    //   output.SR = input.SR
+    if (numberOfSourceChannels == 4 && numberOfDestinationChannels == 6) {
+        channel(0)->sumFrom(sourceBus.channel(0));
+        channel(1)->sumFrom(sourceBus.channel(1));
+        channel(4)->sumFrom(sourceBus.channel(2));
+        channel(5)->sumFrom(sourceBus.channel(3));
+
+        return;
+    }
+
+    // All other cases, fall back to the discrete sum. This will silence the
+    // excessive channels.
+    discreteSumFrom(sourceBus);
 }
 
-void AudioBus::speakersSumFrom(const AudioBus& sourceBus)
+void AudioBus::processDownMix(const AudioBus& sourceBus)
 {
-    // FIXME: Implement down mixing 5.1 to stereo.
-    // https://bugs.webkit.org/show_bug.cgi?id=79192
-
     unsigned numberOfSourceChannels = sourceBus.numberOfChannels();
     unsigned numberOfDestinationChannels = numberOfChannels();
 
-    if (numberOfDestinationChannels == 2 && numberOfSourceChannels == 1) {
-        // Handle mono -> stereo case (summing mono channel into both left and right).
-        const AudioChannel* sourceChannel = sourceBus.channel(0);
-        channel(0)->sumFrom(sourceChannel);
-        channel(1)->sumFrom(sourceChannel);
-    } else if (numberOfDestinationChannels == 1 && numberOfSourceChannels == 2) {
-        // Handle stereo -> mono case. output += 0.5 * (input.L + input.R).
-        AudioBus& sourceBusSafe = const_cast<AudioBus&>(sourceBus);
-
-        const float* sourceL = sourceBusSafe.channelByType(ChannelLeft)->data();
-        const float* sourceR = sourceBusSafe.channelByType(ChannelRight)->data();
+    // Down-mixing: 2 -> 1
+    //   output = 0.5 * (input.L + input.R)

[Raymond Toy, 2016/03/08 18:04:08]

It would be nice if this comment and the code used the same names: input vs
source; output vs destination.

[hongchan, 2016/03/08 23:53:35]

The situation is different with the test code - here I definitely want to keep
the source/destination because that is our convention throughout the AudioBus
and Channels.

I can change the comment, but I believe leaving as it is on the spec is nice.

+    if (numberOfSourceChannels == 2 && numberOfDestinationChannels == 1) {
+        AudioBus& sourceBusConst = const_cast<AudioBus&>(sourceBus);
+        const float* sourceL = sourceBusConst.channelByType(ChannelLeft)->data();
+        const float* sourceR = sourceBusConst.channelByType(ChannelRight)->data();
 
         float* destination = channelByType(ChannelLeft)->mutableData();

[Raymond Toy, 2016/03/08 18:04:07]

Although you didn't change this, I wonder why destination = channel(0) doesn't
work. There's only one channel in the destination, right?

[hongchan, 2016/03/08 23:53:35]

I believe Chris did this for the readability, but channel(0) makes sense as
well. I see this can go both ways.

         float scale = 0.5;
+
         vsma(sourceL, 1, &scale, destination, 1, length());

[Raymond Toy, 2016/03/08 18:04:07]

Is there some implicit guarantee that destination is zero before you start
summing in to it?

[hongchan, 2016/03/08 23:53:35]

All up/down-mixing is in-place summing. The destination will be zero when 1)
this gets called from 'copyFrom' or 2) the first connection is processed.

         vsma(sourceR, 1, &scale, destination, 1, length());
-    } else if (numberOfDestinationChannels == 6 && numberOfSourceChannels == 1) {
-        // Handle mono -> 5.1 case, sum mono channel into center.
-        channel(2)->sumFrom(sourceBus.channel(0));
-    } else if (numberOfDestinationChannels == 1 && numberOfSourceChannels == 6) {
-        // Handle 5.1 -> mono case.
-        speakersSumFrom5_1_ToMono(sourceBus);
-    } else {
-        // Fallback for unknown combinations.
-        discreteSumFrom(sourceBus);
+        return;
     }
-}
 
-void AudioBus::speakersSumFrom5_1_ToMono(const AudioBus& sourceBus)
-{
-    AudioBus& sourceBusSafe = const_cast<AudioBus&>(sourceBus);
+    // Down-mixing: 4 -> 1
+    //   output = 0.25 * (input.L + input.R + input.SL + input.SR)
+    if (numberOfSourceChannels == 4 && numberOfDestinationChannels == 1) {
+        AudioBus& sourceBusConst = const_cast<AudioBus&>(sourceBus);
+        const float* sourceL = sourceBusConst.channelByType(ChannelLeft)->data();
+        const float* sourceR = sourceBusConst.channelByType(ChannelRight)->data();
+        const float* sourceSL = sourceBusConst.channelByType(ChannelSurroundLeft)->data();
+        const float* sourceSR = sourceBusConst.channelByType(ChannelSurroundRight)->data();
 
-    const float* sourceL = sourceBusSafe.channelByType(ChannelLeft)->data();
-    const float* sourceR = sourceBusSafe.channelByType(ChannelRight)->data();
-    const float* sourceC = sourceBusSafe.channelByType(ChannelCenter)->data();
-    const float* sourceSL = sourceBusSafe.channelByType(ChannelSurroundLeft)->data();
-    const float* sourceSR = sourceBusSafe.channelByType(ChannelSurroundRight)->data();
+        float* destination = channelByType(ChannelLeft)->mutableData();

[Raymond Toy, 2016/03/08 18:04:07]

Is destination guaranteed to be zero?

[hongchan, 2016/03/08 23:53:35]

All up/down-mixing is in-place summing. The destination will be zero when 1)
this gets called from 'copyFrom' or 2) the first connection is processed.

+        float scale = 0.25;
 
-    float* destination = channelByType(ChannelLeft)->mutableData();
+        vsma(sourceL, 1, &scale, destination, 1, length());
+        vsma(sourceR, 1, &scale, destination, 1, length());
+        vsma(sourceSL, 1, &scale, destination, 1, length());
+        vsma(sourceSR, 1, &scale, destination, 1, length());
+        return;
+    }
 
-    AudioFloatArray temp(length());
-    float* tempData = temp.data();
+    // Down-mixing: 5.1 -> 1
+    //   output = sqrt(1/2) * (input.L + input.R) + input.C
+    //            + 0.5 * (input.SL + input.SR)
+    if (numberOfSourceChannels == 6 && numberOfDestinationChannels == 1) {
+        AudioBus& sourceBusConst = const_cast<AudioBus&>(sourceBus);
+        const float* sourceL = sourceBusConst.channelByType(ChannelLeft)->data();
+        const float* sourceR = sourceBusConst.channelByType(ChannelRight)->data();
+        const float* sourceC = sourceBusConst.channelByType(ChannelCenter)->data();
+        const float* sourceSL = sourceBusConst.channelByType(ChannelSurroundLeft)->data();
+        const float* sourceSR = sourceBusConst.channelByType(ChannelSurroundRight)->data();
 
-    // Sum in L and R.
-    vadd(sourceL, 1, sourceR, 1, tempData, 1, length());
-    float scale = 0.7071;
-    vsmul(tempData, 1, &scale, tempData, 1, length());
-    vadd(tempData, 1, destination, 1, destination, 1, length());
+        float* destination = channelByType(ChannelLeft)->mutableData();
+        float scaleSqrtHalf = sqrtf(0.5);
+        float scaleHalf = 0.5;
+
+        vsma(sourceL, 1, &scaleSqrtHalf, destination, 1, length());
+        vsma(sourceR, 1, &scaleSqrtHalf, destination, 1, length());
+        vadd(sourceC, 1, destination, 1, destination, 1, length());
+        vsma(sourceSL, 1, &scaleHalf, destination, 1, length());
+        vsma(sourceSR, 1, &scaleHalf, destination, 1, length());
+        return;
+    }
 
-    // Sum in SL and SR.
-    vadd(sourceSL, 1, sourceSR, 1, tempData, 1, length());
-    scale = 0.5;
-    vsmul(tempData, 1, &scale, tempData, 1, length());
-    vadd(tempData, 1, destination, 1, destination, 1, length());
+    // Down-mixing: 4 -> 2
+    //   output.L = 0.5 * (input.L + input.SL)
+    //   output.R = 0.5 * (input.R + input.SR)
+    if (numberOfSourceChannels == 4 && numberOfDestinationChannels == 2) {
+        AudioBus& sourceBusConst = const_cast<AudioBus&>(sourceBus);
+        const float* sourceL = sourceBusConst.channelByType(ChannelLeft)->data();
+        const float* sourceR = sourceBusConst.channelByType(ChannelRight)->data();
+        const float* sourceSL = sourceBusConst.channelByType(ChannelSurroundLeft)->data();
+        const float* sourceSR = sourceBusConst.channelByType(ChannelSurroundRight)->data();
+
+        float* destinationL = channelByType(ChannelLeft)->mutableData();
+        float* destinationR = channelByType(ChannelRight)->mutableData();

[Raymond Toy, 2016/03/08 18:04:07]

Again, are destinationL and destinationR guaranteed to contain 0?

[hongchan, 2016/03/08 23:53:35]

All up/down-mixing is in-place summing. The destination will be zero when 1)
this gets called from 'copyFrom' or 2) the first connection is processed.

+        float scaleHalf = 0.5;
+
+        vsma(sourceL, 1, &scaleHalf, destinationL, 1, length());
+        vsma(sourceSL, 1, &scaleHalf, destinationL, 1, length());
+        vsma(sourceR, 1, &scaleHalf, destinationR, 1, length());
+        vsma(sourceSR, 1, &scaleHalf, destinationR, 1, length());
+        return;
+    }
+
+    // Down-mixing: 5.1 -> 2
+    //   output.L = input.L + sqrt(1/2) * (input.C + input.SL)
+    //   output.R = input.R + sqrt(1/2) * (input.C + input.SR)
+    if (numberOfSourceChannels == 6 && numberOfDestinationChannels == 2) {
+        AudioBus& sourceBusConst = const_cast<AudioBus&>(sourceBus);
+        const float* sourceL = sourceBusConst.channelByType(ChannelLeft)->data();
+        const float* sourceR = sourceBusConst.channelByType(ChannelRight)->data();
+        const float* sourceC = sourceBusConst.channelByType(ChannelCenter)->data();
+        const float* sourceSL = sourceBusConst.channelByType(ChannelSurroundLeft)->data();
+        const float* sourceSR = sourceBusConst.channelByType(ChannelSurroundRight)->data();
+
+        float* destinationL = channelByType(ChannelLeft)->mutableData();
+        float* destinationR = channelByType(ChannelRight)->mutableData();

[Raymond Toy, 2016/03/08 18:04:07]

Are destinationL and destinationR guaranteed to be 0?

[hongchan, 2016/03/08 23:53:35]

All up/down-mixing is in-place summing. The destination will be zero when 1)
this gets called from 'copyFrom' or 2) the first connection is processed.

+        float scaleSqrtHalf = sqrtf(0.5);
+
+        vadd(sourceL, 1, destinationL, 1, destinationL, 1, length());
+        vsma(sourceC, 1, &scaleSqrtHalf, destinationL, 1, length());
+        vsma(sourceSL, 1, &scaleSqrtHalf, destinationL, 1, length());
+        vadd(sourceR, 1, destinationR, 1, destinationR, 1, length());
+        vsma(sourceC, 1, &scaleSqrtHalf, destinationR, 1, length());
+        vsma(sourceSR, 1, &scaleSqrtHalf, destinationR, 1, length());
+        return;
+    }
+
+    // Down-mixing: 5.1 -> 4
+    //   output.L = input.L + sqrt(1/2) * input.C
+    //   output.R = input.R + sqrt(1/2) * input.C
+    //   output.SL = input.SL
+    //   output.SR = input.SR
+    if (numberOfSourceChannels == 6 && numberOfDestinationChannels == 4) {
+        AudioBus& sourceBusConst = const_cast<AudioBus&>(sourceBus);
+        const float* sourceL = sourceBusConst.channelByType(ChannelLeft)->data();
+        const float* sourceR = sourceBusConst.channelByType(ChannelRight)->data();
+        const float* sourceC = sourceBusConst.channelByType(ChannelCenter)->data();
+
+        float* destinationL = channelByType(ChannelLeft)->mutableData();
+        float* destinationR = channelByType(ChannelRight)->mutableData();
+        float scaleSqrtHalf = sqrtf(0.5);
+
+        vadd(sourceL, 1, destinationL, 1, destinationL, 1, length());
+        vsma(sourceC, 1, &scaleSqrtHalf, destinationL, 1, length());
+        vadd(sourceR, 1, destinationR, 1, destinationR, 1, length());
+        vsma(sourceC, 1, &scaleSqrtHalf, destinationR, 1, length());
+        channel(2)->sumFrom(sourceBus.channel(4));
+        channel(3)->sumFrom(sourceBus.channel(5));
+        return;
+    }
 
-    // Sum in center.
-    vadd(sourceC, 1, destination, 1, destination, 1, length());
+    // All other cases, fall back to the discrete sum. This will perform
+    // channel-wise sum until the destination channels run out.
+    discreteSumFrom(sourceBus);
 }
 
 void AudioBus::discreteCopyFrom(const AudioBus& sourceBus)