Add UMA to track software fallback in VTVideoDecodeAccelerator.

Add UMA to track software fallback in VTVideoDecodeAccelerator.

VideoToolbox can fall back to software internally for two reasons:
  1) The resolution is below the minimum (or other format incompatibility). Eventually we want Chromium to handle
fallback in this case.
  2) The sandbox was not initialized properly. This we want to know about and fix.

This CL does not distinguish the two cases.

BUG=133828
Committed: https://crrev.com/3125ca55d73650bd37ce9d9daa3dfe5182a7cf3a
Cr-Commit-Position: refs/heads/master@{#306692}

[sandersd (OOO until July 31), 2014-12-02 00:57:34]

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

[Pawel Osciak, 2014-12-02 01:10:08]

On 2014/12/02 00:57:35, sandersd wrote:

We already have Media.GpuVideoDecoderInitializeStatus:
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/gpu_....

[sandersd (OOO until July 31), 2014-12-02 01:15:27]

On 2014/12/02 01:10:08, Pawel Osciak wrote:
> On 2014/12/02 00:57:35, sandersd wrote:
> 
> We already have Media.GpuVideoDecoderInitializeStatus:
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/gpu_....

This is a very different UMA, it tracks whether VideoToolbox is falling back on
software decode *internally*. This can happen in two cases:
  1) The resolution is below the minimum. Eventually we want Chrome to handle
fallback in this case.
  2) The sandbox was not initialized properly and hardware decoding failed. This
I want to know about and fix.

[sandersd (OOO until July 31), 2014-12-02 01:18:16]

On 2014/12/02 01:15:27, sandersd wrote:
> On 2014/12/02 01:10:08, Pawel Osciak wrote:
> > On 2014/12/02 00:57:35, sandersd wrote:
> > 
> > We already have Media.GpuVideoDecoderInitializeStatus:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/gpu_....
> 
> This is a very different UMA, it tracks whether VideoToolbox is falling back
on
> software decode *internally*. This can happen in two cases:
>   1) The resolution is below the minimum. Eventually we want Chrome to handle
> fallback in this case.
>   2) The sandbox was not initialized properly and hardware decoding failed.
This
> I want to know about and fix.

Addendum: I can't actually distinguish the two cases from the session
properties, but a separate UMA to track this specifically for video >= 480P
should only count (2).

[Pawel Osciak, 2014-12-02 01:21:45]

On 2014/12/02 01:15:27, sandersd wrote:
> On 2014/12/02 01:10:08, Pawel Osciak wrote:
> > On 2014/12/02 00:57:35, sandersd wrote:
> > 
> > We already have Media.GpuVideoDecoderInitializeStatus:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/gpu_....
> 
> This is a very different UMA, it tracks whether VideoToolbox is falling back
on
> software decode *internally*. This can happen in two cases:

I see, sorry, thanks for the clarification. CL description could've been a
little bit less succinct ;)

[Pawel Osciak, 2014-12-02 01:21:50]

posciak@chromium.org changed reviewers:
+ posciak@chromium.org

[Pawel Osciak, 2014-12-02 01:21:51]

https://codereview.chromium.org/773673002/diff/1/content/common/gpu/media/vt_...
File content/common/gpu/media/vt_video_decode_accelerator.cc (right):

https://codereview.chromium.org/773673002/diff/1/content/common/gpu/media/vt_...
content/common/gpu/media/vt_video_decode_accelerator.cc:322: if
(!FinishDelayedFrames())
This is I think not related to this CL?

[sandersd (OOO until July 31), 2014-12-02 01:25:31]

https://codereview.chromium.org/773673002/diff/1/content/common/gpu/media/vt_...
File content/common/gpu/media/vt_video_decode_accelerator.cc (right):

https://codereview.chromium.org/773673002/diff/1/content/common/gpu/media/vt_...
content/common/gpu/media/vt_video_decode_accelerator.cc:322: if
(!FinishDelayedFrames())
On 2014/12/02 01:21:51, Pawel Osciak wrote:
> This is I think not related to this CL?

Done.

