EricssonResearch / cerbero

OpenWebRTC cerbero fork
GNU Lesser General Public License v2.1
19 stars 58 forks source link

libnice: integrate patch to spin the agent's mainloop in a separate thread #32

Closed alessandrod closed 8 years ago

alessandrod commented 8 years ago

This commit adds the following patch to the libnice{-static} recipes. It's needed as part of landing https://github.com/EricssonResearch/openwebrtc/issues/186 which i'm hoping to do soon.

Subject: [PATCH] nicesrc: spin the agent mainloop in a separate thread

Don't run the mainloop from the srcpad task, since that can get blocked in the pipeline and cause unnecessary STUN retrasmissions (at best) and completely block the agent (at worst).

superdump commented 8 years ago

:+1:

sdroege commented 8 years ago

Looks good to me and makes sense. Can you also forward this upstream to https://phabricator.freedesktop.org/project/view/20/ ? If you don't want to create yet another account and stuff, I can also forward it there for you :)

sdroege commented 8 years ago

Put it there for discussion https://phabricator.freedesktop.org/T3359

superdump commented 8 years ago

Tested as noted in https://github.com/EricssonResearch/openwebrtc/issues/186 - it helps a lot!

sdroege commented 8 years ago

@alessandrod Can you fix the threading problem? Just needs a GCond, e.g. like here: https://github.com/sdroege/gst-player/blob/master/lib/gst/player/gstplayer.c#L451 https://github.com/sdroege/gst-player/blob/master/lib/gst/player/gstplayer.c#L680 https://github.com/sdroege/gst-player/blob/master/lib/gst/player/gstplayer.c#L2371

alessandrod commented 8 years ago

Updated. I made both _start() and _stop() wait for the agent thread so that we're sure to call g_main_loop_run and g_main_loop_quit in the right order.

superdump commented 8 years ago

If tested then good for me. @sdroege?

sdroege commented 8 years ago

Looks good except for the small leak

alessandrod commented 8 years ago

Done. Thanks!

sdroege commented 8 years ago

Great, let's get this in then :) Do you want to put it into fdo phabricator for review?