750869 - Support WebRTC for Android in our build system

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

Need to disable a whole much of linker and config stuff for Android from the gyp files, and modify configure.in to invoke gyp with the right options for Android.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Android patches for make system and gyp files (WIP, not working)

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

In particular, android-9 (2.3) needed for OpenSLES is not supported by the ELFLoader.  Probably more bugs past that...

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Android patches for make system and gyp files (WIP, not working) #2

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

FYI, this is the bug I told Joey about which captures partial implementation of Android support for WebRTC.

Without significant back-porting of OpenSLES, this would bump our minimum Android version to 2.3

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Android patches for make system and gyp files (WIP, not working) #2

same patch, un-bitrotted, applies against alder/default

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Try output from that build.  Next steps are obvious (muck with gyp files).

https://tbpl.mozilla.org/php/getParsedLog.php?id=12976705&tree=Try

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Android patches for make system and gyp files (WIP, not working) #2

Now builds on try(!)  https://tbpl.mozilla.org/?tree=Try&rev=850fb110f2d3  Not sure why we're getting all orange on the tests; could be the dependency on OpenSLES which is only in Android 2.3 and above.

Comment 8

[(not currently active) Ted Mielczarek]

(In reply to Randell Jesup [:jesup] from comment #7)
> Now builds on try(!)  https://tbpl.mozilla.org/?tree=Try&rev=850fb110f2d3 
> Not sure why we're getting all orange on the tests; could be the dependency
> on OpenSLES which is only in Android 2.3 and above.

Yes, our current Android test boxes are Tegras which run Android 2.2. We have Pandaboards coming online in the near future which run ICS.

As I see it we have two options here:
1) Bump our minimum supported version to 2.3. We might be able to get away with this if we make our separate armv6 builds continue to support 2.2, but it will wreak havoc with our automated testing, per above.
2) Dynamically load the OpenSLES lib so as to not introduce a hard dependency. WebRTC will still fail on < 2.3, but at least the builds will run.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

In last Wednesday's call, Ted said that the current plan is to go with option 2 here.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Android patches for make system and gyp files (WIP, not working), #3

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

There were two patches here which were similar but not identical.  My guess is that the oldest one was just forgotten to be marked out-of-date?  jesup, do you recall?

In any case, I took the newer one, and attempted to apply it, and almost all the hunks failed.  So I hand-rebased it, with a couple of exceptions, and it now builds again and Fennec runs.  I've attached that patch as a checkpoint, and marked the version that used to be most recent as obsolete.  

My suspicion is that if one tried to actually use webrtc it would crash.  So, my next steps are:

* get something that runs in mochitest on desktop to try, possibly jsmith's getUserMedia tests that are currently under review

* work on making that pass

* fix up the bits that currently are Android only

Of note are that the current iteration of the patch leaves the Android sdk version at v5 and removed the explicit linkage to OpenSLES, so my expectation is that the first thing I'll need to do to on my way to making a test pass will be to figure out where to the dynamic loading of that library and add that in.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Great!

It's a surprisingly small set of mods in the end; I was surprised originally how few things I had to touch to get it built.

I checked; there is active support for Android in media/webrtc/trunk for capturing audio and video (I actually checked video), so in theory getUserMedia (at least ignoring the UI) should work.  You might need to set the pref to disable UI permission requests.

The mochitests will be a first cut but don't exercise actual hardware cameras, as there are one on the test servers.

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

Interestingly, even with appropriate prefs set, mozSrcObject on the video element is coming up null, so getUserMedia isn't quite there yet.  Time to debug a bit...

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

After discussion with jesup, the next step here is to get a version of this patch landed on mozilla-inbound but defaulting to disabled, since it's going to add bloat and possibly other issues that we don't want to work on just yet.  Doing this will make collaboration among the WebRTC devs much easier, because getting an Android build that runs will be as simple adding --enable-webrtc to anyone's .mozconfig.

Next steps after that which will be required along the way to getting things working (probably spinoff bugs) will include getting netwerk/sctp building and dynamically linking with OpenSLES.

Comment 15

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Android patches for make system and gyp files (WIP, not working), #4

This patch seems to fix a regression in earlier versions of the patch that broke the build for x86 targets (i.e. lots of desktop machines).  It also removes the code that defaulted webrtc to on for variety of builds.

Next step is probably to throw this at a try server.

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

rjesup suggested the following try runs:

For this, a try run (all platforms) would be good with the default state (not enabled on android); one enabled would be good too though the tests would fail on non-pandaboards I assume

Comment 17

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 681729 [details] [diff] [review]
Android patches for make system and gyp files (WIP, not working), #4

As mentioned previously in the bug, the intent here is to simply cause this to build a firefox that runs on Android, so that it's straightforward for people to work on the Android port without having to apply this patch.

The current default of --disable-webrtc for Android will remain, and this patch will now generate a running Fennec for --enable-webrtc on Android as well.

