Closed Bug 1374204 Opened 3 years ago Closed 3 years ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: mchiang, Assigned: mchiang)

References

Details

Attachments

(3 files)

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 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?
Attachment #8879084 - Flags: review?(jib) → review-
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
Attachment #8879082 - Flags: review?(jib) → review+
Comment on attachment 8879083 [details]
Bug 1374204 - remove test_peerConnection_basicVideoRemoteHwEncoder.html.

https://reviewboard.mozilla.org/r/150414/#review155352
Attachment #8879083 - Flags: review?(jib) → review+
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 = {}) {
Attachment #8879084 - Flags: review?(jib) → review+
Assignee: nobody → mchiang
Rank: 25
Priority: -- → P2
Pushed by ryanvm@gmail.com:
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
Keywords: checkin-needed
Depends on: 1375634
You need to log in before you can comment on or make changes to this bug.