Replace AudioSilenceDetector with an AudioPowerMonitor.

chrome/browser/media/audio_stream_indicator.h

 // 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.
 
 #ifndef CHROME_BROWSER_MEDIA_AUDIO_STREAM_INDICATOR_H_
 #define CHROME_BROWSER_MEDIA_AUDIO_STREAM_INDICATOR_H_
 
 #include <map>
-#include <set>
 
 #include "base/memory/ref_counted.h"
 
 namespace content {
 class WebContents;
 }
 
 class AudioStreamIndicator
     : public base::RefCountedThreadSafe<AudioStreamIndicator> {
  public:
   AudioStreamIndicator();
 
   // This method should be called on the IO thread.
   void UpdateWebContentsStatus(int render_process_id,
                                int render_view_id,
                                int stream_id,
-                               bool is_playing_and_audible);
+                               bool is_playing,
+                               float power_dBFS,

[DaleCurtis, 2013/07/02 22:26:34]

Style should be all lowercase I think.

[miu, 2013/07/09 00:59:56]

Done.

+                               bool clipped);
 
   // This method should be called on the UI thread.
-  bool IsPlayingAudio(content::WebContents* contents);
+  bool IsPlayingAudio(const content::WebContents* contents);
+
+  // Returns the audible |level| in the range [0.0,1.0], where 0.0 means the
+  // audio signal is imperceivably silent and 1.0 means it is at maximum
+  // volume.  |signal_has_clipped| is set to true if any part of the audio
+  // signal has clipped since the last call to this method.
+  //
+  // This method should be called on the UI thread.
+  //
+  // TODO(miu): Consider replacing this "pull" API with an event-driven "push"
+  // API.
+  void CurrentAudibleLevel(const content::WebContents* contents,
+                           float* level, bool* signal_has_clipped);
 
  private:
   struct RenderViewId {

[DaleCurtis, 2013/07/02 22:26:34]

If you just use std::pair<int, int> you can avoid a lot of code and take
advantage of binary savings via folding with other maps using pairs of ints.

[miu, 2013/07/09 00:59:56]

Done.

I absolutely agree.  There was a past discussion on a previous code review; but
IIRC, it centered strictly around one reviewer's style preference.  I think use
of std::pair<> here has technical merit and is inline with the "eliminate code
bloat" philosophy of the Chromium project.

     RenderViewId(int render_process_id,
                  int render_view_id);
 
-    // Required to use this struct in the std::multiset below.
+    // Required to use this struct in the std::map below.
     bool operator<(const RenderViewId& other) const;
 
     int render_process_id;
     int render_view_id;
   };
 
+  struct SignalPower {
+    float power_dBFS;
+    bool clipped;
+  };
+
   friend class base::RefCountedThreadSafe<AudioStreamIndicator>;
   virtual ~AudioStreamIndicator();
 
   void UpdateWebContentsStatusOnUIThread(int render_process_id,
                                          int render_view_id,
                                          int stream_id,
-                                         bool playing);
+                                         bool is_playing,
+                                         float power_dBFS,
+                                         bool clipped);
 
-  // A map from RenderViews to sets of streams playing in them (each RenderView
-  // might have more than one stream).
-  std::map<RenderViewId, std::set<int> > audio_streams_;
+  // A map from RenderViews to the streams playing in them (each RenderView
+  // might have more than one stream).  An inner map associates each stream with
+  // its last-known power level.
+  typedef std::map<int, SignalPower> StreamPowerLevelMap;

[DaleCurtis, 2013/07/02 22:26:34]

About how many streams/renderview are we expecting? I wonder if just using a
vector of structs here would be more efficient than a nested map.

Alternatively, I think if you're going make clipped sticky across calls, you can
extend the concept to power levels.  What do you think about this:

struct TabPowerLevel { bool clipped; set<float> levels; }
typedef map<std::pair<int, int>, TabPowerLevel> RenderViewStreamMap;

On Update(), or the clip flag and add the power level to the set. Each call to
CurrentAudibleLevel() call clears the set and clipped flag.  To return the max
power level you just take set.rbegin() since it's ordered.  If empty, assume
silence.

This should be fine so long as CurrentAudibleLevel is called regularly.  As a
sanity check, if the set gets too large you can clear it during update.

[miu, 2013/07/09 00:59:56]

Took your advice on eliminating the nested map since the find() operations
should be efficient w/ low numbers of streams (as expected).  No more useless
sorting.  ;-)

I realized I was doing the wrong thing here w.r.t. sticky-then-clear handling of
the clip flag.  The reason is that Update() will only be called when the power
level/clip state changes.  So, for example, if sound is continuously clipping at
max power, there will never be any calls to Update() until that changes. 
Therefore, it was erroneous for me to be clearing the clip flag when the UI
calls CurrentAudibleLevel().

Overall, I think it makes sense to simply maintain the power and clip state
per-stream and have the CurrentAudibleLevel() call scan all streams' power
levels.

+  typedef std::map<RenderViewId, StreamPowerLevelMap> RenderViewStreamMap;
+  RenderViewStreamMap audio_streams_;
 };
 
 #endif  // CHROME_BROWSER_MEDIA_AUDIO_STREAM_INDICATOR_H_