Cast:Updating logging

Cast:Updating logging

- Re-enabling the logging unittest
- Differentiating between stats and raw data in config and reset.
- Setting a const string for all UMA stats.

The above changes are a consequence of the following bugs/feature requests:
BUG=327194, 327960, 327958
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=246060

[mikhal1, 2014-01-13 19:32:45]

Please review, 
-Mikhal

[hguihot, 2014-01-13 19:51:10]

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
File media/cast/logging/logging_impl.cc (right):

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:90:
UMA_HISTOGRAM_TIMES("Cast.VideoRendertDelay", delay);
Typo.

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:168: UMA_HISTOGRAM_COUNTS("Cast.Rtt", value);
Cast.RttMs?

(Cast.JitterMs is used below)

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:212:
UMA_HISTOGRAM_COUNTS(CastLoggingToString(it->first),
This is not using a constant.

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:221:
UMA_HISTOGRAM_COUNTS("Cast.VideoBitRateKbps",
Bitrate? (no upper case R)

(RembBitrate does not use upper case R)

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:224:
UMA_HISTOGRAM_COUNTS("Cast.AudioBitRateKbps",
Bitrate?

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:256: switch (it->first) {
Will these histograms make sense considering these numbers can represent some
arbitrary duration (i.e. the time between calls to Reset() and
GetPacketsStatsData() can be anything)?

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:263: case kPacketRetransmited:
Typo (transmitted).

https://codereview.chromium.org/134843005/diff/1/media/cast/logging/logging_i...
media/cast/logging/logging_impl.cc:264:
UMA_HISTOGRAM_COUNTS("Cast.Stats.PacketRetransmited", it->second);
Same typo here too.

[mikhal1, 2014-01-14 20:09:58]

PTAL

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
File media/cast/logging/logging_impl.cc (right):

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:90:
UMA_HISTOGRAM_TIMES("Cast.VideoRendertDelay", delay);
On 2014/01/13 19:51:10, hguihot wrote:
> Typo.

Done.

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:168: UMA_HISTOGRAM_COUNTS("Cast.Rtt", value);
On 2014/01/13 19:51:10, hguihot wrote:
> Cast.RttMs?
> 
> (Cast.JitterMs is used below)

Done.

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:212:
UMA_HISTOGRAM_COUNTS(CastLoggingToString(it->first),
On 2014/01/13 19:51:10, hguihot wrote:
> This is not using a constant.

Done.

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:221:
UMA_HISTOGRAM_COUNTS("Cast.VideoBitRateKbps",
On 2014/01/13 19:51:10, hguihot wrote:
> Bitrate? (no upper case R)
> 
> (RembBitrate does not use upper case R)

Done.

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:224:
UMA_HISTOGRAM_COUNTS("Cast.AudioBitRateKbps",
On 2014/01/13 19:51:10, hguihot wrote:
> Bitrate?

Done.

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:256: switch (it->first) {
That is up to the user. I'm assuming the user will call reset and get these
numbers every ~1S.
On 2014/01/13 19:51:10, hguihot wrote:
> Will these histograms make sense considering these numbers can represent some
> arbitrary duration (i.e. the time between calls to Reset() and
> GetPacketsStatsData() can be anything)?

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:263: case kPacketRetransmited:
On 2014/01/13 19:51:10, hguihot wrote:
> Typo (transmitted).

Done.

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:264:
UMA_HISTOGRAM_COUNTS("Cast.Stats.PacketRetransmited", it->second);
On 2014/01/13 19:51:10, hguihot wrote:
> Same typo here too.

Done.

[hguihot, 2014-01-14 20:15:02]

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
File media/cast/logging/logging_impl.cc (right):

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:256: switch (it->first) {
I don't think we can assume that for all receivers though (making reading the
histograms more difficult, or impossible if the reader does not know the
sampling frequency). Can we convert those to packets/second (or other unit of
time) instead of just packets?

On 2014/01/14 20:09:58, mikhal1 wrote:
> That is up to the user. I'm assuming the user will call reset and get these
> numbers every ~1S.
> On 2014/01/13 19:51:10, hguihot wrote:
> > Will these histograms make sense considering these numbers can represent
some
> > arbitrary duration (i.e. the time between calls to Reset() and
> > GetPacketsStatsData() can be anything)?
>

[pwestin, 2014-01-17 23:32:26]

lgtm

[commit-bot: I haz the power, 2014-01-17 23:34:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mikhal@chromium.org/134843005/160001

[commit-bot: I haz the power, 2014-01-18 00:35:24]

Retried try job too often on linux_aura for step(s) compositor_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2014-01-21 16:21:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mikhal@chromium.org/134843005/160001

[commit-bot: I haz the power, 2014-01-21 17:11:53]

Change committed as 246060

[mikhal1, 2014-01-22 15:59:57]

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
File media/cast/logging/logging_impl.cc (right):

https://chromiumcodereview.appspot.com/134843005/diff/1/media/cast/logging/lo...
media/cast/logging/logging_impl.cc:256: switch (it->first) {
This implementation will be obsolete once my changes to the stats are submitted
(which follow our discussions). You will be added as a reviewer once I'll upload
that cl. Feel free to comment there.
On 2014/01/14 20:15:03, hguihot wrote:
> I don't think we can assume that for all receivers though (making reading the
> histograms more difficult, or impossible if the reader does not know the
> sampling frequency). Can we convert those to packets/second (or other unit of
> time) instead of just packets?
> 
> On 2014/01/14 20:09:58, mikhal1 wrote:
> > That is up to the user. I'm assuming the user will call reset and get these
> > numbers every ~1S.
> > On 2014/01/13 19:51:10, hguihot wrote:
> > > Will these histograms make sense considering these numbers can represent
> some
> > > arbitrary duration (i.e. the time between calls to Reset() and
> > > GetPacketsStatsData() can be anything)?
> > 
>