UMA stats for browser/renderer audio rendering buffer size mismatch.

media/audio/audio_output_resampler.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 "media/audio/audio_output_resampler.h"
 
 #include <stdint.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/single_thread_task_runner.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "media/audio/audio_output_proxy.h"
 #include "media/audio/sample_rates.h"
 #include "media/base/audio_converter.h"
 #include "media/base/limits.h"
 
 namespace media {
@@ skipping 104 matching lines @@
     UMA_HISTOGRAM_ENUMERATION(
         "Media.FallbackHardwareAudioSamplesPerSecond",
         asr, kAudioSampleRateMax + 1);
   } else {
     UMA_HISTOGRAM_COUNTS(
         "Media.FallbackHardwareAudioSamplesPerSecondUnexpected",
         output_params.sample_rate());
   }
 }
 
+// Record UMA statistics for input/output rebuffering.
+static void RecordRebufferingStats(const AudioParameters& input_params,
+                                   const AudioParameters& output_params) {
+  const int& input_buffer_size = input_params.frames_per_buffer();
+  const int& output_buffer_size = output_params.frames_per_buffer();

[Ilya Sherman, 2016/08/24 22:20:23]

nit: An int is generally going to be smaller or equally sized to a pointer, and
have one less level of indirection than a reference.  I'll defer to owners of
this code, but IMO it's generally preferable to copy ints than to have const
references to them ;-)

[o1ka, 2016/08/25 11:48:34]

It's extending lifetime of a temporary rather than having a reference (aka
pointer), so it should not require any additional memory, AFAIK. On the other
hand, "const int" should be optimized by copy elision => does not cause any
copy. I'll leave out references, just to make the code clearer.

+  DCHECK(input_buffer_size);
+  DCHECK(output_buffer_size);
+  int value = (output_buffer_size - input_buffer_size) * 1000 /
+              std::min(input_buffer_size, output_buffer_size);

[Ilya Sherman, 2016/08/24 22:20:23]

What is the possible range for these values?  It's not appropriate to use a
sparse histogram to record a value that might take on many different for a
single client.

[o1ka, 2016/08/25 11:48:34]

For both buffer sizes the range is between 128 and 4096.
A buffer size is usually: (a) a power of 2, or (b) a sample rate
(https://en.wikipedia.org/wiki/Sampling_(signal_processing)#Sampling_rate)
divided by 100 (10 ms buffers) or 50 (20 ms buffers).

So, the range of the values we want to record is [-31, 31] (before scaling), but
we'll see only a subset of integers in that range, and for some of those
integers a very limited subset of non-round values.

I'm not sure what is considered to be a client in this context. We expect that
each (audio hardware + drivers) combination will produce 1 value per histogram
(might be 2-3 in some cases if code paths for the specific audio latency diverge
on renderer side)

[Ilya Sherman, 2016/08/26 00:35:34]

I think you are trying to pack too much into a single histogram.  It would be
great if we could record 2D histograms -- and, actually, if you know the precise
set of inputs, you could just flatten the cross-product.

OTOH, if you're not sure that you know the exact set of inputs, then I'd
recommend recording two histograms: One indicating whether the sizes are
compatible, and another indicating the precise periodicity if they do not match
(or, in all cases, if you prefer).  That would both (a) give you clearer data,
and (b) make me more confident that I can correctly reason about the structure
of the histogram.

+
+  switch (input_params.latency_tag()) {
+    case AudioLatency::LATENCY_EXACT_MS:
+      UMA_HISTOGRAM_SPARSE_SLOWLY(
+          "Media.Audio.Render.BufferSizeMismatch.LatencyExactMs", value);
+      return;
+    case AudioLatency::LATENCY_INTERACTIVE:
+      UMA_HISTOGRAM_SPARSE_SLOWLY(
+          "Media.Audio.Render.BufferSizeMismatch.LatencyInteractive", value);
+      return;
+    case AudioLatency::LATENCY_RTC:
+      UMA_HISTOGRAM_SPARSE_SLOWLY(
+          "Media.Audio.Render.BufferSizeMismatch.LatencyRtc", value);
+      return;
+    case AudioLatency::LATENCY_PLAYBACK:
+      UMA_HISTOGRAM_SPARSE_SLOWLY(
+          "Media.Audio.Render.BufferSizeMismatch.LatencyPlayback", value);
+      return;
+    default:

[Ilya Sherman, 2016/08/24 22:20:23]

Please explicitly list all possible cases.  AFAICT there are no other valid
cases, so there is no reason to have an additional histogram.

[o1ka, 2016/08/25 11:48:34]

Agree. Done

+      UMA_HISTOGRAM_SPARSE_SLOWLY(
+          "Media.Audio.Render.BufferSizeMismatch.LatencyOther", value);
+  }
+}
+
 // Converts low latency based |output_params| into high latency appropriate
 // output parameters in error situations.
 void AudioOutputResampler::SetupFallbackParams() {
 // Only Windows has a high latency output driver that is not the same as the low
 // latency path.
 #if defined(OS_WIN)
   // Choose AudioParameters appropriate for opening the device in high latency
   // mode.  |kMinLowLatencyFrameSize| is arbitrarily based on Pepper Flash's
   // MAXIMUM frame size for low latency.
   static const int kMinLowLatencyFrameSize = 2048;
@@ skipping 199 matching lines @@
 
 OnMoreDataConverter::OnMoreDataConverter(const AudioParameters& input_params,
                                          const AudioParameters& output_params)
     : io_ratio_(static_cast<double>(input_params.GetBytesPerSecond()) /
                 output_params.GetBytesPerSecond()),
       source_callback_(nullptr),
       input_bytes_per_frame_(input_params.GetBytesPerFrame()),
       audio_converter_(input_params, output_params, false),
       error_occurred_(false),
       input_buffer_size_(input_params.frames_per_buffer()),
-      output_buffer_size_(output_params.frames_per_buffer()) {}
+      output_buffer_size_(output_params.frames_per_buffer()) {
+  RecordRebufferingStats(input_params, output_params);
+}
 
 OnMoreDataConverter::~OnMoreDataConverter() {
   // Ensure Stop() has been called so we don't end up with an AudioOutputStream
   // calling back into OnMoreData() after destruction.
   CHECK(!source_callback_);
 }
 
 void OnMoreDataConverter::Start(
     AudioOutputStream::AudioSourceCallback* callback) {
   CHECK(!source_callback_);
@@ skipping 43 matching lines @@
     dest->ZeroFramesPartial(frames, dest->frames() - frames);
   return frames > 0 ? 1 : 0;
 }
 
 void OnMoreDataConverter::OnError(AudioOutputStream* stream) {
   error_occurred_ = true;
   source_callback_->OnError(stream);
 }
 
 }  // namespace media