GNOME Bugzilla – Bug 705371
souphttpsrc: Does network operations from the state change thread
Last modified: 2013-09-18 16:48:27 UTC
For development purposes, I am using Rygel as both the client and the server (single process on the same machine). This bug ONLY occurs when playing media in this scenario. With the following change in GStreamer (gst-plugins-good, souphttpsrc), Rygel hangs when playing any media content: -- 5bc5f4a0f6d9b82f72b39926d8ad81600dac4604 is the first bad commit commit 5bc5f4a0f6d9b82f72b39926d8ad81600dac4604 Author: Thiago Santos <thiago.sousa.santos@collabora.com> Date: Wed Jun 19 14:06:40 2013 -0300 souphttpsrc: allow seeks in ready On is_seekable, check if the server's headers have already been received. If not, do a HEAD request to get them before responding to basesrc. https://bugzilla.gnome.org/show_bug.cgi?id=702206 -- The reason for the hang is that souphttpsrc now makes an HTTP request (HEAD) during pipeline setup prior to moving to PLAY state. It does this so that it can determine whether or not the element can report itself as "seekable" (by looking for range request support in the HTTP HEAD response headers). This request is being initiated on the Rygel main g_main_loop thread. Once the request has been submitted to libsoup, the thread enters a new g_main_loop and waits for a response. Since the Rygel main loop thread is now blocked in a new main loop in souphttpsrc, the request is never processed by the Rygel server and we are now deadlocked.
We do a HEAD reaquest in rygel for the exact same reason, wonder if it's possible to pass that information to souphttpsrc, at least as a work-around...
Created attachment 250755 [details] Minimal testcase Minimal testcase showing the problem without rygel
Anything souphttpsrc does should not depend on or be run in Rygel's main loop, but its own custom main loop / main context. Regression, so marking as blocker.
Tim's latest comments confuse me. Jens marked this bug as being a GStreamer problem. But Tim's comment seems to indicate that Rygel should not be running GStreamer from its main loop. So, is this problem really a GStreamer bug or a Rygel bug?
*If* Jens's comment that > This [HEAD] request is being initiated on the Rygel main g_main_loop thread is correct, then that would indicate a bug in souphttpsrc (or - less likely - libsoup). The unit tests also fail because of this.
> *If* Jens's comment that Sorry, that was your comment of course Greg. Anyway, I'm quite sure it's a souphttpsrc problem.
Well, this testcase obviously hangs because the HEAD request is done when souphttpsrc is set from READY to PAUSED, which is done from the main thread when setting playbin to PLAYING. That means that the server can't reply yet because its main loop is not running yet. souphttpsrc then waits (apparently forever) for the server to reply to the request (opening a connection works it seems). Is that the same problem in your original case of this bug?
(In reply to comment #7) > Is that the same problem in your original case of this bug? Yes, thats why I originally posted this bug in Rygel. I wasn't sure whether the playbin initialization (client) and server threads should be the same.
Ok, so that's just another case of doing synchronous network operations from the main thread. Our network elements should not do that
The souphttpsrc code before the HEAD request was added did all network operations from different threads, so this is actually a regression. Should be moved to a different thread (without waiting for it on the main thread) or be reverted. CC'ing Thiago who wrote the original patch
Thiago, any ideas here? In the worst case we just have to revert the commits that added the HEAD requests before 1.2.
The objective of the patch was allowing gsturidownloader (used by dashdemux) to fetch only the range of the file that was defined by the dash fragment. So doing this operation in READY isn't acceptable. It is also possible for dashdemux to set souphttpsrc to playing and then issue the seek for the desired range, but this would make it start downloading from 0 until the seek was processed. How can souphttpsrc fetch only a range of bytes without attempting to start from 0? Use properties to define the range? Set it to live to prevent producing data in paused and seek in paused?
What about storing the seek in READY and then handle this seek when going to PAUSED?
Created attachment 255057 [details] [review] souphttpsrc: do not do http requests in READY Patch that prevents doing HEAD requests if in READY. This still keeps dash working for range requests as the seek requested start will be stored for later use when doing the GET. I'm still unsure if we need to keep the 'is_seekable' call to the new check_seekable function as I'm not sure if it is acceptable to do the request from query/event handlers as its when basesrc seems to call is_seekable
commit c84282b9a6bfe9383172c4f8ed28b655cfe84f2f Author: Thiago Santos <thiago.sousa.santos@collabora.com> Date: Mon Sep 16 13:53:45 2013 -0300 souphttpsrc: do not do http requests in READY HEAD requests to discover if the server is seekable shouldn't be done in READY as it might lock the main thread that is doing the state change. https://bugzilla.gnome.org/show_bug.cgi?id=705371