[DaleCurtis, 2014-12-02 18:20:22]

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
File content/common/gpu/media/vt_video_decode_accelerator.cc (right):

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
content/common/gpu/media/vt_video_decode_accelerator.cc:338: if
(VTSessionCopyProperty(
Does clang-format bless this alignment? It looks off.

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
content/common/gpu/media/vt_video_decode_accelerator.cc:344:
UMA_HISTOGRAM_BOOLEAN("Media.VTVDA.UsingHardware",
Don't you need to log this for some of the other failures above? Probably not
the session reinitialization ones, but the others may cause early abort which
falls back to the software path.

[sandersd (OOO until July 31), 2014-12-02 18:28:29]

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
File content/common/gpu/media/vt_video_decode_accelerator.cc (right):

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
content/common/gpu/media/vt_video_decode_accelerator.cc:338: if
(VTSessionCopyProperty(
On 2014/12/02 18:20:22, DaleCurtis wrote:
> Does clang-format bless this alignment? It looks off.

Done.

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
content/common/gpu/media/vt_video_decode_accelerator.cc:344:
UMA_HISTOGRAM_BOOLEAN("Media.VTVDA.UsingHardware",
On 2014/12/02 18:20:22, DaleCurtis wrote:
> Don't you need to log this for some of the other failures above? Probably not
> the session reinitialization ones, but the others may cause early abort which
> falls back to the software path.

This is the only path with internal software fallback, if it fails before this
the whole VDA fails.

[DaleCurtis, 2014-12-02 18:29:59]

lgtm % updating the cl description

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
File content/common/gpu/media/vt_video_decode_accelerator.cc (right):

https://codereview.chromium.org/773673002/diff/20001/content/common/gpu/media...
content/common/gpu/media/vt_video_decode_accelerator.cc:344:
UMA_HISTOGRAM_BOOLEAN("Media.VTVDA.UsingHardware",
On 2014/12/02 18:28:29, sandersd wrote:
> On 2014/12/02 18:20:22, DaleCurtis wrote:
> > Don't you need to log this for some of the other failures above? Probably
not
> > the session reinitialization ones, but the others may cause early abort
which
> > falls back to the software path.
> 
> This is the only path with internal software fallback, if it fails before this
> the whole VDA fails.

Ahhhh, as posciak@ noted, you definitely need a better CL description then :) I
thought this was tracking global software fallback.

[sandersd (OOO until July 31), 2014-12-02 18:35:19]

sandersd@chromium.org changed reviewers:
+ isherman@chromium.org

[sandersd (OOO until July 31), 2014-12-02 18:35:20]

isherman@chromium.org: Please review changes in histograms.xml.

[sandersd (OOO until July 31), 2014-12-02 20:40:37]

Patchset #4 (id:60001) has been deleted

[Ilya Sherman, 2014-12-02 23:26:40]

histograms lgtm % a nit:

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

https://codereview.chromium.org/773673002/diff/80001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:12750: +<histogram
name="Media.VTVDA.UsingHardware" enum="Boolean">
nit: Please create a boolean histogram with more customized labels.

[sandersd (OOO until July 31), 2014-12-03 18:33:14]

Patchset #6 (id:120001) has been deleted

[sandersd (OOO until July 31), 2014-12-03 18:33:35]

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

https://codereview.chromium.org/773673002/diff/80001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:12750: +<histogram
name="Media.VTVDA.UsingHardware" enum="Boolean">
On 2014/12/02 23:26:40, Ilya Sherman wrote:
> nit: Please create a boolean histogram with more customized labels.

Done.

[Ilya Sherman, 2014-12-03 19:22:27]

Thanks.  (Still LGTM.)

[sandersd (OOO until July 31), 2014-12-03 20:48:52]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-12-03 20:49:36]

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

[commit-bot: I haz the power, 2014-12-03 22:33:27]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2014-12-03 22:34:08]

Patchset 5 (id:??) landed as
https://crrev.com/3125ca55d73650bd37ce9d9daa3dfe5182a7cf3a
Cr-Commit-Position: refs/heads/master@{#306692}