Add UMA stats for silence AudioInputController.

Add UMA stats for silence AudioInputController.

4 cases of silence during a session: no measurement, no silence, only silence and partial silence.

NOTRY=true

BUG=399835
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289688

[Henrik Grunell, 2014-08-12 14:59:40]

Tommi: media/audio/*
Ilya: histograms.xml

I wonder if we'll actually get useful data from this. I think it would be good
to see 0 exactly on it's own. It should also be compared to the length of the
session. Maybe percentage is better?

[tommi (sloooow) - chröme, 2014-08-12 15:13:48]

On 2014/08/12 14:59:40, Henrik Grunell wrote:
> Tommi: media/audio/*
> Ilya: histograms.xml
> 
> I wonder if we'll actually get useful data from this. I think it would be good
> to see 0 exactly on it's own. It should also be compared to the length of the
> session. Maybe percentage is better?

I think this approach is interesting but it's also not exactly clear to me
either how practical this is (maybe we can only know by trying it out?).  Can we
chat with xians and henrika tomorrow to see what they think?  They have more
experience with this type of issues so hopefully can reflect on what's
practical.

[Ilya Sherman, 2014-08-12 20:15:53]

Histogram LGTM.  You can check me on this by viewing what gets logged at
chrome://histograms, but I believe that 0 is already going to be logged
precisely.

https://codereview.chromium.org/462813004/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/462813004/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:10554: +<histogram
name="Media.AudioInputControllerSilenceCount">
nit: Please add a units attribute.

[Henrik Grunell, 2014-08-13 11:59:25]

Changed to report 4 silence cases instead. Ptal.

[tommi (sloooow) - chröme, 2014-08-13 18:28:04]

looks pretty good.  a couple of suggestions

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
File media/audio/audio_input_controller.cc (right):

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
media/audio/audio_input_controller.cc:492: else if (silence_state_ ==
SILENCE_STATE_NO_SILENCE)
nit: I find "NO_SILENCE" to be a bit confusing when reading the other constants
too (e.g. ONLY_SILENCE)... can we change it to e.g. "HAVE_AUDIO", "AUDIO_GOOD",
"AUDIO_CAPTURED" or something like that?

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
media/audio/audio_input_controller.cc:493: silence_state_ =
SILENCE_STATE_PARTIAL_SILENCE;
nit:
else
  DCHECK_EQ(silence_state_, SILENCE_STATE_PARTIAL_SILENCE);

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
media/audio/audio_input_controller.cc:498: silence_state_ =
SILENCE_STATE_PARTIAL_SILENCE;
nit: same else case here.  good for sanity checking and readability (imho)

https://codereview.chromium.org/462813004/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/462813004/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:36747: +  <int value="1" label="No
silence"/>
"Have audio", "Audio good", "Audio received", etc... "no silence" sounds like
cacophony or something to me for some reason :)

[Henrik Grunell, 2014-08-14 11:21:45]

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
File media/audio/audio_input_controller.cc (right):

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
media/audio/audio_input_controller.cc:492: else if (silence_state_ ==
SILENCE_STATE_NO_SILENCE)
On 2014/08/13 18:28:04, tommi wrote:
> nit: I find "NO_SILENCE" to be a bit confusing when reading the other
constants
> too (e.g. ONLY_SILENCE)... can we change it to e.g. "HAVE_AUDIO",
"AUDIO_GOOD",
> "AUDIO_CAPTURED" or something like that?

I changed it to ONLY_AUDIO. And the PARTIAL_SILENCE to AUDIO_AND_SILENCE.

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
media/audio/audio_input_controller.cc:493: silence_state_ =
SILENCE_STATE_PARTIAL_SILENCE;
On 2014/08/13 18:28:04, tommi wrote:
> nit:
> else
>   DCHECK_EQ(silence_state_, SILENCE_STATE_PARTIAL_SILENCE);

Well it can be that or ONLY_SILENCE. Done.

https://codereview.chromium.org/462813004/diff/40001/media/audio/audio_input_...
media/audio/audio_input_controller.cc:498: silence_state_ =
SILENCE_STATE_PARTIAL_SILENCE;
On 2014/08/13 18:28:04, tommi wrote:
> nit: same else case here.  good for sanity checking and readability (imho)

Done.

https://codereview.chromium.org/462813004/diff/40001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/462813004/diff/40001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:36747: +  <int value="1" label="No
silence"/>
On 2014/08/13 18:28:04, tommi wrote:
> "Have audio", "Audio good", "Audio received", etc... "no silence" sounds like
> cacophony or something to me for some reason :)

Done.

[tommi (sloooow) - chröme, 2014-08-14 14:06:43]

lgtm

[Henrik Grunell, 2014-08-14 14:20:11]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-08-14 14:23:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/grunell@chromium.org/462813004/60001

[commit-bot: I haz the power, 2014-08-14 18:19:45]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-14 19:42:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-14 19:42:22]

Try jobs failed on following builders:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Henrik Grunell, 2014-08-14 20:15:42]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-08-14 20:17:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/grunell@chromium.org/462813004/60001

[commit-bot: I haz the power, 2014-08-14 20:20:36]

Committed patchset #4 (60001) as 289688