Modified Windows buffer size to avoid audio glitches.

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 <windows.h>
 #include <audioclient.h>
 #include <avrt.h>
 #include <mmdeviceapi.h>
 #include <mmreg.h>
 #include <mmsystem.h>
 
+#include <algorithm>
 #include <stdlib.h>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/timer.h"
 #include "base/win/scoped_co_mem.h"
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_comptr.h"
 #include "remoting/host/audio_capturer.h"
 #include "remoting/proto/audio.pb.h"
 
 namespace {
 const int kChannels = 2;
 const int kBitsPerSample = 16;
 const int kBitsPerByte = 8;
 // Conversion factor from 100ns to 1ms.
 const int kHnsToMs = 10000;

[Wez, 2012/08/16 18:49:36]

nit: I'd suggest calling this k100nsToMs, but check whether the style guide
allows names of that form?

[Wez, 2012/08/16 18:49:36]

nit: Shouldn't this be kMsTo100ns or kMillisecondIn100ns, since it is the number
of 100s of ns in a millisecond, rather than the other way around?

[kxing, 2012/08/16 20:17:21]

Done.

[kxing, 2012/08/16 20:17:21]

Yeah, I'll name it to k100nsInMillisecond, because it seems clearer than
kMillisecondIn100ns.

 
 // 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 kMinTimerInteveral = 30;

[Wez, 2012/08/16 18:49:36]

typo: kMinTimerInterval

[kxing, 2012/08/16 20:17:21]

Done.

+
+// Upper bound for the timer precision error, in milliseconds.

[Wez, 2012/08/16 18:49:36]

nit: Clarify where this value comes from, even if it's just a an estimate based
on experimentation.

[kxing, 2012/08/16 20:17:21]

Done.

+const int kMaxTimerLag = 30;

[Wez, 2012/08/16 18:49:36]

nit: Consider calling this kMaxExpectedTimerLag; it's possible for the timer to
lag by longer but we don't expect that in normal usage!

[kxing, 2012/08/16 20:17:21]

Done.

 }  // namespace
 
 namespace remoting {
 
 class AudioCapturerWin : public AudioCapturer {
  public:
   AudioCapturerWin();
   virtual ~AudioCapturerWin();
 
   // AudioCapturer interface.
@@ skipping 75 matching lines @@
     return false;
   }
 
   REFERENCE_TIME device_period;
   hr = audio_client_->GetDevicePeriod(&device_period, NULL);
   if (FAILED(hr)) {
     LOG(ERROR) << "IAudioClient::GetDevicePeriod failed. Error " << hr;
     return false;
   }
   audio_device_period_ = base::TimeDelta::FromMilliseconds(
-      device_period / kChannels / kHnsToMs);
+      std::max(static_cast<int>(device_period / kHnsToMs), kMinTimerInteveral));

[Wez, 2012/08/16 18:49:36]

nit: If |device_period| is not an integer number of milliseconds then this will
round-down; is that a problem?

[kxing, 2012/08/16 20:17:21]

Done.

 
   // 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) {
@@ skipping 43 matching lines @@
         return false;
       }
       break;
     }
     default:
       LOG(ERROR) << "Failed to force 16-bit PCM";
       return false;
   }
 
   // Initialize the IAudioClient.
-  hr = audio_client_->Initialize(AUDCLNT_SHAREMODE_SHARED,
-                                 AUDCLNT_STREAMFLAGS_LOOPBACK,
-                                 0,
-                                 0,
-                                 wave_format_ex_,
-                                 NULL);
+  hr = audio_client_->Initialize(
+      AUDCLNT_SHAREMODE_SHARED,
+      AUDCLNT_STREAMFLAGS_LOOPBACK,
+      (kMaxTimerLag + audio_device_period_.InMilliseconds()) * kHnsToMs,

[Wez, 2012/08/16 18:49:36]

You're multiplying a value in milliseconds by the 100ns->ms conversion ratio;
surely you want to multiply by the ms->100ns ratio?

[kxing, 2012/08/16 20:17:21]

Done.

+      0,
+      wave_format_ex_,
+      NULL);
   if (FAILED(hr)) {
     LOG(ERROR) << "Failed to initialize IAudioClient. Error " << hr;
     return false;
   }
 
   // Get an IAudioCaptureClient.
   hr = audio_client_->GetService(__uuidof(IAudioCaptureClient),
                                  audio_capture_client_.ReceiveVoid());
   if (FAILED(hr)) {
     LOG(ERROR) << "Failed to get an IAudioCaptureClient. Error " << hr;
@@ skipping 56 matching lines @@
     UINT32 frames;
     DWORD flags;
     hr = audio_capture_client_->GetBuffer(
         &data, &frames, &flags, NULL, NULL);
     if (FAILED(hr)) {
       LOG(ERROR) << "Failed to GetBuffer. Error " << hr;
       return;
     }
 
     if (!IsPacketOfSilence(
           reinterpret_cast<const int16*>(data),

[Wez, 2012/08/16 18:49:36]

nit: Indent reinterpret... by four spaces from the if's open bracket.

[kxing, 2012/08/16 20:17:21]

Done.

           frames * kChannels)) {

[Wez, 2012/08/16 18:49:36]

What's the rationale for performing the silence check on the raw data rather
than constructing the packet first? To simplify the IsPacketOfSilence check, or
to avoid the memory copy?

[kxing, 2012/08/16 20:17:21]

Yes, it's to avoid the memory copy.

       scoped_ptr<AudioPacket> packet =
           scoped_ptr<AudioPacket>(new AudioPacket());
       packet->add_data(data, frames * wave_format_ex_->nBlockAlign);
       packet->set_sampling_rate(sampling_rate_);
       packet->set_bytes_per_sample(
           static_cast<AudioPacket::BytesPerSample>(sizeof(int16)));
       packet->set_encoding(AudioPacket::ENCODING_RAW);
 
       callback_.Run(packet.Pass());
     }
@@ skipping 16 matching lines @@
       return false;
   }
   return true;
 }
 
 scoped_ptr<AudioCapturer> AudioCapturer::Create() {
   return scoped_ptr<AudioCapturer>(new AudioCapturerWin());
 }
 
 }  // namespace remoting