Add Media.AudioRendererEvents histogram to measure how often OnRenderError() is called.

media/filters/audio_renderer_impl.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/filters/audio_renderer_impl.h"
 
 #include <math.h>
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
+#include "base/metrics/histogram.h"
 #include "media/audio/audio_util.h"
 #include "media/base/audio_splicer.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/data_buffer.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/media_switches.h"
 #include "media/filters/audio_decoder_selector.h"
 #include "media/filters/decrypting_demuxer_stream.h"
 
 namespace media {
@@ skipping 250 matching lines @@
   int bytes_per_frame = channels * decoder_->bits_per_channel() / 8;
   splicer_.reset(new AudioSplicer(bytes_per_frame, sample_rate));
 
   // We're all good! Continue initializing the rest of the audio renderer based
   // on the decoder format.
   algorithm_.reset(new AudioRendererAlgorithm());
   algorithm_->Initialize(0, audio_parameters_);
 
   state_ = kPaused;
 
+  HISTOGRAM_BOOLEAN("Media.AudioRendererSinkErrors", false);

[DaleCurtis, 2013/04/24 20:56:39]

I believe you want UMA_*, otherwise this won't get uploaded?  Or is w/o UMA_ the
new public pattern?

I'm also not sure histograms work this way.  Won't this end up recording a false
and true value if an error occurs?  Is it smart enough to only record one value?

[jar (doing other things), 2013/04/24 21:51:07]

+1 for needing the prefix UMA_HISTOGRAM_BOOLEAN

The code, as written, will work, producing counts in two distinct buckets.  I
always hate the chance of (for example) misspelling the histogram in one place
or the other, so I'd suggest factoring out:

void histogram_renderer_status(bool error) {
  UMA_HISTOGRAM_BOOLEAN("Media.AudioRendererSinkErrors", error);
}

You can then call this from line 288 and 635.


On 2013/04/24 20:56:39, DaleCurtis wrote:

[scherkus (not reviewing), 2013/04/25 00:04:46]

Done.

+
   sink_->Initialize(audio_parameters_, weak_this_);
   sink_->Start();
 
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 void AudioRendererImpl::ResumeAfterUnderflow(bool buffer_more_audio) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
   if (state_ == kUnderflow) {
@@ skipping 325 matching lines @@
   base::TimeDelta predicted_play_time = base::TimeDelta::FromMicroseconds(
       static_cast<float>(frames_filled) * base::Time::kMicrosecondsPerSecond /
       audio_parameters_.sample_rate());
 
   lock_.AssertAcquired();
   earliest_end_time_ = std::max(
       earliest_end_time_, time_now + playback_delay + predicted_play_time);
 }
 
 void AudioRendererImpl::OnRenderError() {
+  HISTOGRAM_BOOLEAN("Media.AudioRendererSinkErrors", true);

[scherkus (not reviewing), 2013/04/24 20:27:41]

we can also record audio_parameters_ when hitting an error, but I'm inclined to
wait to see how often this gets hit (I long for way to record histograms for
parameters without having a histogram for each individual parameter!)

[jar (doing other things), 2013/04/24 21:51:07]

Some of this stuff *might* be pulled from dremel logs, looking for correlation
between failure reports, and parameters seen in other histograms.  I agree that
first you should see how significant the issue is. ;-)

On 2013/04/24 20:27:41, scherkus wrote:

   disabled_cb_.Run();
 }
 
 void AudioRendererImpl::DisableUnderflowForTesting() {
   underflow_disabled_ = true;
 }
 
 void AudioRendererImpl::HandleAbortedReadOrDecodeError(bool is_decode_error) {
   PipelineStatus status = is_decode_error ? PIPELINE_ERROR_DECODE : PIPELINE_OK;
   switch (state_) {
@@ skipping 15 matching lines @@
     case kUnderflow:
     case kRebuffering:
     case kStopped:
       if (status != PIPELINE_OK)
         error_cb_.Run(status);
       return;
   }
 }
 
 }  // namespace media