make pulseaudio available for all posix platforms because it's not linux only

make pulseaudio available for all posix platforms because it's not linux only

+ add support for pulseaudio on OpenBSD

BUG=
TEST=compile

[Robert Nagy, 2011-11-08 21:19:50]

please review

[scherkus (not reviewing), 2011-11-09 01:10:54]

http://codereview.chromium.org/8499029/diff/1/media/audio/openbsd/audio_manag...
File media/audio/openbsd/audio_manager_openbsd.cc (right):

http://codereview.chromium.org/8499029/diff/1/media/audio/openbsd/audio_manag...
media/audio/openbsd/audio_manager_openbsd.cc:42: return HasAudioHardware();
does the presence of /dev/audioctl (or similar) guarantee input/output devices
or do you intend to fail later on if someone attempts to open an input device
when one doesn't exist?

http://codereview.chromium.org/8499029/diff/1/media/audio/openbsd/audio_manag...
media/audio/openbsd/audio_manager_openbsd.cc:68: } else {
nit: you don't need the since you're returning

http://codereview.chromium.org/8499029/diff/1/media/audio/pulse/pulse_output.h
File media/audio/pulse/pulse_output.h (right):

http://codereview.chromium.org/8499029/diff/1/media/audio/pulse/pulse_output....
media/audio/pulse/pulse_output.h:34: #if defined(OS_LINUX)
Does using the base class AudioManager work here?

If not a typedef might be cleaner here

#if defined(OS_LINUX)
typedef AudioManagerLinux AudioManagerPosix;
#elif defined(OS_OPENBSD)
typedef AudioManagerOpenBSD AudioManagerPosix;
#else
#error Unsupported platform
#end

[Robert Nagy, 2011-11-09 07:44:39]

http://codereview.chromium.org/8499029/diff/1/media/audio/openbsd/audio_manag...
File media/audio/openbsd/audio_manager_openbsd.cc (right):

http://codereview.chromium.org/8499029/diff/1/media/audio/openbsd/audio_manag...
media/audio/openbsd/audio_manager_openbsd.cc:42: return HasAudioHardware();
On 2011/11/09 01:10:54, scherkus wrote:
> does the presence of /dev/audioctl (or similar) guarantee input/output devices
> or do you intend to fail later on if someone attempts to open an input device
> when one doesn't exist?
If opening the audioctl device succeeds then there is an audio device that can
be used.
Later on there is only the pulseaudio and other sound server code for OpenBSD.

http://codereview.chromium.org/8499029/diff/1/media/audio/openbsd/audio_manag...
media/audio/openbsd/audio_manager_openbsd.cc:68: } else {
On 2011/11/09 01:10:54, scherkus wrote:
> nit: you don't need the since you're returning
I need it in case kUsePulseAudio is not set through the command line.

http://codereview.chromium.org/8499029/diff/1/media/audio/pulse/pulse_output.h
File media/audio/pulse/pulse_output.h (right):

http://codereview.chromium.org/8499029/diff/1/media/audio/pulse/pulse_output....
media/audio/pulse/pulse_output.h:34: #if defined(OS_LINUX)
On 2011/11/09 01:10:54, scherkus wrote:
> Does using the base class AudioManager work here?
> 
> If not a typedef might be cleaner here
> 
> #if defined(OS_LINUX)
> typedef AudioManagerLinux AudioManagerPosix;
> #elif defined(OS_OPENBSD)
> typedef AudioManagerOpenBSD AudioManagerPosix;
> #else
> #error Unsupported platform
> #end
Unfortunately i still need to have class AudioManagerPLATFORM; but using a
typedef is much better indeed.

[scherkus (not reviewing), 2011-11-16 00:57:56]

few nits

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
File media/audio/openbsd/audio_manager_openbsd.cc (right):

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
media/audio/openbsd/audio_manager_openbsd.cc:62: AudioOutputStream* stream;
nit: I think it'd be clearer if it was written as:

#if defined(USE_PULSEAUDIO)
  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUsePulseAudio)) {
    AudioOutputStream* stream =
        new PulseAudioOutputStream(params, this, GetMessageLoop());
    active_streams_.insert(stream);
    return stream;
  }    
#endif

  NOTIMPLEMENTED();
  return NULL;
}

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
File media/audio/openbsd/audio_manager_openbsd.h (right):

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
media/audio/openbsd/audio_manager_openbsd.h:11: #include
"base/memory/ref_counted.h"
II don't see ref_counted.h nor thread.h used in the .h -- move to .cc ?

