[Chromoting] Implement down mixing in AudioPump

remoting/host/audio_capturer_win.cc

Index: remoting/host/audio_capturer_win.cc
diff --git a/remoting/host/audio_capturer_win.cc b/remoting/host/audio_capturer_win.cc
index 2610e6e2b692eed2523b0dfe2047d3222b379996..3dd6dc7b7c1d2226e74742b425c1d661cd86b8c1 100644
--- a/remoting/host/audio_capturer_win.cc
+++ b/remoting/host/audio_capturer_win.cc
@@ -21,7 +21,6 @@
 #include "remoting/host/win/default_audio_device_change_detector.h"
 
 namespace {
-const int kChannels = 2;
 const int kBytesPerSample = 2;
 const int kBitsPerSample = kBytesPerSample * 8;
 // Conversion factor from 100ns to 1ms.
@@ -39,6 +38,7 @@ const int kMinTimerInterval = 30;
 // Upper bound for the timer precision error, in milliseconds.
 // Timers are supposed to be accurate to 20ms, so we use 30ms to be safe.
 const int kMaxExpectedTimerLag = 30;
+
 }  // namespace
 
 namespace remoting {
@@ -150,58 +150,50 @@ bool AudioCapturerWin::Initialize() {
     return false;
   }
 
-  // Set the wave format
-  switch (wave_format_ex_->wFormatTag) {
-    case WAVE_FORMAT_IEEE_FLOAT:
-      // Intentional fall-through.
-    case WAVE_FORMAT_PCM:
-      if (!AudioCapturer::IsValidSampleRate(wave_format_ex_->nSamplesPerSec)) {
-        LOG(ERROR) << "Host sampling rate is neither 44.1 kHz nor 48 kHz.";
-        return false;
-      }
-      sampling_rate_ = static_cast<AudioPacket::SamplingRate>(
-          wave_format_ex_->nSamplesPerSec);
-
-      wave_format_ex_->wFormatTag = WAVE_FORMAT_PCM;
-      wave_format_ex_->nChannels = kChannels;
-      wave_format_ex_->wBitsPerSample = kBitsPerSample;
-      wave_format_ex_->nBlockAlign = kChannels * kBytesPerSample;
-      wave_format_ex_->nAvgBytesPerSec =
-          sampling_rate_ * kChannels * kBytesPerSample;
-      break;
-    case WAVE_FORMAT_EXTENSIBLE: {
-      PWAVEFORMATEXTENSIBLE wave_format_extensible =
-          reinterpret_cast<WAVEFORMATEXTENSIBLE*>(
-          static_cast<WAVEFORMATEX*>(wave_format_ex_));
-      if (IsEqualGUID(KSDATAFORMAT_SUBTYPE_IEEE_FLOAT,
-                      wave_format_extensible->SubFormat)) {
-        if (!AudioCapturer::IsValidSampleRate(
-                wave_format_extensible->Format.nSamplesPerSec)) {
-          LOG(ERROR) << "Host sampling rate is neither 44.1 kHz nor 48 kHz.";
-          return false;
-        }
-        sampling_rate_ = static_cast<AudioPacket::SamplingRate>(
-            wave_format_extensible->Format.nSamplesPerSec);
-
-        wave_format_extensible->SubFormat = KSDATAFORMAT_SUBTYPE_PCM;
-        wave_format_extensible->Samples.wValidBitsPerSample = kBitsPerSample;
-
-        wave_format_extensible->Format.nChannels = kChannels;
-        wave_format_extensible->Format.nSamplesPerSec = sampling_rate_;
-        wave_format_extensible->Format.wBitsPerSample = kBitsPerSample;
-        wave_format_extensible->Format.nBlockAlign =
-            kChannels * kBytesPerSample;
-        wave_format_extensible->Format.nAvgBytesPerSec =
-            sampling_rate_ * kChannels * kBytesPerSample;
-      } else {
-        LOG(ERROR) << "Failed to force 16-bit samples";
-        return false;
-      }
-      break;
-    }
-    default:
-      LOG(ERROR) << "Failed to force 16-bit PCM";
+  if (wave_format_ex_->wFormatTag != WAVE_FORMAT_IEEE_FLOAT &&
+      wave_format_ex_->wFormatTag != WAVE_FORMAT_PCM &&
+      wave_format_ex_->wFormatTag != WAVE_FORMAT_EXTENSIBLE) {
+    LOG(ERROR) << "Failed to force 16-bit PCM";
+    return false;
+  }
+
+  if (!AudioCapturer::IsValidSampleRate(wave_format_ex_->nSamplesPerSec)) {
+    LOG(ERROR) << "Host sampling rate is neither 44.1 kHz nor 48 kHz. "
+               << wave_format_ex_->nSamplesPerSec;
+    return false;
+  }
+
+  // We support from mono to 7.1. This check should be consistent with
+  // AudioPacket::Channels.
+  if (wave_format_ex_->nChannels > 8 || wave_format_ex_->nChannels <= 0) {
+    LOG(ERROR) << "Unsupported channels " << wave_format_ex_->nChannels;
+    return false;
+  }
+
+  sampling_rate_ = static_cast<AudioPacket::SamplingRate>(
+      wave_format_ex_->nSamplesPerSec);
+
+  wave_format_ex_->wBitsPerSample = kBitsPerSample;
+  wave_format_ex_->nBlockAlign = wave_format_ex_->nChannels * kBytesPerSample;
+  wave_format_ex_->nAvgBytesPerSec =
+      sampling_rate_ * wave_format_ex_->nBlockAlign;
+
+  if (wave_format_ex_->wFormatTag == WAVE_FORMAT_EXTENSIBLE) {
+    PWAVEFORMATEXTENSIBLE wave_format_extensible =
+        reinterpret_cast<WAVEFORMATEXTENSIBLE*>(
+        static_cast<WAVEFORMATEX*>(wave_format_ex_));
+    if (!IsEqualGUID(KSDATAFORMAT_SUBTYPE_IEEE_FLOAT,
+                     wave_format_extensible->SubFormat) &&
+        !IsEqualGUID(KSDATAFORMAT_SUBTYPE_PCM,
+                     wave_format_extensible->SubFormat)) {
+      LOG(ERROR) << "Failed to force 16-bit samples";
       return false;
+    }
+
+    wave_format_extensible->SubFormat = KSDATAFORMAT_SUBTYPE_PCM;
+    wave_format_extensible->Samples.wValidBitsPerSample = kBitsPerSample;
+  } else {
+    wave_format_ex_->wFormatTag = WAVE_FORMAT_PCM;
   }
 
   // Initialize the IAudioClient.
@@ -233,7 +225,7 @@ bool AudioCapturerWin::Initialize() {
   }
 
   volume_filter_.ActivateBy(mm_device_.Get());
-  volume_filter_.Initialize(sampling_rate_, kChannels);
+  volume_filter_.Initialize(sampling_rate_, wave_format_ex_->nChannels);
 
   return true;
 }
@@ -280,7 +272,11 @@ void AudioCapturerWin::DoCapture() {
       packet->set_encoding(AudioPacket::ENCODING_RAW);
       packet->set_sampling_rate(sampling_rate_);
       packet->set_bytes_per_sample(AudioPacket::BYTES_PER_SAMPLE_2);
-      packet->set_channels(AudioPacket::CHANNELS_STEREO);
+      // Only the count of channels is taken into account now, we should also
+      // consider dwChannelMask.

[chcunningham, 2017/06/08 16:39:45]

as-is, what happens if you have a some mask (that you don't consider)? Will the
captured packets just be padded with empty bytes for channels not in the mask? 

I'm reading the docs for the first time, but It seems like you may only get data
for the masked channels without padding. And the order of the channels appears
to be dependent on the mask. Internally, media::ChannelMixer assumes the orders
will match those from FFmpeg. Its hard coded here. 

https://cs.chromium.org/chromium/src/media/base/channel_layout.cc?type=cs&sq=...

Seems like things could go pretty wrong if we fail to consider the mask and
reconcile the order with that from ffmpeg. I don't know how often this will bite
you. Like, how often is a mask provided... How standard is ffmpeg's ordering...

+DaleCurtis to advise. 

[DaleCurtis, 2017/06/08 17:05:47]

I don't remember the details around channel masks, but we do have code for
converting to media::ChannelLayout here
https://cs.chromium.org/chromium/src/media/audio/win/core_audio_util_win.cc?l=39

[Sergey Ulanov, 2017/06/08 18:27:59]

We do take into account nChannels, and number of bits set in dwChannelMask
should be equal nChannels.
This order matches the order used by windows, see
https://msdn.microsoft.com/en-us/library/windows/hardware/ff538802(v=vs.85).aspx
ffmpeg's channel order matches channel order used by Windows as far as I can
tell. Please let me know if I'm wrong.

[Hzj_jie, 2017/06/08 19:07:48]

Thank you for the suggestion, but we do not always have dwChannelMask. I have no
idea when will WAVE_FORMAT_EXTENSIBLE be returned, but we still need to support
WAVE_FORMAT_PCM and WAVE_FORMAT_IEEE_FLOAT. Meanwhile by using dwChannelMask for
WAVE_FORMAT_EXTENSIBLE would make the logic more complex, and I believe in most
of the cases it does not make any difference.

[chcunningham, 2017/06/09 19:25:28]

This is a key insight that I overlooked. I now appreciate that, in the worst
case you may put something in the wrong ear, but at least you're not going to
read out of bounds or anything. 
You're right, looks the same to me.

[Hzj_jie, 2017/06/09 21:33:47]

Done.

+      // TODO(zijiehe): Support also layouts.

[chcunningham, 2017/06/08 16:39:45]

Not sure what this TODO means. Do you mean "Support masked layouts"?

[Hzj_jie, 2017/06/08 19:07:48]

Yes. Updated.

+      packet->set_channels(static_cast<AudioPacket::Channels>(
+          wave_format_ex_->nChannels));
 
       callback_.Run(std::move(packet));
     }