Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android thread scheduling issue #126

Closed
Rugvip opened this issue Jan 18, 2015 · 23 comments
Closed

Android thread scheduling issue #126

Rugvip opened this issue Jan 18, 2015 · 23 comments

Comments

@Rugvip
Copy link
Contributor

Rugvip commented Jan 18, 2015

There is an issue with glib and thread scheduling on Android that causes the main owr thread to not get awoken instantly when e.g. a idle GSource is attached. This can cause a delay of up to several minutes when setting up a call.

The first step in debugging this is to move to a newer glib version, meanwhile, here's a temporary fix for the issue:
Rugvip@038146a

@Rugvip Rugvip added the bug label Jan 18, 2015
@sdroege
Copy link
Contributor

sdroege commented Jan 18, 2015

You can easily test a new glib version with cerbero. 2.42 iirc

@sdroege
Copy link
Contributor

sdroege commented Jan 26, 2015

@Rugvip did you test that already? If it's still broken with 2.42 this is really a big problem and will bite us sooner or later in other situations too (it would mean that probably GPoll is broken, which is also used in other places).

@Rugvip
Copy link
Contributor Author

Rugvip commented Jan 26, 2015

I haven't been able to build for Android using cerbero, there's some issue with the icu build. The build fails when trying to build icu for OS X, which is needed for a cross build. I haven't had time to look any further into that issue yet.

@sdroege
Copy link
Contributor

sdroege commented Jan 26, 2015

Can you send me details about that build failure?

@Rugvip
Copy link
Contributor Author

Rugvip commented Jan 26, 2015

I just tried building glib 2.42 and glib-networking with the current build scripts, and it doesn't seem to have fixed the issue. I'm doing a full rebuild of dependencies atm, just in case.

And here's the failed cerbero build:

./cerbero-uninstalled -c config/cross-android-armv7.cbc package openwebrtc

...

[(28/35) icu -> configure ]
WARNING: Using default configuration because /Users/epatold/.cerbero/cerbero.cbc is missing
-----> Configuring icu on the host platform
-----> copying /Users/epatold/cerbero/data/autotools/config.guess to /Users/epatold/cerbero/sources/android_armv7/icu/source/config.guess
-----> copying /Users/epatold/cerbero/data/autotools/config.sub to /Users/epatold/cerbero/sources/android_armv7/icu/source/config.sub
configure: WARNING: unrecognized options: --disable-maintainer-mode, --disable-silent-rules, --enable-introspection
checking for ICU version numbers... release 53.1, library 53.1, unicode version 6.3
checking build system type... x86_64-apple-darwin12
checking host system type... x86_64-apple-darwin12
checking whether to build debug libraries... no
checking whether to build release libraries... yes
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether clang accepts -g... yes
checking for clang option to accept ISO C89... none needed
checking whether we are using the GNU C++ compiler... yes
checking whether clang++ accepts -g... yes
checking how to run the C preprocessor... arm-linux-androideabi-cpp
configure: error: in `/Users/epatold/cerbero/sources/android_armv7/icu/source/host':
configure: error: C preprocessor "arm-linux-androideabi-cpp" fails sanity check
See `config.log' for more details
Running command 'autoreconf -f -i'
Running command '../configure --prefix /Users/epatold/cerbero/dist/darwin_x86_64 --libdir /Users/epatold/cerbero/dist/darwin_x86_64/lib --enable-shared --enable-static         --disable-renaming --disable-samples --disable-debug         --disable-layout --disable-extras --disable-maintainer-mode  --disable-silent-rules  --enable-introspection  --build=x86_64-apple-darwin12'

Recipe 'icu' failed at the build step 'configure'

config.log

@sdroege
Copy link
Contributor

sdroege commented Jan 27, 2015

Ok, we're going to fix the icu build. If the problem still exists with 2.42 you won the price for finding the worst GLib bug on Android so far.

I assume you didn't succeed in creating a simpler testcase for it?

@nirbheek
Copy link
Contributor

I can reproduce the ICU problem, and I'm working on fixing it in the recipe.

Edit: Fix turned out to be simpler than I expected; this should be working now.

