The default bug view has changed. See this FAQ.

WebRTC voice engine code disabled on Android

RESOLVED FIXED in mozilla20

Status

()

Core
WebRTC: Audio/Video
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dmose, Assigned: dmose)

Tracking

(Blocks: 1 bug)

Trunk
mozilla20
ARM
Android
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Created attachment 690288 [details] [diff] [review]
explicitly un-disable voice engine code for android WIP, v1

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.

Updated

4 years ago
Whiteboard: [WebRTC][blocking-webrtc-]
(Assignee)

Comment 1

4 years ago
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
(Assignee)

Comment 2

4 years ago
Created attachment 694957 [details] [diff] [review]
re-enable voice engine code for Android, v2
Attachment #690288 - Attachment is obsolete: true
Attachment #694957 - Flags: review?(rjesup)
(Assignee)

Comment 3

4 years ago
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+
(Assignee)

Comment 5

4 years ago
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+
(Assignee)

Updated

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

Updated

4 years ago
Whiteboard: [WebRTC][blocking-webrtc-] → [WebRTC][blocking-webrtc-][qa-]
You need to log in before you can comment on or make changes to this bug.