Fix AudioPipeReader to use default pipe buffer size.

remoting/host/linux/audio_pipe_reader.cc

 // Copyright (c) 2012 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.
 
 #include "remoting/host/linux/audio_pipe_reader.h"
 
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
 
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/stl_util.h"
 
 namespace remoting {
 
 namespace {
 
 const int kSampleBytesPerSecond = AudioPipeReader::kSamplingRate *
                                   AudioPipeReader::kChannels *
                                   AudioPipeReader::kBytesPerSample;
 
-// Read data from the pipe every 40ms.
-const int kCapturingPeriodMs = 40;
-
-// Size of the pipe buffer in milliseconds.
-const int kPipeBufferSizeMs = kCapturingPeriodMs * 2;
-
-// Size of the pipe buffer in bytes.
-const int kPipeBufferSizeBytes = kPipeBufferSizeMs * kSampleBytesPerSecond /
-    base::Time::kMillisecondsPerSecond;
-
 #if !defined(F_SETPIPE_SZ)
 // F_SETPIPE_SZ is supported only starting linux 2.6.35, but we want to be able
 // to compile this code on machines with older kernel.
 #define F_SETPIPE_SZ 1031
 #endif  // defined(F_SETPIPE_SZ)
 
 }  // namespace
 
 // static
 scoped_refptr<AudioPipeReader> AudioPipeReader::Create(
@@ skipping 86 matching lines @@
   pipe_ = new_pipe.Pass();
 
   if (pipe_.IsValid()) {
     // Set O_NONBLOCK flag.
     if (HANDLE_EINTR(fcntl(pipe_.GetPlatformFile(), F_SETFL, O_NONBLOCK)) < 0) {
       PLOG(ERROR) << "fcntl";
       pipe_.Close();
       return;
     }
 
-    // Set buffer size for the pipe.
-    if (HANDLE_EINTR(fcntl(
-            pipe_.GetPlatformFile(), F_SETPIPE_SZ, kPipeBufferSizeBytes)) < 0) {
-      PLOG(ERROR) << "fcntl";
-    }
+    pipe_buffer_size_ = fpathconf(pipe_.GetPlatformFile(), _PC_PIPE_BUF);

[Lambros, 2014/12/23 23:07:18]

I don't think this call can fail, can it?

[Sergey Ulanov, 2014/12/24 00:01:28]

it shouldn't, but it's still better to check if it does. Done.

+    capture_period_ = base::TimeDelta::FromSeconds(1) * pipe_buffer_size_ /
+                      kSampleBytesPerSecond / 2;

[Lambros, 2014/12/23 23:07:18]

I'm curious to know why we divide by 2? Is it simply so that we capture twice as
often as we really need?

[Sergey Ulanov, 2014/12/24 00:01:26]

Yes. We need to capture more often than the buffer length. E.g. if the buffer
can fit 20ms of audio samples then we need to capture a more often than every
20ms. If we try scheduling the timer every 20ms, then, because the timer is not
precise, sometimes more than 20ms will pass between between DoCapture() calls
and as result it won't be reading enough audio samples, starving the stream on
the client.

 
     WaitForPipeReadable();
   }
 }
 
 void AudioPipeReader::StartTimer() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   started_time_ = base::TimeTicks::Now();
   last_capture_position_ = 0;
-  timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kCapturingPeriodMs),
-               this, &AudioPipeReader::DoCapture);
+  timer_.Start(FROM_HERE, capture_period_, this, &AudioPipeReader::DoCapture);
 }
 
 void AudioPipeReader::DoCapture() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(pipe_.IsValid());
 
   // Calculate how much we need read from the pipe. Pulseaudio doesn't control
   // how much data it writes to the pipe, so we need to pace the stream.
   base::TimeDelta stream_position = base::TimeTicks::Now() - started_time_;
   int64 stream_position_bytes = stream_position.InMilliseconds() *
@@ skipping 28 matching lines @@
   int incomplete_samples_bytes = pos % (kChannels * kBytesPerSample);
   left_over_bytes_.assign(data, pos - incomplete_samples_bytes,
                           incomplete_samples_bytes);
   data.resize(pos - incomplete_samples_bytes);
 
   last_capture_position_ += data.size();
   // Normally PulseAudio will keep pipe buffer full, so we should always be able
   // to read |bytes_to_read| bytes, but in case it's misbehaving we need to make
   // sure that |stream_position_bytes| doesn't go out of sync with the current
   // stream position.
-  if (stream_position_bytes - last_capture_position_ > kPipeBufferSizeBytes)
-    last_capture_position_ = stream_position_bytes - kPipeBufferSizeBytes;
+  if (stream_position_bytes - last_capture_position_ > pipe_buffer_size_)
+    last_capture_position_ = stream_position_bytes - pipe_buffer_size_;
   DCHECK_LE(last_capture_position_, stream_position_bytes);
 
   // Dispatch asynchronous notification to the stream observers.
   scoped_refptr<base::RefCountedString> data_ref =
       base::RefCountedString::TakeString(&data);
   observers_->Notify(&StreamObserver::OnDataRead, data_ref);
 }
 
 void AudioPipeReader::WaitForPipeReadable() {
   timer_.Stop();
   base::MessageLoopForIO::current()->WatchFileDescriptor(
       pipe_.GetPlatformFile(), false, base::MessageLoopForIO::WATCH_READ,
       &file_descriptor_watcher_, this);
 }
 
 // static
 void AudioPipeReaderTraits::Destruct(const AudioPipeReader* audio_pipe_reader) {
   audio_pipe_reader->task_runner_->DeleteSoon(FROM_HERE, audio_pipe_reader);
 }
 
 }  // namespace remoting