Add a histogram for video decoder fallback.

Add a histogram for video decoder fallback.

BUG=447732
TEST=Manually inspect chrome:histograms.

Committed: https://crrev.com/c85d60e78f5e252bde45aacc2d61548f915ba53a
Cr-Commit-Position: refs/heads/master@{#311525}

[watk, 2015-01-13 18:25:32]

watk@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[watk, 2015-01-13 18:25:33]

I tested that a fall back results in both a Success and Failure, and that a
normal video with audio results in only one Failure.

[DaleCurtis, 2015-01-13 19:20:50]

As discussed offline I think this will be unbalanced, both a true and a false
will be recorded in the fallback case; double check with the UMA team to be sure
though.

Instead I think the statistic should be recorded once within MediaInternals as
we do with some of the other pipeline statistics.

https://codereview.chromium.org/847223002/diff/1/media/filters/decoder_stream.cc
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/847223002/diff/1/media/filters/decoder_stream...
media/filters/decoder_stream.cc:262: if (GetStreamType() ==
DemuxerStream::VIDEO)
Why the get StreamType() method? Does StreamType == not work?

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

https://codereview.chromium.org/847223002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:12907: +    previously intialized one
failed to reinitialize. Failure is recorded when a
Everything after "Failure..." seems a bit more than necessary.  We generally try
to keep these concise.  How about: "Whether Chrome had to fallback to a
secondary decoder after the primary decoder failed reinitialization."

[watk, 2015-01-13 22:23:17]

Patchset #2 (id:20001) has been deleted

[watk, 2015-01-13 23:19:08]

On 2015/01/13 19:20:50, DaleCurtis wrote:
> As discussed offline I think this will be unbalanced, both a true and a false
> will be recorded in the fallback case; double check with the UMA team to be
sure
> though.
> 
> Instead I think the statistic should be recorded once within MediaInternals as
> we do with some of the other pipeline statistics.

Done.
 
>
https://codereview.chromium.org/847223002/diff/1/media/filters/decoder_stream.cc
> File media/filters/decoder_stream.cc (right):
> 
>
https://codereview.chromium.org/847223002/diff/1/media/filters/decoder_stream...
> media/filters/decoder_stream.cc:262: if (GetStreamType() ==
> DemuxerStream::VIDEO)
> Why the get StreamType() method? Does StreamType == not work?

It does.. silly me.

>
https://codereview.chromium.org/847223002/diff/1/tools/metrics/histograms/his...
> File tools/metrics/histograms/histograms.xml (right):
> 
>
https://codereview.chromium.org/847223002/diff/1/tools/metrics/histograms/his...
> tools/metrics/histograms/histograms.xml:12907: +    previously intialized one
> failed to reinitialize. Failure is recorded when a
> Everything after "Failure..." seems a bit more than necessary.  We generally
try
> to keep these concise.  How about: "Whether Chrome had to fallback to a
> secondary decoder after the primary decoder failed reinitialization."

Sure, no longer necessary now that the values don't need extra interpretation.

[watk, 2015-01-13 23:20:10]

watk@chromium.org changed reviewers:
+ asvitkine@chromium.org

[watk, 2015-01-13 23:20:11]

asvitkine@ could you please take a look at histograms.xml?

[DaleCurtis, 2015-01-13 23:36:16]

I'd double check that setting the src= to a new codec won't trigger this code
too. It shouldn't, but best to check :)

lgtm % that.

[watk, 2015-01-14 00:56:06]

On 2015/01/13 23:36:16, DaleCurtis wrote:
> I'd double check that setting the src= to a new codec won't trigger this code
> too. It shouldn't, but best to check :)

Confirmed that it's not a problem. (Has a separate player_id).

> lgtm % that.

[Alexei Svitkine (slow), 2015-01-14 17:25:58]

lgtm

[watk, 2015-01-14 17:29:32]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2015-01-14 17:30:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/847223002/40001

[commit-bot: I haz the power, 2015-01-14 19:08:59]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-01-14 19:10:56]

Patchset 2 (id:??) landed as
https://crrev.com/c85d60e78f5e252bde45aacc2d61548f915ba53a
Cr-Commit-Position: refs/heads/master@{#311525}