[Webkit-unassigned] [Bug 170003] [GStreamer] Deadlock in MediaPlayerPrivateGStreamer::changePipelineState, web process often locks up on seeking in a youtube video that has already fully buffered

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=170003

--- Comment #14 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170605/e680c31f/attachment.html>