Adding GetAudioInputDeviceNames to AudioManager, this function is supposed to do device enumerati...

media/audio/mac/audio_manager_mac.cc

Index: media/audio/mac/audio_manager_mac.cc
===================================================================
--- media/audio/mac/audio_manager_mac.cc	(revision 86276)
+++ media/audio/mac/audio_manager_mac.cc	(working copy)
@@ -4,6 +4,7 @@
 
 #include <CoreAudio/AudioHardware.h>
 
+#include "base/stringprintf.h"
 #include "base/sys_info.h"
 #include "media/audio/fake_audio_input_stream.h"
 #include "media/audio/fake_audio_output_stream.h"
@@ -127,6 +128,21 @@
   // TODO(cpu): implement.
 }
 
+void AudioManagerMac::GetAudioInputDeviceNames(
+    media::AudioDeviceNames* device_names) {
+  // TODO(xians): query a full list of valid devices.

[scherkus (not reviewing), 2011/05/26 06:13:12]

would it hurt to leave this as NOTIMPLEMENTED()?

right now it seems like GetAudioInputDeviceNames() is a wrapper around
HasAudioInputDevices() -- how soon do you expect to follow up w/ implementing
audio input device name querying?

[xians, 2011/05/26 13:54:57]

They are actually very good questions.
The story is that this GetAudioInputDeviceName is used by
AudioInputDeviceManager(I am currently working on, and will submit a patch
soon). So I need to prepend the default device to the list either in
GetAudioInputDeviceNames or inside AudioInputDeviceManager.
The reason why I prefer to do it in GetAudioInputDeviceNames is that I want to
use the same function for now and the future. But I understand it is a bit
against the chrome concept "dont design for the future".

My current plan is that I am going to make a patch for AudioInputDeviceManager,
then hooks it up to media stream and chrome recording. After that I suppose I
may have time to work on this input device querying. But the risk here is that
it is a good-to-have feature, and its priority might be lowered if something
else jumps in. So I dont really have a concrete time for it, but I am hoping to
start it soon.

A question about the approach to add features in chrome, can I add a feature to
some specific platforms first while putting others in a TODO list? I ask this
because this device selection is easier in windows and mac, but quite
complicated in linux, as we have ALSA, PulseAudio, and other sound servers
mixing around there.

+  if (HasAudioInputDevices()) {
+    // Add the default device to the list.
+    // We use (device_name)_(index) to make up the unique_ids to identify the

[scherkus (not reviewing), 2011/05/26 06:13:12]

I don't quite understand the need for unique IDs

does this method of generating IDs also apply to non-default devices?

if we have indices wouldn't it make sense to use an integer to specify the
index?

right now for a device "Foo" with 3 indices, we'd end up with the following
AudioDeviceNames:
{ "Foo", "Foo_0" },
{ "Foo", "Foo_1" },
{ "Foo", "Foo_2" },

...since we already have an AudioDeviceName struct we can probably do away with
the duplicated "Foo" in the device ID

Hope I'm making sense here and feel free to correct me if I'm misunderstanding
things here!

[xians, 2011/05/26 13:54:57]

I was also thinking about it before, but I decided not to do it because it
reduce the readability. But I am open to the idea. I pasted the code here to let
you decide which one is better:
static const char* kNotWantedDevices[] = {
      "default",
      "null",
      "pulse",
      "dmix",
      "surround"};

    // Now check if if it is a valid device.
    for (int i = 0; i < arraysize(kNotWantedDevices); ++i) {
      if (strncmp(kNotWantedDevices[i],
                  hint_device_name.get(),
                  strlen(kNotWantedDevices[i])) == 0)
         break;  // Invalid device.
      else if (i == (arraysize(kNotWantedDevices) -1))
        return true;  // Valid device, return.
    }
  }

[xians, 2011/05/26 13:54:57]

Sorry for the bad design here.
There are two ways to generate the IDs.
1, make our own internal unique id by the index. 
2, get the IDs from the OS.
Option 2 is slightly better than 1 because it handles the following user case:
Assuming we have USB0 and USB1, they are having a same name.
If the user chooses USB0, but unplugs the device before the call starts, then
USB1 will become USB0 and will be used for the call if we are using index. But
if we are using a string unique id from OS, we will find out the selected device
does not exist any more and return an error at the early state.

I confess this scenario is hidden in the corner and may not be important at all.
I was trying to have the framework cover both option 1 and 2, so I made the
unique id a string but indicating it can be made up by the index, and will make
the decision when I get there. But right now I slightly prefer to get the IDs
from the OS.
How do you think here?

Anyhow, it is quite a bad idea to use default_0 for the default id, 0 should be
enough. I did it because it used to be the style for my team.

[scherkus (not reviewing), 2011/05/27 19:10:36]

Ignore my other comment... but I do like this better :)

[scherkus (not reviewing), 2011/05/27 19:10:36]

OK... what about integers?  If we're enumerating?

+    // devices. For default device, the index is 0, so its unique_id is
+    // Default_0.
+    media::AudioDeviceName name;
+    name.device_name = AudioManagerBase::kDefaultDeviceName;
+    name.unique_id = StringPrintf("%s_0", AudioManagerBase::kDefaultDeviceName);
+    device_names->push_back(name);
+  }
+}
+
 // Called by the stream when it has been released by calling Close().
 void AudioManagerMac::ReleaseOutputStream(AudioOutputStream* stream) {
   DCHECK(stream);