[Chromoting] Implement down mixing in AudioPump

remoting/host/audio_capturer_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/audio_capturer_win.h"
 
 #include <avrt.h>
 #include <mmreg.h>
 #include <mmsystem.h>
 #include <objbase.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <windows.h>
 
 #include <algorithm>
 #include <utility>
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/synchronization/lock.h"
 #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.
 const int k100nsPerMillisecond = 10000;
 
 // Tolerance for catching packets of silence. If all samples have absolute
 // value less than this threshold, the packet will be counted as a packet of
 // silence. A value of 2 was chosen, because Windows can give samples of 1 and
 // -1, even when no audio is playing.
 const int kSilenceThreshold = 2;
 
 // Lower bound for timer intervals, in milliseconds.
 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 {
 
 AudioCapturerWin::AudioCapturerWin()
     : sampling_rate_(AudioPacket::SAMPLING_RATE_INVALID),
       volume_filter_(kSilenceThreshold),
       last_capture_error_(S_OK) {
   thread_checker_.DetachFromThread();
 }
@@ skipping 91 matching lines @@
   audio_device_period_ = base::TimeDelta::FromMilliseconds(
       std::max(device_period_in_milliseconds, kMinTimerInterval));
 
   // Get the wave format.
   hr = audio_client_->GetMixFormat(&wave_format_ex_);
   if (FAILED(hr)) {
     LOG(ERROR) << "Failed to get WAVEFORMATEX. Error " << hr;
     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);
+  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;
+  }
 
-      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);
+  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;
+  }
 
-        wave_format_extensible->SubFormat = KSDATAFORMAT_SUBTYPE_PCM;
-        wave_format_extensible->Samples.wValidBitsPerSample = kBitsPerSample;
+  // 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;
+  }
 
-        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;
+  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;
     }
-    default:
-      LOG(ERROR) << "Failed to force 16-bit PCM";
-      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.
   hr = audio_client_->Initialize(
       AUDCLNT_SHAREMODE_SHARED,
       AUDCLNT_STREAMFLAGS_LOOPBACK,
       (kMaxExpectedTimerLag + audio_device_period_.InMilliseconds()) *
       k100nsPerMillisecond,
       0,
       wave_format_ex_,
@@ skipping 11 matching lines @@
   }
 
   // Start the IAudioClient.
   hr = audio_client_->Start();
   if (FAILED(hr)) {
     LOG(ERROR) << "Failed to start IAudioClient. Error " << hr;
     return false;
   }
 
   volume_filter_.ActivateBy(mm_device_.Get());
-  volume_filter_.Initialize(sampling_rate_, kChannels);
+  volume_filter_.Initialize(sampling_rate_, wave_format_ex_->nChannels);
 
   return true;
 }
 
 bool AudioCapturerWin::is_initialized() const {
   // All Com components should be initialized / deinitialized together.
   return !!audio_client_;
 }
 
 void AudioCapturerWin::DoCapture() {
@@ skipping 26 matching lines @@
                                           nullptr);
     if (FAILED(hr))
       break;
 
     if (volume_filter_.Apply(reinterpret_cast<int16_t*>(data), frames)) {
       std::unique_ptr<AudioPacket> packet(new AudioPacket());
       packet->add_data(data, frames * wave_format_ex_->nBlockAlign);
       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));
     }
 
     hr = audio_capture_client_->ReleaseBuffer(frames);
     if (FAILED(hr))
       break;
   }
 
   // There is nothing to capture if the audio endpoint device has been unplugged
@@ skipping 11 matching lines @@
 
 bool AudioCapturer::IsSupported() {
   return true;
 }
 
 std::unique_ptr<AudioCapturer> AudioCapturer::Create() {
   return base::WrapUnique(new AudioCapturerWin());
 }
 
 }  // namespace remoting