1599799 - Mochitest crashes when Android H.264 support for WebRTC is turned on.

Status: RESOLVED FIXED

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

Description

[John Lin [:jhlin][:jolin]]

Follow-up of bug 1592140 comment 9.
With the patch in bug 1592140, treeherder tests failed:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=277110101&resultStatus=testfailed%2Cbusted%2Cexception&revision=f4a9e675a33f01ce97adce2296b9b21ce5e3111f

After investigation, WebrtcMediaDataEncoder has two defects:

1. ProcessEncode() can resolve after mCallback is cleared by RegisterEncodeCompleteCallback()
2. ProcessEncode() can be preempted by Shutdown() and calls Java method with deleted mEnocoder.

Comment 1

[John Lin [:jhlin][:jolin]]

Attachment: Bug 1599799 - p1: check Java encoder existence before processing output. r?jya

Java callback runs on Android binder thread and can be preempted by
encoder task queue so that ProcessOutput() is queued after
ProcessShutdown(). Discard the output when that happens.

Comment 2

[John Lin [:jhlin][:jolin]]

Attachment: Bug 1599799 - p2: discard frame when callback is invalid. r?jya

Client of WebrtcMediaDataEncoder unregisters callback before calling
Shutdown(). When a pending encode promise gets resolved after that,
it will cause null pointer error.

Depends on D55759

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51a360648ea2
p1: check Java encoder existence before processing output. r=jya
https://hg.mozilla.org/integration/autoland/rev/fc9ac6839a9e
p2: discard frame when callback is invalid. r=jya

Comment 4

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/51a360648ea2
https://hg.mozilla.org/mozilla-central/rev/fc9ac6839a9e

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on these patches if they block bug 1592140.

Comment 6

[John Lin [:jhlin][:jolin]]

Comment on attachment 9113372 [details]
Bug 1599799 - p1: check Java encoder existence before processing output. r?jya

Beta/Release Uplift Approval Request

• User impact if declined: Geckoview-based apps won't be able to communicate with WebRTC apps that support only H.264 video.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1592140
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patches include only minor code changes to address automation tests failures.
• String changes made/needed: N/A

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9113372 [details]
Bug 1599799 - p1: check Java encoder existence before processing output. r?jya

Needed for bug 1592140 so platform decoder can be used for H264 WebRTC calls. Approved for GV73.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9288fb4aefd5
https://hg.mozilla.org/releases/mozilla-beta/rev/2301535d0cb1