Ensure that data is not overwritten in the audio input shared memory ring buffer that sits on the b…

content/browser/renderer_host/media/audio_input_sync_writer.cc

Index: content/browser/renderer_host/media/audio_input_sync_writer.cc
diff --git a/content/browser/renderer_host/media/audio_input_sync_writer.cc b/content/browser/renderer_host/media/audio_input_sync_writer.cc
index 600881af9a93799011f993ebaae27a766a2414c0..7eb0507f7a358d81d72f510908d577212e03d6b2 100644
--- a/content/browser/renderer_host/media/audio_input_sync_writer.cc
+++ b/content/browser/renderer_host/media/audio_input_sync_writer.cc
@@ -7,12 +7,31 @@
 #include <algorithm>
 
 #include "base/memory/shared_memory.h"
+#include "base/metrics/histogram.h"
+#include "base/strings/stringprintf.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 
 using media::AudioBus;
 
 namespace content {
 
+namespace {
+
+  // TODO BEFORE COMMIT: Which value makes sense? I.e. how long can we accept
+  // blocking the soundcard thread and wait before dropping data.

[tommi (sloooow) - chröme, 2015/08/28 13:43:06]

Could this be dynamically chosen somehow based on the buffer size in use? 
Bigger buffers would then have more resiliency which would be naturally
expected.

[Henrik Grunell, 2015/09/02 14:27:55]

I assume you mean number of segments in the ring buffer. I'm not sure that would
make things better for us. I don't see the benefits. Despite that, what value
should we have here? My gut feeling tells me 10 ms is a reasonable value - the
usual size of each data chunk and average interval between each processing
round.

+  const base::TimeDelta kReadCheckTimeout =
+      base::TimeDelta::FromMilliseconds(10);
+
+// Used to log if any audio glitches have been detected during an audio session.
+// Elements in this enum should not be added, deleted or rearranged.
+enum AudioGlitchResult {
+  AUDIO_CAPTURER_NO_AUDIO_GLITCHES = 0,
+  AUDIO_CAPTURER_AUDIO_GLITCHES = 1,
+  AUDIO_CAPTURER_AUDIO_GLITCHES_MAX = AUDIO_CAPTURER_AUDIO_GLITCHES
+};
+
+}  // namespace
+
 AudioInputSyncWriter::AudioInputSyncWriter(base::SharedMemory* shared_memory,
                                            int shared_memory_segment_count,
                                            const media::AudioParameters& params)
@@ -21,7 +40,10 @@ AudioInputSyncWriter::AudioInputSyncWriter(base::SharedMemory* shared_memory,
       current_segment_id_(0),
       creation_time_(base::Time::Now()),
       audio_bus_memory_size_(AudioBus::CalculateMemorySize(params)),
-      next_buffer_id_(0) {
+      next_buffer_id_(0),
+      expected_buffer_reads_(0),
+      read_verification_count_(0),
+      read_verification_timeout_count_(0) {
   DCHECK_GT(shared_memory_segment_count, 0);
   DCHECK_EQ(shared_memory->requested_size() % shared_memory_segment_count, 0u);
   shared_memory_segment_size_ =
@@ -45,7 +67,27 @@ AudioInputSyncWriter::AudioInputSyncWriter(base::SharedMemory* shared_memory,
   }
 }
 
-AudioInputSyncWriter::~AudioInputSyncWriter() {}
+AudioInputSyncWriter::~AudioInputSyncWriter() {
+  if (read_verification_count_ == 0)
+    return;
+
+  UMA_HISTOGRAM_PERCENTAGE(
+      "Media.AudioCapturerMissedReadDeadline",
+      100.0 * read_verification_timeout_count_ / read_verification_count_);
+
+  UMA_HISTOGRAM_ENUMERATION("Media.AudioCapturerAudioGlitches",
+                            read_verification_timeout_count_ == 0 ?
+                                AUDIO_CAPTURER_NO_AUDIO_GLITCHES :
+                                AUDIO_CAPTURER_AUDIO_GLITCHES,
+                            AUDIO_CAPTURER_AUDIO_GLITCHES_MAX + 1);
+
+  std::string log_string = base::StringPrintf(
+      "AISW: number of detected audio glitches: %lu out of %lu",
+      read_verification_timeout_count_,
+      read_verification_count_);
+  MediaStreamManager::SendMessageToNativeLog(log_string);
+  DVLOG(1) << log_string;
+}
 
 void AudioInputSyncWriter::Write(const media::AudioBus* data,
                                  double volume,
@@ -77,6 +119,34 @@ void AudioInputSyncWriter::Write(const media::AudioBus* data,
   last_write_time_ = base::Time::Now();
 #endif
 
+  // Check that the renderer side has read data so that we don't overwrite.
+  // The renderer side sends a signal over the socket each time it has read
+  // data. Here, we count the number of reads we expect to have been done. When
+  // we reach |shared_memory_segment_count_|, we do
+  // (|shared_memory_segment_count_| / 2) socket receives, which normally is
+  // in queue. If we timeout, there's a problem with being able to read  at the
+  // high enough pace, and we throw away the audio buffer.
+  if (expected_buffer_reads_ ==
+      static_cast<int>(shared_memory_segment_count_)) {
+    size_t bytes_received = 0;
+    uint32 dummy = 0;
+    for (uint32 i = 0; i < (shared_memory_segment_count_ + 1) / 2; ++i) {

[tommi (sloooow) - chröme, 2015/08/28 13:43:06]

nit: calculate nr or repetitions outside the loop

[Henrik Grunell, 2015/09/02 14:27:55]

I've removed the loop. I think Dale has a point, we could simply just check once
when wee need.

+      ++read_verification_count_;
+      bytes_received =
+          socket_->ReceiveWithTimeout(&dummy, sizeof(dummy), kReadCheckTimeout);
+      if (bytes_received != sizeof(dummy)) {

[tommi (sloooow) - chröme, 2015/08/28 13:43:06]

if we timeout, should we break from the loop?

Btw, could we use the value we receive to verify the expected read state too?

[Henrik Grunell, 2015/09/02 14:27:55]

Loop is gone.
Yes, that's a good point. Done.

+        const std::string error_message =
+            "Verifying shared memory read timed out. Dropping audio data.";
+        LOG(ERROR) << error_message;
+        MediaStreamManager::SendMessageToNativeLog(error_message);
+        ++read_verification_timeout_count_;
+        return;
+      }
+      --expected_buffer_reads_;
+      DCHECK_GE(expected_buffer_reads_, 0);
+    }
+  }
+
   // Write audio parameters to shared memory.
   uint8* ptr = static_cast<uint8*>(shared_memory_->memory());
   ptr += current_segment_id_ * shared_memory_segment_size_;
@@ -97,6 +167,10 @@ void AudioInputSyncWriter::Write(const media::AudioBus* data,
 
   if (++current_segment_id_ >= shared_memory_segment_count_)
     current_segment_id_ = 0;
+
+  ++expected_buffer_reads_;
+  DCHECK_LE(expected_buffer_reads_,
+            static_cast<int>(shared_memory_segment_count_));
 }
 
 void AudioInputSyncWriter::Close() {