Closed Bug 819856 Opened 7 years ago Closed 7 years ago

WebRTC voice engine code disabled on Android

Categories

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

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: dmose, Assigned: dmose)

References

(Blocks 1 open bug)

Details

(Whiteboard: [WebRTC][blocking-webrtc-][qa-])

Attachments

(1 file, 1 obsolete file)

Right now, the WebRTC voice engine code doesn't work right on Android because it is explicitly disabled for Android.  The attached patch that ekr and I whipped up on Friday re-enables that code.

It's not currently why the code was explicitly disabled, and whether just re-enabling it this way is at all a good idea; which is what needs some research.
Whiteboard: [WebRTC][blocking-webrtc-]
After discussion with jesup and ekr, the decision has been made to go with a version of this patch (re-enabled some stuff for Android), since it puts the code on a better trajectory: more unit tests pass on Android.  Separately, jesup will ping Niklas to try and understand more of the history about why this stuff was disabled, which will, if we're lucky, be helpful in deciding what to do going forward.
Assignee: nobody → dmose
Attachment #690288 - Attachment is obsolete: true
Attachment #694957 - Flags: review?(rjesup)
With this patch, the mediaconduit_unittests and mediapipeline_unittests pass.  When the signaling_unittests aren't failing for one of various as-yet-undiagnosed-but-presumably-unrelated reasons, all of them now pass!

Try server run, in progress:

https://tbpl.mozilla.org/?tree=Try&rev=39a5d1620124
Comment on attachment 694957 [details] [diff] [review]
re-enable voice engine code for Android, v2

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

::: media/webrtc/trunk/src/voice_engine/voice_engine_defines.h
@@ +411,5 @@
>  
> +  // This macro used to cause the calling function to set an error code and return
> +  // However, not doing that seems to cause the unit tests to pass / behave reasonably,
> +  // so it's disabled for now.  Jesup is going to ping Niklas about the original motivation for the 
> +  // macro, which may help us make a more informed choice about what to do about it.

Replace the "Jesup is going to..." with "See bug 819856"
Attachment #694957 - Flags: review?(rjesup) → review+
Pushed a version with the updated comment to mozilla-inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/03759110bbd7

This made existing broken tests pass, so in-testsuite+ set.
Flags: in-testsuite+
Summary: WebRTC voice engine code doesn't work on Android → WebRTC voice engine code disabled on Android
https://hg.mozilla.org/mozilla-central/rev/03759110bbd7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Whiteboard: [WebRTC][blocking-webrtc-] → [WebRTC][blocking-webrtc-][qa-]
You need to log in before you can comment on or make changes to this bug.