Bug 170003

Created attachment 305188 [details] backtrace (this one covers all gst installed plugins)

Same issue with gstreamer 1.12.0 I'm not sure if this helps but youtube streams videos in segments if I understand correctly.

Well the GStreamer portion of the backtrace is good: it's deadlocked on a mutex in gst_base_sink_change_state. But there are no debug symbols for WebKit. Could you install those please? It would be good to get another backtrace with GStreamer 1.12 at any rate.

I managed to finish a debug build of webkit by unsetting the distribution DEBUG_CFLAGS, LDFLAGS, etc... GDB isn't coopering (some weird warning) but I'll figure it out and hopefully post a new backtrace tonight.

Created attachment 311010 [details] gdb output. I don't know how good this is because gdb kept complaining. But this backtrace is with debug webkit.

OK, that's good enough I suppose. It's a shame we can't see local variables inside WebKit (did you use -g1 or some low debugging level?), but we have function names and line numbers and that should be good enough.

To be clear: don't spend any more time on this, it's good. ;)

Additionally, live youtube streams won't even start playing https://www.youtube.com/watch?v=7y7CLWArdFY

*** Bug 172902 has been marked as a duplicate of this bug. ***

I can't reproduce it either, but I think I know what's going on. - Thread 41. Takes preroll mutex in gst_base_sink_chain_main before calling ::render. In WebKit code, the render schedules a repaint in the main thread, and keeps waiting on draw mutex. - Main thread. pause() is requested causing the state change from PLAYING to PAUSE. That ends up changing the state of the video sink to in gst_base_sink_change_state(). It tries to take the preroll lock too, but Thread 41 hasn't released it yet, and it's waiting for us to do the repaint. GStreamer actually handles this case, by calling ::unlock on the sink before trying to take the preroll lock. The problem is that WebKit is not canceling the draw timer and release the draw mutes in that case. The code path not using coordinated graphics does that indeed with its paint timer and mutex. I'll prepare a patch, but I can't reproduce it.

Created attachment 311998 [details] Patch

Comment on attachment 311998 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311998&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:836 > +#if USE(GSTREAMER_GL) || USE(COORDINATED_GRAPHICS_THREADED) > + m_drawTimer.stop(); > + LockHolder locker(m_drawMutex); > + m_drawCondition.notifyOne(); > +#endif This is not going to work when GSTREAMER_GL is enabled. In that case neither drawTimer nor drawMutex or drawCondition are used, as we just push the video frame to the compositor and don't stop the gstreamer thread until the drawing is finished. I would put this as #if !USE(GSTREAMER_GL) && USE(COORDINATED_GRAPHICS_THREADED)

(In reply to Miguel Gomez from comment #12) > Comment on attachment 311998 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=311998&action=review > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:836 > > +#if USE(GSTREAMER_GL) || USE(COORDINATED_GRAPHICS_THREADED) > > + m_drawTimer.stop(); > > + LockHolder locker(m_drawMutex); > > + m_drawCondition.notifyOne(); > > +#endif > > This is not going to work when GSTREAMER_GL is enabled. In that case neither > drawTimer nor drawMutex or drawCondition are used, as we just push the video > frame to the compositor and don't stop the gstreamer thread until the > drawing is finished. > I would put this as #if !USE(GSTREAMER_GL) && > USE(COORDINATED_GRAPHICS_THREADED) Ah, forgot the case when GSTREAMER_GL is enabled and accelerated compositing disabled. In that case we do use the timer, mutex and condition. So we should keep the USE(GSTREAMER_GL) but check that the rendering is not being accelerated in the GSTREAMER_GL case.

(In reply to Miguel Gomez from comment #13) > (In reply to Miguel Gomez from comment #12) > > Comment on attachment 311998 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=311998&action=review > > > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:836 > > > +#if USE(GSTREAMER_GL) || USE(COORDINATED_GRAPHICS_THREADED) > > > + m_drawTimer.stop(); > > > + LockHolder locker(m_drawMutex); > > > + m_drawCondition.notifyOne(); > > > +#endif > > > > This is not going to work when GSTREAMER_GL is enabled. In that case neither > > drawTimer nor drawMutex or drawCondition are used, as we just push the video > > frame to the compositor and don't stop the gstreamer thread until the > > drawing is finished. > > I would put this as #if !USE(GSTREAMER_GL) && > > USE(COORDINATED_GRAPHICS_THREADED) > > Ah, forgot the case when GSTREAMER_GL is enabled and accelerated compositing > disabled. In that case we do use the timer, mutex and condition. So we > should keep the USE(GSTREAMER_GL) but check that the rendering is not being > accelerated in the GSTREAMER_GL case. I think it's harmless to stop the timer and notify the mutex in that case. That's what we do in the destructor. I'm going to update the patch to call cancelRepaint in the destructor indeed, to avoid the duplicated code.

Created attachment 312005 [details] Updated patch

Comment on attachment 312005 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=312005&action=review I don't understand this code, but Miguel does and I'll trust his review, so rs=me. > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:418 > + 0, // Only one parameter You need to change this comment since it's zero parameters

Committed r217786: <http://trac.webkit.org/changeset/217786>