Replace AudioSilenceDetector with an AudioPowerMonitor.

media/audio/audio_power_monitor.h

+// Copyright (c) 2013 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.
+
+#ifndef MEDIA_AUDIO_AUDIO_POWER_MONITOR_H_
+#define MEDIA_AUDIO_AUDIO_POWER_MONITOR_H_
+
+#include <limits>
+
+#include "base/callback.h"
+#include "media/base/media_export.h"
+
+// An audio signal power monitor.  It is periodically provided an AudioBus by
+// the native audio thread, and the audio samples in each channel are analyzed
+// to determine the average power of the signal over a time period.  Here
+// "average power" is a running average calculated by using a first-order
+// low-pass filter over the samples scanned.  Whenever reporting the power

[Chris Rogers, 2013/07/18 00:09:36]

"over the samples" -> "over the square of the samples"

[miu, 2013/07/19 01:08:07]

Done.

+// level, this running average is converted to dBFS (decibels relative to
+// full-scale) units.
+//
+// Note that extreme care has been taken to make the AudioPowerMonitor::Scan()
+// method safe to be called on the native audio thread.  The code acquires no
+// locks, nor engages in any operation that could result in an
+// undetermined/unbounded amount of run-time.
+
+namespace base {
+class MessageLoop;
+class TimeDelta;
+}
+
+namespace media {
+
+class AudioBus;
+
+class MEDIA_EXPORT AudioPowerMonitor {
+ public:
+  // Reports power level in terms of dBFS (see zero_power() and max_power()
+  // below).  |clipped| is true if any *one* sample exceeded maximum amplitude
+  // since the last invocation.

[Chris Rogers, 2013/07/18 00:09:36]

I think to really do this well, the idea of "clipped" needs to be maintained
over a longer time perioed than just every single invocation.  In UIs for peak
detectors (to detect "overs" above 0dbFS) there's usually a hold/timeout period
of a second or two if even one single sample is detected to be over.  That way
the UI can register something sinister like a very obvious red line or bar which
is persistent for a little while.

[miu, 2013/07/19 01:08:07]

Agreed (with the UI details).  I wanted to keep the logic of the power
measurement separate from the UI details.  The callback here simply reports
changes in power levels (and clip status), whereas the UI code will interpret
how the data should be communicated visually.  I'm working with sail@ on this,
and will point this out to him as he proceeds with UI development.

+  typedef base::Callback<void(float power_dbfs, bool clipped)>
+      PowerMeasurementCallback;
+
+  // |sample_rate| is the audio signal sample rate (Hz).  |time_constant|
+  // characterizes how samples are averaged over time to determine the power
+  // level; and is the amount of time it takes a zero power level to increase to
+  // ~63.2% of maximum given a step input signal.  |measurement_period| is the
+  // time length of signal to analyze before invoking the callback to report the
+  // current power level.  |message_loop| is where the |notify_power_level|
+  // callback will be posted.

[Chris Rogers, 2013/07/18 00:09:36]

Not that you have to change this all now, so I'll make this comment as just a
future suggestion.

I don't understand why we're doing the analysis and "pushing" the information to
another thread where the UI is drawn.  In other words, the |measurement_period|
and Post should not really be necessary.  In my experience with desktop/native
audio apps, normally this type of system simply is continuously analysing the
signal in the native audio thread and writing the power and peak value to a
memory location, which is then read by the UI thread which re-draws the metering
UI every 60 or 30 frames per seconds (or whatever the refresh rate), but the
timer triggering re-draw of the UI would be decoupled from this code here.

It seems overly complex to have to communicate with a Post, although I do
appreciate the effort put into making this safe using a "try" version of Post,
so I'm not adamently opposed to it...

[miu, 2013/07/19 01:08:07]

I'll discuss this further with the UI side of the team.  I think the immediate
answer to your question is that the UI design around the existing indicators
uses a push model, and so leveraging that allowed us to get things done faster. 
We should revisit these design decisions once all tab UI indicators reach a more
stable state, and the interactions with the tab strip rendering implementation
is better understood.

+  AudioPowerMonitor(int sample_rate,
+                    const base::TimeDelta& time_constant,
+                    const base::TimeDelta& measurement_period,
+                    base::MessageLoop* message_loop,
+                    const PowerMeasurementCallback& notify_power_level);
+
+  ~AudioPowerMonitor();
+
+  // Scan more |frames| of audio data from |buffer|.  It is safe to call this
+  // from a real-time priority thread.
+  void Scan(const AudioBus& buffer, int frames);
+
+  // dBFS value corresponding to zero power in the audio signal.
+  static float zero_power() { return -std::numeric_limits<float>::infinity(); }
+
+  // dBFS value corresponding to maximum power in the audio signal.
+  static float max_power() { return 0.0f; }
+

[Chris Rogers, 2013/07/18 00:09:36]

Just for completeness, you might consider adding a Reset() method which re-inits
power state variables:
average_power_
clipped_since_last_notification_
frames_since_last_notification_
last_reported_power_
last_reported_clipped_

[miu, 2013/07/19 01:08:07]

The issue here would be around the threading.  I assume the Reset() method would
be called by a thread other than the native audio thread, which means that
somehow these values would have to be reset transactionally.  While it's
possible to do without locking, I don't think I should add that until there's a
need for it.

+ private:
+  // The weight applied when averaging-in each sample.  Computed from the
+  // |sample_rate| and |time_constant|.
+  const float sample_weight_;
+
+  // Number of audio frames to be scanned before reporting the current power
+  // level via callback, as computed from |sample_rate| and
+  // |measurement_period|.
+  const int num_frames_per_callback_;
+
+  // MessageLoop and callback used to notify of the current power level.
+  base::MessageLoop* const message_loop_;
+  const PowerMeasurementCallback notify_power_level_;

[Chris Rogers, 2013/07/18 00:09:36]

maybe a better name would be "power_level_callback_"

[miu, 2013/07/19 01:08:07]

Done.

+
+  // Accumulated results over one or more calls to Scan().
+  float average_power_;
+  bool clipped_since_last_notification_;
+  int frames_since_last_notification_;
+
+  // Keep track of last reported results to forgo making redundant
+  // notifications.

[Chris Rogers, 2013/07/18 00:09:36]

nit: notifications can go on previous comment line

[miu, 2013/07/19 01:08:07]

It's one character too long.  :(  I s/notifications/callbacks/ to fit.

+  float last_reported_power_;
+  bool last_reported_clipped_;
+
+  DISALLOW_COPY_AND_ASSIGN(AudioPowerMonitor);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_AUDIO_AUDIO_POWER_MONITOR_H_