1392583 - Crash on Android when requesting audio gUM with agc:true

Status: RESOLVED FIXED

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

Description

[Andreas Pehrson [:pehrsons]]

This bug was filed from the Socorro interface and is 
report bp-766b43b7-f798-477c-8dd1-dc3b40170816.
=============================================================

This crash reproduces consistently on Android (I have a Sony Xperia Z5) with https://codepen.io/brainshave/pen/JyOeYg.

Seems triggered by requesting agc for the audio capture.

Comment 1

[Dan Minor [:dminor]]

I'm thinking that rather than:

    FATAL() << "Should never be called";

it would be sufficient to just log the function call and return an error code like many of the other functions in media/webrtc/trunk/webrtc/modules/audio_device/android/audio_device_template.h do.

Comment 2

[Dan Minor [:dminor]]

Attachment: Bug 1392583 - Remove calls to FATAL() from audio_device_template.h for unimplemented functions;

This removes calls to FATAL() and replaces them with LOG() statements. The
unimplemented functions already returned error codes.

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

Comment 3

[Paul Adenot (:padenot)]

Comment on attachment 8899851 [details]
Bug 1392583 - Remove calls to FATAL() from audio_device_template.h for unimplemented functions;

https://reviewboard.mozilla.org/r/171176/#review176322

Can you please nominate for uplift when landed?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4025523b22a3
Remove calls to FATAL() from audio_device_template.h for unimplemented functions; r=padenot

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4025523b22a3

Comment 6

[Dan Minor [:dminor]]

Comment on attachment 8899851 [details]
Bug 1392583 - Remove calls to FATAL() from audio_device_template.h for unimplemented functions;

Approval Request Comment
[Feature/Bug causing the regression]:
Most likely the webrtc.org update in Bug 1341285, although similar code could have existed before.
[User impact if declined]:
Crashes when attempting to make a WebRTC call with AGC enabled on Android.
[Is this code covered by automated tests?]:
No, we only run a subset of tests on Android due to reliability problems on the test devices.
[Has the fix been verified in Nightly?]:
Yes.
[Needs manual test from QE? If yes, steps to reproduce]: 
No.
[List of other uplifts needed for the feature/fix]:
None.
[Is the change risky?]:
No.
[Why is the change risky/not risky?]:
The changes the behaviour for calling an unimplemented function from crashing to logging the function call and returning an error code.
[String changes made/needed]:
None.

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8899851 [details]
Bug 1392583 - Remove calls to FATAL() from audio_device_template.h for unimplemented functions;

Crash fix for WebRTC, let's take it for beta 6. 

Andreas, would you mind verifying this on beta, once beta 6 is out (after Friday)?

Comment 8

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-beta/rev/9ea3c3462336

Comment 9

[(Out Sep 17-23) Ryan VanderMeulen]

There isn't a Fennec b6, so verification will need to wait until after next week's b7 build is created.

Comment 10

[Andreas Pehrson [:pehrsons]]

Verified working on b7 with the codepen in comment 0.