1374204 - [Firefox for Android] set preference for hardware encoder to true in nightly

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, enhancement, P2)

Description

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

We introduced the android hardware encoder in bug 1265755 and isolated the change with preference. Now we plan to enable it in nightly and ensure mochitest still test the legacy encoder.

Comment 1

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1374204 - set the preference for android hardware encoder to true in nightly.

Review commit: https://reviewboard.mozilla.org/r/150412/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150412/

Comment 2

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1374204 - remove test_peerConnection_basicVideoRemoteHwEncoder.html.

Review commit: https://reviewboard.mozilla.org/r/150414/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150414/

Comment 3

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1374204 - verify webrtc peerconnection tests with android hardware encoder and legacy encoder.

Review commit: https://reviewboard.mozilla.org/r/150416/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150416/

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8879084 [details]
Bug 1374204 - verify webrtc peerconnection tests with android hardware encoder and legacy encoder.

https://reviewboard.mozilla.org/r/150416/#review155340

r- for breaking error propagation in tests that rely on the default error path [1].

[1] https://dxr.mozilla.org/mozilla-central/rev/95543bdc59bd038a3d5d084b85a4fec493c349ee/dom/media/tests/mochitest/head.js#371-376

::: dom/media/tests/mochitest/pc.js:2037
(Diff revision 1)
>  };
>  
>  function runNetworkTest(testFunction, fixtureOptions) {
>    fixtureOptions = fixtureOptions || {}
> +
> +  let version = SpecialPowers.Cc["@mozilla.org/xre/app-info;1"].getService(SpecialPowers.Ci.nsIXULAppInfo).version;

wordwrap or break up

::: dom/media/tests/mochitest/pc.js:2056
(Diff revision 1)
> +            }, function (options = {}) {
> +              testFunction(options)
> +            }))

This breaks error propagation if testFunction() returns a promise.

SpecialPowers.pushPrefEnv() returns a promise. We should use that instead of the legacy callback API.

Also, the if-else here duplicates too much code IMHO.

Can we rewrite runNetworkTest() using async/await?

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8879082 [details]
Bug 1374204 - set the preference for android hardware encoder to true in nightly.

https://reviewboard.mozilla.org/r/150412/#review155350

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8879083 [details]
Bug 1374204 - remove test_peerConnection_basicVideoRemoteHwEncoder.html.

https://reviewboard.mozilla.org/r/150414/#review155352

Comment 7

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8879084 [details]
Bug 1374204 - verify webrtc peerconnection tests with android hardware encoder and legacy encoder.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150416/diff/1-2/

Comment 8

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8879084 [details]
Bug 1374204 - verify webrtc peerconnection tests with android hardware encoder and legacy encoder.

https://reviewboard.mozilla.org/r/150416/#review155896

Awesome, thanks!

::: dom/media/tests/mochitest/pc.js:2034
(Diff revisions 1 - 2)
> -function runNetworkTest(testFunction, fixtureOptions) {
> +async function runNetworkTest(testFunction, fixtureOptions) {
>    fixtureOptions = fixtureOptions || {}

We can save a line here with argument defaults:

    async function runNetworkTest(testFunction, fixtureOptions = {}) {

Comment 9

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8879082 [details]
Bug 1374204 - set the preference for android hardware encoder to true in nightly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150412/diff/1-2/

Comment 10

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8879083 [details]
Bug 1374204 - remove test_peerConnection_basicVideoRemoteHwEncoder.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150414/diff/1-2/

Comment 11

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Comment on attachment 8879084 [details]
Bug 1374204 - verify webrtc peerconnection tests with android hardware encoder and legacy encoder.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150416/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41bf073edcae
set the preference for android hardware encoder to true in nightly. r=jib
https://hg.mozilla.org/integration/autoland/rev/a15608555656
remove test_peerConnection_basicVideoRemoteHwEncoder.html. r=jib
https://hg.mozilla.org/integration/autoland/rev/99a5d811d40f
verify webrtc peerconnection tests with android hardware encoder and legacy encoder. r=jib

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/41bf073edcae
https://hg.mozilla.org/mozilla-central/rev/a15608555656
https://hg.mozilla.org/mozilla-central/rev/99a5d811d40f