Add ALSA support to volume keys

chrome/browser/chromeos/audio_mixer.h

+// Copyright (c) 2010 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 CHROME_BROWSER_CHROMEOS_AUDIO_MIXER_H_
+#define CHROME_BROWSER_CHROMEOS_AUDIO_MIXER_H_
+#pragma once
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+
+namespace chromeos {
+
+class AudioMixer {
+ public:
+  enum State {
+    UNINITIALIZED = 0,
+    INITIALIZING,
+    READY,
+    SHUTTING_DOWN,
+    IN_ERROR,
+  };
+
+  AudioMixer() {}

[scherkus (not reviewing), 2011/01/11 00:24:12]

don't inline ctors / dtors (might have to write a .cc unfortunately!)

[davejcool, 2011/01/11 02:52:54]

The style guide says it is fine to inline virtual destructors in pure abstract
base classes.

+  virtual ~AudioMixer() {}
+
+  // Non-Blocking call.  Connect to Mixer, find a default device, then call the
+  // callback when complete with success code.
+  typedef Callback1<bool>::Type InitDoneCallback;
+  virtual bool Init(InitDoneCallback* callback) = 0;

[scherkus (not reviewing), 2011/01/11 00:24:12]

I'm inclined to say either use the callback to report status or use a return
value but avoid both as its confusing in the "false" case (what happens to my
callback?  does it get executed with false as well?  does it ever get executed? 
cleanup etc...?)

currently both the implementations would return false only if creating a thread
failed, in which case we have bigger issues to worry about!

also you can achieve the same effect of reporting thread-creation-failure by
executing the callback on the callee's thread with a value of false

[davejcool, 2011/01/11 02:52:54]

Thanks, done.

+
+  // Call may block.  Mixer will be connected before returning, unless error.
+  virtual bool InitSync() = 0;

[scherkus (not reviewing), 2011/01/11 00:24:12]

we can consider deferring to a later change but I'm wondering if it makes sense
to push the async+sync initialization down to each AudioMixer implementation
when it's only AudioHandler that really need to have that flexibility

what do you think about standardizing on an asynchronous Initialization() for
AudioMixers and then having AudioHandler use a WaitableEvent to implement its
synchronous version?

Just a thought (and probably suitable after this CL lands)

[davejcool, 2011/01/11 02:52:54]

I like the idea- it would simplify the AudioMixer interface just that much more.

+
+  // Call may block.  Returns a default of kSilenceDb on error.
+  virtual double GetVolumeDb() const = 0;
+
+  // Non-Blocking call.  Returns the actual volume limits possible according
+  // to the mixer.  Limits are left unchanged on error
+  virtual bool GetVolumeLimits(double* vol_min, double* vol_max) = 0;
+
+  // Non-blocking call.
+  virtual void SetVolumeDb(double vol_db) = 0;
+
+  // Call may block.  Gets the mute state of the default device (true == mute).
+  // Returns a default of false on error.
+  virtual bool IsMute() const = 0;
+
+  // Non-Blocking call.
+  virtual void SetMute(bool mute) = 0;
+
+  // Returns READY if we have a valid working connection to the Mixer.
+  // This can return IN_ERROR if we lose the connection, even after an original
+  // successful init.  Non-blocking call.
+  virtual State CheckState() const = 0;

[scherkus (not reviewing), 2011/01/11 00:24:12]

nit: I think this used to be called CheckState() because it would attempt a
re-connection... given that it simply is returning the state we can call this
GetState()

what do you think? (my memory might be a big foggy as well here so correct me if
wrong!)

[davejcool, 2011/01/11 02:52:54]

Yes, GetState() Now makes more sense.

+
+  // Approximation of pure silence expressed in decibels.
+  static const double kSilenceDb = -200.0;

[scherkus (not reviewing), 2011/01/11 00:24:12]

constants at top of class declarations + move the assignment to .cc

[davejcool, 2011/01/11 02:52:54]

Moved to the top where it belongs.  I've seen constants like this defined in
similar header files, so I'd like to keep it here... (plus, otherwise a .cc
would be required for just this single constant?)

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AudioMixer);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_AUDIO_MIXER_H_
+