Jean-Yves Avenard [:jya] Assignee
	Description • 9 years ago

There appears to be severe performance issue with the Apple VideoToolbox decoder even with hardware acceleration is enabled.

A 4K YouTube videos show lots of dropped frame when using VideoToolBox with Intel GPU.

On a late-2013 macbook air (Intel HD5000), the average time it takes to decode a 4K frame is:
- Software decode: 95ms average, max 469ms
- VideoToolbox with HW acceleration: 31ms average, max 326ms
- VDA (pure HW acceleration): .63ms average, max 97ms

To not have dropped frames, we must decode in less than 30ms a 4K @ 30Hz. As it is the VideoToolbox decoder is borderline in the average case, and way over at its worse.

This patch track the attempt to prefer the VDA decoder for higher resolution video.

	Jean-Yves Avenard [:jya] Assignee
	Comment 1 • 9 years ago

Add detection if decoding is hardware accelerated and related log

	Ralph Giles (:rillian)
	Comment 2 • 9 years ago

Comment on attachment 8565776 [details] [diff] [review]
Part1. Add logging reporting if we are using HW accelerated decode

Review of attachment 8565776 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with question addressed.

::: dom/media/fmp4/apple/AppleVTDecoder.cpp
@@ +317,5 @@
> +    LOG("AppleVTDecoder: %s hardware accelerated decoding",
> +        (rv == noErr && isUsingHW == kCFBooleanTrue) ? "using" : "not using");
> +  } else {
> +    LOG("AppleVTDecoder: using software decoding "
> +        "(platform doesn't support HW acceleration)");

Is this true? Do we know MacOS X 10.8 and earlier won't use hardware acceleration, despite the property key not being defined in the API?

	Jean-Yves Avenard [:jya] Assignee
	Comment 3 • 9 years ago

As far as I can say, is that there's no way to tell.
Even the doc about kVTVideoDecoderSpecification_EnableHardwareAcceleratedVideoDecoder are confusing at best as the only guaranteed behaviour is when you set it to false, in which case HW acceleration will be disabled.

It is suggested that the VT decoder will, when it's true or not set decide what to use and when.
However, reports like bug 1132994, indicates to me that unless it's set to true, we won't get HW acceleration (as that's the only difference between 35 and 36)

On my mac pro, while the key exist, I can't query it, and when requesting all VTSession keys anything related to HW is missing (and VDA always return an error)

	Ralph Giles (:rillian)
	Comment 4 • 9 years ago

Who knows then. I just wanted to make sure we didn't leave a confusing message in the log. How about "Couldn't determine hardware acceleration status."?

	Jean-Yves Avenard [:jya] Assignee
	Comment 5 • 9 years ago

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/5d68cdc80ddf

	Carsten Book [:Tomcat]
	Comment 6 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/5d68cdc80ddf

	Ralph Giles (:rillian)
	Comment 7 • 9 years ago

Comment on attachment 8565776 [details] [diff] [review]
Part1. Add logging reporting if we are using HW accelerated decode

Requestint 36 uplift of teh initial logging patch.

Approval Request Comment
[Feature/regressing bug #]: MSE and mp4 playback on Mac.
[User impact if declined]: Slow video decoding on some systems.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: This just adds logging plumbing. Risk should be low.
[String/UUID change made/needed]: None.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 8 • 9 years ago

Comment on attachment 8565776 [details] [diff] [review]
Part1. Add logging reporting if we are using HW accelerated decode

Looks like this may do a bit more than just logging but we definitely want the change in 37 Beta 1 in which we're testing MSE on OSX. 37 is now Beta so clearing Aurora request and approving for Beta.

Beta+

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 9 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/85e3bc280be6

	Jean-Yves Avenard [:jya] Assignee
	Comment 10 • 9 years ago

enable async decoding on apple VT.

	Jean-Yves Avenard [:jya] Assignee
	Comment 11 • 9 years ago

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a05f09431d9c

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/a05f09431d9c

	Jean-Yves Avenard [:jya] Assignee
	Comment 13 • 9 years ago

Comment on attachment 8585883 [details] [diff] [review]
Part2. Enable async decoding on mac

Approval Request Comment
[Feature/regressing bug #]: 1133633
[User impact if declined]: stuttering playback, dropped frames
[Describe test coverage new/current, TreeHerder]: This patch significantly improve the performance of the decoder on mac, a 4K video will go from 60% dropped frames to 0%. Patch will have to be rebase for beta.
[Risks and why]: Low.
[String/UUID change made/needed]: None

	Sylvestre Ledru [:Sylvestre]
	Comment 14 • 9 years ago

Jean-Yves, are you planning to provide the beta patch?

	Jean-Yves Avenard [:jya] Assignee
	Comment 15 • 9 years ago

Comment on attachment 8585883 [details] [diff] [review]
Part2. Enable async decoding on mac

I remove the request for beta uplift.

MSE isn't enabled on mac in beta, so let's urgent there.

	Ryan VanderMeulen [:RyanVM]
	Comment 16 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/8ddb0e3d862d

	Jean-Yves Avenard [:jya] Assignee
	Comment 17 • 9 years ago

Comment on attachment 8585883 [details] [diff] [review]
Part2. Enable async decoding on mac

We may enable MSE in 38.. So we would want that in as well.

	Sylvestre Ledru [:Sylvestre]
	Comment 18 • 9 years ago

Comment on attachment 8585883 [details] [diff] [review]
Part2. Enable async decoding on mac

Taking it in case we decide to do something with MSE under Mac.
Should be in 38 beta 4.

	Jean-Yves Avenard [:jya] Assignee
	Comment 19 • 9 years ago

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/10f75583d21a