1185388 - [EME] Limit cores used by WMF in gmp-clearkey

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P2)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

As in bug 1172841, we should limit the number of cores that WMF uses to decode in gmp-clearkey, so that Firefox's rendering doesn't get starved on low end hardware or at high framerates.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Edwin, can you take this please?

Comment 2

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1185388.patch

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8642210 [details] [diff] [review]
1185388.patch

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

::: media/gmp-clearkey/0.1/WMFUtils.cpp
@@ +257,5 @@
>  
> +static int32_t
> +ComputeNumThreads()
> +{
> +  SYSTEM_INFO sysInfo;

Use the aCoreCount argument passed to VideoDecoder::InitDecode instead of Win32 APIs here so there's no risk of the sandbox breaking this and so that Firefox can control this.

Comment 4

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1185388.patch

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8642632 [details] [diff] [review]
1185388.patch

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

::: media/gmp-clearkey/0.1/WMFUtils.cpp
@@ +256,5 @@
>  
> +int32_t
> +GetNumThreads(int32_t aCoreCount)
> +{
> +  return aCoreCount > 4 ? -1 : (std::max)(aCoreCount - 1, 1);

Nit: Do you need the "()" around (std::max)? i.e. can this be:

std::max(aCoreCount - 1, 1);

?

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/66a7b65bdff9

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/66a7b65bdff9