http://codereview.chromium.org/8499029/diff/4002/media/audio/pulse/pulse_outp...
File media/audio/pulse/pulse_output.h (right):

http://codereview.chromium.org/8499029/diff/4002/media/audio/pulse/pulse_outp...
media/audio/pulse/pulse_output.h:19: #ifndef MEDIA_AUDIO_LINUX_PULSE_OUTPUT_H_
update header guard

http://codereview.chromium.org/8499029/diff/4002/media/audio/pulse/pulse_outp...
media/audio/pulse/pulse_output.h:140: #endif  //
MEDIA_AUDIO_LINUX_PULSE_OUTPUT_H_
down here too

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.cc
File media/base/media_switches.cc (right):

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.cc...
media/base/media_switches.cc:16: // Use PulseAudio on platforms that support it.
nit: move comment by switch

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.h
File media/base/media_switches.h (right):

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.h#...
media/base/media_switches.h:23: MEDIA_EXPORT extern const char kVideoThreads[];
nit: add blank line above here

[Robert Nagy, 2011-11-16 11:59:59]

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
File media/audio/openbsd/audio_manager_openbsd.cc (right):

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
media/audio/openbsd/audio_manager_openbsd.cc:62: AudioOutputStream* stream;
On 2011/11/16 00:57:56, scherkus wrote:
> nit: I think it'd be clearer if it was written as:
> 
> #if defined(USE_PULSEAUDIO)
>   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kUsePulseAudio)) {
>     AudioOutputStream* stream =
>         new PulseAudioOutputStream(params, this, GetMessageLoop());
>     active_streams_.insert(stream);
>     return stream;
>   }    
> #endif
> 
>   NOTIMPLEMENTED();
>   return NULL;
> }

Done.

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
File media/audio/openbsd/audio_manager_openbsd.h (right):

http://codereview.chromium.org/8499029/diff/4002/media/audio/openbsd/audio_ma...
media/audio/openbsd/audio_manager_openbsd.h:11: #include
"base/memory/ref_counted.h"
On 2011/11/16 00:57:56, scherkus wrote:
> II don't see ref_counted.h nor thread.h used in the .h -- move to .cc ?
They are not needed at all. Removed.

http://codereview.chromium.org/8499029/diff/4002/media/audio/pulse/pulse_outp...
File media/audio/pulse/pulse_output.h (right):

http://codereview.chromium.org/8499029/diff/4002/media/audio/pulse/pulse_outp...
media/audio/pulse/pulse_output.h:19: #ifndef MEDIA_AUDIO_LINUX_PULSE_OUTPUT_H_
On 2011/11/16 00:57:56, scherkus wrote:
> update header guard

Done.

http://codereview.chromium.org/8499029/diff/4002/media/audio/pulse/pulse_outp...
media/audio/pulse/pulse_output.h:140: #endif  //
MEDIA_AUDIO_LINUX_PULSE_OUTPUT_H_
On 2011/11/16 00:57:56, scherkus wrote:
> down here too

Done.

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.cc
File media/base/media_switches.cc (right):

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.cc...
media/base/media_switches.cc:16: // Use PulseAudio on platforms that support it.
On 2011/11/16 00:57:56, scherkus wrote:
> nit: move comment by switch

Done.

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.h
File media/base/media_switches.h (right):

http://codereview.chromium.org/8499029/diff/4002/media/base/media_switches.h#...
media/base/media_switches.h:23: MEDIA_EXPORT extern const char kVideoThreads[];
On 2011/11/16 00:57:56, scherkus wrote:
> nit: add blank line above here

Done.

[scherkus (not reviewing), 2011-11-17 01:18:07]

LGTM

[commit-bot: I haz the power, 2011-11-17 10:46:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robert.nagy@gmail.com/8499029/17001

[commit-bot: I haz the power, 2011-11-17 10:46:45]

Can't process patch for file media/base/media_switches.h.
File's status is None, patchset upload is incomplete.

[Robert Nagy, 2011-11-17 12:54:59]

On 2011/11/17 10:46:45, I haz the power (commit-bot) wrote:
> Can't process patch for file media/base/media_switches.h.
> File's status is None, patchset upload is incomplete.

scherkus, can you please commit this by hand?