[Chromoting] Implement down mixing in AudioPump

remoting/proto/audio.proto

Index: remoting/proto/audio.proto
diff --git a/remoting/proto/audio.proto b/remoting/proto/audio.proto
index f64daaed748a1492f3fc7999a017e68a4001006c..c522db55fbf5fd567160441968b5e3d47dbe36e0 100644
--- a/remoting/proto/audio.proto
+++ b/remoting/proto/audio.proto
@@ -47,6 +47,12 @@ message AudioPacket {
     CHANNELS_INVALID = -1;
     CHANNELS_MONO = 1;
     CHANNELS_STEREO = 2;
+    CHANNELS_SURROUND = 3;
+    CHANNELS_QUAD = 4;

[Sergey Ulanov, 2017/05/31 00:44:43]

There are several different speaker layouts for 3 or 4 speakers. See
https://codesearch.chromium.org/chromium/src/media/base/channel_layout.h

E.g. for 4 channels it can be (L, R, C, LFE), or (L, R, BL, BR), or (L, R, C,
BC)

So speaker layout is not uniquely identified by number of channels.

There are several approaches we could take
1. Extend this struct to identify broader set of speaker layouts, either by
adding a layout field, or by renaming channels field to layout (channels count
can be deduced from layout).
2. Limit support to widely used speaker layouts, i.e. 5.1 and 7.1 in addition to
stereo.
3. Use a different struct for non-encoded frames. Then it will be possible to
use media::ChannelLayout enum to identify various layouts.

I think (2) is close what you had initially, and it seems reasonable to me. We
just need to have clear expectations on how the capturer will work on systems
with other speaker configurations.

Or if you think we want to support all possible layouts then (3) is the right
approach.

[Hzj_jie, 2017/05/31 02:49:28]

Yes, I simply choose the most common used configurations. And both 5.1 and 7.1
also have different configurations. Windows supports from 1 to 8 channels, while
supporting 3 / 4 / 5 / 7 channels in CRD is still better than nothing. Since we
will downmix anyway, layouts is not that important. In ChannelMixingMatrix
(https://cs.chromium.org/chromium/src/media/base/channel_mixing_matrix.cc?type...),
the layout impacts only which channels are merged. Using 4 channels as an
example,
L / R / C / BC will be merged to,
(L + C + BC) / (R + C + BC)
L / R / BL / BR will be merged to,
(L + BL) / (R + BR)
If we always use L / R / C / BC, we will merge L / R / BL / BR layout into,
(L + BL + BR) / (R + BL + BR)
which IMO is still acceptable for remote desktop usage. Better than nothing.

I also believe the count of users who require multiple channels is limited,
using approach (3) is not worthy.

[Sergey Ulanov, 2017/05/31 18:37:23]

This may work (though it's not ideal because it gives suboptimal experience for
a more common quadraphonic configuration). But
it needs to be documented somewhere, here or in the capturer. Also, if we treat
all 4-channel layouts as L/R/C/BC, then it shouldn't be called CHANNELS_QUAD.
Quadraphonic layout is L/R/BL/BR.
Maybe call these CHANNELS_SURROUND_3 and CHANNELS_SURROUND_4?
As we discussed before it is suboptimal that we currently use protobufs outside
of the protocol layer, regardless of multichannel audio support.
BTW, potentially we could use media::AudioBus to represent unecoded frames. That
would also make the encoder more efficient by avoiding unnecessary float->int
conversion. The encoder already uses AudioBus internally for resampling, WDYT?

[Hzj_jie, 2017/05/31 19:23:19]

I will add comments to both AudioCapturerWin and AudioPump: the capturer treats
all the 4-channel configurations as QUAD, while the pump does the downmixing.
Yes, it was my original plan (indeed the branch name of this change is called
"return-audio-bus"). But then I found it was not worthy due to the very limited
user cases. But for the performance concern, yes, I think it's still a good
solution.
IMO, this change is still reasonable to solve the users' need. How about let's
submit this change first and optimize in a different change? Then we may not be
that urgent: after all, converting 48k * 8 int16s from or to floats per second
is not a big deal for a modern CPU.

+    CHANNELS_4_1 = 5;
+    CHANNELS_5_1 = 6;
+    CHANNELS_6_1 = 7;
+    CHANNELS_7_1 = 8;
   }
 
   optional Channels channels = 6 [default = CHANNELS_INVALID];