nirbheek added a commit to nirbheek/cerbero-old that referenced this issue Jan 27, 2015
The host platform doesn't set CPP, so the cross-build value gets used instead.
This causes a build failure on OS X (although it's wrong everywhere). Just to
be sure, do the same for CC/CXX as well.

EricssonResearch/openwebrtc#126 (comment)
@jrosssavant
Copy link

This fix resolved the icu build issue for me, now I'm down to a failure on the compile step of the openwebrtc recipe:

$ ./cerbero-uninstalled -c config/cross-android-armv7.cbc package openwebrtc
 ... -o .libs/libopenwebrtc_jni.so.0.0.0
owr_jni.c:3337: error: undefined reference to 'owr_media_renderer_dump_dot_file'
owr_jni.c:4004: error: undefined reference to 'owr_media_source_dump_dot_file'
owr_jni.c:5150: error: undefined reference to 'owr_transport_agent_dump_dot_file'
collect2: error: ld returned 1 exit status
make[4]: *** [libopenwebrtc_jni.la] Error 1
make[3]: *** [all] Error 2
make[2]: *** [all-recursive] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
Running command 'make'

Recipe 'openwebrtc' failed at the build step 'compile'

Look familiar?

@superdump
Copy link
Contributor

@Rugvip - looks like those bindings are out of sync perhaps?

@Rugvip
Copy link
Contributor Author

Rugvip commented Jan 29, 2015

@jrosssavant Yep, I'm running into the same issue.

Turns out that you still need a updated build for your host platform when building for android, since cerbero also copies Owr-0.1.gir from the host build.

Try building for osx/linux first and then for Android.

@Rugvip
Copy link
Contributor Author

Rugvip commented Jan 29, 2015

I rebuilt it for OS X, and that itself didn't fix the issue, but when the install failed I chose "[1] rebuild the recipe from scratch", and that did fix it.

@Rugvip
Copy link
Contributor Author

Rugvip commented Jan 29, 2015

The jars end up in ~/cerbero/dist/android_armv7/lib/jni/, and are not included in any .pkg.

I'm getting a loadLibrary error when loading the .jar thought, which I'm fixing atm.

@Rugvip
Copy link
Contributor Author

Rugvip commented Jan 29, 2015

From discussion on irc: the .jars are a bit broken atm, standby :)

@jrosssavant
Copy link

Thanks for the info. After rebuilding for OS X I was able to get the Android build to complete without any errors per your suggestion. I'll hold off testing with this for now until you confirm the jars are in working order.

@sdroege
Copy link
Contributor

sdroege commented Feb 12, 2015

Ok, let's go back to the original issue here instead of build problems. Please create new tickets for that.

@Rugvip did you have any success with trying a newer GLib version? If not we should start creating a minimal testcase for this and make sure to fix it. If it breaks in Owr, it will also cause problems in GStreamer elements that use a custom GMainContext internally.

@nirbheek
Copy link
Contributor

Rugvip created a ticket for the jar problems a while ago: #161

@sdroege
Copy link
Contributor

sdroege commented Feb 12, 2015

Yes, I don't care about that in this ticket 😛 This is about GMainContext being broken on Android

@superdump
Copy link
Contributor

@Rugvip any response?

@superdump
Copy link
Contributor

@Rugvip - this seems to no longer happen when using glib built with cerbero, right?

@sdroege
Copy link
Contributor

sdroege commented Apr 1, 2015

Or is that just gone because of the workaround that was merged?

@superdump
Copy link
Contributor

Could be but I think @Rugvip tested updating glib with the build scripts and still saw the issue. I'll let him respond about that though.

@Rugvip
Copy link
Contributor Author

Rugvip commented Apr 1, 2015

@sdroege The workaround was removed as a part of the owr_init patch, since it didn't seem to be needed anymore.
As @superdump pointed out I've never seen the issue when using a cerbero build, but I did test with glib-2.42, built with the old build scripts.

So either something else that was different between the builds script and cerbero builds fixed the issue, or the issue is still there and we haven't noticed it yet.

For now the workaround is removed and I'm keeping an eye out in case the issue isn't gone.
This also means that we don't have any good way of reproducing the issue with an updated glib version, so ... meh.

@Rugvip
Copy link
Contributor Author

Rugvip commented Jul 18, 2015

Haven't had any more trouble with this.

@Rugvip Rugvip closed this as completed Jul 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants