Improved AGC update scheme for the audio backend in Chrome

media/audio/agc_audio_stream.h

Index: media/audio/agc_audio_stream.h
diff --git a/media/audio/agc_audio_stream.h b/media/audio/agc_audio_stream.h
new file mode 100644
index 0000000000000000000000000000000000000000..127777da20bbe04f53588a41b473c5e9c3bbba34
--- /dev/null
+++ b/media/audio/agc_audio_stream.h
@@ -0,0 +1,152 @@
+// 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.
+
+#ifndef MEDIA_AUDIO_AGC_AUDIO_STREAM_H_
+#define MEDIA_AUDIO_AGC_AUDIO_STREAM_H_
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
+#include "base/timer.h"
+#include "media/audio/audio_io.h"
+
+namespace media {
+
+// TODO(henrika): add info about threading

[tommi (sloooow) - chröme, 2013/05/27 13:54:36]

Will you do this now?

[henrika (OOO until Aug 14), 2013/05/27 15:37:36]

Done. Sorry.

+
+// The template based AgcAudioStream implements platform-independent parts
+// of the AudioInterface interface. Supported interfaces to pass as
+// AudioInterface are AudioIntputStream and AudioOutputStream. Each platform-
+// dependent implementation should derive from this class.
+//
+// Usage example (on Windows):
+//
+//  class WASAPIAudioInputStream : public AgcAudioStream<AudioInputStream> {
+//   public:
+//    WASAPIAudioInputStream();
+//    ...
+//  };
+//
+template <typename AudioInterface>
+class MEDIA_EXPORT AgcAudioStream : public AudioInterface {
+ public:
+  // Time between two successive timer events.
+  enum { kIntervalBetweenVolumeUpdatesMs = 1000 };

[tommi (sloooow) - chröme, 2013/05/27 13:54:36]

I don't think Chrome in general declares constants this way. Instead declare it
as a static const int.

[henrika (OOO until Aug 14), 2013/05/27 15:37:36]

Done.

+
+  AgcAudioStream()
+      : agc_is_enabled_(false), max_volume_(0.0), normalized_volume_(0.0) {
+    DVLOG(1) << __FUNCTION__;
+  }
+
+  virtual ~AgcAudioStream() {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    DVLOG(1) << __FUNCTION__;
+  }
+
+ protected:
+  // Stores a new microphone volume level by checking the audio input device.
+  // Called on the audio manager thread.
+  void UpdateAgcVolume() {
+    DCHECK(thread_checker_.CalledOnValidThread());
+
+    // We take new volume samples once every second when the AGC is enabled.
+    // To ensure that a new setting has an immediate effect, the new volume
+    // setting is cached here. It will ensure that the next OnData() callback
+    // will contain a new valid volume level. If this approach was not taken,
+    // we could report invalid volume levels to the client for a time period
+    // of up to one second.
+    QueryAndStoreNewMicrophoneVolume();
+  }
+
+  //  the latest stored volume level if AGC is enabled.
+  // Called at each capture callback on a real-time capture thread (platform
+  // dependent).
+  void GetAgcVolume(double* normalized_volume) {
+    base::AutoLock lock(lock_);
+    *normalized_volume = normalized_volume_;
+  }
+
+ private:
+  // Sets the automatic gain control (AGC) to on or off. When AGC is enabled,
+  // the microphone volume is queried periodically and the volume level can
+  // be read in each AudioInputCallback::OnData() callback and fed to the
+  // render-side AGC.
+  virtual void SetAutomaticGainControl(bool enabled) OVERRIDE {
+    DVLOG(1) << "SetAutomaticGainControl(enabled=" << enabled << ")";
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (enabled) {
+      timer_.Start(FROM_HERE,
+          base::TimeDelta::FromMilliseconds(kIntervalBetweenVolumeUpdatesMs),
+          this, &AgcAudioStream::OnTimer);
+    } else if (timer_.IsRunning())

[tommi (sloooow) - chröme, 2013/05/27 13:54:36]

{}
(if one side of an else has a brace, the other should as well)

[henrika (OOO until Aug 14), 2013/05/27 15:37:36]

Done.

+      timer_.Stop();
+    agc_is_enabled_ = enabled;

[tommi (sloooow) - chröme, 2013/05/27 13:54:36]

What I meant here is that what if these two values are already equal?  Aren't we
then going through an unnecessary amount of trouble to set the value?

I think this could be covered by a check at the top of the function:
if (agc_is_enabled_ == enabled)
  return;

[henrika (OOO until Aug 14), 2013/05/27 15:37:36]

Done.

+  }
+
+  // Gets the current automatic gain control state.
+  virtual bool GetAutomaticGainControl() OVERRIDE {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    return agc_is_enabled_;
+  }
+
+  // Takes a new microphone volume sample and stores it in |normalized_volume_|.
+  // Range is normalized to [0.0,1.0] or [0.0, 1.5] on Linux.
+  void QueryAndStoreNewMicrophoneVolume() {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    DCHECK(timer_.IsRunning());
+
+    // Cach the maximum volume if this is the first time we ask for it.
+    if (max_volume_ == 0.0)
+      max_volume_ = GetMaxVolume();
+
+    // Retrieve the current volume level by asking the audio hardware.
+    // Range is normalized to [0.0,1.0] or [0.0, 1.5] on Linux.
+    if (max_volume_ != 0.0) {
+      double normalized_volume = GetVolume() / max_volume_;
+      {

[tommi (sloooow) - chröme, 2013/05/27 13:54:36]

nit: there's no need for this scope since the next two lines contain the only
objects in the current scope and they are also the two last lines in the scope.

[henrika (OOO until Aug 14), 2013/05/27 15:37:36]

Done.

+        base::AutoLock auto_lock(lock_);
+        normalized_volume_ = normalized_volume;
+      }
+    }
+  }
+
+  // This method is called periodically when AGC is enabled and always on the
+  // audio manager thread. We use it to read the current microphone level and
+  // to store it so it can be read by the main capture thread. By using this
+  // approach, we can avoid accessing audio hardware from a real-time audio
+  // thread and it leads to a more stable capture performance.
+  void OnTimer() {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    QueryAndStoreNewMicrophoneVolume();
+  }
+
+  // Ensures that this class is created and destroyed on the same thread.
+  base::ThreadChecker thread_checker_;
+
+  // Repeating timer which cancels itself when it goes out of scope.
+  // Used to check the microphone volume periodically.
+  base::RepeatingTimer<AgcAudioStream<AudioInterface> > timer_;
+
+  // True when automatic gain control is enabled, false otherwise.
+  bool agc_is_enabled_;
+
+  // Stores the maximum volume which is used for normalization to a volume
+  // range of [0.0, 1.0].
+  double max_volume_;
+
+  // Contains last result of internal call to GetVolume(). We save resources
+  // by not querying the capture volume for each callback. Guarded by |lock_|.
+  // The range is normalized to [0.0, 1.0].
+  double normalized_volume_;
+
+  // Protects |normalized_volume_| .
+  base::Lock lock_;
+
+  DISALLOW_COPY_AND_ASSIGN(AgcAudioStream<AudioInterface>);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_AUDIO_AGC_AUDIO_STREAM_H_