Fix AudioCapturer implementation to read from audio pipe even when there are no active clients.

remoting/host/audio_capturer_linux.cc

Index: remoting/host/audio_capturer_linux.cc
diff --git a/remoting/host/audio_capturer_linux.cc b/remoting/host/audio_capturer_linux.cc
index b4a8fd0eeedfef1919d4ce2983e1405392cbdcc5..a0f0e9105623153aa3a5ba381af2f24bc8c6abab 100644
--- a/remoting/host/audio_capturer_linux.cc
+++ b/remoting/host/audio_capturer_linux.cc
@@ -36,11 +36,9 @@ const int kCapturingPeriodMs = 40;
 #define F_SETPIPE_SZ 1031
 #endif  // defined(F_SETPIPE_SZ)
 
-// Pipename used to capture audio stream from.
-// TODO(sergeyu): Pass this to AudioCapturerLinux constructor once we have
-// Linux-specific DesktopEnvironmentFactory
-base::LazyInstance<FilePath>::Leaky
-    g_audio_pipe_name = LAZY_INSTANCE_INITIALIZER;
+
+base::LazyInstance<scoped_refptr<AudioCapturerLinuxCore> >::Leaky
+    g_audio_capturer_core = LAZY_INSTANCE_INITIALIZER;
 
 const int IsPacketOfSilence(const std::string& data) {
   const int64* int_buf = reinterpret_cast<const int64*>(data.data());
@@ -58,7 +56,23 @@ const int IsPacketOfSilence(const std::string& data) {
 
 }  // namespace
 
-AudioCapturerLinux::AudioCapturerLinux(const FilePath& pipe_name) {
+void AudioCapturerLinuxCore::Initialize(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    const FilePath& pipe_name) {
+  g_audio_capturer_core.Get() =
+      new AudioCapturerLinuxCore(task_runner, pipe_name);
+}
+
+AudioCapturerLinuxCore::AudioCapturerLinuxCore(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    const FilePath& pipe_name)
+    : main_task_runner_(task_runner),
+      capturers_(new ObserverListThreadSafe<AudioCapturerLinux>()) {
+  main_task_runner_->PostTask(FROM_HERE, base::Bind(
+      &AudioCapturerLinuxCore::StartReading, this, pipe_name));
+}
+
+void AudioCapturerLinuxCore::StartReading(const FilePath& pipe_name) {
   pipe_fd_ = HANDLE_EINTR(open(
       pipe_name.value().c_str(), O_RDONLY | O_NONBLOCK));
   if (pipe_fd_ < 0) {
@@ -78,43 +92,33 @@ AudioCapturerLinux::AudioCapturerLinux(const FilePath& pipe_name) {
   WaitForPipeReadable();
 }
 
-AudioCapturerLinux::~AudioCapturerLinux() {
-}
-
-bool AudioCapturerLinux::Start(const PacketCapturedCallback& callback) {
-  if (pipe_fd_ < 0)
-    return false;
-
-  callback_ = callback;
-
-  return true;
+AudioCapturerLinuxCore::~AudioCapturerLinuxCore() {
 }
 
-void AudioCapturerLinux::Stop() {
-  callback_.Reset();
+void AudioCapturerLinuxCore::AddCapturer(AudioCapturerLinux* capturer) {
+  capturers_->AddObserver(capturer);
 }
-
-bool AudioCapturerLinux::IsStarted() {
-  return !callback_.is_null();
+void AudioCapturerLinuxCore::RemoveCapturer(AudioCapturerLinux* capturer) {
+  capturers_->RemoveObserver(capturer);
 }
 
-void AudioCapturerLinux::OnFileCanReadWithoutBlocking(int fd) {
+void AudioCapturerLinuxCore::OnFileCanReadWithoutBlocking(int fd) {
   DCHECK_EQ(fd, pipe_fd_);
   StartTimer();
 }
 
-void AudioCapturerLinux::OnFileCanWriteWithoutBlocking(int fd) {
+void AudioCapturerLinuxCore::OnFileCanWriteWithoutBlocking(int fd) {
   NOTREACHED();
 }
 
-void AudioCapturerLinux::StartTimer() {
+void AudioCapturerLinuxCore::StartTimer() {
   started_time_ = base::TimeTicks::Now();
   last_capture_samples_ = 0;
   timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kCapturingPeriodMs),
-               this, &AudioCapturerLinux::DoCapture);
+               this, &AudioCapturerLinuxCore::DoCapture);
 }
 
-void AudioCapturerLinux::DoCapture() {
+void AudioCapturerLinuxCore::DoCapture() {
   DCHECK_GT(pipe_fd_, 0);
 
   // Calculate how much we need read from the pipe. Pulseaudio doesn't control
@@ -158,41 +162,66 @@ void AudioCapturerLinux::DoCapture() {
                           incomplete_samples_bytes);
   data.resize(pos - incomplete_samples_bytes);
 
-  if (callback_.is_null())
-    return;
-
   if (IsPacketOfSilence(data))
     return;
 
-  scoped_ptr<AudioPacket> packet(new AudioPacket());
-  packet->add_data(data);
-  packet->set_encoding(AudioPacket::ENCODING_RAW);
-  packet->set_sampling_rate(kSamplingRate);
-  packet->set_bytes_per_sample(AudioPacket::BYTES_PER_SAMPLE_2);
-  packet->set_channels(AudioPacket::CHANNELS_STEREO);
-  callback_.Run(packet.Pass());
+  scoped_refptr<base::RefCountedString> data_ref =

[Wez, 2012/10/30 04:20:11]

nit: Add a comment e.g. "Dispatch asynchronous notification to capturers"

[Sergey Ulanov, 2012/10/30 23:59:48]

Done.

+      base::RefCountedString::TakeString(&data);
+  capturers_->Notify(&AudioCapturerLinux::OnDataRead, data_ref);
 }
 
-void AudioCapturerLinux::WaitForPipeReadable() {
+void AudioCapturerLinuxCore::WaitForPipeReadable() {
   timer_.Stop();
   MessageLoopForIO::current()->WatchFileDescriptor(
       pipe_fd_, false, MessageLoopForIO::WATCH_READ,
       &file_descriptor_watcher_, this);
 }
 
-void AudioCapturerLinux::SetPipeName(const FilePath& pipe_name) {
-  g_audio_pipe_name.Get() = pipe_name;
+AudioCapturerLinux::AudioCapturerLinux(
+    scoped_refptr<AudioCapturerLinuxCore> core)
+    : core_(core) {
+}
+
+AudioCapturerLinux::~AudioCapturerLinux() {
+}
+
+bool AudioCapturerLinux::Start(const PacketCapturedCallback& callback) {
+  callback_ = callback;
+  core_->AddCapturer(this);

[Wez, 2012/10/30 04:20:11]

nit: Use |g_audio_capturer_core| directly here?

[Sergey Ulanov, 2012/10/30 23:59:48]

See my other comments. I'd like to avoid usage of globals as much as possible.

+  return true;
+}
+
+void AudioCapturerLinux::Stop() {
+  core_->RemoveCapturer(this);
+  callback_.Reset();
+}
+
+bool AudioCapturerLinux::IsStarted() {
+  return !callback_.is_null();
+}
+
+void AudioCapturerLinux::OnDataRead(
+    scoped_refptr<base::RefCountedString> data) {
+  DCHECK(!callback_.is_null());
+
+  scoped_ptr<AudioPacket> packet(new AudioPacket());
+  packet->add_data(data->data());
+  packet->set_encoding(AudioPacket::ENCODING_RAW);
+  packet->set_sampling_rate(kSamplingRate);
+  packet->set_bytes_per_sample(AudioPacket::BYTES_PER_SAMPLE_2);
+  packet->set_channels(AudioPacket::CHANNELS_STEREO);
+  callback_.Run(packet.Pass());
 }
 
 bool AudioCapturer::IsSupported() {
-  return !g_audio_pipe_name.Get().empty();
+  return g_audio_capturer_core.Get() != NULL;
 }
 
 scoped_ptr<AudioCapturer> AudioCapturer::Create() {
-  FilePath path = g_audio_pipe_name.Get();
-  if (path.empty())
+  scoped_refptr<AudioCapturerLinuxCore> core = g_audio_capturer_core.Get();
+  if (!core)
     return scoped_ptr<AudioCapturer>();
-  return scoped_ptr<AudioCapturer>(new AudioCapturerLinux(path));
+  return scoped_ptr<AudioCapturer>(new AudioCapturerLinux(core));
 }
 
 }  // namespace remoting