Implement Linux audio capturer.

Implement audio capturer for chromoting host on Linux.

This change adds AudioCapturerLinux which works by reading audio stream from
a named pipe. The name of the pipe is passed via command line. 
me2me_virtual_host.py will need to configure pulseaudio to stream audio to the 
the same pipe , that's coming in a separate CL.

BUG=145891
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=156060

[Wez, 2012-09-04 21:41:45]

http://codereview.chromium.org/10907041/diff/1/remoting/host/audio_capturer_l...
File remoting/host/audio_capturer_linux.cc (right):

http://codereview.chromium.org/10907041/diff/1/remoting/host/audio_capturer_l...
remoting/host/audio_capturer_linux.cc:81: void
AudioCapturerLinux::OnFileCanWriteWithoutBlocking(int fd) {
nit: NOTREACHED()?

http://codereview.chromium.org/10907041/diff/1/remoting/host/audio_capturer_l...
remoting/host/audio_capturer_linux.cc:98: stream_position_samples -
last_capture_time_samples_;
Do you need these calculations, and timers going off? Can't you just wait for
the pipe to be readable and then read until EWOULDBLOCK to keep in sync with
PulseAudio?

[Sergey Ulanov, 2012-09-05 01:35:49]

http://codereview.chromium.org/10907041/diff/1/remoting/host/audio_capturer_l...
File remoting/host/audio_capturer_linux.cc (right):

http://codereview.chromium.org/10907041/diff/1/remoting/host/audio_capturer_l...
remoting/host/audio_capturer_linux.cc:81: void
AudioCapturerLinux::OnFileCanWriteWithoutBlocking(int fd) {
On 2012/09/04 21:41:45, Wez wrote:
> nit: NOTREACHED()?

Done.

http://codereview.chromium.org/10907041/diff/1/remoting/host/audio_capturer_l...
remoting/host/audio_capturer_linux.cc:98: stream_position_samples -
last_capture_time_samples_;
On 2012/09/04 21:41:46, Wez wrote:
> Do you need these calculations, and timers going off? Can't you just wait for
> the pipe to be readable and then read until EWOULDBLOCK to keep in sync with
> PulseAudio?

No. PulseAudio's pipe sink plugin doesn't control rate with which it pushes data
to the pipe, so we have to do it here. If we read all available data then pulse
will keep pushing it, which means we will be receiving much more than 44100
samples per second.

[Wez, 2012-09-06 21:47:29]

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
File remoting/host/audio_capturer_linux.cc (right):

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:53: Sleep();
Why do this here, rather than in Start?

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:95: base::TimeDelta stream_position =
base::TimeTicks::Now() - started_time_;
Add a comment explaining what this calculation is for.
Why do you calculate the current position in the stream relative to the
start-time-ticks, rather than calculating the additional data you need to read
purely in terms of Now() - last_sample_ticks_?

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:137: void AudioCapturerLinux::Sleep() {
Sleep -> e.g. WaitForDataReady / WaitForPipe / WaitForPipeReadable /
WaitForFileCanRead

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:148: return scoped_ptr<AudioCapturer>();
You'll need to update this to provide a SupportsAudioCapture() that depends on
the audio pipe option, and to have Create() CHECK if called when audio's not
enabled.

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
File remoting/host/audio_capturer_linux.h (right):

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.h:27: virtual bool IsRunning() OVERRIDE;
nit: IsStarted()

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.h:47: int64 last_capture_time_samples_;
Express this simply as the stream position of the latest capture, i.e. no need
to mention time since it's not a time at all.

[Sergey Ulanov, 2012-09-06 23:18:53]

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
File remoting/host/audio_capturer_linux.cc (right):

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:53: Sleep();
On 2012/09/06 21:47:29, Wez wrote:
> Why do this here, rather than in Start?

Capturer is started only when a user is connected, but we want to read from the
pipe even when the session is inactive, so that we don't block any app that
tries to play a sound while the user is not connected.

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:95: base::TimeDelta stream_position =
base::TimeTicks::Now() - started_time_;
On 2012/09/06 21:47:29, Wez wrote:
> Add a comment explaining what this calculation is for.

Done.

> Why do you calculate the current position in the stream relative to the
> start-time-ticks, rather than calculating the additional data you need to read
> purely in terms of Now() - last_sample_ticks_?

That's to avoid accumulation of rounding errors when converting ticks to
samples.

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:137: void AudioCapturerLinux::Sleep() {
On 2012/09/06 21:47:29, Wez wrote:
> Sleep -> e.g. WaitForDataReady / WaitForPipe / WaitForPipeReadable /
> WaitForFileCanRead

Done.

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:148: return scoped_ptr<AudioCapturer>();
On 2012/09/06 21:47:29, Wez wrote:
> You'll need to update this to provide a SupportsAudioCapture() that depends on
> the audio pipe option, and to have Create() CHECK if called when audio's not
> enabled.

Done.

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
File remoting/host/audio_capturer_linux.h (right):

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.h:27: virtual bool IsRunning() OVERRIDE;
On 2012/09/06 21:47:29, Wez wrote:
> nit: IsStarted()

Done

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.h:47: int64 last_capture_time_samples_;
On 2012/09/06 21:47:29, Wez wrote:
> Express this simply as the stream position of the latest capture, 
Done

> i.e. no need to mention time since it's not a time at all.

It is time, but measured in number of samples. It doesn't correspond to number
of samples we were actually able to read from the pipe. E.g. if we were read
less than 44100 samples on one particular second, this value is still increased
by 44100.

[Wez, 2012-09-06 23:32:39]

lgtm

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
File remoting/host/audio_capturer_linux.cc (right):

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:53: Sleep();
On 2012/09/06 23:18:53, sergeyu wrote:
> On 2012/09/06 21:47:29, Wez wrote:
> > Why do this here, rather than in Start?
> 
> Capturer is started only when a user is connected, but we want to read from
the
> pipe even when the session is inactive, so that we don't block any app that
> tries to play a sound while the user is not connected.

Do we actually need to do that, given that there will be no user connected to
hear the audio?

[Sergey Ulanov, 2012-09-07 00:22:02]

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
File remoting/host/audio_capturer_linux.cc (right):

http://codereview.chromium.org/10907041/diff/5001/remoting/host/audio_capture...
remoting/host/audio_capturer_linux.cc:53: Sleep();
On 2012/09/06 23:32:39, Wez wrote:
> On 2012/09/06 23:18:53, sergeyu wrote:
> > On 2012/09/06 21:47:29, Wez wrote:
> > > Why do this here, rather than in Start?
> > 
> > Capturer is started only when a user is connected, but we want to read from
> the
> > pipe even when the session is inactive, so that we don't block any app that
> > tries to play a sound while the user is not connected.
> 
> Do we actually need to do that, given that there will be no user connected to
> hear the audio?

Yes, we do need to do it. Otherwise we may break some apps and UX. E.g. some
widgets or apps may want to play some notification sounds and expect them to
finish playing at some point. If we don't read from the pipe while user is not
connected then all these sound may be queued until the user is connected. This
may lead to OOM, and also will bombard the user with all notification sounds
that were played while the user was away.

[commit-bot: I haz the power, 2012-09-07 00:39:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10907041/9001

[commit-bot: I haz the power, 2012-09-07 00:39:58]

Failed to apply patch for remoting/host/desktop_environment_factory.cc:
While running patch -p1 --forward --force;
patching file remoting/host/desktop_environment_factory.cc
Hunk #1 FAILED at 32.
1 out of 1 hunk FAILED -- saving rejects to file
remoting/host/desktop_environment_factory.cc.rej

[commit-bot: I haz the power, 2012-09-10 19:34:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10907041/9

[commit-bot: I haz the power, 2012-09-10 19:51:00]

Try job failure for 10907041-9 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2012-09-10 23:13:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10907041/14007

[commit-bot: I haz the power, 2012-09-10 23:19:32]

Failed to request the patch to try. Please note that binary filesare still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 404: Not Found

[commit-bot: I haz the power, 2012-09-10 23:19:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10907041/10003

[commit-bot: I haz the power, 2012-09-10 23:33:05]

Try job failure for 10907041-10003 (retry) on win for step "runhooks".
It's a second try, previously, step "runhooks" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-09-10 23:41:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10907041/10003

[commit-bot: I haz the power, 2012-09-11 00:00:44]

Try job failure for 10907041-10003 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2012-09-11 00:09:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10907041/10004

[commit-bot: I haz the power, 2012-09-11 00:44:02]

Try job failure for 10907041-10004 (retry) on win_rel for step "runhooks"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...