Comment 18

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 681729 [details] [diff] [review]
Android patches for make system and gyp files (WIP, not working), #4

Try server finds that this has busted linux; removing review requests.  Thanks for the try server help, jesup.  If someone else doesn't get around to fixing the linux bustage by the morning, I'll start poking...

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Build system support for --enable-webrtc for Android (off by default)

Comment 20

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 682455 [details] [diff] [review]
Build system support for --enable-webrtc for Android (off by default)

https://tbpl.mozilla.org/?tree=Try&rev=a9da4605048f
that's with webrtc still disabled for android.  

With it enabled, see:
https://tbpl.mozilla.org/?tree=Try&rev=62229bd072ca
and after a small fix for gyp:
https://tbpl.mozilla.org/?tree=Try&rev=61627676c022
(the SCTP failures were expected)

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

Latest try (webrtc disabled on android) https://tbpl.mozilla.org/?tree=Try&rev=a9da4605048f is all green except for a B2G unagi that I retriggered (infra it appears).

Comment 22

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Build system support for --enable-webrtc for Android (off by default)

Put back the explicit linkage to OpenSLES to make more build errors go away.  Getting something building is currently a higher priority than making it maximally back-ported.  I've filed bug 815905 to track making this dynamically linked before we enable WebRTC by default on Android.

Comment 23

[(not currently active) Ted Mielczarek]

Comment on attachment 685912 [details] [diff] [review]
Build system support for --enable-webrtc for Android (off by default)

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

::: configure.in
@@ +5216,5 @@
>      MOZ_VP8_ENCODER=1
>      MOZ_VP8_ERROR_CONCEALMENT=1
> +
> +    if test "$OS_TARGET" = "Android"; then
> +       LDFLAGS="$LDFLAGS -lOpenSLES"

You should probably put a comment here mentioning that we want to remove the hard link dependency on OpenSLES.

@@ +8838,5 @@
> +*)
> +# unsupported arch for webrtc
> +    WEBRTC_TARGET_ARCH=unknown
> +# XXX we probably want to disable it entirely for unknown archs
> +#    MOZ_WEBRTC=

I'd rather you either do this or just leave this out entirely.

@@ +8853,5 @@
>     fi
>     EXTRA_GYP_DEFINES="-D MSVS_VERSION=${_MSVS_VERSION} -D MSVS_OS_BITS=${OS_BITS}"
> +
> +elif test "${OS_TARGET}" = "Android"; then
> +   # Not sure why this comes up as 'linux', perhaps due to cross-compile?

This comment isn't super helpful.

::: media/webrtc/trunk/build/common.gypi
@@ +2239,5 @@
> +#                      '-m32',
> +#                      '-mmmx',
> +#                    ],
> +#                  }],
> +#                ],

This doesn't seem like a good thing to disable globally. should this get put inside a "!android" block?

Comment 24

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: 685912: Build system support for --enable-webrtc for Android (off by default)

Update patch; r+ carried forward.  Still testing in various configs.

Comment 25

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #23)

> You should probably put a comment here mentioning that we want to remove the
> hard link dependency on OpenSLES.

Done.

> 
> @@ +8838,5 @@
> > +*)
> > +# unsupported arch for webrtc
> > +    WEBRTC_TARGET_ARCH=unknown
> > +# XXX we probably want to disable it entirely for unknown archs
> > +#    MOZ_WEBRTC=
> 
> I'd rather you either do this or just leave this out entirely.

Uncommented the MOZ_WEBRTC line and got rid of the XXX comment.
 
> > +   # Not sure why this comes up as 'linux', perhaps due to cross-compile?
> 
> This comment isn't super helpful.

Removed.

> ::: media/webrtc/trunk/build/common.gypi
> @@ +2239,5 @@
> > +#                      '-m32',
> > +#                      '-mmmx',
> > +#                    ],
> > +#                  }],
> > +#                ],
> 
> This doesn't seem like a good thing to disable globally. should this get put
> inside a "!android" block?

New patch has what I suspect to be an appropriate fix here.  I'm compiling locally now; if someone wants to eyeball my changes that'd rock.

Next step: throw this patch at the try server.

Comment 26

[Dan Mosedale (:dmosedale, :dmose)]

Just looking for feedback on common.gypi change.

Comment 27

[(not currently active) Ted Mielczarek]

Comment on attachment 687306 [details] [diff] [review]
685912: Build system support for --enable-webrtc for Android (off by default)

Looks good, thanks.

Comment 28

[Gian-Carlo Pascutto [:gcp]]

https://tbpl.mozilla.org/?tree=Try&rev=1ddd03e88658

Comment 29

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. v2 Build system support for --enable-webrtc for Android (off by default)

Typo fix. 
https://tbpl.mozilla.org/?tree=Try&rev=70eda5de7a00

Comment 30

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c5dad17fece

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0c5dad17fece